IndexedDB: Handle missing object stores in object store operations (#38115)

These changes fix a large number of panics that can manifest as
intermittent test failures. They also add more specification text to
various IDBObjectStore methods and implement missing steps that check
for whether an object store is deleted.

Testing: Existing test coverage.

Signed-off-by: Josh Matthews <josh@joshmatthews.net>

components/script/dom/idbdatabase.rs

@@ -92,6 +92,13 @@ impl IDBDatabase {
         )
     }
 
+    pub(crate) fn object_store_exists(&self, name: &DOMString) -> bool {
+        self.object_store_names
+            .borrow()
+            .iter()
+            .any(|store_name| store_name == name)
+    }
+
     pub fn version(&self) -> u64 {
         let (sender, receiver) = ipc::channel(self.global().time_profiler_chan().clone()).unwrap();
         let operation = SyncOperation::Version(

components/script/dom/idbobjectstore.rs

@@ -10,11 +10,14 @@ use net_traits::indexeddb_thread::{
     IndexedDBThreadMsg, SyncOperation,
 };
 use profile_traits::ipc;
+use script_bindings::error::ErrorResult;
 
 use crate::dom::bindings::cell::DomRefCell;
 use crate::dom::bindings::codegen::Bindings::IDBDatabaseBinding::IDBObjectStoreParameters;
 use crate::dom::bindings::codegen::Bindings::IDBObjectStoreBinding::IDBObjectStoreMethods;
-use crate::dom::bindings::codegen::Bindings::IDBTransactionBinding::IDBTransactionMode;
+use crate::dom::bindings::codegen::Bindings::IDBTransactionBinding::{
+    IDBTransactionMethods, IDBTransactionMode,
+};
 // We need to alias this name, otherwise test-tidy complains at &String reference.
 use crate::dom::bindings::codegen::UnionTypes::StringOrStringSequence as StrOrStringSequence;
 use crate::dom::bindings::error::{Error, Fallible};
@@ -171,6 +174,14 @@ impl IDBObjectStore {
         self.key_path.is_some()
     }
 
+    fn verify_not_deleted(&self) -> ErrorResult {
+        let db = self.transaction.Db();
+        if !db.object_store_exists(&self.name.borrow()) {
+            return Err(Error::InvalidState);
+        }
+        Ok(())
+    }
+
     /// Checks if the transation is active, throwing a "TransactionInactiveError" DOMException if not.
     fn check_transaction_active(&self) -> Fallible<()> {
         // Let transaction be this object store handle's transaction.
@@ -191,9 +202,7 @@ impl IDBObjectStore {
         let transaction = &self.transaction;
 
         // If transaction is not active, throw a "TransactionInactiveError" DOMException.
-        if !transaction.is_active() {
+        self.check_transaction_active()?;
-            return Err(Error::TransactionInactive);
-        }
 
         if let IDBTransactionMode::Readonly = transaction.get_mode() {
             return Err(Error::ReadOnly);
@@ -210,14 +219,14 @@ impl IDBObjectStore {
         overwrite: bool,
         can_gc: CanGc,
     ) -> Fallible<DomRoot<IDBRequest>> {
-        // Step 1: Unneeded, handled by self.check_readwrite_transaction_active()
+        // Step 1. Let transaction be handle’s transaction.
         // Step 2: Let store be this object store handle's object store.
         // This is resolved in the `execute_async` function.
-
         // Step 3: If store has been deleted, throw an "InvalidStateError" DOMException.
-        // FIXME:(rasviitanen)
+        self.verify_not_deleted()?;
 
-        // Steps 4-5
+        // Step 4. If transaction’s state is not active, then throw a "TransactionInactiveError" DOMException.
+        // Step 5. If transaction is a read-only transaction, throw a "ReadOnlyError" DOMException.
         self.check_readwrite_transaction_active()?;
 
         // Step 6: If store uses in-line keys and key was given, throw a "DataError" DOMException.
@@ -253,10 +262,13 @@ impl IDBObjectStore {
             }
         }
 
+        // Step 10. Let clone be a clone of value in targetRealm during transaction. Rethrow any exceptions.
         let serialized_value = structuredclone::write(cx, value, None)?;
 
         let (sender, receiver) = indexed_db::create_channel(self.global());
 
+        // Step 12. Let operation be an algorithm to run store a record into an object store with store, clone, key, and no-overwrite flag.
+        // Step 13. Return the result (an IDBRequest) of running asynchronously execute a request with handle and operation.
         IDBRequest::execute_async(
             self,
             AsyncOperation::ReadWrite(AsyncReadWriteOperation::PutItem {
@@ -295,15 +307,20 @@ impl IDBObjectStoreMethods<crate::DomTypeHolder> for IDBObjectStore {
 
     // https://www.w3.org/TR/IndexedDB-2/#dom-idbobjectstore-delete
     fn Delete(&self, cx: SafeJSContext, query: HandleValue) -> Fallible<DomRoot<IDBRequest>> {
-        // Step 1: Unneeded, handled by self.check_readwrite_transaction_active()
+        // Step 1. Let transaction be this’s transaction.
-        // TODO: Step 2
+        // Step 2. Let store be this's object store.
-        // TODO: Step 3
+        // Step 3. If store has been deleted, throw an "InvalidStateError" DOMException.
-        // Steps 4-5
+        self.verify_not_deleted()?;
+
+        // Step 4. If transaction’s state is not active, then throw a "TransactionInactiveError" DOMException.
+        // Step 5. If transaction is a read-only transaction, throw a "ReadOnlyError" DOMException.
         self.check_readwrite_transaction_active()?;
+
         // Step 6
         // TODO: Convert to key range instead
         let serialized_query = convert_value_to_key(cx, query, None);
-        // Step 7
+        // 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());
         serialized_query.and_then(|q| {
             IDBRequest::execute_async(
@@ -318,11 +335,17 @@ impl IDBObjectStoreMethods<crate::DomTypeHolder> for IDBObjectStore {
 
     // https://www.w3.org/TR/IndexedDB-2/#dom-idbobjectstore-clear
     fn Clear(&self) -> Fallible<DomRoot<IDBRequest>> {
-        // Step 1: Unneeded, handled by self.check_readwrite_transaction_active()
+        // Step 1. Let transaction be this’s transaction.
-        // TODO: Step 2
+        // Step 2. Let store be this's object store.
-        // TODO: Step 3
+        // Step 3. If store has been deleted, throw an "InvalidStateError" DOMException.
-        // Steps 4-5
+        self.verify_not_deleted()?;
+
+        // Step 4. If transaction’s state is not active, then throw a "TransactionInactiveError" DOMException.
+        // Step 5. If transaction is a read-only transaction, throw a "ReadOnlyError" DOMException.
         self.check_readwrite_transaction_active()?;
+
+        // Step 6. Let operation be an algorithm to run clear an object store with store.
+        // Stpe 7. Return the result (an IDBRequest) of running asynchronously execute a request with this and operation.
         let (sender, receiver) = indexed_db::create_channel(self.global());
 
         IDBRequest::execute_async(
@@ -336,14 +359,19 @@ impl IDBObjectStoreMethods<crate::DomTypeHolder> for IDBObjectStore {
 
     // https://www.w3.org/TR/IndexedDB-2/#dom-idbobjectstore-get
     fn Get(&self, cx: SafeJSContext, query: HandleValue) -> Fallible<DomRoot<IDBRequest>> {
-        // Step 1: Unneeded, handled by self.check_transaction_active()
+        // Step 1. Let transaction be this’s transaction.
-        // TODO: Step 2
+        // Step 2. Let store be this's object store.
-        // TODO: Step 3
+        // Step 3. If store has been deleted, throw an "InvalidStateError" DOMException.
-        // Step 4
+        self.verify_not_deleted()?;
+
+        // Step 4. If transaction’s state is not active, then throw a "TransactionInactiveError" DOMException.
         self.check_transaction_active()?;
-        // Step 5
+
+        // Step 5. Let range be the result of converting a value to a key range with query and true. Rethrow any exceptions.
         let serialized_query = convert_value_to_key_range(cx, query, None);
-        // Step 6
+
+        // Step 6. Let operation be an algorithm to run retrieve a value from an object store with the current Realm record, store, and range.
+        // Step 7. Return the result (an IDBRequest) of running asynchronously execute a request with this and operation.
         let (sender, receiver) = indexed_db::create_channel(self.global());
         serialized_query.and_then(|q| {
             IDBRequest::execute_async(
@@ -361,14 +389,20 @@ impl IDBObjectStoreMethods<crate::DomTypeHolder> for IDBObjectStore {
 
     // https://www.w3.org/TR/IndexedDB-2/#dom-idbobjectstore-getkey
     fn GetKey(&self, cx: SafeJSContext, query: HandleValue) -> Result<DomRoot<IDBRequest>, Error> {
-        // Step 1: Unneeded, handled by self.check_transaction_active()
+        // Step 1. Let transaction be this’s transaction.
-        // TODO: Step 2
+        // Step 2. Let store be this's object store.
-        // TODO: Step 3
+        // Step 3. If store has been deleted, throw an "InvalidStateError" DOMException.
-        // Step 4
+        self.verify_not_deleted()?;
+
+        // Step 4. If transaction’s state is not active, then throw a "TransactionInactiveError" DOMException.
         self.check_transaction_active()?;
-        // Step 5
+
+        // Step 5. Let range be the result of running the steps to convert a value to a key range with query and null disallowed flag set. Rethrow any exceptions.
         let serialized_query = convert_value_to_key_range(cx, query, None);
-        // Step 6
+
+        // Step 6. Run the steps to asynchronously execute a request and return the IDBRequest created by these steps.
+        // The steps are run with this object store handle as source and the steps to retrieve a key from an object
+        // store as operation, using store and range.
         let (sender, receiver) = indexed_db::create_channel(self.global());
         serialized_query.and_then(|q| {
             IDBRequest::execute_async(
@@ -406,16 +440,19 @@ impl IDBObjectStoreMethods<crate::DomTypeHolder> for IDBObjectStore {
 
     // https://www.w3.org/TR/IndexedDB-2/#dom-idbobjectstore-count
     fn Count(&self, cx: SafeJSContext, query: HandleValue) -> Fallible<DomRoot<IDBRequest>> {
-        // Step 1: Unneeded, handled by self.check_transaction_active()
+        // Step 1. Let transaction be this’s transaction.
-        // TODO: Step 2
+        // Step 2. Let store be this's object store.
-        // TODO: Step 3
+        // Step 3. If store has been deleted, throw an "InvalidStateError" DOMException.
-        // Steps 4
+        self.verify_not_deleted()?;
+
+        // Step 4. If transaction’s state is not active, then throw a "TransactionInactiveError" DOMException.
         self.check_transaction_active()?;
 
-        // Step 5
+        // Step 5. Let range be the result of converting a value to a key range with query. Rethrow any exceptions.
         let serialized_query = convert_value_to_key_range(cx, query, None);
 
-        // Step 6
+        // Step 6. Let operation be an algorithm to run count the records in a range with store and range.
+        // Step 7. Return the result (an IDBRequest) of running asynchronously execute a request with this and operation.
         let (sender, receiver) = indexed_db::create_channel(self.global());
         serialized_query.and_then(|q| {
             IDBRequest::execute_async(
@@ -437,8 +474,23 @@ impl IDBObjectStoreMethods<crate::DomTypeHolder> for IDBObjectStore {
     }
 
     // https://www.w3.org/TR/IndexedDB-2/#dom-idbobjectstore-setname
-    fn SetName(&self, value: DOMString) {
+    fn SetName(&self, value: DOMString) -> ErrorResult {
+        // Step 2. Let transaction be this’s transaction.
+        let transaction = &self.transaction;
+
+        // Step 3. Let store be this's object store.
+        // Step 4. If store has been deleted, throw an "InvalidStateError" DOMException.
+        self.verify_not_deleted()?;
+
+        // Step 5. If transaction is not an upgrade transaction, throw an "InvalidStateError" DOMException.
+        if transaction.Mode() != IDBTransactionMode::Versionchange {
+            return Err(Error::InvalidState);
+        }
+        // Step 6. If transaction’s state is not active, throw a "TransactionInactiveError" DOMException.
+        self.check_transaction_active()?;
+
         *self.name.borrow_mut() = value;
+        Ok(())
     }
 
     // https://www.w3.org/TR/IndexedDB-2/#dom-idbobjectstore-keypath

components/script_bindings/webidls/IDBObjectStore.webidl

@@ -10,7 +10,7 @@
 // https://w3c.github.io/IndexedDB/#idbobjectstore
 [Pref="dom_indexeddb_enabled", Exposed=(Window,Worker)]
 interface IDBObjectStore {
-  attribute DOMString name;
+  [SetterThrows] attribute DOMString name;
   // readonly attribute any keyPath;
   // readonly attribute DOMStringList indexNames;
   [SameObject] readonly attribute IDBTransaction transaction;

tests/wpt/meta/IndexedDB/idbobjectstore-add-put-exception-order.any.js.ini

@@ -5,28 +5,16 @@
   expected: ERROR
 
 [idbobjectstore-add-put-exception-order.any.html]
-  [IDBObjectStore.put exception order: InvalidStateError vs. TransactionInactiveError]
-    expected: FAIL
-
   [IDBObjectStore.put exception order: TransactionInactiveError vs. ReadOnlyError]
     expected: FAIL
 
-  [IDBObjectStore.add exception order: InvalidStateError vs. TransactionInactiveError]
-    expected: FAIL
-
   [IDBObjectStore.add exception order: TransactionInactiveError vs. ReadOnlyError]
     expected: FAIL
 
 
 [idbobjectstore-add-put-exception-order.any.worker.html]
-  [IDBObjectStore.put exception order: InvalidStateError vs. TransactionInactiveError]
-    expected: FAIL
-
   [IDBObjectStore.put exception order: TransactionInactiveError vs. ReadOnlyError]
     expected: FAIL
 
-  [IDBObjectStore.add exception order: InvalidStateError vs. TransactionInactiveError]
-    expected: FAIL
-
   [IDBObjectStore.add exception order: TransactionInactiveError vs. ReadOnlyError]
     expected: FAIL

tests/wpt/meta/IndexedDB/idbobjectstore-clear-exception-order.any.js.ini

@@ -1,7 +1,4 @@
 [idbobjectstore-clear-exception-order.any.worker.html]
-  [IDBObjectStore.clear exception order: InvalidStateError vs. TransactionInactiveError]
-    expected: FAIL
-
   [IDBObjectStore.clear exception order: TransactionInactiveError vs. ReadOnlyError]
     expected: FAIL
 
@@ -10,9 +7,6 @@
   expected: ERROR
 
 [idbobjectstore-clear-exception-order.any.html]
-  [IDBObjectStore.clear exception order: InvalidStateError vs. TransactionInactiveError]
-    expected: FAIL
-
   [IDBObjectStore.clear exception order: TransactionInactiveError vs. ReadOnlyError]
     expected: FAIL
 

tests/wpt/meta/IndexedDB/idbobjectstore-delete-exception-order.any.js.ini

@@ -2,9 +2,6 @@
   expected: ERROR
 
 [idbobjectstore-delete-exception-order.any.html]
-  [IDBObjectStore.delete exception order: InvalidStateError vs. TransactionInactiveError]
-    expected: FAIL
-
   [IDBObjectStore.delete exception order: TransactionInactiveError vs. ReadOnlyError]
     expected: FAIL
 
@@ -13,8 +10,5 @@
   expected: ERROR
 
 [idbobjectstore-delete-exception-order.any.worker.html]
-  [IDBObjectStore.delete exception order: InvalidStateError vs. TransactionInactiveError]
-    expected: FAIL
-
   [IDBObjectStore.delete exception order: TransactionInactiveError vs. ReadOnlyError]
     expected: FAIL

tests/wpt/meta/IndexedDB/idbobjectstore-query-exception-order.any.js.ini

@@ -1,7 +1,4 @@
 [idbobjectstore-query-exception-order.any.html]
-  [IDBObjectStore.get exception order: InvalidStateError vs. TransactionInactiveError]
-    expected: FAIL
-
   [IDBObjectStore.get exception order: TransactionInactiveError vs. DataError]
     expected: FAIL
 
@@ -17,9 +14,6 @@
   [IDBObjectStore.getAllKeys exception order: TransactionInactiveError vs. DataError]
     expected: FAIL
 
-  [IDBObjectStore.count exception order: InvalidStateError vs. TransactionInactiveError]
-    expected: FAIL
-
   [IDBObjectStore.count exception order: TransactionInactiveError vs. DataError]
     expected: FAIL
 
@@ -40,9 +34,6 @@
   expected: ERROR
 
 [idbobjectstore-query-exception-order.any.worker.html]
-  [IDBObjectStore.get exception order: InvalidStateError vs. TransactionInactiveError]
-    expected: FAIL
-
   [IDBObjectStore.get exception order: TransactionInactiveError vs. DataError]
     expected: FAIL
 
@@ -58,9 +49,6 @@
   [IDBObjectStore.getAllKeys exception order: TransactionInactiveError vs. DataError]
     expected: FAIL
 
-  [IDBObjectStore.count exception order: InvalidStateError vs. TransactionInactiveError]
-    expected: FAIL
-
   [IDBObjectStore.count exception order: TransactionInactiveError vs. DataError]
     expected: FAIL
 

tests/wpt/meta/IndexedDB/idbobjectstore_clear.any.js.ini

@@ -5,9 +5,6 @@
   [Clear removes all records from an index ]
     expected: FAIL
 
-  [If the object store has been deleted, the implementation must throw a DOMException of type InvalidStateError ]
-    expected: FAIL
-
 
 [idbobjectstore_clear.any.sharedworker.html]
   expected: ERROR
@@ -21,6 +18,3 @@
 
   [Clear removes all records from an index ]
     expected: FAIL
-
-  [If the object store has been deleted, the implementation must throw a DOMException of type InvalidStateError ]
-    expected: FAIL

tests/wpt/meta/IndexedDB/idbobjectstore_count.any.js.ini

@@ -5,9 +5,6 @@
   [Returns the number of records that have keys with the key]
     expected: FAIL
 
-  [If the object store has been deleted, the implementation must throw a DOMException of type InvalidStateError ]
-    expected: FAIL
-
 
 [idbobjectstore_count.any.sharedworker.html]
   expected: ERROR
@@ -19,9 +16,6 @@
   [Returns the number of records that have keys with the key]
     expected: FAIL
 
-  [If the object store has been deleted, the implementation must throw a DOMException of type InvalidStateError ]
-    expected: FAIL
-
 
 [idbobjectstore_count.any.serviceworker.html]
   expected: ERROR

tests/wpt/meta/IndexedDB/idbobjectstore_delete.any.js.ini

@@ -12,9 +12,6 @@
   [delete() removes all of the records in the range]
     expected: FAIL
 
-  [If the object store has been deleted, the implementation must throw a DOMException of type InvalidStateError]
-    expected: FAIL
-
 
 [idbobjectstore_delete.any.serviceworker.html]
   expected: ERROR
@@ -33,9 +30,6 @@
   [delete() removes all of the records in the range]
     expected: FAIL
 
-  [If the object store has been deleted, the implementation must throw a DOMException of type InvalidStateError]
-    expected: FAIL
-
 
 [idbobjectstore_delete.any.sharedworker.html]
   expected: ERROR

tests/wpt/meta/IndexedDB/nested-cloning-basic.any.js.ini

@@ -2,7 +2,6 @@
   expected: ERROR
 
 [nested-cloning-basic.any.worker.html]
-  expected: CRASH
   [small typed array]
     expected: FAIL