Fix crash when enumerating properties of global object (#36491)

These changes make our implementation of the enumeration hook for
globals [match
Gecko's](https://searchfox.org/mozilla-central/rev/1f65969e57c757146e3e548614b49d3a4168eeb8/dom/base/nsGlobalWindowInner.cpp#3297),
fixing an assertion failure that occurred in the previous
implementation.

Our enumeration hook is supposed to fill a vector with names of
properties on the global object without modifying the global in any way;
instead we were defining all of the missing webidl interfaces. We now do
much less work and crash less.

Testing: New crashtest based on manual testcase.
Fixes: #34686

---------

Signed-off-by: Josh Matthews <josh@joshmatthews.net>

Cargo.lock

@@ -526,7 +526,7 @@ dependencies = [
  "bitflags 2.9.0",
  "cexpr",
  "clang-sys",
- "itertools 0.13.0",
+ "itertools 0.10.5",
  "proc-macro2",
  "quote",
  "regex",
@@ -1071,7 +1071,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "117725a109d387c937a1533ce01b450cbde6b88abceea8473c4d7a85853cda3c"
 dependencies = [
  "lazy_static",
- "windows-sys 0.59.0",
+ "windows-sys 0.52.0",
 ]
 
 [[package]]
@@ -2026,7 +2026,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "976dd42dc7e85965fe702eb8164f21f450704bdde31faefd6471dba214cb594e"
 dependencies = [
  "libc",
- "windows-sys 0.59.0",
+ "windows-sys 0.52.0",
 ]
 
 [[package]]
@@ -2552,7 +2552,7 @@ dependencies = [
  "gobject-sys",
  "libc",
  "system-deps",
- "windows-sys 0.59.0",
+ "windows-sys 0.52.0",
 ]
 
 [[package]]
@@ -4002,7 +4002,7 @@ checksum = "e04d7f318608d35d4b61ddd75cbdaee86b023ebe2bd5a66ee0915f0bf93095a9"
 dependencies = [
  "hermit-abi 0.5.0",
  "libc",
- "windows-sys 0.59.0",
+ "windows-sys 0.52.0",
 ]
 
 [[package]]
@@ -6168,7 +6168,7 @@ dependencies = [
  "errno",
  "libc",
  "linux-raw-sys",
- "windows-sys 0.59.0",
+ "windows-sys 0.52.0",
 ]
 
 [[package]]
@@ -7505,7 +7505,7 @@ dependencies = [
  "getrandom",
  "once_cell",
  "rustix",
- "windows-sys 0.59.0",
+ "windows-sys 0.52.0",
 ]
 
 [[package]]
@@ -8768,7 +8768,7 @@ version = "0.1.9"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "cf221c93e13a30d793f7645a0e7762c55d169dbb0a49671918a2319d289b10bb"
 dependencies = [
- "windows-sys 0.59.0",
+ "windows-sys 0.52.0",
 ]
 
 [[package]]

components/script/dom/bindings/utils.rs

@@ -13,7 +13,7 @@ use js::jsapi::{
     CallArgs, DOMCallbacks, HandleObject as RawHandleObject, JS_FreezeObject, JSContext, JSObject,
 };
 use js::rust::{HandleObject, MutableHandleValue, get_object_class, is_dom_class};
-use script_bindings::interfaces::DomHelpers;
+use script_bindings::interfaces::{DomHelpers, Interface};
 use script_bindings::settings_stack::StackEntry;
 
 use crate::DomTypes;
@@ -153,8 +153,7 @@ impl DomHelpers<crate::DomTypeHolder> for crate::DomTypeHolder {
         unsafe { is_platform_object_same_origin(cx, obj) }
     }
 
-    fn interface_map() -> &'static phf::Map<&'static [u8], for<'a> fn(SafeJSContext, HandleObject)>
+    fn interface_map() -> &'static phf::Map<&'static [u8], Interface> {
-    {
         &InterfaceObjectMap::MAP
     }
 

components/script_bindings/build.rs

@@ -43,13 +43,21 @@ fn main() {
     let json: Value = serde_json::from_reader(File::open(json).unwrap()).unwrap();
     let mut map = phf_codegen::Map::new();
     for (key, value) in json.as_object().unwrap() {
-        map.entry(Bytes(key), value.as_str().unwrap());
+        let parts = value.as_array().unwrap();
+        map.entry(
+            Bytes(key),
+            &format!(
+                "Interface {{ define: {}, enabled: {} }}",
+                parts[0].as_str().unwrap(),
+                parts[1].as_str().unwrap()
+            ),
+        );
     }
     let phf = PathBuf::from(env::var_os("OUT_DIR").unwrap()).join("InterfaceObjectMapPhf.rs");
     let mut phf = File::create(phf).unwrap();
     writeln!(
         &mut phf,
-        "pub(crate) static MAP: phf::Map<&'static [u8], fn(JSContext, HandleObject)> = {};",
+        "pub(crate) static MAP: phf::Map<&'static [u8], Interface> = {};",
         map.build(),
     )
     .unwrap();

components/script_bindings/codegen/CodegenRust.py

@@ -2972,7 +2972,8 @@ class CGConstructorEnabled(CGAbstractMethod):
                                   'ConstructorEnabled', 'bool',
                                   [Argument("SafeJSContext", "aCx"),
                                    Argument("HandleObject", "aObj")],
-                                  templateArgs=['D: DomTypes'])
+                                  templateArgs=['D: DomTypes'],
+                                  pub=True)
 
     def definition_body(self):
         conditions = []
@@ -8531,8 +8532,7 @@ class GlobalGenRoots():
     def InterfaceObjectMap(config):
         mods = [
             "crate::dom::bindings::codegen",
-            "crate::script_runtime::JSContext",
+            "script_bindings::interfaces::Interface",
-            "js::rust::HandleObject",
         ]
         imports = CGList([CGGeneric(f"use {mod};") for mod in mods], "\n")
 
@@ -8555,9 +8555,13 @@ class GlobalGenRoots():
             for ctor in d.interface.legacyFactoryFunctions:
                 pairs.append((ctor.identifier.name, binding_mod, binding_ns))
         pairs.sort(key=operator.itemgetter(0))
+
+        def bindingPath(pair):
+            return f'codegen::Bindings::{pair[1]}::{pair[2]}'
+
         mappings = [
-            CGGeneric(f'"{pair[0]}": "codegen::Bindings::{pair[1]}'
+            CGGeneric(f'"{pair[0]}": ["{bindingPath(pair)}::DefineDOMInterface::<crate::DomTypeHolder>", '
-                      f'::{pair[2]}::DefineDOMInterface::<crate::DomTypeHolder>"')
+                      f'"{bindingPath(pair)}::ConstructorEnabled::<crate::DomTypeHolder>"]')
             for pair in pairs
         ]
         return CGWrapper(

components/script_bindings/interfaces.rs

@@ -23,6 +23,17 @@ use crate::script_runtime::{CanGc, JSContext};
 use crate::settings_stack::StackEntry;
 use crate::utils::ProtoOrIfaceArray;
 
+/// Operations that can be invoked for a WebIDL interface against
+/// a global object.
+///
+/// <https://github.com/mozilla/gecko-dev/blob/3fd619f47/dom/bindings/WebIDLGlobalNameHash.h#L24>
+pub struct Interface {
+    /// Define the JS object for this interface on the given global.
+    pub define: fn(JSContext, HandleObject),
+    /// Returns true if this interface's conditions are met for the given global.
+    pub enabled: fn(JSContext, HandleObject) -> bool,
+}
+
 /// Operations that must be invoked from the generated bindings.
 pub trait DomHelpers<D: DomTypes> {
     fn throw_dom_exception(cx: JSContext, global: &D::GlobalScope, result: Error, can_gc: CanGc);
@@ -42,7 +53,7 @@ pub trait DomHelpers<D: DomTypes> {
 
     fn is_platform_object_same_origin(cx: JSContext, obj: RawHandleObject) -> bool;
 
-    fn interface_map() -> &'static phf::Map<&'static [u8], for<'a> fn(JSContext, HandleObject)>;
+    fn interface_map() -> &'static phf::Map<&'static [u8], Interface>;
 
     fn push_new_element_queue();
     fn pop_current_element_queue(can_gc: CanGc);

components/script_bindings/utils.rs

@@ -10,19 +10,19 @@ use std::slice;
 use js::conversions::ToJSValConvertible;
 use js::gc::Handle;
 use js::glue::{
-    CallJitGetterOp, CallJitMethodOp, CallJitSetterOp, JS_GetReservedSlot,
+    AppendToIdVector, CallJitGetterOp, CallJitMethodOp, CallJitSetterOp, JS_GetReservedSlot,
     RUST_FUNCTION_VALUE_TO_JITINFO,
 };
 use js::jsapi::{
     AtomToLinearString, CallArgs, ExceptionStackBehavior, GetLinearStringCharAt,
     GetLinearStringLength, GetNonCCWObjectGlobal, HandleId as RawHandleId,
-    HandleObject as RawHandleObject, Heap, JS_ClearPendingException,
+    HandleObject as RawHandleObject, Heap, JS_AtomizeStringN, JS_ClearPendingException,
-    JS_DeprecatedStringHasLatin1Chars, JS_EnumerateStandardClasses,
+    JS_DeprecatedStringHasLatin1Chars, JS_GetLatin1StringCharsAndLength, JS_IsExceptionPending,
-    JS_GetLatin1StringCharsAndLength, JS_IsExceptionPending, JS_IsGlobalObject,
+    JS_IsGlobalObject, JS_NewEnumerateStandardClasses, JS_ResolveStandardClass, JSAtom, JSContext,
-    JS_ResolveStandardClass, JSAtom, JSContext, JSJitInfo, JSObject, JSTracer,
+    JSJitInfo, JSObject, JSTracer, MutableHandleIdVector as RawMutableHandleIdVector,
-    MutableHandleIdVector as RawMutableHandleIdVector, MutableHandleValue as RawMutableHandleValue,
+    MutableHandleValue as RawMutableHandleValue, ObjectOpResult, StringIsArrayIndex,
-    ObjectOpResult, StringIsArrayIndex,
 };
+use js::jsid::StringId;
 use js::jsval::{JSVal, UndefinedValue};
 use js::rust::wrappers::{
     CallOriginalPromiseReject, JS_DeletePropertyById, JS_ForwardGetPropertyTo,
@@ -576,18 +576,36 @@ impl AsCCharPtrPtr for [u8] {
 }
 
 /// Enumerate lazy properties of a global object.
+/// Modeled after <https://github.com/mozilla/gecko-dev/blob/3fd619f47/dom/bindings/BindingUtils.cpp#L2814>
+/// and <https://github.com/mozilla/gecko-dev/blob/3fd619f47/dom/base/nsGlobalWindowInner.cpp#3297>
 pub(crate) unsafe extern "C" fn enumerate_global<D: DomTypes>(
     cx: *mut JSContext,
     obj: RawHandleObject,
-    _props: RawMutableHandleIdVector,
+    props: RawMutableHandleIdVector,
-    _enumerable_only: bool,
+    enumerable_only: bool,
 ) -> bool {
     assert!(JS_IsGlobalObject(obj.get()));
-    if !JS_EnumerateStandardClasses(cx, obj) {
+    if !JS_NewEnumerateStandardClasses(cx, obj, props, enumerable_only) {
         return false;
     }
-    for init_fun in <D as DomHelpers<D>>::interface_map().values() {
+
-        init_fun(SafeJSContext::from_ptr(cx), Handle::from_raw(obj));
+    if enumerable_only {
+        // All WebIDL interface names are defined as non-enumerable, so there's
+        // no point in checking them if we're only returning enumerable names.
+        return true;
+    }
+
+    let cx = SafeJSContext::from_ptr(cx);
+    let obj = Handle::from_raw(obj);
+    for (name, interface) in <D as DomHelpers<D>>::interface_map() {
+        if !(interface.enabled)(cx, obj) {
+            continue;
+        }
+        let s = JS_AtomizeStringN(*cx, name.as_c_char_ptr(), name.len());
+        rooted!(in(*cx) let id = StringId(s));
+        if s.is_null() || !AppendToIdVector(props, id.handle().into()) {
+            return false;
+        }
     }
     true
 }
@@ -621,8 +639,8 @@ pub(crate) unsafe extern "C" fn resolve_global<D: DomTypes>(
     assert!(!ptr.is_null());
     let bytes = slice::from_raw_parts(ptr, length);
 
-    if let Some(init_fun) = <D as DomHelpers<D>>::interface_map().get(bytes) {
+    if let Some(interface) = <D as DomHelpers<D>>::interface_map().get(bytes) {
-        init_fun(SafeJSContext::from_ptr(cx), Handle::from_raw(obj));
+        (interface.define)(SafeJSContext::from_ptr(cx), Handle::from_raw(obj));
         *rval = true;
     } else {
         *rval = false;

tests/wpt/mozilla/meta/MANIFEST.json

@@ -16,6 +16,13 @@
       {}
      ]
     ],
+    "global-enumerate-crash.html": [
+     "a77e79b1465bf7555340dd5e9bf94a4c8caa85f2",
+     [
+      null,
+      {}
+     ]
+    ],
     "iframe_focus-crash.html": [
      "f991b1a563f3cc44870640ab194708fa239ad89d",
      [
@@ -13517,7 +13524,7 @@
      ]
     ],
     "interfaces.worker.js": [
-     "8d109502622fac7266a4564de09684a3ab94118c",
+     "8af942c0f99218fb39770cdcf299eaa230339010",
      [
       "mozilla/interfaces.worker.html",
       {}

tests/wpt/mozilla/tests/mozilla/global-enumerate-crash.html

@@ -0,0 +1,15 @@
+<html class=test-wait>
+  <body>
+    <iframe id="if"></iframe>
+    <script>
+          const frame = document.getElementById('if').contentWindow;
+          for (const i of Object.getOwnPropertyNames(frame)) {
+            try {
+              frame[i]['foo'];
+            } catch (e) {
+            }
+          }
+          document.documentElement.classList.remove("test-wait");
+    </script>
+  </body>
+</html>

tests/wpt/mozilla/tests/mozilla/interfaces.worker.js

@@ -107,6 +107,7 @@ test_interfaces([
   "Response",
   "SecurityPolicyViolationEvent",
   "ServiceWorkerContainer",
+  "SVGRect",
   "TextDecoder",
   "TextEncoder",
   "TrustedHTML",
@@ -128,6 +129,7 @@ test_interfaces([
   "WebGLShaderPrecisionFormat",
   "WebGLTexture",
   "WebGLUniformLocation",
+  "WebKitCSSMatrix",
   "WebSocket",
   "WeakRef",
   "Worker",