script: Impl safe from_jsval wrapper (#38149)

Implement `SafeFromJSValConvertible`, a safe wrapper for
`ToJSValConvertible`. And, replace unsafe `ToJSValConvertible` with
`SafeFromJSValConvertible` in `script/dom` to reduce the amount of
unsafe code in `script`.

This would support the implementation of `AdoptedStylesheet` where we
will need to have a setter/getter of sequence, that was implemented by
`any` types.

Part of https://github.com/servo/servo/issues/37951

Signed-off-by: Jo Steven Novaryo <jo.steven.novaryo@huawei.com>

components/script/dom/bindings/error.rs

@@ -21,7 +21,9 @@ pub(crate) use script_bindings::error::*;
 
 #[cfg(feature = "js_backtrace")]
 use crate::dom::bindings::cell::DomRefCell;
-use crate::dom::bindings::conversions::{ConversionResult, FromJSValConvertible, root_from_object};
+use crate::dom::bindings::conversions::{
+    ConversionResult, SafeFromJSValConvertible, root_from_object,
+};
 use crate::dom::bindings::str::USVString;
 use crate::dom::domexception::{DOMErrorName, DOMException};
 use crate::dom::globalscope::GlobalScope;
@@ -208,7 +210,7 @@ impl ErrorInfo {
             }
         }
 
