Skip to content

Commit

Permalink
Use safe JSContext in callbacks
Browse files Browse the repository at this point in the history
  • Loading branch information
@marmeladema
marmeladema committed Aug 8, 2019
1 parent 0703a1a commit 357b6c5
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 31 deletions.
46 changes: 21 additions & 25 deletions components/script/dom/bindings/callback.rs
View file
Expand Up @@ -13,10 +13,10 @@ use crate::dom::bindings::settings_stack::{AutoEntryScript, AutoIncumbentScript}
use crate::dom::bindings::utils::AsCCharPtrPtr;
use crate::dom::globalscope::GlobalScope;
use crate::dom::window::Window;
use crate::script_runtime::JSContext as SafeJSContext;
use crate::script_runtime::JSContext;
use js::jsapi::Heap;
use js::jsapi::JSAutoRealm;
use js::jsapi::{AddRawValueRoot, IsCallable, JSContext, JSObject};
use js::jsapi::{AddRawValueRoot, IsCallable, JSObject};
use js::jsapi::{EnterRealm, LeaveRealm, Realm, RemoveRawValueRoot};
use js::jsval::{JSVal, ObjectValue, UndefinedValue};
use js::rust::wrappers::{JS_GetProperty, JS_WrapObject};
Expand Down Expand Up @@ -82,11 +82,11 @@ impl CallbackObject {
}

