bindings: Convert certain Exceptions into Promise rejections (#32923)

* Impl promise exception to rejection for methods Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com> * Impl promise exception to rejection for getters Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com> * Impl promise exception to rejection for static methods Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com> * Add tests for exception to rejection Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com> * Expectations Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com> --------- Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
2025-06-06 16:45:39 +00:00 · 2024-08-03 14:58:37 +02:00 · 2024-08-03 14:58:37 +02:00 · f3bec0aed3
commit f3bec0aed3
parent fd83281657
11 changed files with 291 additions and 20 deletions
--- a/components/script/dom/bindings/codegen/CodegenRust.py
+++ b/components/script/dom/bindings/codegen/CodegenRust.py
@ -1756,7 +1756,8 @@ class MethodDefiner(PropertyDefiner):
                         "methodInfo": not m.isStatic(),
                         "length": methodLength(m),
                         "flags": "JSPROP_READONLY" if crossorigin else "JSPROP_ENUMERATE",
-                         "condition": PropertyDefiner.getControllingCondition(m, descriptor)}
+                         "condition": PropertyDefiner.getControllingCondition(m, descriptor),
+                         "returnsPromise": m.returnsPromise()}
                        for m in methods]

        # TODO: Once iterable is implemented, use tiebreak rules instead of
@ -1877,8 +1878,12 @@ class MethodDefiner(PropertyDefiner):
                    jitinfo = "&%s_methodinfo as *const _ as *const JSJitInfo" % identifier
                    accessor = "Some(generic_method)"
                else:
-                    jitinfo = "ptr::null()"
-                    accessor = 'Some(%s)' % m.get("nativeName", m["name"])
+                    if m.get("returnsPromise", False):
+                        jitinfo = "&%s_methodinfo" % m.get("nativeName", m["name"])
+                        accessor = "Some(generic_static_promise_method)"
+                    else:
+                        jitinfo = "ptr::null()"
+                        accessor = 'Some(%s)' % m.get("nativeName", m["name"])
            if m["name"].startswith("@@"):
                assert not self.crossorigin
                name = 'JSPropertySpec_Name { symbol_: SymbolCode::%s as usize + 1 }' % m["name"][2:]
@ -4044,6 +4049,79 @@ class CGSpecializedMethod(CGAbstractExternMethod):
        return MakeNativeName(nativeName)


+# https://searchfox.org/mozilla-central/rev/b220e40ff2ee3d10ce68e07d8a8a577d5558e2a2/dom/bindings/Codegen.py#10655-10684
+class CGMethodPromiseWrapper(CGAbstractExternMethod):
+    """
+    A class for generating a wrapper around another method that will
+    convert exceptions to promises.
+    """
+
+    def __init__(self, descriptor, methodToWrap):
+        self.method = methodToWrap
+        name = CGMethodPromiseWrapper.makeNativeName(descriptor, self.method)
+        args = [Argument('*mut JSContext', 'cx'),
+                Argument('RawHandleObject', '_obj'),
+                Argument('*mut libc::c_void', 'this'),
+                Argument('*const JSJitMethodCallArgs', 'args')]
+        CGAbstractExternMethod.__init__(self, descriptor, name, 'bool', args)
+
+    def definition_body(self):
+        return CGGeneric(fill(
+            """
+            let ok = ${methodName}(${args});
+            if ok {
+              return true;
+            }
+            return exception_to_promise(cx, (*args).rval());
+            """,
+            methodName=self.method.identifier.name,
+            args=", ".join(arg.name for arg in self.args),
+        ))
+
+    @staticmethod
+    def makeNativeName(descriptor, m):
+        return m.identifier.name + "_promise_wrapper"
+
+
+# https://searchfox.org/mozilla-central/rev/7279a1df13a819be254fd4649e07c4ff93e4bd45/dom/bindings/Codegen.py#11390-11419
+class CGGetterPromiseWrapper(CGAbstractExternMethod):
+    """
+    A class for generating a wrapper around another getter that will
+    convert exceptions to promises.
+    """
+
+    def __init__(self, descriptor, methodToWrap):
+        self.method = methodToWrap
+        name = CGGetterPromiseWrapper.makeNativeName(descriptor, self.method)
+        self.method_call = CGGetterPromiseWrapper.makeOrigName(descriptor, self.method)
+        args = [Argument('*mut JSContext', 'cx'),
+                Argument('RawHandleObject', '_obj'),
+                Argument('*mut libc::c_void', 'this'),
+                Argument('JSJitGetterCallArgs', 'args')]
+        CGAbstractExternMethod.__init__(self, descriptor, name, 'bool', args)
+
+    def definition_body(self):
+        return CGGeneric(fill(
+            """
+            let ok = ${methodName}(${args});
+            if ok {
+              return true;
+            }
+            return exception_to_promise(cx, args.rval());
+            """,
+            methodName=self.method_call,
+            args=", ".join(arg.name for arg in self.args),
+        ))
+
+    @staticmethod
+    def makeOrigName(descriptor, m):
+        return 'get_' + descriptor.internalNameFor(m.identifier.name)
+
+    @staticmethod
+    def makeNativeName(descriptor, m):
+        return CGGetterPromiseWrapper.makeOrigName(descriptor, m) + "_promise_wrapper"
+
+
 class CGDefaultToJSONMethod(CGSpecializedMethod):
    def __init__(self, descriptor, method):
        assert method.isDefaultToJSON()
