From 76ee127af8db545fbc3412cecd7e641be01502f2 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Sun, 20 Apr 2025 23:32:21 -0400 Subject: [PATCH] Eagerly define interfaces on non-Window globals (#36604) These changes make us match Gecko's setup for how Window and non-Window globals are initialized. Since Window globals are much more common than Worker globals, using lazy interface definitions can be a useful memory optimization at the expense of increased complexity for property lookups. Also adds the MayResolve hook for all globals, which is an optimization for the JIT to avoid calling resolve hooks unnecessarily. Testing: Existing test coverage on global interfaces should suffice. --------- Signed-off-by: Josh Matthews --- components/script/dom/bindings/utils.rs | 13 +++ .../script/dom/dedicatedworkerglobalscope.rs | 8 +- .../script/dom/serviceworkerglobalscope.rs | 6 + components/script/dom/workletglobalscope.rs | 12 +- .../script_bindings/codegen/CodegenRust.py | 13 ++- components/script_bindings/import.rs | 9 +- components/script_bindings/utils.rs | 106 ++++++++++++++---- .../script_bindings/webidls/Window.webidl | 2 +- tests/wpt/mozilla/meta/MANIFEST.json | 2 +- .../tests/mozilla/interfaces.worker.js | 2 - 10 files changed, 138 insertions(+), 35 deletions(-) diff --git a/components/script/dom/bindings/utils.rs b/components/script/dom/bindings/utils.rs index f8edab1879d..ba7db745832 100644 --- a/components/script/dom/bindings/utils.rs +++ b/components/script/dom/bindings/utils.rs @@ -118,6 +118,19 @@ pub(crate) const DOM_CALLBACKS: DOMCallbacks = DOMCallbacks { instanceClassMatchesProto: Some(instance_class_has_proto_at_depth), }; +/// Eagerly define all relevant WebIDL interface constructors on the +/// provided global object. +pub(crate) fn define_all_exposed_interfaces( + global: &GlobalScope, + _in_realm: InRealm, + _can_gc: CanGc, +) { + let cx = GlobalScope::get_cx(); + for (_, interface) in &InterfaceObjectMap::MAP { + (interface.define)(cx, global.reflector().get_jsobject()); + } +} + impl DomHelpers for crate::DomTypeHolder { fn throw_dom_exception( cx: SafeJSContext, diff --git a/components/script/dom/dedicatedworkerglobalscope.rs b/components/script/dom/dedicatedworkerglobalscope.rs index b014a85701b..c2a4e4f4c1a 100644 --- a/components/script/dom/dedicatedworkerglobalscope.rs +++ b/components/script/dom/dedicatedworkerglobalscope.rs @@ -42,6 +42,7 @@ use crate::dom::bindings::root::{DomRoot, RootCollection, ThreadLocalStackRoots} use crate::dom::bindings::str::DOMString; use crate::dom::bindings::structuredclone; use crate::dom::bindings::trace::{CustomTraceable, RootedTraceableBox}; +use crate::dom::bindings::utils::define_all_exposed_interfaces; use crate::dom::errorevent::ErrorEvent; use crate::dom::event::{Event, EventBubbles, EventCancelable, EventStatus}; use crate::dom::eventtarget::EventTarget; @@ -493,7 +494,12 @@ impl DedicatedWorkerGlobalScope { { let _ar = AutoWorkerReset::new(&global, worker.clone()); - let _ac = enter_realm(scope); + let realm = enter_realm(scope); + define_all_exposed_interfaces( + global.upcast(), + InRealm::entered(&realm), + CanGc::note(), + ); scope.execute_script(DOMString::from(source), CanGc::note()); } diff --git a/components/script/dom/serviceworkerglobalscope.rs b/components/script/dom/serviceworkerglobalscope.rs index defed40f0ef..9b431da9c0e 100644 --- a/components/script/dom/serviceworkerglobalscope.rs +++ b/components/script/dom/serviceworkerglobalscope.rs @@ -38,6 +38,7 @@ use crate::dom::bindings::root::{DomRoot, RootCollection, ThreadLocalStackRoots} use crate::dom::bindings::str::DOMString; use crate::dom::bindings::structuredclone; use crate::dom::bindings::trace::CustomTraceable; +use crate::dom::bindings::utils::define_all_exposed_interfaces; use crate::dom::dedicatedworkerglobalscope::AutoWorkerReset; use crate::dom::event::Event; use crate::dom::eventtarget::EventTarget; @@ -379,6 +380,11 @@ impl ServiceWorkerGlobalScope { { // TODO: use AutoWorkerReset as in dedicated worker? let realm = enter_realm(scope); + define_all_exposed_interfaces( + scope.upcast(), + InRealm::entered(&realm), + CanGc::note(), + ); scope.execute_script(DOMString::from(source), CanGc::note()); global.dispatch_activate(CanGc::note(), InRealm::entered(&realm)); } diff --git a/components/script/dom/workletglobalscope.rs b/components/script/dom/workletglobalscope.rs index 0196d6a83ea..6e81220731c 100644 --- a/components/script/dom/workletglobalscope.rs +++ b/components/script/dom/workletglobalscope.rs @@ -15,6 +15,7 @@ use js::rust::Runtime; use net_traits::ResourceThreads; use net_traits::image_cache::ImageCache; use profile_traits::{mem, time}; +use script_bindings::realms::InRealm; use script_traits::Painter; use servo_url::{ImmutableOrigin, MutableOrigin, ServoUrl}; use stylo_atoms::Atom; @@ -22,6 +23,7 @@ use stylo_atoms::Atom; use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::root::DomRoot; use crate::dom::bindings::trace::CustomTraceable; +use crate::dom::bindings::utils::define_all_exposed_interfaces; use crate::dom::globalscope::GlobalScope; use crate::dom::paintworkletglobalscope::{PaintWorkletGlobalScope, PaintWorkletTask}; use crate::dom::testworkletglobalscope::{TestWorkletGlobalScope, TestWorkletTask}; @@ -29,6 +31,7 @@ use crate::dom::testworkletglobalscope::{TestWorkletGlobalScope, TestWorkletTask use crate::dom::webgpu::identityhub::IdentityHub; use crate::dom::worklet::WorkletExecutor; use crate::messaging::MainThreadScriptMsg; +use crate::realms::enter_realm; use crate::script_module::ScriptFetchOptions; use crate::script_runtime::{CanGc, JSContext}; @@ -56,7 +59,7 @@ impl WorkletGlobalScope { executor: WorkletExecutor, init: &WorkletGlobalScopeInit, ) -> DomRoot { - match scope_type { + let scope: DomRoot = match scope_type { WorkletGlobalScopeType::Test => DomRoot::upcast(TestWorkletGlobalScope::new( runtime, pipeline_id, @@ -71,7 +74,12 @@ impl WorkletGlobalScope { executor, init, )), - } + }; + + let realm = enter_realm(&*scope); + define_all_exposed_interfaces(scope.upcast(), InRealm::entered(&realm), CanGc::note()); + + scope } /// Create a new stack-allocated `WorkletGlobalScope`. diff --git a/components/script_bindings/codegen/CodegenRust.py b/components/script_bindings/codegen/CodegenRust.py index 107e047ea3b..48f024be70f 100644 --- a/components/script_bindings/codegen/CodegenRust.py +++ b/components/script_bindings/codegen/CodegenRust.py @@ -2405,15 +2405,22 @@ class CGDOMJSClass(CGThing): "flags": "JSCLASS_FOREGROUND_FINALIZE", "name": str_to_cstr_ptr(self.descriptor.interface.identifier.name), "resolveHook": "None", + "mayResolveHook": "None", "slots": "1", "traceHook": f"{TRACE_HOOK_NAME}::", } if self.descriptor.isGlobal(): assert not self.descriptor.weakReferenceable - args["enumerateHook"] = "Some(enumerate_global::)" args["flags"] = "JSCLASS_IS_GLOBAL | JSCLASS_DOM_GLOBAL | JSCLASS_FOREGROUND_FINALIZE" args["slots"] = "JSCLASS_GLOBAL_SLOT_COUNT + 1" - args["resolveHook"] = "Some(resolve_global::)" + if self.descriptor.interface.getExtendedAttribute("NeedResolve"): + args["enumerateHook"] = "Some(enumerate_window::)" + args["resolveHook"] = "Some(resolve_window::)" + args["mayResolveHook"] = "Some(may_resolve_window::)" + else: + args["enumerateHook"] = "Some(enumerate_global)" + args["resolveHook"] = "Some(resolve_global)" + args["mayResolveHook"] = "Some(may_resolve_global)" args["traceHook"] = "js::jsapi::JS_GlobalObjectTraceHook" elif self.descriptor.weakReferenceable: args["slots"] = "2" @@ -2427,7 +2434,7 @@ pub(crate) fn init_class_ops() {{ enumerate: None, newEnumerate: {args['enumerateHook']}, resolve: {args['resolveHook']}, - mayResolve: None, + mayResolve: {args['mayResolveHook']}, finalize: Some({args['finalizeHook']}), call: None, construct: None, diff --git a/components/script_bindings/import.rs b/components/script_bindings/import.rs index 65e9ee30e1d..16cc92f07bf 100644 --- a/components/script_bindings/import.rs +++ b/components/script_bindings/import.rs @@ -127,10 +127,11 @@ pub(crate) mod module { pub(crate) use crate::script_runtime::CanGc; pub(crate) use crate::utils::{ AsVoidPtr, DOM_PROTO_UNFORGEABLE_HOLDER_SLOT, DOMClass, DOMJSClass, JSCLASS_DOM_GLOBAL, - ProtoOrIfaceArray, enumerate_global, exception_to_promise, generic_getter, - generic_lenient_getter, generic_lenient_setter, generic_method, generic_setter, - generic_static_promise_method, get_array_index_from_id, get_property_on_prototype, - has_property_on_prototype, resolve_global, trace_global, + ProtoOrIfaceArray, enumerate_global, enumerate_window, exception_to_promise, + generic_getter, generic_lenient_getter, generic_lenient_setter, generic_method, + generic_setter, generic_static_promise_method, get_array_index_from_id, + get_property_on_prototype, has_property_on_prototype, may_resolve_global, + may_resolve_window, resolve_global, resolve_window, trace_global, }; pub(crate) use crate::weakref::DOM_WEAK_SLOT; pub(crate) use crate::{JSTraceable, proxyhandler}; diff --git a/components/script_bindings/utils.rs b/components/script_bindings/utils.rs index 03a5783958e..fa4982b9904 100644 --- a/components/script_bindings/utils.rs +++ b/components/script_bindings/utils.rs @@ -18,9 +18,10 @@ use js::jsapi::{ GetLinearStringLength, GetNonCCWObjectGlobal, HandleId as RawHandleId, HandleObject as RawHandleObject, Heap, JS_AtomizeStringN, JS_ClearPendingException, JS_DeprecatedStringHasLatin1Chars, JS_GetLatin1StringCharsAndLength, JS_IsExceptionPending, - JS_IsGlobalObject, JS_NewEnumerateStandardClasses, JS_ResolveStandardClass, JSAtom, JSContext, - JSJitInfo, JSObject, JSTracer, MutableHandleIdVector as RawMutableHandleIdVector, - MutableHandleValue as RawMutableHandleValue, ObjectOpResult, StringIsArrayIndex, + JS_IsGlobalObject, JS_MayResolveStandardClass, JS_NewEnumerateStandardClasses, + JS_ResolveStandardClass, JSAtom, JSAtomState, JSContext, JSJitInfo, JSObject, JSTracer, + MutableHandleIdVector as RawMutableHandleIdVector, MutableHandleValue as RawMutableHandleValue, + ObjectOpResult, PropertyKey, StringIsArrayIndex, jsid, }; use js::jsid::StringId; use js::jsval::{JSVal, UndefinedValue}; @@ -30,7 +31,7 @@ use js::rust::wrappers::{ JS_SetPendingException, JS_SetProperty, }; use js::rust::{ - HandleId, HandleObject, HandleValue, MutableHandleValue, ToString, get_object_class, + HandleId, HandleObject, HandleValue, MutableHandleValue, Runtime, ToString, get_object_class, }; use js::{JS_CALLEE, rooted}; use malloc_size_of::MallocSizeOfOps; @@ -577,15 +578,25 @@ impl AsCCharPtrPtr for [u8] { /// Enumerate lazy properties of a global object. /// Modeled after -/// and -pub(crate) unsafe extern "C" fn enumerate_global( +pub(crate) unsafe extern "C" fn enumerate_global( cx: *mut JSContext, obj: RawHandleObject, props: RawMutableHandleIdVector, enumerable_only: bool, ) -> bool { assert!(JS_IsGlobalObject(obj.get())); - if !JS_NewEnumerateStandardClasses(cx, obj, props, enumerable_only) { + JS_NewEnumerateStandardClasses(cx, obj, props, enumerable_only) +} + +/// Enumerate lazy properties of a global object that is a Window. +/// +pub(crate) unsafe extern "C" fn enumerate_window( + cx: *mut JSContext, + obj: RawHandleObject, + props: RawMutableHandleIdVector, + enumerable_only: bool, +) -> bool { + if !enumerate_global(cx, obj, props, enumerable_only) { return false; } @@ -610,34 +621,68 @@ pub(crate) unsafe extern "C" fn enumerate_global( true } +/// Returns true if the resolve hook for this global may resolve the provided id. +/// +/// +pub(crate) unsafe extern "C" fn may_resolve_global( + names: *const JSAtomState, + id: PropertyKey, + maybe_obj: *mut JSObject, +) -> bool { + JS_MayResolveStandardClass(names, id, maybe_obj) +} + +/// Returns true if the resolve hook for this window may resolve the provided id. +/// +/// +pub(crate) unsafe extern "C" fn may_resolve_window( + names: *const JSAtomState, + id: PropertyKey, + maybe_obj: *mut JSObject, +) -> bool { + if may_resolve_global(names, id, maybe_obj) { + return true; + } + + let cx = Runtime::get() + .expect("There must be a JSContext active") + .as_ptr(); + let Ok(bytes) = latin1_bytes_from_id(cx, id) else { + return false; + }; + + >::interface_map().contains_key(bytes) +} + /// Resolve a lazy global property, for interface objects and named constructors. -pub(crate) unsafe extern "C" fn resolve_global( +pub(crate) unsafe extern "C" fn resolve_global( cx: *mut JSContext, obj: RawHandleObject, id: RawHandleId, rval: *mut bool, ) -> bool { assert!(JS_IsGlobalObject(obj.get())); - if !JS_ResolveStandardClass(cx, obj, id, rval) { + JS_ResolveStandardClass(cx, obj, id, rval) +} + +/// Resolve a lazy global property for a Window global. +pub(crate) unsafe extern "C" fn resolve_window( + cx: *mut JSContext, + obj: RawHandleObject, + id: RawHandleId, + rval: *mut bool, +) -> bool { + if !resolve_global(cx, obj, id, rval) { return false; } + if *rval { return true; } - if !id.is_string() { + let Ok(bytes) = latin1_bytes_from_id(cx, *id) else { *rval = false; return true; - } - - let string = id.to_string(); - if !JS_DeprecatedStringHasLatin1Chars(string) { - *rval = false; - return true; - } - let mut length = 0; - let ptr = JS_GetLatin1StringCharsAndLength(cx, ptr::null(), string, &mut length); - assert!(!ptr.is_null()); - let bytes = slice::from_raw_parts(ptr, length); + }; if let Some(interface) = >::interface_map().get(bytes) { (interface.define)(SafeJSContext::from_ptr(cx), Handle::from_raw(obj)); @@ -647,3 +692,22 @@ pub(crate) unsafe extern "C" fn resolve_global( } true } + +/// Returns a slice of bytes corresponding to the bytes in the provided string id. +/// Returns an error if the id is not a string, or the string contains non-latin1 characters. +/// # Safety +/// The slice is only valid as long as the original id is not garbage collected. +unsafe fn latin1_bytes_from_id(cx: *mut JSContext, id: jsid) -> Result<&'static [u8], ()> { + if !id.is_string() { + return Err(()); + } + + let string = id.to_string(); + if !JS_DeprecatedStringHasLatin1Chars(string) { + return Err(()); + } + let mut length = 0; + let ptr = JS_GetLatin1StringCharsAndLength(cx, ptr::null(), string, &mut length); + assert!(!ptr.is_null()); + Ok(slice::from_raw_parts(ptr, length)) +} diff --git a/components/script_bindings/webidls/Window.webidl b/components/script_bindings/webidls/Window.webidl index c7b6dde617d..d42ba22ea66 100644 --- a/components/script_bindings/webidls/Window.webidl +++ b/components/script_bindings/webidls/Window.webidl @@ -3,7 +3,7 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ // https://html.spec.whatwg.org/multipage/#window -[Global=Window, Exposed=Window /*, LegacyUnenumerableNamedProperties */] +[Global=Window, Exposed=Window, LegacyUnenumerableNamedProperties, NeedResolve] /*sealed*/ interface Window : GlobalScope { // the current browsing context [LegacyUnforgeable, CrossOriginReadable] readonly attribute WindowProxy window; diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index 2ef84bb18a6..c18a78786aa 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -13531,7 +13531,7 @@ ] ], "interfaces.worker.js": [ - "8af942c0f99218fb39770cdcf299eaa230339010", + "8d109502622fac7266a4564de09684a3ab94118c", [ "mozilla/interfaces.worker.html", {} diff --git a/tests/wpt/mozilla/tests/mozilla/interfaces.worker.js b/tests/wpt/mozilla/tests/mozilla/interfaces.worker.js index 8af942c0f99..8d109502622 100644 --- a/tests/wpt/mozilla/tests/mozilla/interfaces.worker.js +++ b/tests/wpt/mozilla/tests/mozilla/interfaces.worker.js @@ -107,7 +107,6 @@ test_interfaces([ "Response", "SecurityPolicyViolationEvent", "ServiceWorkerContainer", - "SVGRect", "TextDecoder", "TextEncoder", "TrustedHTML", @@ -129,7 +128,6 @@ test_interfaces([ "WebGLShaderPrecisionFormat", "WebGLTexture", "WebGLUniformLocation", - "WebKitCSSMatrix", "WebSocket", "WeakRef", "Worker",