diff --git a/components/devtools/actor.rs b/components/devtools/actor.rs index 8d6b426f35b..ca2b8576321 100644 --- a/components/devtools/actor.rs +++ b/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>>, old_actors: RefCell>, script_actors: RefCell>, + + /// Lookup table for SourceActor names associated with a given PipelineId. + source_actor_names: RefCell>>, + shareable: Option>>, next: Cell, 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 { + if let Some(source_actor_names) = self.source_actor_names.borrow_mut().get(&pipeline_id) { + return source_actor_names.clone(); + } + + vec![] + } } diff --git a/components/devtools/actors/source.rs b/components/devtools/actors/source.rs index 3cfc7b354e5..9f8dfcde3cd 100644 --- a/components/devtools/actors/source.rs +++ b/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 { /// pub is_black_boxed: bool, - pub content: String, + pub content: Option, 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, + 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, 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("") + .to_owned(), }; let _ = stream.write_json_packet(&reply); ActorMessageStatus::Processed diff --git a/components/devtools/lib.rs b/components/devtools/lib.rs index 0adbf0ded36..bf0c43a79ad 100644 --- a/components/devtools/lib.rs +++ b/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 { diff --git a/components/script/dom/dedicatedworkerglobalscope.rs b/components/script/dom/dedicatedworkerglobalscope.rs index 9263edeb3ef..1e87cf02687 100644 --- a/components/script/dom/dedicatedworkerglobalscope.rs +++ b/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::().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, )); diff --git a/components/script/dom/htmlscriptelement.rs b/components/script/dom/htmlscriptelement.rs index 17838ace9fc..325000e773b 100644 --- a/components/script/dom/htmlscriptelement.rs +++ b/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 content = match &script.code { - SourceCode::Text(text) => text.to_string(), - SourceCode::Compiled(compiled) => compiled.original_text.to_string(), - }; + 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 - let content_type = Some("text/javascript".to_string()); + // content_type: https://html.spec.whatwg.org/multipage/#scriptingLanguages + (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(), - external: script.external, + url, + 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, )); diff --git a/components/script/dom/servoparser/mod.rs b/components/script/dom/servoparser/mod.rs index bbba8645d44..a0164035a7a 100644 --- a/components/script/dom/servoparser/mod.rs +++ b/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>, } 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, + )); + } } } diff --git a/components/shared/devtools/lib.rs b/components/shared/devtools/lib.rs index 815ad087dd7..3bce57d61fc 100644 --- a/components/shared/devtools/lib.rs +++ b/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, - pub content: String, + pub content: Option, pub content_type: Option, } diff --git a/python/servo/devtools_tests.py b/python/servo/devtools_tests.py index cbd2b465192..8b3ebf0dbb9 100644 --- a/python/servo/devtools_tests.py +++ b/python/servo/devtools_tests.py @@ -96,9 +96,9 @@ class DevtoolsTests(unittest.IsolatedAsyncioTestCase): self.assert_sources_list(1, set([tuple(["data:text/html,"])])) def test_source_content_inline_script(self): - script_content = "console.log('Hello, world!');" - self.run_servoshell(url=f"data:text/html,") - self.assert_source_content("data:text/html,", script_content) + script_tag = "" + self.run_servoshell(url=f"data:text/html,{script_tag}") + 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 = '
' + 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 = '
' + 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 = '' + 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 diff --git a/python/servo/devtools_tests/sources_content_with_responsexml/empty.html b/python/servo/devtools_tests/sources_content_with_responsexml/empty.html new file mode 100644 index 00000000000..f18028b5d16 --- /dev/null +++ b/python/servo/devtools_tests/sources_content_with_responsexml/empty.html @@ -0,0 +1 @@ + diff --git a/python/servo/devtools_tests/sources_content_with_responsexml/test.html b/python/servo/devtools_tests/sources_content_with_responsexml/test.html new file mode 100644 index 00000000000..35df255d83c --- /dev/null +++ b/python/servo/devtools_tests/sources_content_with_responsexml/test.html @@ -0,0 +1,11 @@ + +