script: make Error::to_jsval safe (#35411)

Signed-off-by: Stephen Muss <stephenmuss@gmail.com>

8 files changed, 41 insertions, 64 deletions

diff --git a/components/script/dom/bindings/error.rs b/components/script/dom/bindings/error.rs
index ac7951e4d4d..931c34eabdb 100644
--- a/components/script/dom/bindings/error.rs
+++ b/components/script/dom/bindings/error.rs
@@ -11,9 +11,7 @@ use backtrace::Backtrace;
 use js::error::{throw_range_error, throw_type_error};
 #[cfg(feature = "js_backtrace")]
 use js::jsapi::StackFormat as JSStackFormat;
-use js::jsapi::{
-    ExceptionStackBehavior, JSContext, JS_ClearPendingException, JS_IsExceptionPending,
-};
+use js::jsapi::{ExceptionStackBehavior, JS_ClearPendingException, JS_IsExceptionPending};
 use js::jsval::UndefinedValue;
 use js::rust::wrappers::{JS_ErrorFromException, JS_GetPendingException, JS_SetPendingException};
 use js::rust::{HandleObject, HandleValue, MutableHandleValue};
@@ -341,19 +339,21 @@ pub(crate) fn throw_constructor_without_new(cx: SafeJSContext, name: &str) {
 impl Error {
     /// Convert this error value to a JS value, consuming it in the process.
     #[allow(clippy::wrong_self_convention)]
-    pub(crate) unsafe fn to_jsval(
+    pub(crate) fn to_jsval(
         self,
-        cx: *mut JSContext,
+        cx: SafeJSContext,
         global: &GlobalScope,
         rval: MutableHandleValue,
     ) {
         match self {
             Error::JSFailed => (),
-            _ => assert!(!JS_IsExceptionPending(cx)),
+            _ => unsafe { assert!(!JS_IsExceptionPending(*cx)) },
+        }
+        throw_dom_exception(cx, global, self);
+        unsafe {
+            assert!(JS_IsExceptionPending(*cx));
+            assert!(JS_GetPendingException(*cx, rval));
+            JS_ClearPendingException(*cx);
         }
-        throw_dom_exception(SafeJSContext::from_ptr(cx), global, self);
-        assert!(JS_IsExceptionPending(cx));
-        assert!(JS_GetPendingException(cx, rval));
-        JS_ClearPendingException(cx);
     }
 }
diff --git a/components/script/dom/promise.rs b/components/script/dom/promise.rs
index 3629f4e53ed..1695e22bf71 100644
--- a/components/script/dom/promise.rs
+++ b/components/script/dom/promise.rs
@@ -219,14 +219,11 @@ impl Promise {
         self.reject(cx, v.handle());
     }
 
-    #[allow(unsafe_code)]
     pub(crate) fn reject_error(&self, error: Error) {
         let cx = GlobalScope::get_cx();
         let _ac = enter_realm(self);
         rooted!(in(*cx) let mut v = UndefinedValue());
-        unsafe {
-            error.to_jsval(*cx, &self.global(), v.handle_mut());
-        }
+        error.to_jsval(cx, &self.global(), v.handle_mut());
         self.reject(cx, v.handle());
     }
 
diff --git a/components/script/dom/readablestream.rs b/components/script/dom/readablestream.rs
index b53c6d496c2..0e49614c533 100644
--- a/components/script/dom/readablestream.rs
+++ b/components/script/dom/readablestream.rs
@@ -417,11 +417,10 @@ impl ReadableStream {
 
     /// <https://streams.spec.whatwg.org/#readable-stream-error>
     /// Note: in other use cases this call happens via the controller.
-    #[allow(unsafe_code)]
     pub(crate) fn error_native(&self, error: Error) {
         let cx = GlobalScope::get_cx();
         rooted!(in(*cx) let mut error_val = UndefinedValue());
-        unsafe { error.to_jsval(*cx, &self.global(), error_val.handle_mut()) };
+        error.to_jsval(cx, &self.global(), error_val.handle_mut());
         self.error(error_val.handle());
     }
 
diff --git a/components/script/dom/readablestreambyobreader.rs b/components/script/dom/readablestreambyobreader.rs
index 427bee60b9a..9b5045f3380 100644
--- a/components/script/dom/readablestreambyobreader.rs
+++ b/components/script/dom/readablestreambyobreader.rs
@@ -153,20 +153,17 @@ impl ReadableStreamBYOBReader {
     }
 
     /// <https://streams.spec.whatwg.org/#abstract-opdef-readablestreambyobreaderrelease>
-    #[allow(unsafe_code)]
     pub(crate) fn release(&self) -> Fallible<()> {
         // Perform ! ReadableStreamReaderGenericRelease(reader).
         self.generic_release()?;
         // Let e be a new TypeError exception.
         let cx = GlobalScope::get_cx();
         rooted!(in(*cx) let mut error = UndefinedValue());
-        unsafe {
-            Error::Type("Reader is released".to_owned()).to_jsval(
-                *cx,
-                &self.global(),
-                error.handle_mut(),
-            )
-        };
+        Error::Type("Reader is released".to_owned()).to_jsval(
+            cx,
+            &self.global(),
+            error.handle_mut(),
+        );
 
         // Perform ! ReadableStreamBYOBReaderErrorReadIntoRequests(reader, e).
         self.error_read_into_requests(error.handle());
diff --git a/components/script/dom/readablestreamdefaultcontroller.rs b/components/script/dom/readablestreamdefaultcontroller.rs
index d1dde18518f..6aa17253282 100644
--- a/components/script/dom/readablestreamdefaultcontroller.rs
+++ b/components/script/dom/readablestreamdefaultcontroller.rs
@@ -450,7 +450,6 @@ impl ReadableStreamDefaultController {
     }
 
     /// <https://streams.spec.whatwg.org/#readable-stream-default-controller-call-pull-if-needed>
-    #[allow(unsafe_code)]
     fn call_pull_if_needed(&self, can_gc: CanGc) {
         if !self.should_call_pull() {
             return;
@@ -500,11 +499,9 @@ impl ReadableStreamDefaultController {
             let cx = GlobalScope::get_cx();
             rooted!(in(*cx) let mut rval = UndefinedValue());
             // TODO: check if `self.global()` is the right globalscope.
-            unsafe {
-                error
-                    .clone()
-                    .to_jsval(*cx, &self.global(), rval.handle_mut())
-            };
+            error
+                .clone()
+                .to_jsval(cx, &self.global(), rval.handle_mut());
             let promise = Promise::new(&global, can_gc);
             promise.reject_native(&rval.handle());
             promise
@@ -513,7 +510,6 @@ impl ReadableStreamDefaultController {
     }
 
     /// <https://streams.spec.whatwg.org/#rs-default-controller-private-cancel>
-    #[allow(unsafe_code)]
     pub(crate) fn perform_cancel_steps(
         &self,
         reason: SafeHandleValue,
@@ -540,11 +536,9 @@ impl ReadableStreamDefaultController {
             let cx = GlobalScope::get_cx();
             rooted!(in(*cx) let mut rval = UndefinedValue());
             // TODO: check if `self.global()` is the right globalscope.
-            unsafe {
-                error
-                    .clone()
-                    .to_jsval(*cx, &self.global(), rval.handle_mut())
-            };
+            error
+                .clone()
+                .to_jsval(cx, &self.global(), rval.handle_mut());
             let promise = Promise::new(&global, can_gc);
             promise.reject_native(&rval.handle());
             promise
diff --git a/components/script/dom/readablestreamdefaultreader.rs b/components/script/dom/readablestreamdefaultreader.rs
index ea3c60cb065..d62f7c122bd 100644
--- a/components/script/dom/readablestreamdefaultreader.rs
+++ b/components/script/dom/readablestreamdefaultreader.rs
@@ -245,20 +245,17 @@ impl ReadableStreamDefaultReader {
     }
 
     /// <https://streams.spec.whatwg.org/#abstract-opdef-readablestreamdefaultreaderrelease>
-    #[allow(unsafe_code)]
     pub(crate) fn release(&self) -> Fallible<()> {
         // Perform ! ReadableStreamReaderGenericRelease(reader).
         self.generic_release()?;
         // Let e be a new TypeError exception.
         let cx = GlobalScope::get_cx();
         rooted!(in(*cx) let mut error = UndefinedValue());
-        unsafe {
-            Error::Type("Reader is released".to_owned()).to_jsval(
-                *cx,
-                &self.global(),
-                error.handle_mut(),
-            )
-        };
+        Error::Type("Reader is released".to_owned()).to_jsval(
+            cx,
+            &self.global(),
+            error.handle_mut(),
+        );
 
         // Perform ! ReadableStreamDefaultReaderErrorReadRequests(reader, e).
         self.error_read_requests(error.handle());
@@ -363,19 +360,16 @@ impl ReadableStreamDefaultReaderMethods<crate::DomTypeHolder> for ReadableStream
     }
 
     /// <https://streams.spec.whatwg.org/#default-reader-read>
-    #[allow(unsafe_code)]
     fn Read(&self, can_gc: CanGc) -> Rc<Promise> {
         // If this.[[stream]] is undefined, return a promise rejected with a TypeError exception.
         if self.stream.get().is_none() {
             let cx = GlobalScope::get_cx();
             rooted!(in(*cx) let mut error = UndefinedValue());
-            unsafe {
-                Error::Type("stream is undefined".to_owned()).to_jsval(
-                    *cx,
-                    &self.global(),
-                    error.handle_mut(),
-                )
-            };
+            Error::Type("stream is undefined".to_owned()).to_jsval(
+                cx,
+                &self.global(),
+                error.handle_mut(),
+            );
             return Promise::new_rejected(&self.global(), cx, error.handle()).unwrap();
         }
         // Let promise be a new promise.
diff --git a/components/script/dom/readablestreamgenericreader.rs b/components/script/dom/readablestreamgenericreader.rs
index ecc6255a161..72a02c05476 100644
--- a/components/script/dom/readablestreamgenericreader.rs
+++ b/components/script/dom/readablestreamgenericreader.rs
@@ -101,13 +101,11 @@ pub(crate) trait ReadableStreamGenericReader {
                 // Otherwise, set reader.[[closedPromise]] to a promise rejected with a TypeError exception.
                 let cx = GlobalScope::get_cx();
                 rooted!(in(*cx) let mut error = UndefinedValue());
-                unsafe {
-                    Error::Type("Cannot release lock due to stream state.".to_owned()).to_jsval(
-                        *cx,
-                        &stream.global(),
-                        error.handle_mut(),
-                    )
-                };
+                Error::Type("Cannot release lock due to stream state.".to_owned()).to_jsval(
+                    cx,
+                    &stream.global(),
+                    error.handle_mut(),
+                );
 
                 self.set_closed_promise(
                     Promise::new_rejected(&stream.global(), cx, error.handle()).unwrap(),
diff --git a/components/script/script_module.rs b/components/script/script_module.rs
index e6647d3f460..5cb58e7f471 100644
--- a/components/script/script_module.rs
+++ b/components/script/script_module.rs
@@ -76,10 +76,9 @@ use crate::realms::{enter_realm, AlreadyInRealm, InRealm};
 use crate::script_runtime::{CanGc, JSContext as SafeJSContext};
 use crate::task::TaskBox;
 
-#[allow(unsafe_code)]
-unsafe fn gen_type_error(global: &GlobalScope, string: String) -> RethrowError {
+fn gen_type_error(global: &GlobalScope, string: String) -> RethrowError {
     rooted!(in(*GlobalScope::get_cx()) let mut thrown = UndefinedValue());
-    Error::Type(string).to_jsval(*GlobalScope::get_cx(), global, thrown.handle_mut());
+    Error::Type(string).to_jsval(GlobalScope::get_cx(), global, thrown.handle_mut());
 
     RethrowError(RootedTraceableBox::from_box(Heap::boxed(thrown.get())))
 }
@@ -1428,8 +1427,7 @@ fn fetch_an_import_module_script_graph(
 
     // Step 2.
     if url.is_err() {
-        let specifier_error =
-            unsafe { gen_type_error(global, "Wrong module specifier".to_owned()) };
+        let specifier_error = gen_type_error(global, "Wrong module specifier".to_owned());
         return Err(specifier_error);
     }