From 3a04f4195eb650f092c44d5a05fee178b9e84fbe Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Sat, 24 May 2025 23:21:05 -0400 Subject: [PATCH] script: Return global objects for DOM objects in the relevant realm (#37120) DomObject::global is a tricky API because it's used pervasively but has subtle requirements that are not documented and not yet enforced by the type system (#36116). The method returns the relevant global object for a given DOM object, but that operation is only meaningful if there is an active realm. We usually, but not always, have an active realm. This change avoids a footgun by following the principle of least surprise. Rather than making every single caller of `something.global()` both prove that there is an active realm and think about which realm they want active, we implement the obvious behaviour: always activate the realm of the callee before obtaining the relevant global. Testing: Existing WPT coverage is sufficient; this method is called all over the codebase. Fixes: #37070 #27037 Signed-off-by: Josh Matthews --- components/script/dom/bindings/reflector.rs | 14 ++++++++++++-- components/script_bindings/reflector.rs | 13 +------------ 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/components/script/dom/bindings/reflector.rs b/components/script/dom/bindings/reflector.rs index 0a5afbce487..c888686974e 100644 --- a/components/script/dom/bindings/reflector.rs +++ b/components/script/dom/bindings/reflector.rs @@ -11,7 +11,7 @@ use crate::DomTypes; use crate::dom::bindings::conversions::DerivedFrom; use crate::dom::bindings::root::DomRoot; use crate::dom::globalscope::GlobalScope; -use crate::realms::InRealm; +use crate::realms::{InRealm, enter_realm}; use crate::script_runtime::CanGc; /// Create the reflector for a new DOM object and yield ownership to the @@ -42,7 +42,16 @@ where } pub(crate) trait DomGlobal { + /// Returns the [relevant global] in whatever realm is currently active. + /// + /// [relevant global]: https://html.spec.whatwg.org/multipage/#concept-relevant-global fn global_(&self, realm: InRealm) -> DomRoot; + + /// Returns the [relevant global] in the same realm as the callee object. + /// If you know the callee's realm is already the current realm, it is + /// more efficient to call [DomGlobal::global_] instead. + /// + /// [relevant global]: https://html.spec.whatwg.org/multipage/#concept-relevant-global fn global(&self) -> DomRoot; } @@ -51,7 +60,8 @@ impl> DomGlobal for T { >::global_(self, realm) } fn global(&self) -> DomRoot { - >::global(self) + let realm = enter_realm(self); + >::global_(self, InRealm::entered(&realm)) } } diff --git a/components/script_bindings/reflector.rs b/components/script_bindings/reflector.rs index 6b6ae03cb69..4b91b0536fc 100644 --- a/components/script_bindings/reflector.rs +++ b/components/script_bindings/reflector.rs @@ -8,7 +8,7 @@ use malloc_size_of_derive::MallocSizeOf; use crate::interfaces::GlobalScopeHelpers; use crate::iterable::{Iterable, IterableIterator}; -use crate::realms::{AlreadyInRealm, InRealm}; +use crate::realms::InRealm; use crate::root::{Dom, DomRoot, Root}; use crate::script_runtime::{CanGc, JSContext}; use crate::{DomTypes, JSTraceable}; @@ -108,17 +108,6 @@ pub trait DomGlobalGeneric: DomObject { { D::GlobalScope::from_reflector(self, realm) } - - /// Returns the [`GlobalScope`] of the realm that the [`DomObject`] was created in. If this - /// object is a `Node`, this will be different from it's owning `Document` if adopted by. For - /// `Node`s it's almost always better to use `NodeTraits::owning_global`. - fn global(&self) -> DomRoot - where - Self: Sized, - { - let realm = AlreadyInRealm::assert_for_cx(D::GlobalScope::get_cx()); - D::GlobalScope::from_reflector(self, InRealm::already(&realm)) - } } impl DomGlobalGeneric for T {}