#[allow(unsafe_code)]
unsafe fn init(&mut self, cx: *mut JSContext, callback: *mut JSObject) {
unsafe fn init(&mut self, cx: JSContext, callback: *mut JSObject) {
self.callback.set(callback);
self.permanent_js_root.set(ObjectValue(callback));
assert!(AddRawValueRoot(
cx,
*cx,
self.permanent_js_root.get_unsafe(),
b"CallbackObject::root\n".as_c_char_ptr()
));
Expand All @@ -113,7 +113,7 @@ impl PartialEq for CallbackObject {
/// callback interface types.
pub trait CallbackContainer {
/// Create a new CallbackContainer object for the given `JSObject`.
unsafe fn new(cx: SafeJSContext, callback: *mut JSObject) -> Rc<Self>;
unsafe fn new(cx: JSContext, callback: *mut JSObject) -> Rc<Self>;
/// Returns the underlying `CallbackObject`.
fn callback_holder(&self) -> &CallbackObject;
/// Returns the underlying `JSObject`.
Expand Down Expand Up @@ -152,8 +152,8 @@ impl CallbackFunction {

/// Initialize the callback function with a value.
/// Should be called once this object is done moving.
pub unsafe fn init(&mut self, cx: SafeJSContext, callback: *mut JSObject) {
self.object.init(*cx, callback);
pub unsafe fn init(&mut self, cx: JSContext, callback: *mut JSObject) {
self.object.init(cx, callback);
}
}

Expand All @@ -179,18 +179,18 @@ impl CallbackInterface {

/// Initialize the callback function with a value.
/// Should be called once this object is done moving.
pub unsafe fn init(&mut self, cx: SafeJSContext, callback: *mut JSObject) {
self.object.init(*cx, callback);
pub unsafe fn init(&mut self, cx: JSContext, callback: *mut JSObject) {
self.object.init(cx, callback);
}

/// Returns the property with the given `name`, if it is a callable object,
/// or an error otherwise.
pub fn get_callable_property(&self, cx: *mut JSContext, name: &str) -> Fallible<JSVal> {
rooted!(in(cx) let mut callable = UndefinedValue());
rooted!(in(cx) let obj = self.callback_holder().get());
pub fn get_callable_property(&self, cx: JSContext, name: &str) -> Fallible<JSVal> {
rooted!(in(*cx) let mut callable = UndefinedValue());
rooted!(in(*cx) let obj = self.callback_holder().get());
unsafe {
let c_name = CString::new(name).unwrap();
if !JS_GetProperty(cx, obj.handle(), c_name.as_ptr(), callable.handle_mut()) {
if !JS_GetProperty(*cx, obj.handle(), c_name.as_ptr(), callable.handle_mut()) {
return Err(Error::JSFailed);
}

Expand All @@ -206,16 +206,12 @@ impl CallbackInterface {
}

/// Wraps the reflector for `p` into the compartment of `cx`.
pub fn wrap_call_this_object<T: DomObject>(
cx: *mut JSContext,
p: &T,
mut rval: MutableHandleObject,
) {
pub fn wrap_call_this_object<T: DomObject>(cx: JSContext, p: &T, mut rval: MutableHandleObject) {
rval.set(p.reflector().get_jsobject().get());
assert!(!rval.get().is_null());

unsafe {
if !JS_WrapObject(cx, rval) {
if !JS_WrapObject(*cx, rval) {
rval.set(ptr::null_mut());
}
}
Expand All @@ -228,7 +224,7 @@ pub struct CallSetup {
/// (possibly wrapped) callback object.
exception_global: DomRoot<GlobalScope>,
/// The `JSContext` used for the call.
cx: *mut JSContext,
cx: JSContext,
/// The compartment we were in before the call.
old_realm: *mut Realm,
/// The exception handling used for the call.
Expand All @@ -255,7 +251,7 @@ impl CallSetup {
let ais = callback.incumbent().map(AutoIncumbentScript::new);
CallSetup {
exception_global: global,
cx: *cx,
cx: cx,
old_realm: unsafe { EnterRealm(*cx, callback.callback()) },
handling: handling,
entry_script: Some(aes),
Expand All @@ -264,21 +260,21 @@ impl CallSetup {
}

/// Returns the `JSContext` used for the call.
pub fn get_context(&self) -> *mut JSContext {
pub fn get_context(&self) -> JSContext {
self.cx
}
}

impl Drop for CallSetup {
fn drop(&mut self) {
unsafe {
LeaveRealm(self.cx, self.old_realm);
LeaveRealm(*self.cx, self.old_realm);
if self.handling == ExceptionHandling::Report {
let _ac = JSAutoRealm::new(
self.cx,
*self.cx,
self.exception_global.reflector().get_jsobject().get(),
);
report_pending_exception(self.cx, true);
report_pending_exception(*self.cx, true);
}
drop(self.incumbent_script.take());
drop(self.entry_script.take().unwrap());
Expand Down
12 changes: 6 additions & 6 deletions components/script/dom/bindings/codegen/CodegenRust.py
View file
Expand Up @@ -6828,8 +6828,8 @@ def getMethodImpls(self, method):
# 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 = ["SafeJSContext::from_ptr(s.get_context())", "thisObjJS.handle()"] + argnames
argnamesWithoutThis = ["SafeJSContext::from_ptr(s.get_context())", "thisObjJS.handle()"] + argnames
argnamesWithThis = ["s.get_context()", "thisObjJS.handle()"] + argnames
argnamesWithoutThis = ["s.get_context()", "thisObjJS.handle()"] + 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.
Expand All @@ -6849,7 +6849,7 @@ def getMethodImpls(self, method):

bodyWithThis = string.Template(
setupCall +
"rooted!(in(s.get_context()) let mut thisObjJS = ptr::null_mut::<JSObject>());\n"
"rooted!(in(*s.get_context()) let mut thisObjJS = ptr::null_mut::<JSObject>());\n"
"wrap_call_this_object(s.get_context(), thisObj, thisObjJS.handle_mut());\n"
"if thisObjJS.is_null() {\n"
" return Err(JSFailed);\n"
Expand All @@ -6860,7 +6860,7 @@ def getMethodImpls(self, method):
})
bodyWithoutThis = string.Template(
setupCall +
"rooted!(in(s.get_context()) let thisObjJS = ptr::null_mut::<JSObject>());\n"
"rooted!(in(*s.get_context()) let thisObjJS = ptr::null_mut::<JSObject>());\n"
"unsafe { ${methodName}(${callArgs}) }").substitute({
"callArgs": ", ".join(argnamesWithoutThis),
"methodName": 'self.' + method.name,
Expand Down Expand Up @@ -7118,7 +7118,7 @@ def getCallSetup(self):
return ""
return (
"CallSetup s(CallbackPreserveColor(), aRv, aExceptionHandling);\n"
"JSContext* cx = s.get_context();\n"
"JSContext* cx = *s.get_context();\n"
"if (!cx) {\n"
" return Err(JSFailed);\n"
"}\n")
Expand Down Expand Up @@ -7219,7 +7219,7 @@ def getCallableDecl(self):
"methodName": self.methodName
}
getCallableFromProp = string.Template(
'r#try!(self.parent.get_callable_property(*cx, "${methodName}"))'
'r#try!(self.parent.get_callable_property(cx, "${methodName}"))'
).substitute(replacements)
if not self.singleOperation:
return 'rooted!(in(*cx) let callable =\n' + getCallableFromProp + ');\n'
Expand Down

0 comments on commit 357b6c5

Please sign in to comment.