Remove some usage of unsafe code in Promise

components/script/dom/bindings/codegen/CodegenRust.py

@@ -810,7 +810,7 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None,
                 if !JS_WrapValue(*cx, valueToResolve.handle_mut()) {
                 $*{exceptionCode}
                 }
-                match Promise::new_resolved(&promiseGlobal, *cx, valueToResolve.handle()) {
+                match Promise::new_resolved(&promiseGlobal, cx, valueToResolve.handle()) {
                     Ok(value) => value,
                     Err(error) => {
                     throw_dom_exception(*cx, &promiseGlobal, error);

components/script/dom/promise.rs

@@ -18,6 +18,7 @@ use crate::dom::bindings::reflector::{DomObject, MutDomObject, Reflector};
 use crate::dom::bindings::utils::AsCCharPtrPtr;
 use crate::dom::globalscope::GlobalScope;
 use crate::dom::promisenativehandler::PromiseNativeHandler;
+use crate::script_runtime::JSContext as SafeJSContext;
 use dom_struct::dom_struct;
 use js::conversions::ToJSValConvertible;
 use js::jsapi::{AddRawValueRoot, CallArgs, GetFunctionNativeReserved};
@@ -49,20 +50,21 @@ pub struct Promise {
 
 /// Private helper to enable adding new methods to Rc<Promise>.
 trait PromiseHelper {
-    #[allow(unsafe_code)]
+    fn initialize(&self, cx: SafeJSContext);
-    unsafe fn initialize(&self, cx: *mut JSContext);
 }
 
 impl PromiseHelper for Rc<Promise> {
     #[allow(unsafe_code)]
-    unsafe fn initialize(&self, cx: *mut JSContext) {
+    fn initialize(&self, cx: SafeJSContext) {
         let obj = self.reflector().get_jsobject();
         self.permanent_js_root.set(ObjectValue(*obj));
-        assert!(AddRawValueRoot(
+        unsafe {
-            cx,
+            assert!(AddRawValueRoot(
-            self.permanent_js_root.get_unsafe(),
+                *cx,
-            b"Promise::root\0".as_c_char_ptr()
+                self.permanent_js_root.get_unsafe(),
-        ));
+                b"Promise::root\0".as_c_char_ptr()
+            ));
+        }
     }
 }
 
@@ -86,75 +88,72 @@ impl Promise {
         Promise::new_in_current_compartment(global, comp)
     }
 
-    #[allow(unsafe_code)]
     pub fn new_in_current_compartment(global: &GlobalScope, _comp: InCompartment) -> Rc<Promise> {
         let cx = global.get_cx();
         rooted!(in(*cx) let mut obj = ptr::null_mut::<JSObject>());
-        unsafe {
+        Promise::create_js_promise(cx, HandleObject::null(), obj.handle_mut());
-            Promise::create_js_promise(*cx, HandleObject::null(), obj.handle_mut());
+        Promise::new_with_js_promise(obj.handle(), cx)
-            Promise::new_with_js_promise(obj.handle(), *cx)
-        }
     }
 
     #[allow(unsafe_code)]
     pub fn duplicate(&self) -> Rc<Promise> {
         let cx = self.global().get_cx();
-        unsafe { Promise::new_with_js_promise(self.reflector().get_jsobject(), *cx) }
+        Promise::new_with_js_promise(self.reflector().get_jsobject(), cx)
     }
 
     #[allow(unsafe_code, unrooted_must_root)]
-    pub unsafe fn new_with_js_promise(obj: HandleObject, cx: *mut JSContext) -> Rc<Promise> {
+    pub fn new_with_js_promise(obj: HandleObject, cx: SafeJSContext) -> Rc<Promise> {
-        assert!(IsPromiseObject(obj));
+        unsafe {
-        let promise = Promise {
+            assert!(IsPromiseObject(obj));
-            reflector: Reflector::new(),
+            let promise = Promise {
-            permanent_js_root: Heap::default(),
+                reflector: Reflector::new(),
-        };
+                permanent_js_root: Heap::default(),
-        let mut promise = Rc::new(promise);
+            };
-        Rc::get_mut(&mut promise).unwrap().init_reflector(obj.get());
+            let mut promise = Rc::new(promise);
-        promise.initialize(cx);
+            Rc::get_mut(&mut promise).unwrap().init_reflector(obj.get());
-        promise
+            promise.initialize(cx);
+            promise
+        }
     }
 
     #[allow(unsafe_code)]
-    unsafe fn create_js_promise(
+    fn create_js_promise(cx: SafeJSContext, proto: HandleObject, mut obj: MutableHandleObject) {
-        cx: *mut JSContext,
+        unsafe {
-        proto: HandleObject,
+            let do_nothing_func = JS_NewFunction(
-        mut obj: MutableHandleObject,
+                *cx,
-    ) {
+                Some(do_nothing_promise_executor),
-        let do_nothing_func = JS_NewFunction(
+                /* nargs = */ 2,
-            cx,
+                /* flags = */ 0,
-            Some(do_nothing_promise_executor),
+                ptr::null(),
-            /* nargs = */ 2,
+            );
-            /* flags = */ 0,
+            assert!(!do_nothing_func.is_null());
-            ptr::null(),
+            rooted!(in(*cx) let do_nothing_obj = JS_GetFunctionObject(do_nothing_func));
-        );
+            assert!(!do_nothing_obj.is_null());
-        assert!(!do_nothing_func.is_null());
+            obj.set(NewPromiseObject(*cx, do_nothing_obj.handle(), proto));
-        rooted!(in(cx) let do_nothing_obj = JS_GetFunctionObject(do_nothing_func));
+            assert!(!obj.is_null());
-        assert!(!do_nothing_obj.is_null());
+        }
-        obj.set(NewPromiseObject(cx, do_nothing_obj.handle(), proto));
-        assert!(!obj.is_null());
     }
 
     #[allow(unrooted_must_root, unsafe_code)]
-    pub unsafe fn new_resolved(
+    pub fn new_resolved(
         global: &GlobalScope,
-        cx: *mut JSContext,
+        cx: SafeJSContext,
         value: HandleValue,
     ) -> Fallible<Rc<Promise>> {
-        let _ac = JSAutoRealm::new(cx, global.reflector().get_jsobject().get());
+        let _ac = JSAutoRealm::new(*cx, global.reflector().get_jsobject().get());
-        rooted!(in(cx) let p = CallOriginalPromiseResolve(cx, value));
+        rooted!(in(*cx) let p = unsafe { CallOriginalPromiseResolve(*cx, value) });
         assert!(!p.handle().is_null());
         Ok(Promise::new_with_js_promise(p.handle(), cx))
     }
 
     #[allow(unrooted_must_root, unsafe_code)]
-    pub unsafe fn new_rejected(
+    pub fn new_rejected(
         global: &GlobalScope,
-        cx: *mut JSContext,
+        cx: SafeJSContext,
         value: HandleValue,
     ) -> Fallible<Rc<Promise>> {
-        let _ac = JSAutoRealm::new(cx, global.reflector().get_jsobject().get());
+        let _ac = JSAutoRealm::new(*cx, global.reflector().get_jsobject().get());
-        rooted!(in(cx) let p = CallOriginalPromiseReject(cx, value));
+        rooted!(in(*cx) let p = unsafe { CallOriginalPromiseReject(*cx, value) });
         assert!(!p.handle().is_null());
         Ok(Promise::new_with_js_promise(p.handle(), cx))
     }
@@ -169,14 +168,16 @@ impl Promise {
         rooted!(in(*cx) let mut v = UndefinedValue());
         unsafe {
             val.to_jsval(*cx, v.handle_mut());
-            self.resolve(*cx, v.handle());
         }
+        self.resolve(cx, v.handle());
     }
 
     #[allow(unrooted_must_root, unsafe_code)]
-    pub unsafe fn resolve(&self, cx: *mut JSContext, value: HandleValue) {
+    pub fn resolve(&self, cx: SafeJSContext, value: HandleValue) {
-        if !ResolvePromise(cx, self.promise_obj(), value) {
+        unsafe {
-            JS_ClearPendingException(cx);
+            if !ResolvePromise(*cx, self.promise_obj(), value) {
+                JS_ClearPendingException(*cx);
+            }
         }
     }
 
@@ -190,8 +191,8 @@ impl Promise {
         rooted!(in(*cx) let mut v = UndefinedValue());
         unsafe {
             val.to_jsval(*cx, v.handle_mut());
-            self.reject(*cx, v.handle());
         }
+        self.reject(cx, v.handle());
     }
 
     #[allow(unsafe_code)]
@@ -201,14 +202,16 @@ impl Promise {
         rooted!(in(*cx) let mut v = UndefinedValue());
         unsafe {
             error.to_jsval(*cx, &self.global(), v.handle_mut());
-            self.reject(*cx, v.handle());
         }
+        self.reject(cx, v.handle());
     }
 
     #[allow(unrooted_must_root, unsafe_code)]
-    pub unsafe fn reject(&self, cx: *mut JSContext, value: HandleValue) {
+    pub fn reject(&self, cx: SafeJSContext, value: HandleValue) {
-        if !RejectPromise(cx, self.promise_obj(), value) {
+        unsafe {
-            JS_ClearPendingException(cx);
+            if !RejectPromise(*cx, self.promise_obj(), value) {
+                JS_ClearPendingException(*cx);
+            }
         }
     }
 

components/script/dom/testbinding.rs

@@ -941,29 +941,21 @@ impl TestBindingMethods for TestBinding {
     }
 
     #[allow(unrooted_must_root)]
-    #[allow(unsafe_code)]
     fn ReturnResolvedPromise(&self, cx: SafeJSContext, v: HandleValue) -> Fallible<Rc<Promise>> {
-        unsafe { Promise::new_resolved(&self.global(), *cx, v) }
+        Promise::new_resolved(&self.global(), cx, v)
     }
 
     #[allow(unrooted_must_root)]
-    #[allow(unsafe_code)]
     fn ReturnRejectedPromise(&self, cx: SafeJSContext, v: HandleValue) -> Fallible<Rc<Promise>> {
-        unsafe { Promise::new_rejected(&self.global(), *cx, v) }
+        Promise::new_rejected(&self.global(), cx, v)
     }
 
-    #[allow(unsafe_code)]
     fn PromiseResolveNative(&self, cx: SafeJSContext, p: &Promise, v: HandleValue) {
-        unsafe {
+        p.resolve(cx, v);
-            p.resolve(*cx, v);
-        }
     }
 
-    #[allow(unsafe_code)]
     fn PromiseRejectNative(&self, cx: SafeJSContext, p: &Promise, v: HandleValue) {
-        unsafe {
+        p.reject(cx, v);
-            p.reject(*cx, v);
-        }
     }
 
     fn PromiseRejectWithTypeError(&self, p: &Promise, s: USVString) {

components/script/script_runtime.rs

@@ -201,7 +201,8 @@ unsafe extern "C" fn promise_rejection_tracker(
     // TODO: Step 2 - If script's muted errors is true, terminate these steps.
 
     // Step 3.
-    let global = GlobalScope::from_context(cx);
+    let cx = JSContext::from_ptr(cx);
+    let global = GlobalScope::from_context(*cx);
 
     wrap_panic(
         AssertUnwindSafe(|| {
@@ -281,7 +282,7 @@ pub fn notify_about_rejected_promises(global: &GlobalScope) {
                 .iter()
                 .map(|promise| {
                     let promise =
-                        Promise::new_with_js_promise(Handle::from_raw(promise.handle()), *cx);
+                        Promise::new_with_js_promise(Handle::from_raw(promise.handle()), cx);
 
                     TrustedPromise::new(promise)
                 })