DevTools: sources for HTML files should be the whole HTML file (#37456)

To show the contents of inline scripts in the Sources panel, we need to
send the whole HTML file from script to devtools, not just the script
code. This is trickier than the external script case, but we can look to
[how Firefox does
it](https://servo.zulipchat.com/#narrow/channel/263398-general/topic/Getting.20the.20original.20page.20HTML.20from.20script/near/524392861)
for some inspiration. The process is as follows:

- when we execute a script
  - notify devtools to create the source actor
- if it’s an external script, send the script code to the devtools
server
  - if it’s an inline script, don’t send any source contents yet
  - devtools stores the contents in the source actor
- while loading a new document
  - buffer the markup, so we can send it to devtools
- when we finish loading a new document
  - send the buffered markup to the devtools server
- devtools stores the contents in any source actors with no contents yet
- when a source actor gets a `source` request
  - if we have the contents, send those contents to the client
- if we don’t have the contents (inline script that loaded while
devtools was closed)
    - FUTURE: try to fetch the markup out of cache
    - otherwise send `<!-- not available; please reload! -->`

Testing: Several tests added to test the changes, also updates an
existing test with correct assertion
Fixes: https://github.com/servo/servo/issues/36874

---------

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

components/devtools/actor.rs

@@ -10,10 +10,10 @@ use std::net::TcpStream;
 use std::sync::{Arc, Mutex};
 
 use base::cross_process_instant::CrossProcessInstant;
+use base::id::PipelineId;
 use log::debug;
 use serde_json::{Map, Value};
 
-/// General actor system infrastructure.
 use crate::StreamId;
 
 #[derive(PartialEq)]
@@ -58,6 +58,10 @@ pub struct ActorRegistry {
     new_actors: RefCell<Vec<Box<dyn Actor + Send>>>,
     old_actors: RefCell<Vec<String>>,
     script_actors: RefCell<HashMap<String, String>>,
+
+    /// Lookup table for SourceActor names associated with a given PipelineId.
+    source_actor_names: RefCell<HashMap<PipelineId, Vec<String>>>,
+
     shareable: Option<Arc<Mutex<ActorRegistry>>>,
     next: Cell<u32>,
     start_stamp: CrossProcessInstant,
@@ -71,6 +75,7 @@ impl ActorRegistry {
             new_actors: RefCell::new(vec![]),
             old_actors: RefCell::new(vec![]),
             script_actors: RefCell::new(HashMap::new()),
+            source_actor_names: RefCell::new(HashMap::new()),
             shareable: None,
             next: Cell::new(0),
             start_stamp: CrossProcessInstant::now(),
@@ -215,4 +220,20 @@ impl ActorRegistry {
         let mut actors = self.old_actors.borrow_mut();
         actors.push(name);
     }
+
+    pub fn register_source_actor(&self, pipeline_id: PipelineId, actor_name: &str) {
+        self.source_actor_names
+            .borrow_mut()
+            .entry(pipeline_id)
+            .or_default()
+            .push(actor_name.to_owned());
+    }
+
+    pub fn source_actor_names_for_pipeline(&mut self, pipeline_id: PipelineId) -> Vec<String> {
+        if let Some(source_actor_names) = self.source_actor_names.borrow_mut().get(&pipeline_id) {
+            return source_actor_names.clone();
+        }
+
+        vec![]
+    }
 }

components/devtools/actors/source.rs

@@ -6,6 +6,7 @@ use std::cell::RefCell;
 use std::collections::BTreeSet;
 use std::net::TcpStream;
 
+use base::id::PipelineId;
 use serde::Serialize;
 use serde_json::{Map, Value};
 use servo_url::ServoUrl;
@@ -51,7 +52,7 @@ pub struct SourceActor {
     /// <https://firefox-source-docs.mozilla.org/devtools/backend/protocol.html#black-boxing-sources>
     pub is_black_boxed: bool,
 
-    pub content: String,
+    pub content: Option<String>,
     pub content_type: String,
 }
 
@@ -86,7 +87,12 @@ impl SourceManager {
 }
 
 impl SourceActor {
-    pub fn new(name: String, url: ServoUrl, content: String, content_type: String) -> SourceActor {
+    pub fn new(
+        name: String,
+        url: ServoUrl,
+        content: Option<String>,
+        content_type: String,
+    ) -> SourceActor {
         SourceActor {
             name,
             url,
@@ -98,14 +104,16 @@ impl SourceActor {
 
     pub fn new_registered(
         actors: &mut ActorRegistry,
+        pipeline_id: PipelineId,
         url: ServoUrl,
-        content: String,
+        content: Option<String>,
         content_type: String,
     ) -> &SourceActor {
         let source_actor_name = actors.new_name("source");
 
         let source_actor = SourceActor::new(source_actor_name.clone(), url, content, content_type);
         actors.register(Box::new(source_actor));
+        actors.register_source_actor(pipeline_id, &source_actor_name);
 
         actors.find(&source_actor_name)
     }
@@ -138,7 +146,13 @@ impl Actor for SourceActor {
                 let reply = SourceContentReply {
                     from: self.name(),
                     content_type: self.content_type.clone(),
-                    source: self.content.clone(),
+                    // TODO: do we want to wait instead of giving up immediately, in cases where the content could
+                    // become available later (e.g. after a fetch)?
+                    source: self
+                        .content
+                        .as_deref()
+                        .unwrap_or("<!-- not available; please reload! -->")
+                        .to_owned(),
                 };
                 let _ = stream.write_json_packet(&reply);
                 ActorMessageStatus::Processed

components/devtools/lib.rs

@@ -252,10 +252,13 @@ impl DevtoolsInstance {
                     console_message,
                     worker_id,
                 )) => self.handle_console_message(pipeline_id, worker_id, console_message),
-                DevtoolsControlMsg::FromScript(ScriptToDevtoolsControlMsg::ScriptSourceLoaded(
+                DevtoolsControlMsg::FromScript(ScriptToDevtoolsControlMsg::CreateSourceActor(
                     pipeline_id,
                     source_info,
-                )) => self.handle_script_source_info(pipeline_id, source_info),
+                )) => self.handle_create_source_actor(pipeline_id, source_info),
+                DevtoolsControlMsg::FromScript(
+                    ScriptToDevtoolsControlMsg::UpdateSourceContent(pipeline_id, source_content),
+                ) => self.handle_update_source_content(pipeline_id, source_content),
                 DevtoolsControlMsg::FromScript(ScriptToDevtoolsControlMsg::ReportPageError(
                     pipeline_id,
                     page_error,
@@ -515,13 +518,14 @@ impl DevtoolsInstance {
         }
     }
 
-    fn handle_script_source_info(&mut self, pipeline_id: PipelineId, source_info: SourceInfo) {
+    fn handle_create_source_actor(&mut self, pipeline_id: PipelineId, source_info: SourceInfo) {
         let mut actors = self.actors.lock().unwrap();
 
         let source_actor = SourceActor::new_registered(
             &mut actors,
+            pipeline_id,
             source_info.url,
-            source_info.content.clone(),
+            source_info.content,
             source_info.content_type.unwrap(),
         );
         let source_actor_name = source_actor.name.clone();
@@ -577,6 +581,17 @@ impl DevtoolsInstance {
             }
         }
     }
+
+    fn handle_update_source_content(&mut self, pipeline_id: PipelineId, source_content: String) {
+        let mut actors = self.actors.lock().unwrap();
+
+        for actor_name in actors.source_actor_names_for_pipeline(pipeline_id) {
+            let source_actor: &mut SourceActor = actors.find_mut(&actor_name);
+            if source_actor.content.is_none() {
+                source_actor.content = Some(source_content.clone());
+            }
+        }
+    }
 }
 
 fn allow_devtools_client(stream: &mut TcpStream, embedder: &EmbedderProxy, token: &str) -> bool {

components/script/dom/dedicatedworkerglobalscope.rs

@@ -488,10 +488,10 @@ impl DedicatedWorkerGlobalScope {
                         url: metadata.final_url,
                         external: true, // Worker scripts are always external.
                         worker_id: Some(global.upcast::<WorkerGlobalScope>().get_worker_id()),
-                        content: source.to_string(),
+                        content: Some(source.to_string()),
                         content_type: metadata.content_type.map(|c_type| c_type.0.to_string()),
                     };
-                    let _ = chan.send(ScriptToDevtoolsControlMsg::ScriptSourceLoaded(
+                    let _ = chan.send(ScriptToDevtoolsControlMsg::CreateSourceActor(
                         pipeline_id,
                         source_info,
                     ));

components/script/dom/htmlscriptelement.rs

@@ -16,6 +16,7 @@ use content_security_policy as csp;
 use devtools_traits::{ScriptToDevtoolsControlMsg, SourceInfo};
 use dom_struct::dom_struct;
 use encoding_rs::Encoding;
+use html5ever::serialize::TraversalScope;
 use html5ever::{LocalName, Prefix, local_name, namespace_url, ns};
 use ipc_channel::ipc;
 use js::jsval::UndefinedValue;
@@ -27,7 +28,7 @@ use net_traits::request::{
     RequestBuilder, RequestId,
 };
 use net_traits::{
-    FetchMetadata, FetchResponseListener, Metadata, NetworkError, ResourceFetchTiming,
+    FetchMetadata, FetchResponseListener, IpcSend, Metadata, NetworkError, ResourceFetchTiming,
     ResourceTimingType,
 };
 use servo_config::pref;
@@ -72,7 +73,7 @@ use crate::dom::trustedscript::TrustedScript;
 use crate::dom::trustedscripturl::TrustedScriptURL;
 use crate::dom::virtualmethods::VirtualMethods;
 use crate::dom::window::Window;
-use crate::fetch::create_a_potential_cors_request;
+use crate::fetch::{create_a_potential_cors_request, load_whole_resource};
 use crate::network_listener::{self, NetworkListener, PreInvoke, ResourceTimingListener};
 use crate::realms::enter_realm;
 use crate::script_module::{
@@ -1085,23 +1086,32 @@ impl HTMLScriptElement {
         if let Some(chan) = self.global().devtools_chan() {
             let pipeline_id = self.global().pipeline_id();
 
-            // TODO: https://github.com/servo/servo/issues/36874
+            let (url, content, content_type, is_external) = if script.external {
                 let content = match &script.code {
                     SourceCode::Text(text) => text.to_string(),
                     SourceCode::Compiled(compiled) => compiled.original_text.to_string(),
                 };
 
-            // https://html.spec.whatwg.org/multipage/#scriptingLanguages
+                // content_type: https://html.spec.whatwg.org/multipage/#scriptingLanguages
-            let content_type = Some("text/javascript".to_string());
+                (script.url.clone(), Some(content), "text/javascript", true)
+            } else {
+                // TODO: if needed, fetch the page again, in the same way as in the original request.
+                // Fetch it from cache, even if the original request was non-idempotent (e.g. POST).
+                // If we can’t fetch it from cache, we should probably give up, because with a real
+                // fetch, the server could return a different response.
+
+                // TODO: handle cases where Content-Type is not text/html.
+                (doc.url(), None, "text/html", false)
+            };
 
             let source_info = SourceInfo {
-                url: script.url.clone(),
+                url,
-                external: script.external,
+                external: is_external,
                 worker_id: None,
                 content,
-                content_type,
+                content_type: Some(content_type.to_string()),
             };
-            let _ = chan.send(ScriptToDevtoolsControlMsg::ScriptSourceLoaded(
+            let _ = chan.send(ScriptToDevtoolsControlMsg::CreateSourceActor(
                 pipeline_id,
                 source_info,
             ));

components/script/dom/servoparser/mod.rs

@@ -10,6 +10,7 @@ use base::id::PipelineId;
 use base64::Engine as _;
 use base64::engine::general_purpose;
 use content_security_policy as csp;
+use devtools_traits::ScriptToDevtoolsControlMsg;
 use dom_struct::dom_struct;
 use embedder_traits::resources::{self, Resource};
 use encoding_rs::Encoding;
@@ -137,6 +138,9 @@ pub(crate) struct ServoParser {
     #[ignore_malloc_size_of = "Defined in html5ever"]
     #[no_trace]
     prefetch_input: BufferQueue,
+    // The whole input as a string, if needed for the devtools Sources panel.
+    // TODO: use a faster type for concatenating strings?
+    content_for_devtools: Option<DomRefCell<String>>,
 }
 
 pub(crate) struct ElementAttribute {
@@ -457,6 +461,13 @@ impl ServoParser {
 
     #[cfg_attr(crown, allow(crown::unrooted_must_root))]
     fn new_inherited(document: &Document, tokenizer: Tokenizer, kind: ParserKind) -> Self {
+        // Store the whole input for the devtools Sources panel, if the devtools server is running
+        // and we are parsing for a document load (not just things like innerHTML).
+        // TODO: check if a devtools client is actually connected and/or wants the sources?
+        let content_for_devtools = (document.global().devtools_chan().is_some() &&
+            document.has_browsing_context())
+        .then_some(DomRefCell::new(String::new()));
+
         ServoParser {
             reflector: Reflector::new(),
             document: Dom::from_ref(document),
@@ -472,6 +483,7 @@ impl ServoParser {
             script_created_parser: kind == ParserKind::ScriptCreated,
             prefetch_tokenizer: prefetch::Tokenizer::new(document),
             prefetch_input: BufferQueue::default(),
+            content_for_devtools,
         }
     }
 
@@ -490,6 +502,15 @@ impl ServoParser {
     }
 
     fn push_tendril_input_chunk(&self, chunk: StrTendril) {
+        if let Some(mut content_for_devtools) = self
+            .content_for_devtools
+            .as_ref()
+            .map(|content| content.borrow_mut())
+        {
+            // TODO: append these chunks more efficiently
+            content_for_devtools.push_str(chunk.as_ref());
+        }
+
         if chunk.is_empty() {
             return;
         }
@@ -687,6 +708,21 @@ impl ServoParser {
         // Steps 3-12 are in another castle, namely finish_load.
         let url = self.tokenizer.url().clone();
         self.document.finish_load(LoadType::PageSource(url), can_gc);
+
+        // Send the source contents to devtools, if needed.
+        if let Some(content_for_devtools) = self
+            .content_for_devtools
+            .as_ref()
+            .map(|content| content.take())
+        {
+            let global = self.document.global();
+            let chan = global.devtools_chan().expect("Guaranteed by new");
+            let pipeline_id = self.document.global().pipeline_id();
+            let _ = chan.send(ScriptToDevtoolsControlMsg::UpdateSourceContent(
+                pipeline_id,
+                content_for_devtools,
+            ));
+        }
     }
 }
 

components/shared/devtools/lib.rs

@@ -105,7 +105,9 @@ pub enum ScriptToDevtoolsControlMsg {
     TitleChanged(PipelineId, String),
 
     /// Get source information from script
-    ScriptSourceLoaded(PipelineId, SourceInfo),
+    CreateSourceActor(PipelineId, SourceInfo),
+
+    UpdateSourceContent(PipelineId, String),
 }
 
 /// Serialized JS return values
@@ -563,6 +565,6 @@ pub struct SourceInfo {
     pub url: ServoUrl,
     pub external: bool,
     pub worker_id: Option<WorkerId>,
-    pub content: String,
+    pub content: Option<String>,
     pub content_type: Option<String>,
 }

python/servo/devtools_tests.py

@@ -96,9 +96,9 @@ class DevtoolsTests(unittest.IsolatedAsyncioTestCase):
         self.assert_sources_list(1, set([tuple(["data:text/html,<script type=module>;</script>"])]))
 
     def test_source_content_inline_script(self):
-        script_content = "console.log('Hello, world!');"
+        script_tag = "<script>console.log('Hello, world!')</script>"
-        self.run_servoshell(url=f"data:text/html,<script>{script_content}</script>")
+        self.run_servoshell(url=f"data:text/html,{script_tag}")
-        self.assert_source_content("data:text/html,<script>console.log('Hello, world!');</script>", script_content)
+        self.assert_source_content(f"data:text/html,{script_tag}", script_tag)
 
     def test_source_content_external_script(self):
         self.start_web_server(test_dir=os.path.join(DevtoolsTests.script_path, "devtools_tests/sources"))
@@ -106,6 +106,41 @@ class DevtoolsTests(unittest.IsolatedAsyncioTestCase):
         expected_content = 'console.log("external classic");\n'
         self.assert_source_content(f"{self.base_url}/classic.js", expected_content)
 
+    def test_source_content_html_file(self):
+        self.start_web_server(test_dir=self.get_test_path("sources"))
+        self.run_servoshell()
+        expected_content = open(self.get_test_path("sources/test.html")).read()
+        self.assert_source_content(f"{self.base_url}/test.html", expected_content)
+
+    # Test case that uses innerHTML and would actually need the HTML parser
+    # (innerHTML has a fast path for values that don’t contain b'&' | b'\0' | b'<' | b'\r')
+    def test_source_content_inline_script_with_inner_html(self):
+        script_tag = '<div id="el"></div><script>el.innerHTML="<p>test"</script>'
+        self.run_servoshell(url=f"data:text/html,{script_tag}")
+        self.assert_source_content(f"data:text/html,{script_tag}", script_tag)
+
+    # Test case that uses outerHTML and would actually need the HTML parser
+    # (innerHTML has a fast path for values that don’t contain b'&' | b'\0' | b'<' | b'\r')
+    def test_source_content_inline_script_with_outer_html(self):
+        script_tag = '<div id="el"></div><script>el.outerHTML="<p>test"</script>'
+        self.run_servoshell(url=f"data:text/html,{script_tag}")
+        self.assert_source_content(f"data:text/html,{script_tag}", script_tag)
+
+    # Test case that uses DOMParser and would actually need the HTML parser
+    # (innerHTML has a fast path for values that don’t contain b'&' | b'\0' | b'<' | b'\r')
+    def test_source_content_inline_script_with_domparser(self):
+        script_tag = '<script>(new DOMParser).parseFromString("<p>test","text/html")</script>'
+        self.run_servoshell(url=f"data:text/html,{script_tag}")
+        self.assert_source_content(f"data:text/html,{script_tag}", script_tag)
+
+    # Test case that uses XMLHttpRequest#responseXML and would actually need the HTML parser
+    # (innerHTML has a fast path for values that don’t contain b'&' | b'\0' | b'<' | b'\r')
+    def test_source_content_inline_script_with_responsexml(self):
+        self.start_web_server(test_dir=self.get_test_path("sources_content_with_responsexml"))
+        self.run_servoshell()
+        expected_content = open(self.get_test_path("sources_content_with_responsexml/test.html")).read()
+        self.assert_source_content(f"{self.base_url}/test.html", expected_content)
+
     # Sets `base_url` and `web_server` and `web_server_thread`.
     def start_web_server(self, *, test_dir=None):
         if test_dir is None:
@@ -270,6 +305,9 @@ class DevtoolsTests(unittest.IsolatedAsyncioTestCase):
 
         client.disconnect()
 
+    def get_test_path(self, path: str) -> str:
+        return os.path.join(DevtoolsTests.script_path, os.path.join("devtools_tests", path))
+
 
 def run_tests(script_path, build_type: BuildType):
     DevtoolsTests.script_path = script_path

python/servo/devtools_tests/sources_content_with_responsexml/empty.html

@@ -0,0 +1 @@
+<!doctype html><meta charset=utf-8>

python/servo/devtools_tests/sources_content_with_responsexml/test.html

@@ -0,0 +1,11 @@
+<!doctype html><meta charset=utf-8>
+<script>
+    const req = new XMLHttpRequest;
+    req.responseType = "document";
+    req.open("GET", "empty.html");
+    req.send(null);
+    req.onreadystatechange = () => {
+        if (req.readyState == XMLHttpRequest.DONE)
+            console.log(req.responseXML);
+    };
+</script>