From 4784668fa96466524cf8e6df76252f08373281eb Mon Sep 17 00:00:00 2001 From: shuppy Date: Mon, 11 Aug 2025 14:04:51 +0800 Subject: [PATCH] devtools: Create source actors from Debugger API notifications (#38334) currently our devtools impl creates source actors in script, when executing scripts in HTMLScriptElement or DedicatedWorkerGlobalScope. this approach is cumbersome, and it means that many pathways to running scripts are missed, such as imported ES modules. with the [SpiderMonkey Debugger API](https://firefox-source-docs.mozilla.org/js/Debugger/), we can pick up all of the scripts and all of their sources without any extra code, as long as we tell it about every global we create (#38333, #38551). this patch adds a [Debugger#onNewScript() hook](https://firefox-source-docs.mozilla.org/js/Debugger/Debugger.html#onnewscript-script-global) to the debugger script, which calls DebuggerGlobalScope#notifyNewSource() to notify our script system when a new script runs. if the source is relevant to the file tree in the Sources tab, script tells devtools to create a source actor. Testing: adds several new automated devtools tests Fixes: part of #36027 Signed-off-by: Delan Azabani Co-authored-by: atbrakhi --- components/devtools/actors/source.rs | 11 + components/devtools/lib.rs | 1 + components/script/dom/debuggerglobalscope.rs | 109 +++++- .../script/dom/dedicatedworkerglobalscope.rs | 24 +- components/script/dom/htmlscriptelement.rs | 54 --- components/script/script_runtime.rs | 24 +- .../webidls/DebuggerGlobalScope.webidl | 16 + components/shared/devtools/lib.rs | 3 +- python/servo/devtools_tests.py | 367 +++++++++++++++++- resources/debugger.js | 31 +- 10 files changed, 522 insertions(+), 118 deletions(-) diff --git a/components/devtools/actors/source.rs b/components/devtools/actors/source.rs index 7f26f5e979d..fb31af49156 100644 --- a/components/devtools/actors/source.rs +++ b/components/devtools/actors/source.rs @@ -56,6 +56,9 @@ pub struct SourceActor { pub content: Option, pub content_type: Option, + // TODO: use it in #37667, then remove this allow + #[allow(unused)] + pub spidermonkey_id: u32, /// `introductionType` in SpiderMonkey `CompileOptionsWrapper`. pub introduction_type: String, } @@ -96,6 +99,7 @@ impl SourceActor { url: ServoUrl, content: Option, content_type: Option, + spidermonkey_id: u32, introduction_type: String, ) -> SourceActor { SourceActor { @@ -104,6 +108,7 @@ impl SourceActor { content, content_type, is_black_boxed: false, + spidermonkey_id, introduction_type, } } @@ -114,6 +119,7 @@ impl SourceActor { url: ServoUrl, content: Option, content_type: Option, + spidermonkey_id: u32, introduction_type: String, ) -> &SourceActor { let source_actor_name = actors.new_name("source"); @@ -123,6 +129,7 @@ impl SourceActor { url, content, content_type, + spidermonkey_id, introduction_type, ); actors.register(Box::new(source_actor)); @@ -160,6 +167,10 @@ impl Actor for SourceActor { let reply = SourceContentReply { from: self.name(), content_type: self.content_type.clone(), + // 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: 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 diff --git a/components/devtools/lib.rs b/components/devtools/lib.rs index 93bb7b18d22..695c98dba4a 100644 --- a/components/devtools/lib.rs +++ b/components/devtools/lib.rs @@ -552,6 +552,7 @@ impl DevtoolsInstance { source_info.url, source_content, source_info.content_type, + source_info.spidermonkey_id, source_info.introduction_type, ); let source_actor_name = source_actor.name.clone(); diff --git a/components/script/dom/debuggerglobalscope.rs b/components/script/dom/debuggerglobalscope.rs index 118539bfbdb..2231fb433f1 100644 --- a/components/script/dom/debuggerglobalscope.rs +++ b/components/script/dom/debuggerglobalscope.rs @@ -2,9 +2,9 @@ * 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 base::id::PipelineId; +use base::id::{Index, PipelineId, PipelineNamespaceId}; use constellation_traits::ScriptToConstellationChan; -use devtools_traits::{ScriptToDevtoolsControlMsg, WorkerId}; +use devtools_traits::{ScriptToDevtoolsControlMsg, SourceInfo, WorkerId}; use dom_struct::dom_struct; use embedder_traits::JavaScriptEvaluationError; use embedder_traits::resources::{self, Resource}; @@ -14,6 +14,9 @@ use js::rust::Runtime; use js::rust::wrappers::JS_DefineDebuggerObject; use net_traits::ResourceThreads; use profile_traits::{mem, time}; +use script_bindings::codegen::GenericBindings::DebuggerGlobalScopeBinding::{ + DebuggerGlobalScopeMethods, NotifyNewSource, +}; use script_bindings::realms::InRealm; use script_bindings::reflector::DomObject; use servo_url::{ImmutableOrigin, MutableOrigin, ServoUrl}; @@ -30,7 +33,7 @@ use crate::dom::types::{DebuggerAddDebuggeeEvent, Event}; use crate::dom::webgpu::identityhub::IdentityHub; use crate::realms::enter_realm; use crate::script_module::ScriptFetchOptions; -use crate::script_runtime::{CanGc, JSContext}; +use crate::script_runtime::{CanGc, IntroductionType, JSContext}; #[dom_struct] /// Global scope for interacting with the devtools Debugger API. @@ -105,6 +108,10 @@ impl DebuggerGlobalScope { GlobalScope::get_cx() } + pub(crate) fn as_global_scope(&self) -> &GlobalScope { + self.upcast::() + } + fn evaluate_js(&self, script: &str, can_gc: CanGc) -> Result<(), JavaScriptEvaluationError> { rooted!(in (*Self::get_cx()) let mut rval = UndefinedValue()); self.global_scope.evaluate_js_on_global_with_result( @@ -149,3 +156,99 @@ impl DebuggerGlobalScope { ); } } + +impl DebuggerGlobalScopeMethods for DebuggerGlobalScope { + // check-tidy: no specs after this line + fn NotifyNewSource(&self, args: &NotifyNewSource) { + let Some(devtools_chan) = self.as_global_scope().devtools_chan() else { + return; + }; + let pipeline_id = PipelineId { + namespace_id: PipelineNamespaceId(args.pipelineId.namespaceId), + index: Index::new(args.pipelineId.index).expect("`pipelineId.index` must not be zero"), + }; + + if let Some(introduction_type) = args.introductionType.as_ref() { + // Check the `introductionType` and `url`, decide whether or not to create a source actor, and if so, + // tell the devtools server to create a source actor. Based on the Firefox impl in: + // - getDebuggerSourceURL() + // - getSourceURL() + // - resolveSourceURL() + // - SourceActor#_isInlineSource + // - SourceActor#url + + // Firefox impl: getDebuggerSourceURL(), getSourceURL() + // TODO: handle `about:srcdoc` case (see Firefox getDebuggerSourceURL()) + // TODO: remove trailing details that may have been appended by SpiderMonkey + // (currently impossible to do robustly due to ) + let url_original = args.url.str(); + // FIXME: use page/worker url as base here + let url_original = ServoUrl::parse(url_original).ok(); + + // If the source has a `urlOverride` (aka `displayURL` aka `//# sourceURL`), it should be a valid url, + // possibly relative to the page/worker url, and we should treat the source as coming from that url for + // devtools purposes, including the file tree in the Sources tab. + // Firefox impl: getSourceURL() + let url_override = args + .urlOverride + .as_ref() + .map(|url| url.str()) + // FIXME: use page/worker url as base here, not `url_original` + .and_then(|url| ServoUrl::parse_with_base(url_original.as_ref(), url).ok()); + + // If the `introductionType` is “eval or eval-like”, the `url` won’t be meaningful, so ignore these + // sources unless we have a `urlOverride` (aka `displayURL` aka `//# sourceURL`). + // Firefox impl: getDebuggerSourceURL(), getSourceURL() + if [ + IntroductionType::INJECTED_SCRIPT_STR, + IntroductionType::EVAL_STR, + IntroductionType::DEBUGGER_EVAL_STR, + IntroductionType::FUNCTION_STR, + IntroductionType::JAVASCRIPT_URL_STR, + IntroductionType::EVENT_HANDLER_STR, + IntroductionType::DOM_TIMER_STR, + ] + .contains(&introduction_type.str()) && + url_override.is_none() + { + debug!( + "Not creating debuggee: `introductionType` is `{introduction_type}` but no valid url" + ); + return; + } + + // Sources with an `introductionType` of `inlineScript` are generally inline, meaning their contents + // are a substring of the page markup (hence not known to SpiderMonkey, requiring plumbing in Servo). + // But sources with a `urlOverride` are not inline, since they get their own place in the Sources tree. + // nor are sources created for `