From b7cdd88b8ee6e53bd2567702956c436cca7738b8 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Fri, 22 Aug 2025 18:04:34 -0400 Subject: [PATCH] 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 --- components/script/dom/idbdatabase.rs | 7 + components/script/dom/idbobjectstore.rs | 124 +++++++++++++----- .../webidls/IDBObjectStore.webidl | 2 +- ...ctstore-add-put-exception-order.any.js.ini | 12 -- ...jectstore-clear-exception-order.any.js.ini | 6 - ...ectstore-delete-exception-order.any.js.ini | 6 - ...jectstore-query-exception-order.any.js.ini | 12 -- .../IndexedDB/idbobjectstore_clear.any.js.ini | 6 - .../IndexedDB/idbobjectstore_count.any.js.ini | 6 - .../idbobjectstore_delete.any.js.ini | 6 - .../IndexedDB/nested-cloning-basic.any.js.ini | 1 - 11 files changed, 96 insertions(+), 92 deletions(-) diff --git a/components/script/dom/idbdatabase.rs b/components/script/dom/idbdatabase.rs index d1877e88350..44e27980f00 100644 --- a/components/script/dom/idbdatabase.rs +++ b/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( diff --git a/components/script/dom/idbobjectstore.rs b/components/script/dom/idbobjectstore.rs index 6961a0e10f0..30b9f3345c0 100644 --- a/components/script/dom/idbobjectstore.rs +++ b/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() { - return Err(Error::TransactionInactive); - } + self.check_transaction_active()?; if let IDBTransactionMode::Readonly = transaction.get_mode() { return Err(Error::ReadOnly); @@ -210,14 +219,14 @@ impl IDBObjectStore { overwrite: bool, can_gc: CanGc, ) -> Fallible> { - // 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 for IDBObjectStore { // https://www.w3.org/TR/IndexedDB-2/#dom-idbobjectstore-delete fn Delete(&self, cx: SafeJSContext, query: HandleValue) -> Fallible> { - // Step 1: Unneeded, handled by self.check_readwrite_transaction_active() - // TODO: Step 2 - // TODO: Step 3 - // Steps 4-5 + // Step 1. Let transaction be this’s transaction. + // Step 2. Let store be this's object store. + // Step 3. If store has been deleted, throw an "InvalidStateError" DOMException. + 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 for IDBObjectStore { // https://www.w3.org/TR/IndexedDB-2/#dom-idbobjectstore-clear fn Clear(&self) -> Fallible> { - // Step 1: Unneeded, handled by self.check_readwrite_transaction_active() - // TODO: Step 2 - // TODO: Step 3 - // Steps 4-5 + // Step 1. Let transaction be this’s transaction. + // Step 2. Let store be this's object store. + // Step 3. If store has been deleted, throw an "InvalidStateError" DOMException. + 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 for IDBObjectStore { // https://www.w3.org/TR/IndexedDB-2/#dom-idbobjectstore-get fn Get(&self, cx: SafeJSContext, query: HandleValue) -> Fallible> { - // Step 1: Unneeded, handled by self.check_transaction_active() - // TODO: Step 2 - // TODO: Step 3 - // Step 4 + // Step 1. Let transaction be this’s transaction. + // Step 2. Let store be this's object store. + // Step 3. If store has been deleted, throw an "InvalidStateError" DOMException. + 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 for IDBObjectStore { // https://www.w3.org/TR/IndexedDB-2/#dom-idbobjectstore-getkey fn GetKey(&self, cx: SafeJSContext, query: HandleValue) -> Result, Error> { - // Step 1: Unneeded, handled by self.check_transaction_active() - // TODO: Step 2 - // TODO: Step 3 - // Step 4 + // Step 1. Let transaction be this’s transaction. + // Step 2. Let store be this's object store. + // Step 3. If store has been deleted, throw an "InvalidStateError" DOMException. + 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 for IDBObjectStore { // https://www.w3.org/TR/IndexedDB-2/#dom-idbobjectstore-count fn Count(&self, cx: SafeJSContext, query: HandleValue) -> Fallible> { - // Step 1: Unneeded, handled by self.check_transaction_active() - // TODO: Step 2 - // TODO: Step 3 - // Steps 4 + // Step 1. Let transaction be this’s transaction. + // Step 2. Let store be this's object store. + // Step 3. If store has been deleted, throw an "InvalidStateError" DOMException. + 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 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 diff --git a/components/script_bindings/webidls/IDBObjectStore.webidl b/components/script_bindings/webidls/IDBObjectStore.webidl index ce29c506e61..5d88beef407 100644 --- a/components/script_bindings/webidls/IDBObjectStore.webidl +++ b/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; diff --git a/tests/wpt/meta/IndexedDB/idbobjectstore-add-put-exception-order.any.js.ini b/tests/wpt/meta/IndexedDB/idbobjectstore-add-put-exception-order.any.js.ini index 0bc51ecdde6..bfbbe7ec1cf 100644 --- a/tests/wpt/meta/IndexedDB/idbobjectstore-add-put-exception-order.any.js.ini +++ b/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 diff --git a/tests/wpt/meta/IndexedDB/idbobjectstore-clear-exception-order.any.js.ini b/tests/wpt/meta/IndexedDB/idbobjectstore-clear-exception-order.any.js.ini index 11c723d8778..b271dc9c6d2 100644 --- a/tests/wpt/meta/IndexedDB/idbobjectstore-clear-exception-order.any.js.ini +++ b/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 diff --git a/tests/wpt/meta/IndexedDB/idbobjectstore-delete-exception-order.any.js.ini b/tests/wpt/meta/IndexedDB/idbobjectstore-delete-exception-order.any.js.ini index f58f65515c2..c446c2f1679 100644 --- a/tests/wpt/meta/IndexedDB/idbobjectstore-delete-exception-order.any.js.ini +++ b/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 diff --git a/tests/wpt/meta/IndexedDB/idbobjectstore-query-exception-order.any.js.ini b/tests/wpt/meta/IndexedDB/idbobjectstore-query-exception-order.any.js.ini index b13d31eba8f..4ec4c261338 100644 --- a/tests/wpt/meta/IndexedDB/idbobjectstore-query-exception-order.any.js.ini +++ b/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 diff --git a/tests/wpt/meta/IndexedDB/idbobjectstore_clear.any.js.ini b/tests/wpt/meta/IndexedDB/idbobjectstore_clear.any.js.ini index 8d401ad5d00..47949bf3dc8 100644 --- a/tests/wpt/meta/IndexedDB/idbobjectstore_clear.any.js.ini +++ b/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 diff --git a/tests/wpt/meta/IndexedDB/idbobjectstore_count.any.js.ini b/tests/wpt/meta/IndexedDB/idbobjectstore_count.any.js.ini index ac3117a2c3a..815d61fef93 100644 --- a/tests/wpt/meta/IndexedDB/idbobjectstore_count.any.js.ini +++ b/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 diff --git a/tests/wpt/meta/IndexedDB/idbobjectstore_delete.any.js.ini b/tests/wpt/meta/IndexedDB/idbobjectstore_delete.any.js.ini index 3a00296dee8..1c04005198a 100644 --- a/tests/wpt/meta/IndexedDB/idbobjectstore_delete.any.js.ini +++ b/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 diff --git a/tests/wpt/meta/IndexedDB/nested-cloning-basic.any.js.ini b/tests/wpt/meta/IndexedDB/nested-cloning-basic.any.js.ini index 461928a5677..d1dabe25f10 100644 --- a/tests/wpt/meta/IndexedDB/nested-cloning-basic.any.js.ini +++ b/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