From cb3ecd4417194be58e219d1cf725b3ab6ec02a3a Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Wed, 12 Feb 2025 00:41:00 -0500 Subject: [PATCH] bindings: Support non-object this values for callbacks. (#35427) Signed-off-by: Josh Matthews --- components/script/dom/bindings/callback.rs | 20 +++++++++-------- components/script/dom/bindings/import.rs | 4 ++-- .../script_bindings/codegen/CodegenRust.py | 22 +++++++++---------- 3 files changed, 24 insertions(+), 22 deletions(-) diff --git a/components/script/dom/bindings/callback.rs b/components/script/dom/bindings/callback.rs index d328729a9e3..a33cc49e8f0 100644 --- a/components/script/dom/bindings/callback.rs +++ b/components/script/dom/bindings/callback.rs @@ -7,7 +7,6 @@ use std::default::Default; use std::ffi::CString; use std::mem::drop; -use std::ptr; use std::rc::Rc; use js::jsapi::{ @@ -15,7 +14,7 @@ use js::jsapi::{ }; use js::jsval::{JSVal, ObjectValue, UndefinedValue}; use js::rust::wrappers::{JS_GetProperty, JS_WrapObject}; -use js::rust::{MutableHandleObject, Runtime}; +use js::rust::{MutableHandleValue, Runtime}; use crate::dom::bindings::codegen::Bindings::WindowBinding::Window_Binding::WindowMethods; use crate::dom::bindings::error::{report_pending_exception, Error, Fallible}; @@ -208,19 +207,22 @@ impl CallbackInterface { pub(crate) use script_bindings::callback::ThisReflector; /// Wraps the reflector for `p` into the realm of `cx`. -pub(crate) fn wrap_call_this_object( +pub(crate) fn wrap_call_this_value( cx: JSContext, p: &T, - mut rval: MutableHandleObject, -) { - rval.set(p.jsobject()); - assert!(!rval.get().is_null()); + mut rval: MutableHandleValue, +) -> bool { + rooted!(in(*cx) let mut obj = p.jsobject()); + assert!(!obj.is_null()); unsafe { - if !JS_WrapObject(*cx, rval) { - rval.set(ptr::null_mut()); + if !JS_WrapObject(*cx, obj.handle_mut()) { + return false; } } + + rval.set(ObjectValue(*obj)); + true } /// A class that performs whatever setup we need to safely make a call while diff --git a/components/script/dom/bindings/import.rs b/components/script/dom/bindings/import.rs index 30c23d33e2f..756e8ef9f39 100644 --- a/components/script/dom/bindings/import.rs +++ b/components/script/dom/bindings/import.rs @@ -14,11 +14,11 @@ pub(crate) mod base { }; pub(crate) use js::jsval::{JSVal, NullValue, ObjectOrNullValue, ObjectValue, UndefinedValue}; pub(crate) use js::panic::maybe_resume_unwind; - pub(crate) use js::rust::wrappers::{JS_CallFunctionValue, JS_WrapValue}; + pub(crate) use js::rust::wrappers::{Call, JS_WrapValue}; pub(crate) use js::rust::{HandleObject, HandleValue, MutableHandleObject, MutableHandleValue}; pub(crate) use crate::dom::bindings::callback::{ - wrap_call_this_object, CallSetup, CallbackContainer, CallbackFunction, CallbackInterface, + wrap_call_this_value, CallSetup, CallbackContainer, CallbackFunction, CallbackInterface, CallbackObject, ExceptionHandling, ThisReflector, }; pub(crate) use crate::dom::bindings::codegen::Bindings::AudioNodeBinding::{ diff --git a/components/script_bindings/codegen/CodegenRust.py b/components/script_bindings/codegen/CodegenRust.py index 0ad8de1a567..713291686af 100644 --- a/components/script_bindings/codegen/CodegenRust.py +++ b/components/script_bindings/codegen/CodegenRust.py @@ -7658,13 +7658,13 @@ class CGCallback(CGClass): # Strip out the JSContext*/JSObject* args # that got added. assert args[0].name == "cx" and args[0].argType == "SafeJSContext" - assert args[1].name == "aThisObj" and args[1].argType == "HandleObject" + assert args[1].name == "aThisObj" and args[1].argType == "HandleValue" args = args[2:] # Record the names of all the arguments, so we can use them when we call # the private method. argnames = [arg.name for arg in args] - argnamesWithThis = ["s.get_context()", "thisObjJS.handle()"] + argnames - argnamesWithoutThis = ["s.get_context()", "thisObjJS.handle()"] + argnames + argnamesWithThis = ["s.get_context()", "thisValue.handle()"] + argnames + argnamesWithoutThis = ["s.get_context()", "HandleValue::undefined()"] + argnames # Now that we've recorded the argnames for our call to our private # method, insert our optional argument for deciding whether the # CallSetup should re-throw exceptions on aRv. @@ -7683,14 +7683,14 @@ class CGCallback(CGClass): setupCall = "let s = CallSetup::new(self, aExceptionHandling);\n" bodyWithThis = ( - f"{setupCall}rooted!(in(*s.get_context()) let mut thisObjJS = ptr::null_mut::());\n" - "wrap_call_this_object(s.get_context(), thisObj, thisObjJS.handle_mut());\n" - "if thisObjJS.is_null() {\n" + f"{setupCall}rooted!(in(*s.get_context()) let mut thisValue: JSVal);\n" + "let wrap_result = wrap_call_this_value(s.get_context(), thisObj, thisValue.handle_mut());\n" + "if !wrap_result {\n" " return Err(JSFailed);\n" "}\n" f"unsafe {{ self.{method.name}({', '.join(argnamesWithThis)}) }}") bodyWithoutThis = ( - f"{setupCall}rooted!(in(*s.get_context()) let thisObjJS = ptr::null_mut::());\n" + f"{setupCall}\n" f"unsafe {{ self.{method.name}({', '.join(argnamesWithoutThis)}) }}") return [ClassMethod(f'{method.name}_', method.returnType, args, bodyInHeader=True, @@ -7931,7 +7931,7 @@ class CallbackMember(CGNativeMember): # We want to allow the caller to pass in a "this" object, as # well as a JSContext. return [Argument("SafeJSContext", "cx"), - Argument("HandleObject", "aThisObj")] + args + Argument("HandleValue", "aThisObj")] + args def getCallSetup(self): if self.needThisHandling: @@ -7982,7 +7982,7 @@ class CallbackMethod(CallbackMember): suffix = "" if self.usingOutparam else ".handle_mut()" return (f"{self.getCallableDecl()}" f"rooted!(in(*cx) let rootedThis = {self.getThisObj()});\n" - f"let ok = {self.getCallGuard()}JS_CallFunctionValue(\n" + f"let ok = {self.getCallGuard()}Call(\n" " *cx, rootedThis.handle(), callable.handle(),\n" " &HandleValueArray {\n" f" length_: {argc} as ::libc::size_t,\n" @@ -8023,11 +8023,11 @@ class CallbackOperationBase(CallbackMethod): def getThisObj(self): if not self.singleOperation: - return "self.callback()" + return "ObjectValue(self.callback())" # This relies on getCallableDecl declaring a boolean # isCallable in the case when we're a single-operation # interface. - return "if isCallable { aThisObj.get() } else { self.callback() }" + return "if isCallable { aThisObj.get() } else { ObjectValue(self.callback()) }" def getCallableDecl(self): getCallableFromProp = f'self.parent.get_callable_property(cx, "{self.methodName}")?'