Devtools: refactor source actor state (#37528)

We currently store the source contents in both the SourceActor and the
ThreadActor’s SourceManager, which is redundant. We also currently send
the source contents in thread `sources` responses and watcher
`resources-available-array` messages, but in both cases this is
unnecessary (and ignored by the client).

This patch merges SourceData into SourceActor, making the latter the
single source of truth for source-related state. We also create a
SourceForm type, which represents the subset of source-related state
that gets sent in thread `sources` responses (and for now, watcher
`resources-available-array` messages).

Finally we rename `source_urls` → `source_actor_names` and `new_source`
→ `new_registered` for clarity.

Testing: no changes to client-facing behaviour, and this is covered by
tests

Signed-off-by: Delan Azabani <dazabani@igalia.com>
Co-authored-by: atbrakhi <atbrakhi@igalia.com>

components/devtools/actors/source.rs

@@ -2,7 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
-use std::cell::{Ref, RefCell};
+use std::cell::RefCell;
 use std::collections::BTreeSet;
 use std::net::TcpStream;
 
@@ -14,29 +14,43 @@ use crate::StreamId;
 use crate::actor::{Actor, ActorMessageStatus, ActorRegistry};
 use crate::protocol::JsonPacketStream;
 
+/// A `sourceForm` as used in responses to thread `sources` requests.
+///
+/// For now, we also use this for sources in watcher `resource-available-array` messages,
+/// but in Firefox those have extra fields.
+///
+/// <https://firefox-source-docs.mozilla.org/devtools/backend/protocol.html#loading-script-sources>
 #[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd, Serialize)]
 #[serde(rename_all = "camelCase")]
-pub(crate) struct SourceData {
+pub(crate) struct SourceForm {
     pub actor: String,
     /// URL of the script, or URL of the page for inline scripts.
     pub url: String,
     pub is_black_boxed: bool,
-    pub source_content: String,
 }
 
 #[derive(Serialize)]
 pub(crate) struct SourcesReply {
     pub from: String,
-    pub sources: Vec<SourceData>,
+    pub sources: Vec<SourceForm>,
 }
 
 pub(crate) struct SourceManager {
-    pub source_urls: RefCell<BTreeSet<SourceData>>,
+    source_actor_names: RefCell<BTreeSet<String>>,
 }
 
 #[derive(Clone, Debug)]
 pub struct SourceActor {
+    /// Actor name.
     pub name: String,
+
+    /// URL of the script, or URL of the page for inline scripts.
+    pub url: ServoUrl,
+
+    /// The ‘black-boxed’ flag, which tells the debugger to avoid pausing inside this script.
+    /// <https://firefox-source-docs.mozilla.org/devtools/backend/protocol.html#black-boxing-sources>
+    pub is_black_boxed: bool,
+
     pub content: String,
     pub content_type: String,
 }
@@ -52,40 +66,56 @@ struct SourceContentReply {
 impl SourceManager {
     pub fn new() -> Self {
         Self {
-            source_urls: RefCell::new(BTreeSet::default()),
+            source_actor_names: RefCell::new(BTreeSet::default()),
         }
     }
 
-    pub fn add_source(&self, url: ServoUrl, source_content: String, actor_name: String) {
+    pub fn add_source(&self, actor_name: &str) {
-        self.source_urls.borrow_mut().insert(SourceData {
+        self.source_actor_names
-            actor: actor_name,
+            .borrow_mut()
-            url: url.to_string(),
+            .insert(actor_name.to_owned());
-            is_black_boxed: false,
-            source_content,
-        });
     }
 
-    pub fn sources(&self) -> Ref<BTreeSet<SourceData>> {
+    pub fn source_forms(&self, actors: &ActorRegistry) -> Vec<SourceForm> {
-        self.source_urls.borrow()
+        self.source_actor_names
+            .borrow()
+            .iter()
+            .map(|actor_name| actors.find::<SourceActor>(actor_name).source_form())
+            .collect()
     }
 }
 
 impl SourceActor {
-    pub fn new(name: String, content: String, content_type: String) -> SourceActor {
+    pub fn new(name: String, url: ServoUrl, content: String, content_type: String) -> SourceActor {
         SourceActor {
             name,
+            url,
             content,
             content_type,
+            is_black_boxed: false,
         }
     }
 
-    pub fn new_source(actors: &mut ActorRegistry, content: String, content_type: String) -> String {
+    pub fn new_registered(
+        actors: &mut ActorRegistry,
+        url: ServoUrl,
+        content: String,
+        content_type: String,
+    ) -> &SourceActor {
         let source_actor_name = actors.new_name("source");
 
-        let source_actor = SourceActor::new(source_actor_name.clone(), content, content_type);
+        let source_actor = SourceActor::new(source_actor_name.clone(), url, content, content_type);
         actors.register(Box::new(source_actor));
 
-        source_actor_name
+        actors.find(&source_actor_name)
+    }
+
+    pub fn source_form(&self) -> SourceForm {
+        SourceForm {
+            actor: self.name.clone(),
+            url: self.url.to_string(),
+            is_black_boxed: self.is_black_boxed,
+        }
     }
 }
 

components/devtools/actors/thread.rs

@@ -7,7 +7,7 @@ use std::net::TcpStream;
 use serde::Serialize;
 use serde_json::{Map, Value};
 
-use super::source::{SourceData, SourceManager, SourcesReply};
+use super::source::{SourceManager, SourcesReply};
 use crate::actor::{Actor, ActorMessageStatus, ActorRegistry};
 use crate::protocol::JsonPacketStream;
 use crate::{EmptyReplyMsg, StreamId};
@@ -124,17 +124,9 @@ impl Actor for ThreadActor {
             // Client has attached to the thread and wants to load script sources.
             // <https://firefox-source-docs.mozilla.org/devtools/backend/protocol.html#loading-script-sources>
             "sources" => {
-                let sources: Vec<SourceData> = self
-                    .source_manager
-                    .source_urls
-                    .borrow()
-                    .iter()
-                    .cloned()
-                    .collect();
-
                 let msg = SourcesReply {
                     from: self.name(),
-                    sources,
+                    sources: self.source_manager.source_forms(registry),
                 };
                 let _ = stream.write_json_packet(&msg);
                 ActorMessageStatus::Processed

components/devtools/actors/watcher.rs

@@ -297,9 +297,8 @@ impl Actor for WatcherActor {
                         },
                         "source" => {
                             let thread_actor = registry.find::<ThreadActor>(&target.thread);
-                            let sources = thread_actor.source_manager.sources();
                             target.resources_available(
-                                sources.iter().collect(),
+                                thread_actor.source_manager.source_forms(registry),
                                 "source".into(),
                                 stream,
                             );
@@ -307,10 +306,9 @@ impl Actor for WatcherActor {
                             for worker_name in &root.workers {
                                 let worker = registry.find::<WorkerActor>(worker_name);
                                 let thread = registry.find::<ThreadActor>(&worker.thread);
-                                let worker_sources = thread.source_manager.sources();
 
                                 worker.resources_available(
-                                    worker_sources.iter().collect(),
+                                    thread.source_manager.source_forms(registry),
                                     "source".into(),
                                     stream,
                                 );

components/devtools/lib.rs

@@ -43,7 +43,7 @@ use crate::actors::performance::PerformanceActor;
 use crate::actors::preference::PreferenceActor;
 use crate::actors::process::ProcessActor;
 use crate::actors::root::RootActor;
-use crate::actors::source::{SourceActor, SourceData};
+use crate::actors::source::SourceActor;
 use crate::actors::thread::ThreadActor;
 use crate::actors::worker::{WorkerActor, WorkerType};
 use crate::id::IdMap;
@@ -518,11 +518,14 @@ impl DevtoolsInstance {
     fn handle_script_source_info(&mut self, pipeline_id: PipelineId, source_info: SourceInfo) {
         let mut actors = self.actors.lock().unwrap();
 
-        let source_actor_name = SourceActor::new_source(
+        let source_actor = SourceActor::new_registered(
             &mut actors,
+            source_info.url,
             source_info.content.clone(),
             source_info.content_type.unwrap(),
         );
+        let source_actor_name = source_actor.name.clone();
+        let source_form = source_actor.source_form();
 
         if let Some(worker_id) = source_info.worker_id {
             let Some(worker_actor_name) = self.actor_workers.get(&worker_id) else {
@@ -532,23 +535,12 @@ impl DevtoolsInstance {
             let thread_actor_name = actors.find::<WorkerActor>(worker_actor_name).thread.clone();
             let thread_actor = actors.find_mut::<ThreadActor>(&thread_actor_name);
 
-            thread_actor.source_manager.add_source(
+            thread_actor.source_manager.add_source(&source_actor_name);
-                source_info.url.clone(),
-                source_info.content.clone(),
-                source_actor_name.clone(),
-            );
-
-            let source = SourceData {
-                actor: source_actor_name,
-                url: source_info.url.to_string(),
-                is_black_boxed: false,
-                source_content: source_info.content,
-            };
 
             let worker_actor = actors.find::<WorkerActor>(worker_actor_name);
 
             for stream in self.connections.values_mut() {
-                worker_actor.resource_available(&source, "source".into(), stream);
+                worker_actor.resource_available(&source_form, "source".into(), stream);
             }
         } else {
             let Some(browsing_context_id) = self.pipelines.get(&pipeline_id) else {
@@ -565,24 +557,13 @@ impl DevtoolsInstance {
 
             let thread_actor = actors.find_mut::<ThreadActor>(&thread_actor_name);
 
-            thread_actor.source_manager.add_source(
+            thread_actor.source_manager.add_source(&source_actor_name);
-                source_info.url.clone(),
-                source_info.content.clone(),
-                source_actor_name.clone(),
-            );
-
-            let source = SourceData {
-                actor: source_actor_name,
-                url: source_info.url.to_string(),
-                is_black_boxed: false,
-                source_content: source_info.content,
-            };
 
             // Notify browsing context about the new source
             let browsing_context = actors.find::<BrowsingContextActor>(actor_name);
 
             for stream in self.connections.values_mut() {
-                browsing_context.resource_available(&source, "source".into(), stream);
+                browsing_context.resource_available(&source_form, "source".into(), stream);
             }
         }
     }