@ -4355,6 +4433,8 @@ class CGMemberJITInfo(CGThing):
            internalMemberName = self.descriptor.internalNameFor(self.member.identifier.name)
            getterinfo = ("%s_getterinfo" % internalMemberName)
            getter = ("get_%s" % internalMemberName)
+            if self.member.type.isPromise():
+                getter = CGGetterPromiseWrapper.makeNativeName(self.descriptor, self.member)
            getterinfal = "infallible" in self.descriptor.getExtendedAttributes(self.member, getter=True)

            movable = self.mayBeMovable() and getterinfal
@ -4391,6 +4471,8 @@ class CGMemberJITInfo(CGThing):
        if self.member.isMethod():
            methodinfo = ("%s_methodinfo" % self.member.identifier.name)
            method = ("%s" % self.member.identifier.name)
+            if self.member.returnsPromise():
+                method = CGMethodPromiseWrapper.makeNativeName(self.descriptor, self.member)

            # Methods are infallible if they are infallible, have no arguments
            # to unwrap, and have a return type that's infallible to wrap up for
@ -4608,6 +4690,44 @@ class CGMemberJITInfo(CGThing):
        return "%s | %s" % (existingType, type)


+# https://searchfox.org/mozilla-central/rev/9993372dd72daea851eba4600d5750067104bc15/dom/bindings/Codegen.py#12355-12374
+class CGStaticMethodJitinfo(CGGeneric):
+    """
+    A class for generating the JITInfo for a promise-returning static method.
+    """
+
+    def __init__(self, method):
+        CGGeneric.__init__(
+            self,
+            f"""
+            const {method.identifier.name}_methodinfo: JSJitInfo = JSJitInfo {{
+                __bindgen_anon_1: JSJitInfo__bindgen_ty_1 {{
+                    staticMethod: Some({method.identifier.name})
+                }},
+                __bindgen_anon_2: JSJitInfo__bindgen_ty_2 {{
+                    protoID: PrototypeList::ID::Last as u16,
+                }},
+                __bindgen_anon_3: JSJitInfo__bindgen_ty_3 {{ depth: 0 }},
+                _bitfield_align_1: [],
+                _bitfield_1: __BindgenBitfieldUnit::new(
+                    new_jsjitinfo_bitfield_1!(
+                        JSJitInfo_OpType::StaticMethod as u8,
+                        JSJitInfo_AliasSet::AliasEverything as u8,
+                        JSValueType::JSVAL_TYPE_OBJECT as u8,
+                        false,
+                        false,
+                        false,
+                        false,
+                        false,
+                        false,
+                        0,
+                    ).to_ne_bytes()
+                ),
+            }};
+            """
+        )
+
+
 def getEnumValueName(value):
    # Some enum values can be empty strings.  Others might have weird
    # characters in them.  Deal with the former by returning "_empty",
@ -6355,8 +6475,14 @@ class CGDescriptor(CGThing):
                elif m.isStatic():
                    assert descriptor.interface.hasInterfaceObject()
                    cgThings.append(CGStaticMethod(descriptor, m))
+                    if m.returnsPromise():
+                        cgThings.append(CGStaticMethodJitinfo(m))
                elif not descriptor.interface.isCallback():
                    cgThings.append(CGSpecializedMethod(descriptor, m))
+                    if m.returnsPromise():
+                        cgThings.append(
+                            CGMethodPromiseWrapper(descriptor, m)
+                        )
                    cgThings.append(CGMemberJITInfo(descriptor, m))
            elif m.isAttr():
                if m.getExtendedAttribute("Unscopable"):
@ -6367,6 +6493,10 @@ class CGDescriptor(CGThing):
                    cgThings.append(CGStaticGetter(descriptor, m))
                elif not descriptor.interface.isCallback():
                    cgThings.append(CGSpecializedGetter(descriptor, m))
