jsstring_to_str should accept a NonNull argument for the JS string (#33306)

Instead of asserting the raw pointer is not null, force callers to
produce a NonNull pointer.

Signed-off-by: Andriy Sultanov <sultanovandriy@gmail.com>

6 files changed, 40 insertions, 38 deletions

diff --git a/components/script/devtools.rs b/components/script/devtools.rs
index dacb98683c9..5e68b2d198e 100644
--- a/components/script/devtools.rs
+++ b/components/script/devtools.rs
@@ -71,13 +71,14 @@ pub fn handle_evaluate_js(global: &GlobalScope, eval: String, reply: IpcSender<E
                 },
             )
         } else if rval.is_string() {
-            EvaluateJSReply::StringValue(String::from(jsstring_to_str(*cx, rval.to_string())))
+            let jsstr = std::ptr::NonNull::new(rval.to_string()).unwrap();
+            EvaluateJSReply::StringValue(String::from(jsstring_to_str(*cx, jsstr)))
         } else if rval.is_null() {
             EvaluateJSReply::NullValue
         } else {
             assert!(rval.is_object());
 
-            let jsstr = ToString(*cx, rval.handle());
+            let jsstr = std::ptr::NonNull::new(ToString(*cx, rval.handle())).unwrap();
             let class_name = jsstring_to_str(*cx, jsstr);
 
             EvaluateJSReply::ActorValue {
diff --git a/components/script/dom/bindings/conversions.rs b/components/script/dom/bindings/conversions.rs
index cd092814e0a..df29575b2e6 100644
--- a/components/script/dom/bindings/conversions.rs
+++ b/components/script/dom/bindings/conversions.rs
@@ -163,7 +163,8 @@ where
 pub unsafe fn jsid_to_string(cx: *mut JSContext, id: HandleId) -> Option<DOMString> {
     let id_raw = *id;
     if id_raw.is_string() {
-        return Some(jsstring_to_str(cx, id_raw.to_string()));
+        let jsstr = std::ptr::NonNull::new(id_raw.to_string()).unwrap();
+        return Some(jsstring_to_str(cx, jsstr));
     }
 
     if id_raw.is_int() {
@@ -207,12 +208,12 @@ impl FromJSValConvertible for DOMString {
         if null_behavior == StringificationBehavior::Empty && value.get().is_null() {
             Ok(ConversionResult::Success(DOMString::new()))
         } else {
-            let jsstr = ToString(cx, value);
-            if jsstr.is_null() {
-                debug!("ToString failed");
-                Err(())
-            } else {
-                Ok(ConversionResult::Success(jsstring_to_str(cx, jsstr)))
+            match ptr::NonNull::new(ToString(cx, value)) {
+                Some(jsstr) => Ok(ConversionResult::Success(jsstring_to_str(cx, jsstr))),
+                None => {
+                    debug!("ToString failed");
+                    Err(())
+                },
             }
         }
     }
@@ -220,14 +221,13 @@ impl FromJSValConvertible for DOMString {
 
 /// Convert the given `JSString` to a `DOMString`. Fails if the string does not
 /// contain valid UTF-16.
-pub unsafe fn jsstring_to_str(cx: *mut JSContext, s: *mut JSString) -> DOMString {
-    assert!(!s.is_null());
-    let latin1 = JS_DeprecatedStringHasLatin1Chars(s);
+pub unsafe fn jsstring_to_str(cx: *mut JSContext, s: ptr::NonNull<JSString>) -> DOMString {
+    let latin1 = JS_DeprecatedStringHasLatin1Chars(s.as_ptr());
     DOMString::from_string(if latin1 {
-        latin1_to_string(cx, s)
+        latin1_to_string(cx, s.as_ptr())
     } else {
         let mut length = 0;
-        let chars = JS_GetTwoByteStringCharsAndLength(cx, ptr::null(), s, &mut length);
+        let chars = JS_GetTwoByteStringCharsAndLength(cx, ptr::null(), s.as_ptr(), &mut length);
         assert!(!chars.is_null());
         let potentially_ill_formed_utf16 = slice::from_raw_parts(chars, length);
         let mut s = String::with_capacity(length);
@@ -268,12 +268,11 @@ impl FromJSValConvertible for USVString {
         value: HandleValue,
         _: (),
     ) -> Result<ConversionResult<USVString>, ()> {
-        let jsstr = ToString(cx, value);
-        if jsstr.is_null() {
+        let Some(jsstr) = ptr::NonNull::new(ToString(cx, value)) else {
             debug!("ToString failed");
             return Err(());
-        }
-        let latin1 = JS_DeprecatedStringHasLatin1Chars(jsstr);
+        };
+        let latin1 = JS_DeprecatedStringHasLatin1Chars(jsstr.as_ptr());
         if latin1 {
             // FIXME(ajeffrey): Convert directly from DOMString to USVString
             return Ok(ConversionResult::Success(USVString(String::from(
@@ -281,7 +280,7 @@ impl FromJSValConvertible for USVString {
             ))));
         }
         let mut length = 0;
-        let chars = JS_GetTwoByteStringCharsAndLength(cx, ptr::null(), jsstr, &mut length);
+        let chars = JS_GetTwoByteStringCharsAndLength(cx, ptr::null(), jsstr.as_ptr(), &mut length);
         assert!(!chars.is_null());
         let char_vec = slice::from_raw_parts(chars, length);
         Ok(ConversionResult::Success(USVString(
diff --git a/components/script/dom/bindings/proxyhandler.rs b/components/script/dom/bindings/proxyhandler.rs
index 2baa5374227..315cff7bafb 100644
--- a/components/script/dom/bindings/proxyhandler.rs
+++ b/components/script/dom/bindings/proxyhandler.rs
@@ -259,7 +259,7 @@ unsafe fn id_to_source(cx: SafeJSContext, id: RawHandleId) -> Option<DOMString>
             jsstr.set(jsapi::JS_ValueToSource(*cx, value.handle().into()));
             jsstr.get()
         })
-        .filter(|jsstr| !jsstr.is_null())
+        .and_then(|jsstr| ptr::NonNull::new(jsstr))
         .map(|jsstr| jsstring_to_str(*cx, jsstr))
 }
 
diff --git a/components/script/dom/bindings/utils.rs b/components/script/dom/bindings/utils.rs
index eb1da6617e1..8b6f25d1f80 100644
--- a/components/script/dom/bindings/utils.rs
+++ b/components/script/dom/bindings/utils.rs
@@ -237,19 +237,19 @@ pub unsafe fn find_enum_value<'a, T>(
     v: HandleValue,
     pairs: &'a [(&'static str, T)],
 ) -> Result<(Option<&'a T>, DOMString), ()> {
-    let jsstr = ToString(cx, v);
-    if jsstr.is_null() {
-        return Err(());
+    match ptr::NonNull::new(ToString(cx, v)) {
+        Some(jsstr) => {
+            let search = jsstring_to_str(cx, jsstr);
+            Ok((
+                pairs
+                    .iter()
+                    .find(|&&(key, _)| search == *key)
+                    .map(|(_, ev)| ev),
+                search,
+            ))
+        },
+        None => Err(()),
     }
-
-    let search = jsstring_to_str(cx, jsstr);
-    Ok((
-        pairs
-            .iter()
-            .find(|&&(key, _)| search == *key)
-            .map(|(_, ev)| ev),
-        search,
-    ))
 }
 
 /// Returns wether `obj` is a platform object using dynamic unwrap
diff --git a/components/script/dom/console.rs b/components/script/dom/console.rs
index ae041dfa7da..13bbe1005a2 100644
--- a/components/script/dom/console.rs
+++ b/components/script/dom/console.rs
@@ -72,11 +72,13 @@ where
 #[allow(unsafe_code)]
 unsafe fn handle_value_to_string(cx: *mut jsapi::JSContext, value: HandleValue) -> DOMString {
     rooted!(in(cx) let mut js_string = std::ptr::null_mut::<jsapi::JSString>());
-    js_string.set(JS_ValueToSource(cx, value));
-    if js_string.is_null() {
-        return "<error converting value to string>".into();
+    match std::ptr::NonNull::new(JS_ValueToSource(cx, value)) {
+        Some(js_str) => {
+            js_string.set(js_str.as_ptr());
+            jsstring_to_str(cx, js_str)
+        },
+        None => "<error converting value to string>".into(),
     }
-    jsstring_to_str(cx, *js_string)
 }
 
 #[allow(unsafe_code)]
@@ -84,7 +86,7 @@ fn stringify_handle_value(message: HandleValue) -> DOMString {
     let cx = *GlobalScope::get_cx();
     unsafe {
         if message.is_string() {
-            return jsstring_to_str(cx, message.to_string());
+            return jsstring_to_str(cx, std::ptr::NonNull::new(message.to_string()).unwrap());
         }
         unsafe fn stringify_object_from_handle_value(
             cx: *mut jsapi::JSContext,
diff --git a/components/script/script_module.rs b/components/script/script_module.rs
index f277368d13e..db155fa4f00 100644
--- a/components/script/script_module.rs
+++ b/components/script/script_module.rs
@@ -609,7 +609,7 @@ impl ModuleTree {
         url: &ServoUrl,
         specifier: RawHandle<*mut JSString>,
     ) -> Result<ServoUrl, UrlParseError> {
-        let specifier_str = unsafe { jsstring_to_str(cx, *specifier) };
+        let specifier_str = unsafe { jsstring_to_str(cx, ptr::NonNull::new(*specifier).unwrap()) };
 
         // Step 1.
         if let Ok(specifier_url) = ServoUrl::parse(&specifier_str) {