From d0908b03423652ea042350026aa6ec3b863a5a98 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Sat, 25 Apr 2026 03:45:14 -0400 Subject: [PATCH] script: Wrap debugger global scope pointers in debuggee's realm. (#44386) Debug mozjs builds don't like when we try to trace members of one global object (realm) that originate from another global object (realm). This triggers when we use `Dom` in our global objects, since all `Dom` pointers are expected to in the same realm as the containing object. To address this, we store a JS value of the debugger global's reflector wrapped in the current global's relam, and use value to reconstitute a `DebuggerGlobalScope` pointer as needed. Testing: We don't run debug-mozjs in CI, but this change makes a bunch of tests stop asserting. Fixes: #44385 --------- Signed-off-by: Josh Matthews --- .../dom/workers/dedicatedworkerglobalscope.rs | 30 ++---- .../dom/workers/serviceworkerglobalscope.rs | 92 +++++++------------ .../script/dom/workers/workerglobalscope.rs | 59 +++++++++++- components/script_bindings/root.rs | 19 +++- 4 files changed, 119 insertions(+), 81 deletions(-) diff --git a/components/script/dom/workers/dedicatedworkerglobalscope.rs b/components/script/dom/workers/dedicatedworkerglobalscope.rs index abc0c2cff4a..2a073534355 100644 --- a/components/script/dom/workers/dedicatedworkerglobalscope.rs +++ b/components/script/dom/workers/dedicatedworkerglobalscope.rs @@ -39,7 +39,7 @@ use crate::dom::bindings::error::{ErrorInfo, ErrorResult}; use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::refcounted::Trusted; use crate::dom::bindings::reflector::DomGlobal; -use crate::dom::bindings::root::{Dom, DomRoot}; +use crate::dom::bindings::root::DomRoot; use crate::dom::bindings::str::DOMString; use crate::dom::bindings::structuredclone; use crate::dom::bindings::trace::{CustomTraceable, RootedTraceableBox}; @@ -215,7 +215,6 @@ pub(crate) struct DedicatedWorkerGlobalScope { control_receiver: Receiver, #[no_trace] queued_worker_tasks: DomRefCell>, - debugger_global: Dom, } impl WorkerEventLoopMethods for DedicatedWorkerGlobalScope { @@ -280,7 +279,6 @@ impl DedicatedWorkerGlobalScope { control_receiver: Receiver, insecure_requests_policy: InsecureRequestsPolicy, font_context: Option>, - debugger_global: &DebuggerGlobalScope, ) -> DedicatedWorkerGlobalScope { DedicatedWorkerGlobalScope { workerglobalscope: WorkerGlobalScope::new_inherited( @@ -305,7 +303,6 @@ impl DedicatedWorkerGlobalScope { browsing_context, control_receiver, queued_worker_tasks: Default::default(), - debugger_global: Dom::from_ref(debugger_global), } } @@ -350,9 +347,13 @@ impl DedicatedWorkerGlobalScope { control_receiver, insecure_requests_policy, font_context, - debugger_global, )); - DedicatedWorkerGlobalScopeBinding::Wrap::(cx, scope) + let scope = DedicatedWorkerGlobalScopeBinding::Wrap::(cx, scope); + scope + .upcast::() + .init_debugger_global(debugger_global, cx); + + scope } /// @@ -671,20 +672,9 @@ impl DedicatedWorkerGlobalScope { } // FIXME(#26324): `self.worker` is None in devtools messages. match msg { - MixedMessage::Devtools(msg) => match msg { - DevtoolScriptControlMsg::WantsLiveNotifications(_pipe_id, _bool_val) => {}, - DevtoolScriptControlMsg::Eval(code, id, frame_actor_id, reply) => { - self.debugger_global.fire_eval( - cx, - code.into(), - id, - Some(self.upcast::().worker_id()), - frame_actor_id, - reply, - ); - }, - _ => debug!("got an unusable devtools control message inside the worker!"), - }, + MixedMessage::Devtools(msg) => self + .upcast::() + .handle_devtools_message(msg, cx), MixedMessage::Worker(DedicatedWorkerScriptMsg::CommonWorker(linked_worker, msg)) => { let _ar = AutoWorkerReset::new(self, linked_worker); self.handle_script_event(msg, cx); diff --git a/components/script/dom/workers/serviceworkerglobalscope.rs b/components/script/dom/workers/serviceworkerglobalscope.rs index f199de628f9..1bbc717c636 100644 --- a/components/script/dom/workers/serviceworkerglobalscope.rs +++ b/components/script/dom/workers/serviceworkerglobalscope.rs @@ -8,7 +8,7 @@ use std::thread::{self, JoinHandle}; use std::time::{Duration, Instant}; use crossbeam_channel::{Receiver, Sender, after}; -use devtools_traits::{DebuggerValue, DevtoolScriptControlMsg, EvaluateJSReply}; +use devtools_traits::DevtoolScriptControlMsg; use dom_struct::dom_struct; use fonts::FontContext; use js::jsapi::{JS_AddInterruptCallback, JSContext}; @@ -34,7 +34,7 @@ use crate::dom::bindings::codegen::Bindings::ServiceWorkerGlobalScopeBinding; use crate::dom::bindings::codegen::Bindings::ServiceWorkerGlobalScopeBinding::ServiceWorkerGlobalScopeMethods; use crate::dom::bindings::codegen::Bindings::WorkerBinding::WorkerType; use crate::dom::bindings::inheritance::Castable; -use crate::dom::bindings::root::{Dom, DomRoot}; +use crate::dom::bindings::root::DomRoot; use crate::dom::bindings::str::DOMString; use crate::dom::bindings::structuredclone; use crate::dom::bindings::trace::CustomTraceable; @@ -182,8 +182,6 @@ pub(crate) struct ServiceWorkerGlobalScope { /// currently only used to signal shutdown. #[no_trace] control_receiver: Receiver, - - debugger_global: Option>, } impl WorkerEventLoopMethods for ServiceWorkerGlobalScope { @@ -242,7 +240,6 @@ impl ServiceWorkerGlobalScope { control_receiver: Receiver, closing: Arc, font_context: Arc, - debugger_global: Option<&DebuggerGlobalScope>, ) -> ServiceWorkerGlobalScope { ServiceWorkerGlobalScope { workerglobalscope: WorkerGlobalScope::new_inherited( @@ -265,7 +262,6 @@ impl ServiceWorkerGlobalScope { swmanager_sender, scope_url, control_receiver, - debugger_global: debugger_global.map(Dom::from_ref), } } @@ -283,7 +279,7 @@ impl ServiceWorkerGlobalScope { control_receiver: Receiver, closing: Arc, font_context: Arc, - debugger_global: Option<&DebuggerGlobalScope>, + debugger_global: &DebuggerGlobalScope, cx: &mut js::context::JSContext, ) -> DomRoot { let scope = Box::new(ServiceWorkerGlobalScope::new_inherited( @@ -299,9 +295,13 @@ impl ServiceWorkerGlobalScope { control_receiver, closing, font_context, - debugger_global, )); - ServiceWorkerGlobalScopeBinding::Wrap::(cx, scope) + let scope = ServiceWorkerGlobalScopeBinding::Wrap::(cx, scope); + scope + .upcast::() + .init_debugger_global(debugger_global, cx); + + scope } /// @@ -347,27 +347,23 @@ impl ServiceWorkerGlobalScope { pipeline_id, } = worker_load_origin; - let debugger_global = + let debugger_global = DebuggerGlobalScope::new( + pipeline_id, + init.to_devtools_sender.clone(), init.from_devtools_sender .clone() - .map(|from_devtools_sender| { - let debugger_global = DebuggerGlobalScope::new( - pipeline_id, - init.to_devtools_sender.clone(), - from_devtools_sender, - init.mem_profiler_chan.clone(), - init.time_profiler_chan.clone(), - init.script_to_constellation_chan.clone(), - init.script_to_embedder_chan.clone(), - init.resource_threads.clone(), - init.storage_threads.clone(), - #[cfg(feature = "webgpu")] - Arc::new(IdentityHub::default()), - cx, - ); - debugger_global.execute(cx); - debugger_global - }); + .expect("Guaranteed by update_serviceworker"), + init.mem_profiler_chan.clone(), + init.time_profiler_chan.clone(), + init.script_to_constellation_chan.clone(), + init.script_to_embedder_chan.clone(), + init.resource_threads.clone(), + init.storage_threads.clone(), + #[cfg(feature = "webgpu")] + Arc::new(IdentityHub::default()), + cx, + ); + debugger_global.execute(cx); // Service workers are time limited // https://w3c.github.io/ServiceWorker/#service-worker-lifetime @@ -390,21 +386,19 @@ impl ServiceWorkerGlobalScope { control_receiver, closing, font_context, - debugger_global.as_deref(), + &debugger_global, cx, ); let worker_scope = global.upcast::(); let global_scope = global.upcast::(); - if let Some(debugger_global) = debugger_global.as_deref() { - debugger_global.fire_add_debuggee( - cx, - global_scope, - pipeline_id, - Some(worker_scope.worker_id()), - ); - } + debugger_global.fire_add_debuggee( + cx, + global_scope, + pipeline_id, + Some(worker_scope.worker_id()), + ); let referrer = referrer_url .map(Referrer::ReferrerUrl) @@ -495,27 +489,9 @@ impl ServiceWorkerGlobalScope { fn handle_mixed_message(&self, msg: MixedMessage, cx: &mut js::context::JSContext) -> bool { match msg { - MixedMessage::Devtools(msg) => match msg { - DevtoolScriptControlMsg::WantsLiveNotifications(_pipe_id, _wants_updates) => {}, - DevtoolScriptControlMsg::Eval(code, id, frame_actor_id, reply) => { - if let Some(debugger_global) = self.debugger_global.as_deref() { - debugger_global.fire_eval( - cx, - code.into(), - id, - Some(self.upcast::().worker_id()), - frame_actor_id, - reply, - ); - } else { - let _ = reply.send(EvaluateJSReply { - value: DebuggerValue::VoidValue, - has_exception: true, - }); - } - }, - _ => debug!("got an unusable devtools control message inside the worker!"), - }, + MixedMessage::Devtools(msg) => self + .upcast::() + .handle_devtools_message(msg, cx), MixedMessage::ServiceWorker(msg) => { self.handle_script_event(msg, cx); }, diff --git a/components/script/dom/workers/workerglobalscope.rs b/components/script/dom/workers/workerglobalscope.rs index 20bfaf1e134..9825dc78c69 100644 --- a/components/script/dom/workers/workerglobalscope.rs +++ b/components/script/dom/workers/workerglobalscope.rs @@ -15,7 +15,7 @@ use dom_struct::dom_struct; use encoding_rs::UTF_8; use fonts::FontContext; use headers::{HeaderMapExt, ReferrerPolicy as ReferrerPolicyHeader}; -use js::jsapi::JSContext as RawJSContext; +use js::jsapi::{Heap, JSContext as RawJSContext, Value}; use js::realm::CurrentRealm; use js::rust::{HandleValue, MutableHandleValue, ParentRuntime}; use mime::Mime; @@ -26,6 +26,9 @@ use net_traits::request::{ }; use net_traits::{FetchMetadata, Metadata, NetworkError, ReferrerPolicy, ResourceFetchTiming}; use profile_traits::mem::{ProcessReports, perform_memory_report}; +use script_bindings::conversions::{SafeToJSValConvertible, root_from_handlevalue}; +use script_bindings::reflector::DomObject; +use script_bindings::root::rooted_heap_handle; use servo_base::cross_process_instant::CrossProcessInstant; use servo_base::generic_channel::{GenericSend, GenericSender, RoutedReceiver}; use servo_base::id::{PipelineId, PipelineNamespace}; @@ -59,6 +62,7 @@ use crate::dom::bindings::trace::RootedTraceableBox; use crate::dom::bindings::utils::define_all_exposed_interfaces; use crate::dom::crypto::Crypto; use crate::dom::csp::{GlobalCspReporting, Violation, parse_csp_list_from_metadata}; +use crate::dom::debugger::debuggerglobalscope::DebuggerGlobalScope; use crate::dom::dedicatedworkerglobalscope::DedicatedWorkerGlobalScope; use crate::dom::global_scope_script_execution::{ErrorReporting, RethrowErrors}; use crate::dom::globalscope::GlobalScope; @@ -319,6 +323,13 @@ pub(crate) struct WorkerGlobalScope { /// #[no_trace] endpoints_list: DomRefCell>, + + /// The debugger global object associated with this worker global. + /// All traced members of DOM objects must be same-compartment with the + /// realm being traced, so this is the debugger global object wrapped into + /// this global's compartment. + #[ignore_malloc_size_of = "Measured by the JS engine"] + debugger_global: Heap, } impl WorkerGlobalScope { @@ -384,6 +395,7 @@ impl WorkerGlobalScope { reporting_observer_list: Default::default(), report_list: Default::default(), endpoints_list: Default::default(), + debugger_global: Default::default(), } } @@ -1054,6 +1066,51 @@ impl WorkerGlobalScope { factory.abort_pending_upgrades(); } } + + pub(super) fn init_debugger_global( + &self, + debugger_global: &DebuggerGlobalScope, + cx: &mut js::context::JSContext, + ) { + let mut realm = enter_auto_realm(cx, self); + let cx = &mut realm.current_realm(); + + // Convert the debugger global’s reflector to a Value, wrapping it from its originating realm (debugger realm) + // into the active realm (debuggee realm) so that it can be passed across compartments. + rooted!(&in(cx) let mut wrapped_global: Value); + debugger_global.reflector().safe_to_jsval( + cx.into(), + wrapped_global.handle_mut(), + CanGc::from_cx(cx), + ); + self.debugger_global.set(*wrapped_global); + } + + pub(super) fn handle_devtools_message( + &self, + msg: DevtoolScriptControlMsg, + cx: &mut js::context::JSContext, + ) { + match msg { + DevtoolScriptControlMsg::WantsLiveNotifications(_pipe_id, _wants_updates) => {}, + DevtoolScriptControlMsg::Eval(code, id, frame_actor_id, reply) => { + let debugger_global_handle = rooted_heap_handle(self, |this| &this.debugger_global); + let debugger_global = + root_from_handlevalue::(debugger_global_handle, cx.into()) + .expect("must be a debugger global scope"); + + debugger_global.fire_eval( + cx, + code.into(), + id, + Some(self.worker_id()), + frame_actor_id, + reply, + ); + }, + _ => debug!("got an unusable devtools control message inside the worker!"), + } + } } #[expect(unsafe_code)] diff --git a/components/script_bindings/root.rs b/components/script_bindings/root.rs index 0c8fad038b5..85669afa3e1 100644 --- a/components/script_bindings/root.rs +++ b/components/script_bindings/root.rs @@ -7,8 +7,9 @@ use std::hash::{Hash, Hasher}; use std::ops::Deref; use std::{fmt, mem, ptr}; -use js::gc::Traceable as JSTraceable; -use js::jsapi::{JSObject, JSTracer}; +use js::gc::{Handle, Traceable as JSTraceable}; +use js::jsapi::{Heap, JSObject, JSTracer}; +use js::rust::GCMethods; use malloc_size_of::{MallocSizeOf, MallocSizeOfOps}; use style::thread_state; @@ -469,3 +470,17 @@ where unsafe { &*(self as *const [Dom] as *const [&T]) } } } + +/// Returns a handle to a Heap member of a reflected DOM object. +/// The provided callback acts as a projection of the rooted-ness of +/// the provided DOM object; it must return a reference to a Heap +/// member of the DOM object. +pub fn rooted_heap_handle<'a, T: DomObject, U: GCMethods + Copy>( + object: &'a T, + f: impl Fn(&'a T) -> &'a Heap, +) -> Handle<'a, U> { + // SAFETY: Heap::handle is safe to call when the Heap is a member + // of a rooted object. Our safety invariants for DOM objects + // ensure that a &T is obtained via a root of T. + unsafe { Handle::from_raw(f(object).handle()) } +}