+                    if m.type.isPromise():
+                        cgThings.append(
+                            CGGetterPromiseWrapper(descriptor, m)
+                        )

                if not m.readonly:
                    if m.isStatic():
--- a/components/script/dom/bindings/import.rs
+++ b/components/script/dom/bindings/import.rs
@ -135,11 +135,11 @@ pub mod module {
    pub use crate::dom::bindings::root::{Dom, DomSlice, MaybeUnreflectedDom, Root};
    pub use crate::dom::bindings::trace::JSTraceable;
    pub use crate::dom::bindings::utils::{
-        callargs_is_constructing, enumerate_global, generic_getter, generic_lenient_getter,
-        generic_lenient_setter, generic_method, generic_setter, get_array_index_from_id,
-        get_property_on_prototype, has_property_on_prototype, resolve_global, trace_global,
-        AsVoidPtr, DOMClass, DOMJSClass, ProtoOrIfaceArray, DOM_PROTO_UNFORGEABLE_HOLDER_SLOT,
-        JSCLASS_DOM_GLOBAL,
+        callargs_is_constructing, 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, AsVoidPtr, DOMClass, DOMJSClass,
+        ProtoOrIfaceArray, DOM_PROTO_UNFORGEABLE_HOLDER_SLOT, JSCLASS_DOM_GLOBAL,
    };
    pub use crate::dom::bindings::weakref::{WeakReferenceable, DOM_WEAK_SLOT};
    pub use crate::dom::types::{AnalyserNode, AudioNode, BaseAudioContext, EventTarget};
--- a/components/script/dom/bindings/utils.rs
+++ b/components/script/dom/bindings/utils.rs
@ -6,6 +6,7 @@

 use std::ffi::CString;
 use std::os::raw::{c_char, c_void};
+use std::ptr::NonNull;
 use std::{ptr, slice, str};

 use js::conversions::ToJSValConvertible;
@ -14,17 +15,19 @@ use js::glue::{
    UnwrapObjectDynamic, UnwrapObjectStatic, RUST_FUNCTION_VALUE_TO_JITINFO,
 };
 use js::jsapi::{
-    AtomToLinearString, CallArgs, DOMCallbacks, GetLinearStringCharAt, GetLinearStringLength,
-    GetNonCCWObjectGlobal, HandleId as RawHandleId, HandleObject as RawHandleObject, Heap, JSAtom,
-    JSContext, JSJitInfo, JSObject, JSTracer, JS_DeprecatedStringHasLatin1Chars,
-    JS_EnumerateStandardClasses, JS_FreezeObject, JS_GetLatin1StringCharsAndLength,
-    JS_IsExceptionPending, JS_IsGlobalObject, JS_ResolveStandardClass,
-    MutableHandleIdVector as RawMutableHandleIdVector, ObjectOpResult, StringIsArrayIndex,
+    AtomToLinearString, CallArgs, DOMCallbacks, ExceptionStackBehavior, GetLinearStringCharAt,
+    GetLinearStringLength, GetNonCCWObjectGlobal, HandleId as RawHandleId,
+    HandleObject as RawHandleObject, Heap, JSAtom, JSContext, JSJitInfo, JSObject, JSTracer,
+    JS_ClearPendingException, JS_DeprecatedStringHasLatin1Chars, JS_EnumerateStandardClasses,
+    JS_FreezeObject, JS_GetLatin1StringCharsAndLength, JS_IsExceptionPending, JS_IsGlobalObject,
+    JS_ResolveStandardClass, MutableHandleIdVector as RawMutableHandleIdVector,
+    MutableHandleValue as RawMutableHandleValue, ObjectOpResult, StringIsArrayIndex,
 };
 use js::jsval::{JSVal, UndefinedValue};
 use js::rust::wrappers::{
-    JS_DeletePropertyById, JS_ForwardGetPropertyTo, JS_GetProperty, JS_GetPrototype,
-    JS_HasProperty, JS_HasPropertyById, JS_SetProperty,
+    CallOriginalPromiseReject, JS_DeletePropertyById, JS_ForwardGetPropertyTo,
+    JS_GetPendingException, JS_GetProperty, JS_GetPrototype, JS_HasProperty, JS_HasPropertyById,
+    JS_SetPendingException, JS_SetProperty,
 };
 use js::rust::{
    get_object_class, is_dom_class, GCMethods, Handle, HandleId, HandleObject, HandleValue,
@ -617,3 +620,41 @@ impl AsCCharPtrPtr for [u8] {
 pub unsafe fn callargs_is_constructing(args: &CallArgs) -> bool {
    (*args.argv_.offset(-1)).is_magic()
 }
+
+/// https://searchfox.org/mozilla-central/rev/7279a1df13a819be254fd4649e07c4ff93e4bd45/dom/bindings/BindingUtils.cpp#3300
+pub unsafe extern "C" fn generic_static_promise_method(
+    cx: *mut JSContext,
+    argc: libc::c_uint,
+    vp: *mut JSVal,
+) -> bool {
+    let args = CallArgs::from_vp(vp, argc);
+
+    let info = RUST_FUNCTION_VALUE_TO_JITINFO(JS_CALLEE(cx, vp));
+    assert!(!info.is_null());
+    // TODO: we need safe wrappers for this in mozjs!
+    //assert_eq!((*info)._bitfield_1, JSJitInfo_OpType::StaticMethod as u8)
+    let static_fn = (*info).__bindgen_anon_1.staticMethod.unwrap();
+    if static_fn(cx, argc, vp) {
+        return true;
+    }
+    exception_to_promise(cx, args.rval())
+}
+
+/// Coverts exception to promise rejection
+///
+/// https://searchfox.org/mozilla-central/rev/b220e40ff2ee3d10ce68e07d8a8a577d5558e2a2/dom/bindings/BindingUtils.cpp#3315
+pub unsafe fn exception_to_promise(cx: *mut JSContext, rval: RawMutableHandleValue) -> bool {
+    rooted!(in(cx) let mut exception = UndefinedValue());
+    if !JS_GetPendingException(cx, exception.handle_mut()) {
+        return false;
+    }
+    JS_ClearPendingException(cx);
+    if let Some(promise) = NonNull::new(CallOriginalPromiseReject(cx, exception.handle())) {
+        promise.to_jsval(cx, MutableHandleValue::from_raw(rval));
+        true
+    } else {
+        // We just give up.  Put the exception back.
+        JS_SetPendingException(cx, exception.handle(), ExceptionStackBehavior::Capture);
+        false
+    }
+}
--- a/components/script/dom/testbinding.rs
+++ b/components/script/dom/testbinding.rs
@ -1084,6 +1084,29 @@ impl TestBindingMethods for TestBinding {
            stringMember: Some(s2),
        }
    }
+
+    fn MethodThrowToRejectPromise(&self) -> Fallible<Rc<Promise>> {
+        Err(Error::Type("test".to_string()))
+    }
+
+    fn GetGetterThrowToRejectPromise(&self) -> Fallible<Rc<Promise>> {
+        Err(Error::Type("test".to_string()))
+    }
+
+    fn MethodInternalThrowToRejectPromise(&self, _arg: u64) -> Rc<Promise> {
+        unreachable!("Method should already throw")
+    }
+}
+
+#[allow(non_snake_case)]
+impl TestBinding {
+    pub fn StaticThrowToRejectPromise(_: &GlobalScope) -> Fallible<Rc<Promise>> {
+        Err(Error::Type("test".to_string()))
+    }
+
+    pub fn StaticInternalThrowToRejectPromise(_: &GlobalScope, _arg: u64) -> Rc<Promise> {
+        unreachable!("Method should already throw")
+    }
 }

 #[allow(non_snake_case)]
--- a/components/script/dom/webidls/TestBinding.webidl
+++ b/components/script/dom/webidls/TestBinding.webidl
@ -574,6 +574,16 @@ interface TestBinding {
  undefined promiseRejectWithTypeError(Promise<any> p, USVString message);
  undefined resolvePromiseDelayed(Promise<any> p, DOMString value, unsigned long long ms);

+  [Throws]
+  static Promise<any> staticThrowToRejectPromise();
+  [Throws]
+  Promise<any> methodThrowToRejectPromise();
+  [Throws]
+  readonly attribute Promise<any> getterThrowToRejectPromise;
+
+  static Promise<any> staticInternalThrowToRejectPromise([EnforceRange] unsigned long long arg);
+  Promise<any> methodInternalThrowToRejectPromise([EnforceRange] unsigned long long arg);
+
  undefined panic();

  GlobalScope entryGlobal();
--- a/tests/wpt/meta-legacy-layout/fetch/api/idlharness.any.js.ini
+++ b/tests/wpt/meta-legacy-layout/fetch/api/idlharness.any.js.ini
@ -72,7 +72,6 @@
    expected: FAIL

  [Window interface: calling fetch(RequestInfo, optional RequestInit) on window with too few arguments must throw TypeError]
-    expected: FAIL

  [Window interface: operation fetch(RequestInfo, optional RequestInit)]
    expected: FAIL
@ -170,7 +169,6 @@
    expected: FAIL

  [WorkerGlobalScope interface: calling fetch(RequestInfo, optional RequestInit) on self with too few arguments must throw TypeError]
-    expected: FAIL

  [WorkerGlobalScope interface: operation fetch(RequestInfo, optional RequestInit)]
    expected: FAIL
--- a/tests/wpt/meta/fetch/api/idlharness.any.js.ini
+++ b/tests/wpt/meta/fetch/api/idlharness.any.js.ini
@ -69,7 +69,6 @@
    expected: FAIL

  [WorkerGlobalScope interface: calling fetch(RequestInfo, optional RequestInit) on self with too few arguments must throw TypeError]
-    expected: FAIL

  [Request interface: operation bytes()]
    expected: FAIL
@ -158,7 +157,6 @@
    expected: FAIL

  [Window interface: calling fetch(RequestInfo, optional RequestInit) on window with too few arguments must throw TypeError]
-    expected: FAIL

  [Request interface: operation bytes()]
    expected: FAIL
--- a/tests/wpt/mozilla/meta-legacy-layout/mozilla/exceptionToRejection.any.js.ini
+++ b/tests/wpt/mozilla/meta-legacy-layout/mozilla/exceptionToRejection.any.js.ini
@ -0,0 +1,7 @@
+[exceptionToRejection.any.html]
+  type: testharness
+  prefs: [dom.testbinding.enabled:true]
+
+[exceptionToRejection.any.worker.html]
+  type: testharness
+  prefs: [dom.testbinding.enabled:true]
--- a/tests/wpt/mozilla/meta/MANIFEST.json
+++ b/tests/wpt/mozilla/meta/MANIFEST.json
@ -13130,6 +13130,31 @@
      {}
     ]
    ],
