From f3bec0aed386615e850b9e24b74c697624c32bce Mon Sep 17 00:00:00 2001 From: Samson <16504129+sagudev@users.noreply.github.com> Date: Sat, 3 Aug 2024 14:58:37 +0200 Subject: [PATCH] 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> --- .../dom/bindings/codegen/CodegenRust.py | 136 +++++++++++++++++- components/script/dom/bindings/import.rs | 10 +- components/script/dom/bindings/utils.rs | 57 ++++++-- components/script/dom/testbinding.rs | 23 +++ .../script/dom/webidls/TestBinding.webidl | 10 ++ .../fetch/api/idlharness.any.js.ini | 2 - .../wpt/meta/fetch/api/idlharness.any.js.ini | 2 - .../mozilla/exceptionToRejection.any.js.ini | 7 + tests/wpt/mozilla/meta/MANIFEST.json | 25 ++++ .../mozilla/exceptionToRejection.any.js.ini | 7 + .../tests/mozilla/exceptionToRejection.any.js | 32 +++++ 11 files changed, 291 insertions(+), 20 deletions(-) create mode 100644 tests/wpt/mozilla/meta-legacy-layout/mozilla/exceptionToRejection.any.js.ini create mode 100644 tests/wpt/mozilla/meta/mozilla/exceptionToRejection.any.js.ini create mode 100644 tests/wpt/mozilla/tests/mozilla/exceptionToRejection.any.js diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index cc564a83745..e358233b045 100644 --- 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(): diff --git a/components/script/dom/bindings/import.rs b/components/script/dom/bindings/import.rs index f2c6a5e2b81..44823aa30cd 100644 --- 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}; diff --git a/components/script/dom/bindings/utils.rs b/components/script/dom/bindings/utils.rs index 522b6f46686..764e604620c 100644 --- 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 + } +} diff --git a/components/script/dom/testbinding.rs b/components/script/dom/testbinding.rs index 344d41af031..0177a6d89d1 100644 --- 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> { + Err(Error::Type("test".to_string())) + } + + fn GetGetterThrowToRejectPromise(&self) -> Fallible> { + Err(Error::Type("test".to_string())) + } + + fn MethodInternalThrowToRejectPromise(&self, _arg: u64) -> Rc { + unreachable!("Method should already throw") + } +} + +#[allow(non_snake_case)] +impl TestBinding { + pub fn StaticThrowToRejectPromise(_: &GlobalScope) -> Fallible> { + Err(Error::Type("test".to_string())) + } + + pub fn StaticInternalThrowToRejectPromise(_: &GlobalScope, _arg: u64) -> Rc { + unreachable!("Method should already throw") + } } #[allow(non_snake_case)] diff --git a/components/script/dom/webidls/TestBinding.webidl b/components/script/dom/webidls/TestBinding.webidl index 5c00dbda387..bd6cb2432a4 100644 --- a/components/script/dom/webidls/TestBinding.webidl +++ b/components/script/dom/webidls/TestBinding.webidl @@ -574,6 +574,16 @@ interface TestBinding { undefined promiseRejectWithTypeError(Promise p, USVString message); undefined resolvePromiseDelayed(Promise p, DOMString value, unsigned long long ms); + [Throws] + static Promise staticThrowToRejectPromise(); + [Throws] + Promise methodThrowToRejectPromise(); + [Throws] + readonly attribute Promise getterThrowToRejectPromise; + + static Promise staticInternalThrowToRejectPromise([EnforceRange] unsigned long long arg); + Promise methodInternalThrowToRejectPromise([EnforceRange] unsigned long long arg); + undefined panic(); GlobalScope entryGlobal(); diff --git a/tests/wpt/meta-legacy-layout/fetch/api/idlharness.any.js.ini b/tests/wpt/meta-legacy-layout/fetch/api/idlharness.any.js.ini index 4c76c92b40e..386713b9c51 100644 --- 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 diff --git a/tests/wpt/meta/fetch/api/idlharness.any.js.ini b/tests/wpt/meta/fetch/api/idlharness.any.js.ini index ad5ad110157..ccd8fcfb044 100644 --- 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 diff --git a/tests/wpt/mozilla/meta-legacy-layout/mozilla/exceptionToRejection.any.js.ini b/tests/wpt/mozilla/meta-legacy-layout/mozilla/exceptionToRejection.any.js.ini new file mode 100644 index 00000000000..7d7e4d9cdc7 --- /dev/null +++ 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] \ No newline at end of file diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index ed0714ae18a..1192101b30f 100644 --- 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", [ diff --git a/tests/wpt/mozilla/meta/mozilla/exceptionToRejection.any.js.ini b/tests/wpt/mozilla/meta/mozilla/exceptionToRejection.any.js.ini new file mode 100644 index 00000000000..7d7e4d9cdc7 --- /dev/null +++ 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] \ No newline at end of file diff --git a/tests/wpt/mozilla/tests/mozilla/exceptionToRejection.any.js b/tests/wpt/mozilla/tests/mozilla/exceptionToRejection.any.js new file mode 100644 index 00000000000..738a8bedbcf --- /dev/null +++ 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 staticThrowToRejectPromise(); + Promise methodThrowToRejectPromise(); + readonly attribute Promise getterThrowToRejectPromise; + + static Promise staticInternalThrowToRejectPromise([EnforceRange] unsigned long long arg); + Promise 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");