Invalid return type for key conversion (#39252)

`convert_value_to_key` returns a `ConversionResult` now, so keys can be
considered "Invalid" rather than throwing an exception.

Testing: WPT
Unblocks: #38288

---------

Signed-off-by: Ashwin Naren <arihant2math@gmail.com>

components/script/dom/idbfactory.rs

@@ -98,8 +98,8 @@ impl IDBFactoryMethods<crate::DomTypeHolder> for IDBFactory {
 
     // https://www.w3.org/TR/IndexedDB-2/#dom-idbfactory-cmp
     fn Cmp(&self, cx: SafeJSContext, first: HandleValue, second: HandleValue) -> Fallible<i16> {
-        let first_key = convert_value_to_key(cx, first, None)?;
+        let first_key = convert_value_to_key(cx, first, None)?.into_result()?;
-        let second_key = convert_value_to_key(cx, second, None)?;
+        let second_key = convert_value_to_key(cx, second, None)?.into_result()?;
         let cmp = first_key.partial_cmp(&second_key);
         if let Some(cmp) = cmp {
             match cmp {

components/script/dom/idbkeyrange.rs

@@ -71,7 +71,7 @@ impl IDBKeyRangeMethods<crate::DomTypeHolder> for IDBKeyRange {
         global: &GlobalScope,
         value: HandleValue,
     ) -> Fallible<DomRoot<IDBKeyRange>> {
-        let key = convert_value_to_key(cx, value, None)?;
+        let key = convert_value_to_key(cx, value, None)?.into_result()?;
         let inner = IndexedDBKeyRange::only(key);
         Ok(IDBKeyRange::new(global, inner, CanGc::note()))
     }
@@ -83,7 +83,7 @@ impl IDBKeyRangeMethods<crate::DomTypeHolder> for IDBKeyRange {
         lower: HandleValue,
         open: bool,
     ) -> Fallible<DomRoot<IDBKeyRange>> {
-        let key = convert_value_to_key(cx, lower, None)?;
+        let key = convert_value_to_key(cx, lower, None)?.into_result()?;
         let inner = IndexedDBKeyRange::lower_bound(key, open);
         Ok(IDBKeyRange::new(global, inner, CanGc::note()))
     }
@@ -95,7 +95,7 @@ impl IDBKeyRangeMethods<crate::DomTypeHolder> for IDBKeyRange {
         upper: HandleValue,
         open: bool,
     ) -> Fallible<DomRoot<IDBKeyRange>> {
-        let key = convert_value_to_key(cx, upper, None)?;
+        let key = convert_value_to_key(cx, upper, None)?.into_result()?;
         let inner = IndexedDBKeyRange::upper_bound(key, open);
         Ok(IDBKeyRange::new(global, inner, CanGc::note()))
     }
@@ -112,12 +112,12 @@ impl IDBKeyRangeMethods<crate::DomTypeHolder> for IDBKeyRange {
         // Step 1. Let lowerKey be the result of running the steps to convert a value to a key with
         // lower. Rethrow any exceptions.
         // Step 2. If lowerKey is invalid, throw a "DataError" DOMException.
-        let lower_key = convert_value_to_key(cx, lower, None)?;
+        let lower_key = convert_value_to_key(cx, lower, None)?.into_result()?;
 
         // Step 3. Let upperKey be the result of running the steps to convert a value to a key with
         // upper. Rethrow any exceptions.
         // Step 4. If upperKey is invalid, throw a "DataError" DOMException.
-        let upper_key = convert_value_to_key(cx, upper, None)?;
+        let upper_key = convert_value_to_key(cx, upper, None)?.into_result()?;
 
         // Step 5. If lowerKey is greater than upperKey, throw a "DataError" DOMException.
         if lower_key > upper_key {
@@ -134,7 +134,7 @@ impl IDBKeyRangeMethods<crate::DomTypeHolder> for IDBKeyRange {
 
     // https://www.w3.org/TR/IndexedDB-2/#dom-idbkeyrange-_includes
     fn Includes(&self, cx: SafeJSContext, value: HandleValue) -> Fallible<bool> {
-        let key = convert_value_to_key(cx, value, None)?;
+        let key = convert_value_to_key(cx, value, None)?.into_result()?;
         if self.inner.contains(&key) {
             return Ok(true);
         }

components/script/dom/idbobjectstore.rs

@@ -211,7 +211,7 @@ impl IDBObjectStore {
         let serialized_key: Option<IndexedDBKeyType>;
 
         if !key.is_undefined() {
-            serialized_key = Some(convert_value_to_key(cx, key, None)?);
+            serialized_key = Some(convert_value_to_key(cx, key, None)?.into_result()?);
         } else {
             // Step 11: We should use in-line keys instead
             if let Some(Ok(ExtractionResult::Key(kpk))) = self
@@ -287,7 +287,7 @@ impl IDBObjectStoreMethods<crate::DomTypeHolder> for IDBObjectStore {
 
         // Step 6
         // TODO: Convert to key range instead
-        let serialized_query = convert_value_to_key(cx, query, None);
+        let serialized_query = convert_value_to_key(cx, query, None)?.into_result();
         // Step 7. Let operation be an algorithm to run delete records from an object store with store and range.
         // Stpe 8. Return the result (an IDBRequest) of running asynchronously execute a request with this and operation.
         let (sender, receiver) = indexed_db::create_channel(self.global());

components/script/indexed_db.rs

@@ -101,13 +101,27 @@ pub fn is_valid_key_path(key_path: &StrOrStringSequence) -> bool {
     }
 }
 
+pub(crate) enum ConversionResult {
+    Valid(IndexedDBKeyType),
+    Invalid,
+}
+
+impl ConversionResult {
+    pub fn into_result(self) -> Result<IndexedDBKeyType, Error> {
+        match self {
+            ConversionResult::Valid(key) => Ok(key),
+            ConversionResult::Invalid => Err(Error::Data),
+        }
+    }
+}
+
 // https://www.w3.org/TR/IndexedDB-2/#convert-value-to-key
 #[allow(unsafe_code)]
 pub fn convert_value_to_key(
     cx: SafeJSContext,
     input: HandleValue,
     seen: Option<Vec<HandleValue>>,
-) -> Result<IndexedDBKeyType, Error> {
+) -> Result<ConversionResult, Error> {
     // Step 1: If seen was not given, then let seen be a new empty set.
     let _seen = seen.unwrap_or_default();
 
@@ -121,15 +135,17 @@ pub fn convert_value_to_key(
     // FIXME:(arihant2math) Accept array as well
     if input.is_number() {
         if input.to_number().is_nan() {
-            return Err(Error::Data);
+            return Ok(ConversionResult::Invalid);
         }
-        return Ok(IndexedDBKeyType::Number(input.to_number()));
+        return Ok(ConversionResult::Valid(IndexedDBKeyType::Number(
+            input.to_number(),
+        )));
     }
 
     if input.is_string() {
         let string_ptr = std::ptr::NonNull::new(input.to_string()).unwrap();
         let key = unsafe { jsstr_to_string(*cx, string_ptr) };
-        return Ok(IndexedDBKeyType::String(key));
+        return Ok(ConversionResult::Valid(IndexedDBKeyType::String(key)));
     }
 
     if input.is_object() {
@@ -149,13 +165,15 @@ pub fn convert_value_to_key(
                 if f.is_nan() {
                     return Err(Error::Data);
                 }
-                return Ok(IndexedDBKeyType::Date(f));
+                return Ok(ConversionResult::Valid(IndexedDBKeyType::Date(f)));
             }
 
             if IsArrayBufferObject(*object) || JS_IsArrayBufferViewObject(*object) {
                 // FIXME:(arihant2math) implement it the correct way (is this correct?)
                 let key = structuredclone::write(cx, input, None)?;
-                return Ok(IndexedDBKeyType::Binary(key.serialized.clone()));
+                return Ok(ConversionResult::Valid(IndexedDBKeyType::Binary(
+                    key.serialized.clone(),
+                )));
             }
 
             if let ESClass::Array = built_in_class {
@@ -166,7 +184,7 @@ pub fn convert_value_to_key(
         }
     }
 
-    Err(Error::Data)
+    Ok(ConversionResult::Invalid)
 }
 
 // https://www.w3.org/TR/IndexedDB-2/#convert-a-value-to-a-key-range
@@ -191,7 +209,7 @@ pub fn convert_value_to_key_range(
     if (input.get().is_undefined() || input.get().is_null()) && null_disallowed {
         return Err(Error::Data);
     }
-    let key = convert_value_to_key(cx, input, None)?;
+    let key = convert_value_to_key(cx, input, None)?.into_result()?;
     Ok(IndexedDBKeyRange::only(key))
 }
 
@@ -403,8 +421,6 @@ pub(crate) fn evaluate_key_path_on_value(
 /// <https://www.w3.org/TR/IndexedDB-2/#extract-a-key-from-a-value-using-a-key-path>
 pub(crate) enum ExtractionResult {
     Key(IndexedDBKeyType),
-    // NOTE: Invalid is not used for now. Remove the unused annotation when it is used.
-    #[expect(unused)]
     Invalid,
     Failure,
 }
@@ -434,11 +450,13 @@ pub(crate) fn extract_key(
             // TODO: implement convert_value_to_multientry_key
             unimplemented!("multiEntry keys are not yet supported");
         },
-        _ => convert_value_to_key(cx, r.handle(), None)?,
+        _ => match convert_value_to_key(cx, r.handle(), None)? {
+            ConversionResult::Valid(key) => key,
+            // Step 4. If key is invalid, return invalid.
+            ConversionResult::Invalid => return Ok(ExtractionResult::Invalid),
+        },
     };
 
-    // TODO: Step 4. If key is invalid, return invalid.
-
     // Step 5. Return key.
     Ok(ExtractionResult::Key(key))
 }