-        match unsafe { USVString::from_jsval(*cx, value, ()) } {
+        match USVString::safe_from_jsval(cx, value, ()) {
             Ok(ConversionResult::Success(USVString(string))) => ErrorInfo {
                 message: format!("uncaught exception: {}", string),
                 filename: String::new(),

components/script/dom/customelementregistry.rs

@@ -15,7 +15,7 @@ use js::jsapi::{HandleValueArray, Heap, IsCallable, IsConstructor, JSAutoRealm,
 use js::jsval::{BooleanValue, JSVal, NullValue, ObjectValue, UndefinedValue};
 use js::rust::wrappers::{Construct1, JS_GetProperty, SameValue};
 use js::rust::{HandleObject, HandleValue, MutableHandleValue};
-use script_bindings::conversions::SafeToJSValConvertible;
+use script_bindings::conversions::{SafeFromJSValConvertible, SafeToJSValConvertible};
 
 use super::bindings::trace::HashMapTracedValues;
 use crate::dom::bindings::callback::{CallbackContainer, ExceptionHandling};
@@ -26,9 +26,7 @@ use crate::dom::bindings::codegen::Bindings::CustomElementRegistryBinding::{
 use crate::dom::bindings::codegen::Bindings::ElementBinding::ElementMethods;
 use crate::dom::bindings::codegen::Bindings::FunctionBinding::Function;
 use crate::dom::bindings::codegen::Bindings::WindowBinding::Window_Binding::WindowMethods;
-use crate::dom::bindings::conversions::{
-    ConversionResult, FromJSValConvertible, StringificationBehavior,
-};
+use crate::dom::bindings::conversions::{ConversionResult, StringificationBehavior};
 use crate::dom::bindings::error::{
     Error, ErrorResult, Fallible, report_pending_exception, throw_dom_exception,
 };
@@ -222,13 +220,11 @@ impl CustomElementRegistry {
             return Ok(Vec::new());
         }
 
-        let conversion = unsafe {
-            FromJSValConvertible::from_jsval(
-                *cx,
-                observed_attributes.handle(),
-                StringificationBehavior::Default,
-            )
-        };
+        let conversion = SafeFromJSValConvertible::safe_from_jsval(
+            cx,
+            observed_attributes.handle(),
+            StringificationBehavior::Default,
+        );
         match conversion {
             Ok(ConversionResult::Success(attributes)) => Ok(attributes),
             Ok(ConversionResult::Failure(error)) => Err(Error::Type(error.into())),
@@ -258,7 +254,7 @@ impl CustomElementRegistry {
         }
 
         let conversion =
-            unsafe { FromJSValConvertible::from_jsval(*cx, form_associated_value.handle(), ()) };
+            SafeFromJSValConvertible::safe_from_jsval(cx, form_associated_value.handle(), ());
         match conversion {
             Ok(ConversionResult::Success(flag)) => Ok(flag),
             Ok(ConversionResult::Failure(error)) => Err(Error::Type(error.into())),
@@ -287,13 +283,11 @@ impl CustomElementRegistry {
             return Ok(Vec::new());
         }
 
-        let conversion = unsafe {
-            FromJSValConvertible::from_jsval(
-                *cx,
-                disabled_features.handle(),
-                StringificationBehavior::Default,
-            )
-        };
+        let conversion = SafeFromJSValConvertible::safe_from_jsval(
+            cx,
+            disabled_features.handle(),
+            StringificationBehavior::Default,
+        );
         match conversion {
             Ok(ConversionResult::Success(attributes)) => Ok(attributes),
             Ok(ConversionResult::Failure(error)) => Err(Error::Type(error.into())),
@@ -750,7 +744,7 @@ impl CustomElementDefinition {
 
         rooted!(in(*cx) let element_val = ObjectValue(element.get()));
         let element: DomRoot<Element> =
-            match unsafe { DomRoot::from_jsval(*cx, element_val.handle(), ()) } {
+            match SafeFromJSValConvertible::safe_from_jsval(cx, element_val.handle(), ()) {
                 Ok(ConversionResult::Success(element)) => element,
                 Ok(ConversionResult::Failure(..)) => {
                     return Err(Error::Type(

components/script/dom/readablestream.rs

@@ -35,7 +35,7 @@ use crate::dom::abortsignal::{AbortAlgorithm, AbortSignal};
 use crate::dom::bindings::codegen::Bindings::ReadableStreamDefaultReaderBinding::ReadableStreamDefaultReaderMethods;
 use crate::dom::bindings::codegen::Bindings::ReadableStreamDefaultControllerBinding::ReadableStreamDefaultController_Binding::ReadableStreamDefaultControllerMethods;
 use crate::dom::bindings::codegen::Bindings::UnderlyingSourceBinding::UnderlyingSource as JsUnderlyingSource;
-use crate::dom::bindings::conversions::{ConversionBehavior, ConversionResult};
+use crate::dom::bindings::conversions::{ConversionBehavior, ConversionResult, SafeFromJSValConvertible};
 use crate::dom::bindings::error::{Error, ErrorToJsval, Fallible};
 use crate::dom::bindings::codegen::GenericBindings::WritableStreamDefaultWriterBinding::WritableStreamDefaultWriter_Binding::WritableStreamDefaultWriterMethods;
 use crate::dom::writablestream::WritableStream;
@@ -58,7 +58,6 @@ use crate::dom::underlyingsourcecontainer::UnderlyingSourceType;
 use crate::dom::writablestreamdefaultwriter::WritableStreamDefaultWriter;
 use script_bindings::codegen::GenericBindings::MessagePortBinding::MessagePortMethods;
 use crate::dom::messageport::MessagePort;
-use crate::js::conversions::FromJSValConvertible;
 use crate::realms::{enter_realm, InRealm};
 use crate::script_runtime::{CanGc, JSContext as SafeJSContext};
 use crate::dom::promisenativehandler::{Callback, PromiseNativeHandler};
@@ -2250,10 +2249,8 @@ pub(crate) unsafe fn get_type_and_value_from_message(
         .expect("Getting the value should not fail.");
 
     // Assert: type is a String.
-    let result = unsafe {
-        DOMString::from_jsval(*cx, type_.handle(), StringificationBehavior::Empty)
-            .expect("The type of the message should be a string")
-    };
+    let result = DOMString::safe_from_jsval(cx, type_.handle(), StringificationBehavior::Empty)
+        .expect("The type of the message should be a string");
     let ConversionResult::Success(type_string) = result else {
         unreachable!("The type of the message should be a string");
     };
@@ -2357,7 +2354,7 @@ pub(crate) fn get_read_promise_done(
         rooted!(in(*cx) let object = v.to_object());
         rooted!(in(*cx) let mut done = UndefinedValue());
         match get_dictionary_property(*cx, object.handle(), "done", done.handle_mut(), can_gc) {
-            Ok(true) => match bool::from_jsval(*cx, done.handle(), ()) {
+            Ok(true) => match bool::safe_from_jsval(cx, done.handle(), ()) {
                 Ok(ConversionResult::Success(val)) => Ok(val),
                 Ok(ConversionResult::Failure(error)) => Err(Error::Type(error.to_string())),
                 _ => Err(Error::Type("Unknown format for done property.".to_string())),
@@ -2385,7 +2382,11 @@ pub(crate) fn get_read_promise_bytes(
         rooted!(in(*cx) let mut bytes = UndefinedValue());
         match get_dictionary_property(*cx, object.handle(), "value", bytes.handle_mut(), can_gc) {
             Ok(true) => {
-                match Vec::<u8>::from_jsval(*cx, bytes.handle(), ConversionBehavior::EnforceRange) {
+                match Vec::<u8>::safe_from_jsval(
+                    cx,
+                    bytes.handle(),
+                    ConversionBehavior::EnforceRange,
+                ) {
                     Ok(ConversionResult::Success(val)) => Ok(val),
                     Ok(ConversionResult::Failure(error)) => Err(Error::Type(error.to_string())),
                     _ => Err(Error::Type("Unknown format for bytes read.".to_string())),

components/script/dom/webxr/xrsystem.rs

@@ -18,7 +18,7 @@ use crate::dom::bindings::cell::DomRefCell;
 use crate::dom::bindings::codegen::Bindings::XRSystemBinding::{
     XRSessionInit, XRSessionMode, XRSystemMethods,
 };
-use crate::dom::bindings::conversions::{ConversionResult, FromJSValConvertible};
+use crate::dom::bindings::conversions::{ConversionResult, SafeFromJSValConvertible};
 use crate::dom::bindings::error::Error;
 use crate::dom::bindings::inheritance::Castable;
 use crate::dom::bindings::refcounted::{Trusted, TrustedPromise};
@@ -192,33 +192,29 @@ impl XRSystemMethods<crate::DomTypeHolder> for XRSystem {
 
         if let Some(ref r) = init.requiredFeatures {
             for feature in r {
-                unsafe {
-                    if let Ok(ConversionResult::Success(s)) =
-                        String::from_jsval(*cx, feature.handle(), ())
-                    {
-                        required_features.push(s)
-                    } else {
-                        warn!("Unable to convert required feature to string");
-                        if mode != XRSessionMode::Inline {
-                            self.pending_immersive_session.set(false);
-                        }
-                        promise.reject_error(Error::NotSupported, can_gc);
-                        return promise;
+                if let Ok(ConversionResult::Success(s)) =
+                    String::safe_from_jsval(cx, feature.handle(), ())
+                {
+                    required_features.push(s)
+                } else {
+                    warn!("Unable to convert required feature to string");
+                    if mode != XRSessionMode::Inline {
+                        self.pending_immersive_session.set(false);
                     }
+                    promise.reject_error(Error::NotSupported, can_gc);
+                    return promise;
                 }
             }
         }
 
         if let Some(ref o) = init.optionalFeatures {
             for feature in o {
-                unsafe {
-                    if let Ok(ConversionResult::Success(s)) =
-                        String::from_jsval(*cx, feature.handle(), ())
-                    {
-                        optional_features.push(s)
-                    } else {
-                        warn!("Unable to convert optional feature to string");
-                    }
+                if let Ok(ConversionResult::Success(s)) =
+                    String::safe_from_jsval(cx, feature.handle(), ())
+                {
+                    optional_features.push(s)
+                } else {
+                    warn!("Unable to convert optional feature to string");
                 }
             }
         }

components/script/script_thread.rs

@@ -107,7 +107,7 @@ use crate::dom::bindings::codegen::Bindings::DocumentBinding::{
 use crate::dom::bindings::codegen::Bindings::NavigatorBinding::NavigatorMethods;
 use crate::dom::bindings::codegen::Bindings::WindowBinding::WindowMethods;
 use crate::dom::bindings::conversions::{
-    ConversionResult, FromJSValConvertible, StringificationBehavior,
+    ConversionResult, SafeFromJSValConvertible, StringificationBehavior,
 };
 use crate::dom::bindings::inheritance::Castable;
 use crate::dom::bindings::refcounted::Trusted;
@@ -3681,18 +3681,16 @@ impl ScriptThread {
         );
 
         load_data.js_eval_result = if jsval.get().is_string() {
-            unsafe {
-                let strval = DOMString::from_jsval(
-                    *GlobalScope::get_cx(),
-                    jsval.handle(),
-                    StringificationBehavior::Empty,
-                );
-                match strval {
-                    Ok(ConversionResult::Success(s)) => {
-                        Some(JsEvalResult::Ok(String::from(s).as_bytes().to_vec()))
-                    },
-                    _ => None,
-                }
+            let strval = DOMString::safe_from_jsval(
+                GlobalScope::get_cx(),
+                jsval.handle(),
+                StringificationBehavior::Empty,
+            );
+            match strval {
+                Ok(ConversionResult::Success(s)) => {
+                    Some(JsEvalResult::Ok(String::from(s).as_bytes().to_vec()))
+                },
+                _ => None,
             }
         } else {
             Some(JsEvalResult::NoContent)

components/script_bindings/conversions.rs

@@ -79,6 +79,30 @@ impl ToJSValConvertible for DOMString {
     }
 }
 
+/// A safe wrapper for `FromJSValConvertible`.
+pub trait SafeFromJSValConvertible: Sized {
+    type Config;
+
+    #[allow(clippy::result_unit_err)] // Type definition depends on mozjs
+    fn safe_from_jsval(
+        cx: SafeJSContext,
+        value: HandleValue,
+        option: Self::Config,
+    ) -> Result<ConversionResult<Self>, ()>;
+}
+
+impl<T: FromJSValConvertible> SafeFromJSValConvertible for T {
+    type Config = <T as FromJSValConvertible>::Config;
+
+    fn safe_from_jsval(
+        cx: SafeJSContext,
+        value: HandleValue,
+        option: Self::Config,
+    ) -> Result<ConversionResult<Self>, ()> {
+        unsafe { T::from_jsval(*cx, value, option) }
+    }
+}
+
 // https://heycam.github.io/webidl/#es-DOMString
 impl FromJSValConvertible for DOMString {
     type Config = StringificationBehavior;