+    "exceptionToRejection.any.js": [
+     "738a8bedbcfe83a6f110bc6e6d133e31f80ea9ad",
+     [
+      "mozilla/exceptionToRejection.any.html",
+      {
+       "script_metadata": [
+        [
+         "title",
+         "Test that promise exception are converted to rejections"
+        ]
+       ]
+      }
+     ],
+     [
+      "mozilla/exceptionToRejection.any.worker.html",
+      {
+       "script_metadata": [
+        [
+         "title",
+         "Test that promise exception are converted to rejections"
+        ]
+       ]
+      }
+     ]
+    ],
    "fetch_cannot_overwhelm_system.window.js": [
     "989231e9caedd099f5212bd2f9d377c83f929a22",
     [
--- a/tests/wpt/mozilla/meta/mozilla/exceptionToRejection.any.js.ini
+++ b/tests/wpt/mozilla/meta/mozilla/exceptionToRejection.any.js.ini
@ -0,0 +1,7 @@
+[exceptionToRejection.any.html]
+  type: testharness
+  prefs: [dom.testbinding.enabled:true]
+
+[exceptionToRejection.any.worker.html]
+  type: testharness
+  prefs: [dom.testbinding.enabled:true]
--- a/tests/wpt/mozilla/tests/mozilla/exceptionToRejection.any.js
+++ b/tests/wpt/mozilla/tests/mozilla/exceptionToRejection.any.js
@ -0,0 +1,32 @@
+// META: title=Test that promise exception are converted to rejections
+
+var binding = new TestBinding();
+
+/*
+  static Promise<any> staticThrowToRejectPromise();
+  Promise<any> methodThrowToRejectPromise();
+  readonly attribute Promise<any> getterThrowToRejectPromise;
+
+  static Promise<any> staticInternalThrowToRejectPromise([EnforceRange] unsigned long long arg);
+  Promise<any> methodInternalThrowToRejectPromise([EnforceRange] unsigned long long arg);
+*/
+
+promise_test((t) => {
+    return promise_rejects_js(t, TypeError, TestBinding.staticThrowToRejectPromise());
+}, "staticThrowToRejectPromise");
+
+promise_test((t) => {
+    return promise_rejects_js(t, TypeError, binding.methodThrowToRejectPromise());
+}, "methodThrowToRejectPromise");
+
+promise_test((t) => {
+    return promise_rejects_js(t, TypeError, binding.getterThrowToRejectPromise);
+}, "getterThrowToRejectPromise");
+
+promise_test((t) => {
+    return promise_rejects_js(t, TypeError, TestBinding.staticInternalThrowToRejectPromise(Number.MAX_SAFE_INTEGER + 1));
+}, "staticInternalThrowToRejectPromise");
+
+promise_test((t) => {
+    return promise_rejects_js(t, TypeError, binding.methodInternalThrowToRejectPromise(Number.MAX_SAFE_INTEGER + 1));
+}, "methodInternalThrowToRejectPromise");