From c37d5572fd01dc52022424311c7cc46e47ed6fda Mon Sep 17 00:00:00 2001 From: Gae24 <96017547+Gae24@users.noreply.github.com> Date: Mon, 12 May 2025 13:05:46 +0200 Subject: [PATCH] codegen: use `FromJSValConvertible` trait for `Promise` (#36966) Before it was only used when converting to a `Record`, using it all the times allow us to remove two methods. Plus added a helper method in CodegenRust.py to avoid repeated code. Testing: a successful build and existing tests should cover the changes. Fixes: #36410 --------- Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com> --- components/script/dom/globalscope.rs | 4 - components/script/dom/promise.rs | 17 +--- .../script_bindings/codegen/Bindings.conf | 2 +- .../script_bindings/codegen/CodegenRust.py | 97 +++++-------------- components/script_bindings/import.rs | 6 +- components/script_bindings/interfaces.rs | 19 ---- 6 files changed, 27 insertions(+), 118 deletions(-) diff --git a/components/script/dom/globalscope.rs b/components/script/dom/globalscope.rs index 902d4622db9..55db2e4d248 100644 --- a/components/script/dom/globalscope.rs +++ b/components/script/dom/globalscope.rs @@ -3562,10 +3562,6 @@ impl GlobalScopeHelpers for GlobalScope { GlobalScope::from_reflector(reflector, realm) } - unsafe fn from_object_maybe_wrapped(obj: *mut JSObject, cx: *mut JSContext) -> DomRoot { - GlobalScope::from_object_maybe_wrapped(obj, cx) - } - fn origin(&self) -> &MutableOrigin { GlobalScope::origin(self) } diff --git a/components/script/dom/promise.rs b/components/script/dom/promise.rs index 80b62f161bc..0efffbe6fe2 100644 --- a/components/script/dom/promise.rs +++ b/components/script/dom/promise.rs @@ -29,7 +29,6 @@ use js::rust::wrappers::{ ResolvePromise, SetAnyPromiseIsHandled, SetPromiseUserInputEventHandlingState, }; use js::rust::{HandleObject, HandleValue, MutableHandleObject, Runtime}; -use script_bindings::interfaces::PromiseHelpers; use crate::dom::bindings::conversions::root_from_object; use crate::dom::bindings::error::{Error, ErrorToJsval}; @@ -388,16 +387,6 @@ fn create_native_handler_function( } } -impl PromiseHelpers for Promise { - fn new_resolved( - global: &GlobalScope, - cx: SafeJSContext, - value: impl ToJSValConvertible, - ) -> Rc { - Promise::new_resolved(global, cx, value, CanGc::note()) - } -} - impl FromJSValConvertibleRc for Promise { #[allow(unsafe_code)] unsafe fn from_jsval( @@ -407,16 +396,12 @@ impl FromJSValConvertibleRc for Promise { if value.get().is_null() { return Ok(ConversionResult::Failure("null not allowed".into())); } - if !value.get().is_object() { - return Ok(ConversionResult::Failure("not an object".into())); - } - rooted!(in(cx) let obj = value.get().to_object()); let cx = SafeJSContext::from_ptr(cx); let in_realm_proof = AlreadyInRealm::assert_for_cx(cx); let global_scope = GlobalScope::from_context(*cx, InRealm::Already(&in_realm_proof)); - let promise = Promise::new_resolved(&global_scope, cx, *obj, CanGc::note()); + let promise = Promise::new_resolved(&global_scope, cx, value, CanGc::note()); Ok(ConversionResult::Success(promise)) } } diff --git a/components/script_bindings/codegen/Bindings.conf b/components/script_bindings/codegen/Bindings.conf index 2a9874a386f..875a9498078 100644 --- a/components/script_bindings/codegen/Bindings.conf +++ b/components/script_bindings/codegen/Bindings.conf @@ -538,7 +538,7 @@ DOMInterfaces = { 'Promise': { 'spiderMonkeyInterface': True, - 'additionalTraits': ["crate::interfaces::PromiseHelpers", "js::conversions::FromJSValConvertibleRc"] + 'additionalTraits': ["js::conversions::FromJSValConvertibleRc"] }, 'Range': { diff --git a/components/script_bindings/codegen/CodegenRust.py b/components/script_bindings/codegen/CodegenRust.py index 48f024be70f..458aa7508b0 100644 --- a/components/script_bindings/codegen/CodegenRust.py +++ b/components/script_bindings/codegen/CodegenRust.py @@ -742,6 +742,19 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, "}") return templateBody + # A helper function for types that implement FromJSValConvertible trait + def fromJSValTemplate(config, errorHandler, exceptionCode): + return f"""match FromJSValConvertible::from_jsval(*cx, ${{val}}, {config}) {{ + Ok(ConversionResult::Success(value)) => value, + Ok(ConversionResult::Failure(error)) => {{ + {errorHandler} + }} + _ => {{ + {exceptionCode} + }}, +}} +""" + assert not (isEnforceRange and isClamp) # These are mutually exclusive if type.isSequence() or type.isRecord(): @@ -755,13 +768,7 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, if type.nullable(): declType = CGWrapper(declType, pre="Option<", post=" >") - templateBody = (f"match FromJSValConvertible::from_jsval(*cx, ${{val}}, {config}) {{\n" - " Ok(ConversionResult::Success(value)) => value,\n" - " Ok(ConversionResult::Failure(error)) => {\n" - f"{indent(failOrPropagate, 8)}\n" - " }\n" - f" _ => {{ {exceptionCode} }},\n" - "}") + templateBody = fromJSValTemplate(config, failOrPropagate, exceptionCode) return handleOptional(templateBody, declType, handleDefault("None")) @@ -770,13 +777,7 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, if type.nullable(): declType = CGWrapper(declType, pre="Option<", post=" >") - templateBody = ("match FromJSValConvertible::from_jsval(*cx, ${val}, ()) {\n" - " Ok(ConversionResult::Success(value)) => value,\n" - " Ok(ConversionResult::Failure(error)) => {\n" - f"{indent(failOrPropagate, 8)}\n" - " }\n" - f" _ => {{ {exceptionCode} }},\n" - "}") + templateBody = fromJSValTemplate("()", failOrPropagate, exceptionCode) dictionaries = [ memberType @@ -836,21 +837,7 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, # once again be providing a Promise to signal completion of an # operation, which would then not be exposed to anyone other than # our own implementation code. - templateBody = fill( - """ - { // Scope for our JSAutoRealm. - - rooted!(in(*cx) let globalObj = CurrentGlobalOrNull(*cx)); - let promiseGlobal = D::GlobalScope::from_object_maybe_wrapped(globalObj.handle().get(), *cx); - - rooted!(in(*cx) let mut valueToResolve = $${val}.get()); - if !JS_WrapValue(*cx, valueToResolve.handle_mut()) { - $*{exceptionCode} - } - D::Promise::new_resolved(&promiseGlobal, cx, valueToResolve.handle()) - } - """, - exceptionCode=exceptionCode) + templateBody = fromJSValTemplate("()", failOrPropagate, exceptionCode) if isArgument: declType = CGGeneric("&D::Promise") @@ -960,14 +947,7 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, if type.isDOMString(): nullBehavior = getConversionConfigForType(type, isEnforceRange, isClamp, treatNullAs) - conversionCode = ( - f"match FromJSValConvertible::from_jsval(*cx, ${{val}}, {nullBehavior}) {{\n" - " Ok(ConversionResult::Success(strval)) => strval,\n" - " Ok(ConversionResult::Failure(error)) => {\n" - f"{indent(failOrPropagate, 8)}\n" - " }\n" - f" _ => {{ {exceptionCode} }},\n" - "}") + conversionCode = fromJSValTemplate(nullBehavior, failOrPropagate, exceptionCode) if defaultValue is None: default = None @@ -989,14 +969,7 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, if type.isUSVString(): assert not isEnforceRange and not isClamp - conversionCode = ( - "match FromJSValConvertible::from_jsval(*cx, ${val}, ()) {\n" - " Ok(ConversionResult::Success(strval)) => strval,\n" - " Ok(ConversionResult::Failure(error)) => {\n" - f"{indent(failOrPropagate, 8)}\n" - " }\n" - f" _ => {{ {exceptionCode} }},\n" - "}") + conversionCode = fromJSValTemplate("()", failOrPropagate, exceptionCode) if defaultValue is None: default = None @@ -1018,14 +991,7 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, if type.isByteString(): assert not isEnforceRange and not isClamp - conversionCode = ( - "match FromJSValConvertible::from_jsval(*cx, ${val}, ()) {\n" - " Ok(ConversionResult::Success(strval)) => strval,\n" - " Ok(ConversionResult::Failure(error)) => {\n" - f"{indent(failOrPropagate, 8)}\n" - " }\n" - f" _ => {{ {exceptionCode} }},\n" - "}") + conversionCode = fromJSValTemplate("()", failOrPropagate, exceptionCode) if defaultValue is None: default = None @@ -1056,12 +1022,7 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, else: handleInvalidEnumValueCode = "return true;" - template = ( - "match FromJSValConvertible::from_jsval(*cx, ${val}, ()) {" - f" Err(_) => {{ {exceptionCode} }},\n" - " Ok(ConversionResult::Success(v)) => v,\n" - f" Ok(ConversionResult::Failure(error)) => {{ {handleInvalidEnumValueCode} }},\n" - "}") + template = fromJSValTemplate("()", handleInvalidEnumValueCode, exceptionCode) if defaultValue is not None: assert defaultValue.type.tag() == IDLType.Tags.domstring @@ -1192,14 +1153,7 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, if type_needs_tracing(type): declType = CGTemplatedType("RootedTraceableBox", declType) - template = ( - "match FromJSValConvertible::from_jsval(*cx, ${val}, ()) {\n" - " Ok(ConversionResult::Success(dictionary)) => dictionary,\n" - " Ok(ConversionResult::Failure(error)) => {\n" - f"{indent(failOrPropagate, 8)}\n" - " }\n" - f" _ => {{ {exceptionCode} }},\n" - "}") + template = fromJSValTemplate("()", failOrPropagate, exceptionCode) return handleOptional(template, declType, handleDefault(empty)) @@ -1220,14 +1174,7 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, if type.nullable(): declType = CGWrapper(declType, pre="Option<", post=">") - template = ( - f"match FromJSValConvertible::from_jsval(*cx, ${{val}}, {conversionBehavior}) {{\n" - " Ok(ConversionResult::Success(v)) => v,\n" - " Ok(ConversionResult::Failure(error)) => {\n" - f"{indent(failOrPropagate, 8)}\n" - " }\n" - f" _ => {{ {exceptionCode} }}\n" - "}") + template = fromJSValTemplate(conversionBehavior, failOrPropagate, exceptionCode) if defaultValue is not None: if isinstance(defaultValue, IDLNullValue): diff --git a/components/script_bindings/import.rs b/components/script_bindings/import.rs index 16cc92f07bf..25bd6c38669 100644 --- a/components/script_bindings/import.rs +++ b/components/script_bindings/import.rs @@ -11,12 +11,12 @@ pub(crate) mod base { }; pub(crate) use js::error::throw_type_error; pub(crate) use js::jsapi::{ - CurrentGlobalOrNull, HandleValue as RawHandleValue, HandleValueArray, Heap, IsCallable, - JS_NewObject, JSContext, JSObject, + HandleValue as RawHandleValue, HandleValueArray, Heap, IsCallable, JS_NewObject, JSContext, + JSObject, }; pub(crate) use js::jsval::{JSVal, NullValue, ObjectOrNullValue, ObjectValue, UndefinedValue}; pub(crate) use js::panic::maybe_resume_unwind; - pub(crate) use js::rust::wrappers::{Call, JS_WrapValue}; + pub(crate) use js::rust::wrappers::Call; pub(crate) use js::rust::{HandleObject, HandleValue, MutableHandleObject, MutableHandleValue}; pub(crate) use crate::callback::{ diff --git a/components/script_bindings/interfaces.rs b/components/script_bindings/interfaces.rs index b289737143e..58917283170 100644 --- a/components/script_bindings/interfaces.rs +++ b/components/script_bindings/interfaces.rs @@ -3,10 +3,8 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ use std::cell::RefCell; -use std::rc::Rc; use std::thread::LocalKey; -use js::conversions::ToJSValConvertible; use js::glue::JSPrincipalsCallbacks; use js::jsapi::{CallArgs, HandleObject as RawHandleObject, JSContext as RawJSContext, JSObject}; use js::rust::{HandleObject, MutableHandleObject}; @@ -78,14 +76,6 @@ pub trait GlobalScopeHelpers { unsafe fn from_object(obj: *mut JSObject) -> DomRoot; fn from_reflector(reflector: &impl DomObject, realm: InRealm) -> DomRoot; - /// # Safety - /// `obj` must point to a valid, non-null JSObject. - /// `cx` must point to a valid, non-null RawJSContext. - unsafe fn from_object_maybe_wrapped( - obj: *mut JSObject, - cx: *mut RawJSContext, - ) -> DomRoot; - fn origin(&self) -> &MutableOrigin; fn incumbent() -> Option>; @@ -101,15 +91,6 @@ pub trait DocumentHelpers { fn ensure_safe_to_run_script_or_layout(&self); } -/// Operations that must be invoked from the generated bindings. -pub trait PromiseHelpers { - fn new_resolved( - global: &D::GlobalScope, - cx: JSContext, - value: impl ToJSValConvertible, - ) -> Rc; -} - pub trait ServoInternalsHelpers { fn is_servo_internal(cx: JSContext, global: HandleObject) -> bool; }