diff --git a/components/script/dom/indexeddb/idbdatabase.rs b/components/script/dom/indexeddb/idbdatabase.rs index 617851828ea59..f2d2420031611 100644 --- a/components/script/dom/indexeddb/idbdatabase.rs +++ b/components/script/dom/indexeddb/idbdatabase.rs @@ -6,11 +6,10 @@ use std::cell::Cell; use dom_struct::dom_struct; use js::context::JSContext; -use profile_traits::generic_channel::channel; use script_bindings::cell::DomRefCell; use script_bindings::reflector::reflect_dom_object_with_cx; use servo_base::generic_channel::{GenericSend, GenericSender}; -use storage_traits::indexeddb::{IndexedDBThreadMsg, KeyPath, SyncOperation}; +use storage_traits::indexeddb::{AsyncSchemaOperation, IndexedDBThreadMsg, KeyPath, SyncOperation}; use stylo_atoms::Atom; use uuid::Uuid; @@ -325,36 +324,29 @@ impl IDBDatabaseMethods for IDBDatabase { &transaction, ); - let (sender, receiver) = channel(self.global().time_profiler_chan().clone()).unwrap(); - let key_paths = key_path.map(|p| match p { StringOrStringSequence::String(s) => KeyPath::String(s.to_string()), StringOrStringSequence::StringSequence(s) => { KeyPath::Sequence(s.iter().map(|s| s.to_string()).collect()) }, }); - let operation = SyncOperation::CreateObjectStore( - sender, - self.global().origin().immutable().clone(), - self.name.to_string(), - name.to_string(), - key_paths, + + let operation = AsyncSchemaOperation::CreateObjectStore { + callback: transaction.create_abort_callback(), + key_path: key_paths, auto_increment, - ); + }; self.get_idb_thread() - .send(IndexedDBThreadMsg::Sync(operation)) + .send(IndexedDBThreadMsg::AsyncSchemaOperation { + origin: self.global().origin().immutable().clone(), + database_name: self.name.to_string(), + store_name: name.to_string(), + operation, + transaction_serial_number: transaction.get_serial_number(), + }) .unwrap(); - if receiver - .recv() - .expect("Could not receive object store creation status") - .is_err() - { - warn!("Object store creation failed in idb thread"); - return Err(Error::InvalidState(None)); - }; - self.object_store_names.borrow_mut().push(name); transaction.register_object_store_handle(&object_store.get_name(), &object_store); @@ -390,27 +382,19 @@ impl IDBDatabaseMethods for IDBDatabase { // FIXME:(arihant2math) Remove from index set ... // Step 7 - let (sender, receiver) = channel(self.global().time_profiler_chan().clone()).unwrap(); - - let operation = SyncOperation::DeleteObjectStore( - sender, - self.global().origin().immutable().clone(), - self.name.to_string(), - String::from(name), - ); - + let operation = AsyncSchemaOperation::DeleteObjectStore { + callback: transaction.create_abort_callback(), + }; self.get_idb_thread() - .send(IndexedDBThreadMsg::Sync(operation)) + .send(IndexedDBThreadMsg::AsyncSchemaOperation { + origin: self.global().origin().immutable().clone(), + database_name: self.name.to_string(), + store_name: name.to_string(), + operation, + transaction_serial_number: transaction.get_serial_number(), + }) .unwrap(); - if receiver - .recv() - .expect("Could not receive object store deletion status") - .is_err() - { - warn!("Object store deletion failed in idb thread"); - return Err(Error::InvalidState(None)); - }; Ok(()) } diff --git a/components/script/dom/indexeddb/idbobjectstore.rs b/components/script/dom/indexeddb/idbobjectstore.rs index 6f1af878e63cb..3ebc0e180a290 100644 --- a/components/script/dom/indexeddb/idbobjectstore.rs +++ b/components/script/dom/indexeddb/idbobjectstore.rs @@ -17,8 +17,8 @@ use script_bindings::error::ErrorResult; use script_bindings::reflector::{Reflector, reflect_dom_object_with_cx}; use servo_base::generic_channel::{GenericSend, GenericSender}; use storage_traits::indexeddb::{ - self, AsyncOperation, AsyncReadOnlyOperation, AsyncReadWriteOperation, IndexedDBKeyType, - IndexedDBThreadMsg, SyncOperation, + self, AsyncOperation, AsyncReadOnlyOperation, AsyncReadWriteOperation, AsyncSchemaOperation, + IndexedDBKeyType, IndexedDBThreadMsg, }; use crate::dom::bindings::codegen::Bindings::IDBCursorBinding::IDBCursorDirection; @@ -602,16 +602,25 @@ impl IDBObjectStore { /// The caller must ensure that the original index exists. pub(crate) fn rename_index(&self, name: &DOMString, new_name: &DOMString) { - let rename_index_operation = SyncOperation::RenameIndex( - self.global().origin().immutable().clone(), - self.db_name.to_string(), - self.name.borrow().to_string(), - String::from(name.clone()), - String::from(new_name.clone()), - ); - self.get_idb_thread() - .send(IndexedDBThreadMsg::Sync(rename_index_operation)) - .unwrap(); + let operation = AsyncSchemaOperation::RenameIndex { + callback: self.transaction.create_abort_callback(), + index_name: name.to_string(), + new_name: new_name.to_string(), + }; + + if self + .get_idb_thread() + .send(IndexedDBThreadMsg::AsyncSchemaOperation { + origin: self.global().origin().immutable().clone(), + database_name: self.db_name.to_string(), + store_name: self.name.borrow().clone().into(), + operation, + transaction_serial_number: self.transaction.get_serial_number(), + }) + .is_err() + { + warn!("Could not send AsyncSchemaOperation"); + } // We also need to update the key in the index set let index = self @@ -1013,18 +1022,23 @@ impl IDBObjectStoreMethods for IDBObjectStore { // Step 11. Let index be a new index in store. // Set index’s name to name and key path to keyPath. If unique is set, set index’s unique flag. // If multiEntry is set, set index’s multiEntry flag. - let create_index_operation = SyncOperation::CreateIndex( - self.global().origin().immutable().clone(), - self.db_name.to_string(), - self.name.borrow().to_string(), - name.to_string(), - key_path.clone().into(), - options.unique, - options.multiEntry, - ); + let operation = AsyncSchemaOperation::CreateIndex { + callback: self.transaction.create_abort_callback(), + index_name: name.to_string(), + key_path: key_path.clone().into(), + unique: options.unique, + multi_entry: options.multiEntry, + }; + if self .get_idb_thread() - .send(IndexedDBThreadMsg::Sync(create_index_operation)) + .send(IndexedDBThreadMsg::AsyncSchemaOperation { + origin: self.global().origin().immutable().clone(), + database_name: self.db_name.to_string(), + store_name: self.name.borrow().clone().into(), + operation, + transaction_serial_number: self.transaction.get_serial_number(), + }) .is_err() { return Err(Error::Operation(None)); @@ -1055,15 +1069,23 @@ impl IDBObjectStoreMethods for IDBObjectStore { // Step 7. Remove index from this object store handle's index set. self.index_set.borrow_mut().retain(|n, _| n != &name); // Step 8. Destroy index. - let delete_index_operation = SyncOperation::DeleteIndex( - self.global().origin().immutable().clone(), - self.db_name.to_string(), - self.name.borrow().to_string(), - String::from(name), - ); - self.get_idb_thread() - .send(IndexedDBThreadMsg::Sync(delete_index_operation)) - .unwrap(); + let operation = AsyncSchemaOperation::DeleteIndex { + callback: self.transaction.create_abort_callback(), + index_name: name.to_string(), + }; + if self + .get_idb_thread() + .send(IndexedDBThreadMsg::AsyncSchemaOperation { + origin: self.global().origin().immutable().clone(), + database_name: self.db_name.to_string(), + store_name: self.name.borrow().clone().into(), + operation, + transaction_serial_number: self.transaction.get_serial_number(), + }) + .is_err() + { + return Err(Error::Operation(None)); + } Ok(()) } diff --git a/components/script/dom/indexeddb/idbtransaction.rs b/components/script/dom/indexeddb/idbtransaction.rs index edbd4a763e0bd..495179950c17a 100644 --- a/components/script/dom/indexeddb/idbtransaction.rs +++ b/components/script/dom/indexeddb/idbtransaction.rs @@ -15,7 +15,8 @@ use script_bindings::reflector::reflect_dom_object_with_cx; use servo_base::generic_channel::{GenericSend, GenericSender}; use servo_base::id::ScriptEventLoopId; use storage_traits::indexeddb::{ - IndexedDBIndex, IndexedDBThreadMsg, IndexedDBTxnMode, KeyPath, SyncOperation, TxnCompleteMsg, + BackendError, IndexedDBIndex, IndexedDBThreadMsg, IndexedDBTxnMode, KeyPath, SyncOperation, + TxnCompleteMsg, }; use stylo_atoms::Atom; @@ -41,6 +42,7 @@ use crate::dom::globalscope::GlobalScope; use crate::dom::indexeddb::idbdatabase::IDBDatabase; use crate::dom::indexeddb::idbobjectstore::{IDBObjectStore, IDBObjectStoreAbortState}; use crate::dom::indexeddb::idbrequest::IDBRequest; +use crate::indexeddb::map_backend_error_to_dom_error; #[dom_struct] pub struct IDBTransaction { @@ -721,6 +723,30 @@ impl IDBTransaction { object_store.key_generator_current_number, )) } + + pub(crate) fn create_abort_callback(&self) -> GenericCallback { + let trusted_transaction = Trusted::new(self); + let task_source = self + .global() + .task_manager() + .storage_task_source() + .to_sendable(); + GenericCallback::new( + self.global().time_profiler_chan().clone(), + move |error: Result| { + let Ok(error) = error else { + return; + }; + let trusted_transaction = trusted_transaction.clone(); + task_source.queue(task!(delete_failed: move |cx| { + let transaction = trusted_transaction.root(); + transaction.initiate_abort(cx, map_backend_error_to_dom_error(error)); + transaction.request_backend_abort(); + })); + }, + ) + .expect("Could not create GenericCallback") + } } impl IDBTransactionMethods for IDBTransaction { diff --git a/components/shared/storage/indexeddb.rs b/components/shared/storage/indexeddb.rs index 98773830b87bb..33e7204211231 100644 --- a/components/shared/storage/indexeddb.rs +++ b/components/shared/storage/indexeddb.rs @@ -359,12 +359,62 @@ impl AsyncReadWriteOperation { } } +#[derive(Debug, Deserialize, MallocSizeOf, Serialize)] +pub enum AsyncSchemaOperation { + /// Creates a new index for the database + CreateIndex { + callback: GenericCallback, + index_name: String, + key_path: KeyPath, + unique: bool, + multi_entry: bool, + }, + /// Rename an index + RenameIndex { + callback: GenericCallback, + index_name: String, + new_name: String, + }, + /// Delete an index + DeleteIndex { + callback: GenericCallback, + index_name: String, + }, + /// Creates a new store for the database + CreateObjectStore { + callback: GenericCallback, + key_path: Option, + auto_increment: bool, + }, + /// Delete an existing object store in the database + DeleteObjectStore { + callback: GenericCallback, + }, +} + +impl AsyncSchemaOperation { + pub fn notify_error(&self, error: BackendError) { + match self { + AsyncSchemaOperation::CreateIndex { .. } | + AsyncSchemaOperation::RenameIndex { .. } | + AsyncSchemaOperation::DeleteIndex { .. } => {}, + AsyncSchemaOperation::CreateObjectStore { callback, .. } => { + let _ = callback.send(error); + }, + AsyncSchemaOperation::DeleteObjectStore { callback, .. } => { + let _ = callback.send(error); + }, + }; + } +} + /// Operations that are not executed instantly, but rather added to a /// queue that is eventually run. #[derive(Debug, Deserialize, MallocSizeOf, Serialize)] pub enum AsyncOperation { ReadOnly(AsyncReadOnlyOperation), ReadWrite(AsyncReadWriteOperation), + Schema(AsyncSchemaOperation), } impl AsyncOperation { @@ -372,6 +422,7 @@ impl AsyncOperation { match self { Self::ReadOnly(operation) => operation.notify_error(error), Self::ReadWrite(operation) => operation.notify_error(error), + Self::Schema(operation) => operation.notify_error(error), } } } @@ -527,49 +578,6 @@ pub enum SyncOperation { txn: u64, }, - /// Creates a new index for the database - CreateIndex( - ImmutableOrigin, - String, // Database - String, // Store - String, // Index name - KeyPath, // key path - bool, // unique flag - bool, // multientry flag - ), - /// Rename an index - RenameIndex( - ImmutableOrigin, - String, // Database - String, // Store - String, // Index name - String, // New name - ), - /// Delete an index - DeleteIndex( - ImmutableOrigin, - String, // Database - String, // Store - String, // Index name - ), - - /// Creates a new store for the database - CreateObjectStore( - GenericSender>, - ImmutableOrigin, - String, // Database - String, // Store - Option, // Key Path - bool, - ), - - DeleteObjectStore( - GenericSender>, - ImmutableOrigin, - String, // Database - String, // Store - ), - CloseDatabase( ImmutableOrigin, Uuid, @@ -639,6 +647,13 @@ pub enum IndexedDBThreadMsg { IndexedDBTxnMode, AsyncOperation, ), + AsyncSchemaOperation { + origin: ImmutableOrigin, + database_name: String, + store_name: String, + operation: AsyncSchemaOperation, + transaction_serial_number: u64, + }, EngineTxnBatchComplete { origin: ImmutableOrigin, db_name: String, diff --git a/components/storage/indexeddb/engines/mod.rs b/components/storage/indexeddb/engines/mod.rs index 14234cdd098c2..0cab234bc7a74 100644 --- a/components/storage/indexeddb/engines/mod.rs +++ b/components/storage/indexeddb/engines/mod.rs @@ -67,12 +67,6 @@ pub trait KvsEngine: MallocSizeOf { unique: bool, multi_entry: bool, ) -> Result; - fn rename_index( - &self, - store_name: &str, - index_name: &str, - new_name: &str, - ) -> Result<(), Self::Error>; fn delete_index(&self, store_name: &str, index_name: String) -> Result<(), Self::Error>; fn version(&self) -> Result; diff --git a/components/storage/indexeddb/engines/sqlite.rs b/components/storage/indexeddb/engines/sqlite.rs index 66b855073ad36..d1f285898dce6 100644 --- a/components/storage/indexeddb/engines/sqlite.rs +++ b/components/storage/indexeddb/engines/sqlite.rs @@ -11,9 +11,9 @@ use sea_query::{Condition, Expr, ExprTrait, IntoCondition, SqliteQueryBuilder}; use sea_query_rusqlite::RusqliteBinder; use servo_base::threadpool::ThreadPool; use storage_traits::indexeddb::{ - AsyncOperation, AsyncReadOnlyOperation, AsyncReadWriteOperation, BackendError, - CreateObjectResult, IndexedDBIndex, IndexedDBKeyRange, IndexedDBKeyType, IndexedDBRecord, - IndexedDBTxnMode, KeyPath, PutItemResult, + AsyncOperation, AsyncReadOnlyOperation, AsyncReadWriteOperation, AsyncSchemaOperation, + BackendError, CreateObjectResult, IndexedDBIndex, IndexedDBKeyRange, IndexedDBKeyType, + IndexedDBRecord, IndexedDBTxnMode, KeyPath, PutItemResult, }; use crate::indexeddb::IndexedDBDescription; @@ -315,25 +315,19 @@ impl SqliteEngine { .query_row(&*values.as_params(), |row| row.get(0)) .map(|count: i64| count as usize) } -} - -impl KvsEngine for SqliteEngine { - type Error = Error; fn create_store( - &self, + connection: &Connection, store_name: &str, key_path: Option, auto_increment: bool, - ) -> Result { - let mut stmt = self - .connection - .prepare("SELECT * FROM object_store WHERE name = ?")?; + ) -> Result { + let mut stmt = connection.prepare("SELECT * FROM object_store WHERE name = ?")?; if stmt.exists(params![store_name.to_string()])? { // Store already exists return Ok(CreateObjectResult::AlreadyExists); } - self.connection.execute( + connection.execute( "INSERT INTO object_store (name, key_path, auto_increment) VALUES (?, ?, ?)", params![ store_name.to_string(), @@ -341,32 +335,31 @@ impl KvsEngine for SqliteEngine { auto_increment as i32 ], )?; - Ok(CreateObjectResult::Created) } - fn delete_store(&self, store_name: &str) -> Result<(), Self::Error> { + fn delete_store(connection: &Connection, store_name: &str) -> Result<(), Error> { // https://www.w3.org/TR/IndexedDB-3/#dom-idbdatabase-deleteobjectstore // Step 7. Destroy store. - let object_store = Self::object_store_by_name(&self.connection, store_name)?; + let object_store = Self::object_store_by_name(connection, store_name)?; - self.connection.execute( + connection.execute( "DELETE FROM index_data WHERE object_store_id = ?", params![object_store.id], )?; - self.connection.execute( + connection.execute( "DELETE FROM unique_index_data WHERE object_store_id = ?", params![object_store.id], )?; - self.connection.execute( + connection.execute( "DELETE FROM object_store_index WHERE object_store_id = ?", params![object_store.id], )?; - self.connection.execute( + connection.execute( "DELETE FROM object_data WHERE object_store_id = ?", params![object_store.id], )?; - let result = self.connection.execute( + let result = connection.execute( "DELETE FROM object_store WHERE id = ?", params![object_store.id], )?; @@ -379,6 +372,116 @@ impl KvsEngine for SqliteEngine { } } + fn create_index( + connection: &Connection, + store_name: &str, + index_name: String, + key_path: KeyPath, + unique: bool, + multi_entry: bool, + ) -> Result { + let object_store = connection.query_row( + "SELECT * FROM object_store WHERE name = ?", + params![store_name.to_string()], + |row| object_store_model::Model::try_from(row), + )?; + + let index_exists: bool = connection.query_row( + "SELECT EXISTS(SELECT * FROM object_store_index WHERE name = ? AND object_store_id = ?)", + params![index_name, object_store.id], + |row| row.get(0), + )?; + if index_exists { + return Ok(CreateObjectResult::AlreadyExists); + } + + connection.execute( + "INSERT INTO object_store_index (object_store_id, name, key_path, unique_index, multi_entry_index)\ + VALUES (?, ?, ?, ?, ?)", + params![ + object_store.id, + index_name, + postcard::to_stdvec(&key_path).unwrap(), + unique, + multi_entry, + ], + )?; + Ok(CreateObjectResult::Created) + } + + fn rename_index( + connection: &Connection, + store_name: &str, + index_name: &str, + new_name: &str, + ) -> Result<(), Error> { + let object_store = connection.query_row( + "SELECT * FROM object_store WHERE name = ?", + params![store_name], + |row| object_store_model::Model::try_from(row), + )?; + + // Rename the index if it exists + let _ = connection.execute( + "UPDATE object_store_index SET name = ? WHERE name = ? AND object_store_id = ?", + params![new_name, index_name, object_store.id], + )?; + Ok(()) + } + + fn delete_index( + connection: &Connection, + store_name: &str, + index_name: String, + ) -> Result<(), Error> { + let object_store = connection.query_row( + "SELECT * FROM object_store WHERE name = ?", + params![store_name.to_string()], + |row| Ok(object_store_model::Model::try_from(row).unwrap()), + )?; + + // Delete the index if it exists + let _ = connection.execute( + "DELETE FROM object_store_index WHERE name = ? AND object_store_id = ?", + params![index_name, object_store.id], + )?; + Ok(()) + } +} + +impl KvsEngine for SqliteEngine { + type Error = Error; + + fn create_store( + &self, + store_name: &str, + key_path: Option, + auto_increment: bool, + ) -> Result { + Self::create_store(&self.connection, store_name, key_path, auto_increment) + } + fn create_index( + &self, + store_name: &str, + index_name: String, + key_path: KeyPath, + unique: bool, + multi_entry: bool, + ) -> Result { + Self::create_index( + &self.connection, + store_name, + index_name, + key_path, + unique, + multi_entry, + ) + } + + fn delete_store(&self, store_name: &str) -> Result<(), Self::Error> { + Self::delete_store(&self.connection, store_name) + } + fn close_store(&self, _store_name: &str) -> Result<(), Self::Error> { // TODO: do something Ok(()) @@ -409,6 +512,19 @@ impl KvsEngine for SqliteEngine { }, }; for request in transaction.requests { + if let AsyncOperation::Schema(AsyncSchemaOperation::CreateObjectStore { + callback, + key_path, + auto_increment + }) = &request.operation { + if let Err(error) = + Self::create_store(&connection, &request.store_name, key_path.clone(), *auto_increment) + { + let _ = callback.send(BackendError::DbErr(format!("{error:?}"))); + } + continue; + } + let object_store = connection .prepare("SELECT * FROM object_store WHERE name = ?") .and_then(|mut stmt| { @@ -568,6 +684,47 @@ impl KvsEngine for SqliteEngine { .map_err(|e| BackendError::DbErr(format!("{:?}", e))), ); }, + AsyncOperation::Schema(AsyncSchemaOperation::CreateIndex { + callback, + index_name, + key_path, + unique, + multi_entry + }) => { + if let Err(error) = Self::create_index( + &connection, + &request.store_name, + index_name, + key_path, + unique, + multi_entry + ) { + let _ = callback.send(BackendError::DbErr(format!("{error:?}"))); + } + }, + AsyncOperation::Schema(AsyncSchemaOperation::CreateObjectStore { .. }) => { + unreachable!("Should be handled above"); + }, + AsyncOperation::Schema(AsyncSchemaOperation::DeleteIndex { index_name, callback }) => { + if let Err(error) = Self::delete_index(&connection, &request.store_name, index_name) { + let _ = callback.send(BackendError::DbErr(format!("{error:?}"))); + } + }, + AsyncOperation::Schema(AsyncSchemaOperation::DeleteObjectStore { callback }) => { + if let Err(error) = Self::delete_store(&connection, &request.store_name) { + let _ = callback.send(BackendError::DbErr(format!("{error:?}"))); + } + }, + AsyncOperation::Schema(AsyncSchemaOperation::RenameIndex { index_name, new_name, callback }) => { + if let Err(error) = Self::rename_index( + &connection, + &request.store_name, + &index_name, + &new_name + ) { + let _ = callback.send(BackendError::DbErr(format!("{error:?}"))); + } + }, } } on_complete(); @@ -661,76 +818,8 @@ impl KvsEngine for SqliteEngine { Ok(indexes) } - fn create_index( - &self, - store_name: &str, - index_name: String, - key_path: KeyPath, - unique: bool, - multi_entry: bool, - ) -> Result { - let object_store = self.connection.query_row( - "SELECT * FROM object_store WHERE name = ?", - params![store_name.to_string()], - |row| object_store_model::Model::try_from(row), - )?; - - let index_exists: bool = self.connection.query_row( - "SELECT EXISTS(SELECT * FROM object_store_index WHERE name = ? AND object_store_id = ?)", - params![index_name, object_store.id], - |row| row.get(0), - )?; - if index_exists { - return Ok(CreateObjectResult::AlreadyExists); - } - - self.connection.execute( - "INSERT INTO object_store_index (object_store_id, name, key_path, unique_index, multi_entry_index)\ - VALUES (?, ?, ?, ?, ?)", - params![ - object_store.id, - index_name, - postcard::to_stdvec(&key_path).unwrap(), - unique, - multi_entry, - ], - )?; - Ok(CreateObjectResult::Created) - } - - fn rename_index( - &self, - store_name: &str, - index_name: &str, - new_name: &str, - ) -> Result<(), Self::Error> { - let object_store = self.connection.query_row( - "SELECT * FROM object_store WHERE name = ?", - params![store_name], - |row| object_store_model::Model::try_from(row), - )?; - - // Rename the index if it exists - let _ = self.connection.execute( - "UPDATE object_store_index SET name = ? WHERE name = ? AND object_store_id = ?", - params![new_name, index_name, object_store.id], - )?; - Ok(()) - } - fn delete_index(&self, store_name: &str, index_name: String) -> Result<(), Self::Error> { - let object_store = self.connection.query_row( - "SELECT * FROM object_store WHERE name = ?", - params![store_name.to_string()], - |r| Ok(object_store_model::Model::try_from(r).unwrap()), - )?; - - // Delete the index if it exists - let _ = self.connection.execute( - "DELETE FROM object_store_index WHERE name = ? AND object_store_id = ?", - params![index_name, object_store.id], - )?; - Ok(()) + Self::delete_index(&self.connection, store_name, index_name) } fn version(&self) -> Result { diff --git a/components/storage/indexeddb/mod.rs b/components/storage/indexeddb/mod.rs index 6636e8ef8fe26..9e7c9924c1241 100644 --- a/components/storage/indexeddb/mod.rs +++ b/components/storage/indexeddb/mod.rs @@ -111,6 +111,7 @@ struct IndexedDBEnvironment { handled_next_unhandled_request_id: FxHashMap, handled_pending: FxHashMap>, pending_commit_callbacks: FxHashMap>>, + pending_abort_callbacks: FxHashMap>>, } impl IndexedDBEnvironment { @@ -133,6 +134,7 @@ impl IndexedDBEnvironment { handled_next_unhandled_request_id: FxHashMap::default(), handled_pending: FxHashMap::default(), pending_commit_callbacks: FxHashMap::default(), + pending_abort_callbacks: FxHashMap::default(), } } @@ -638,12 +640,6 @@ impl IndexedDBEnvironment { .map_err(|err| format!("{err:?}")) } - fn rename_index(&self, store_name: &str, index_name: &str, new_name: &str) -> DbResult<()> { - self.engine - .rename_index(store_name, index_name, new_name) - .map_err(|error| format!("{error:?}")) - } - fn delete_index(&self, store_name: &str, index_name: String) -> DbResult<()> { self.engine .delete_index(store_name, index_name) @@ -766,6 +762,48 @@ impl IndexedDBEnvironment { .set_version(version) .map_err(|err| format!("{err:?}")) } + + /// + /// + /// > When a transaction is aborted the implementation must undo (roll back) any changes that + /// > were made to the database during that transaction. + /// + /// This only aborts the transaction if one was previously queued by adding an abort + /// callback to [`Self::pending_abort_callbacks`]. + /// + /// TODO: implement the abort algorithm and rollback for the engine. + fn abort(&mut self, origin: &ImmutableOrigin, database_name: &str, transaction: u64) -> bool { + let message = || TxnCompleteMsg { + origin: origin.clone(), + db_name: database_name.into(), + txn: transaction, + result: Err(BackendError::Abort), + }; + + let Some(abort_callbacks) = self.pending_abort_callbacks.remove(&transaction) else { + return false; + }; + if abort_callbacks.is_empty() { + return false; + } + + for callback in self + .take_pending_commit_callbacks(transaction) + .into_iter() + .chain(abort_callbacks) + { + if callback.send(message()).is_err() { + error!( + "Failed to send deferred abort completion for \ + database '{database_name}' transaction {transaction}.", + ); + } + } + + self.abort_transaction(transaction); + self.schedule_transactions(origin.clone(), database_name); + true + } } fn backend_error_from_sqlite_error(err: RusqliteError) -> BackendError { @@ -1055,8 +1093,9 @@ impl IndexedDBManager { db_name, txn, } => { - let should_notify = - if let Some(db) = self.get_database_mut(origin.clone(), db_name.clone()) { + let should_notify = self + .get_database_mut(origin.clone(), db_name.clone()) + .is_some_and(|db| { // Decide which running flag to clear based on txn mode. let mode = db.transactions.get(&txn).map(|t| t.mode.clone()); @@ -1073,13 +1112,16 @@ impl IndexedDBManager { }, } + if db.abort(&origin, &db_name, txn) { + return false; + } + // If more requests were queued while this batch was running, // schedule again now. db.schedule_transactions(origin.clone(), &db_name); db.can_notify_txn_maybe_commit(txn) - } else { - false - }; + }); + if should_notify { self.handle_sync_operation(SyncOperation::TxnMaybeCommit { origin, @@ -1092,6 +1134,28 @@ impl IndexedDBManager { let reports = self.collect_memory_reports(); sender.send(ProcessReports::new(reports)); }, + IndexedDBThreadMsg::AsyncSchemaOperation { + origin, + database_name, + store_name, + operation, + transaction_serial_number, + } => { + if let Some(database) = + self.get_database_mut(origin.clone(), database_name.clone()) + { + // Queues an operation for a transaction without starting it + database.queue_operation( + &store_name, + transaction_serial_number, + IndexedDBTxnMode::Versionchange, + AsyncOperation::Schema(operation), + ); + database.schedule_transactions(origin, &database_name); + } else { + operation.notify_error(BackendError::DbNotFound); + } + }, } } } @@ -2211,29 +2275,6 @@ impl IndexedDBManager { }); let _ = sender.send(result.ok_or(BackendError::DbNotFound)); }, - SyncOperation::CreateIndex( - origin, - db_name, - store_name, - index_name, - key_path, - unique, - multi_entry, - ) => { - if let Some(db) = self.get_database(origin, db_name) { - let _ = db.create_index(&store_name, index_name, key_path, unique, multi_entry); - } - }, - SyncOperation::RenameIndex(origin, db_name, store_name, index_name, new_name) => { - if let Some(db) = self.get_database(origin, db_name) { - let _ = db.rename_index(&store_name, index_name.as_str(), new_name.as_str()); - } - }, - SyncOperation::DeleteIndex(origin, db_name, store_name, index_name) => { - if let Some(db) = self.get_database(origin, db_name) { - let _ = db.delete_index(&store_name, index_name); - } - }, SyncOperation::Commit(callback, origin, db_name, txn) => { // https://w3c.github.io/IndexedDB/#commit-a-transaction // TODO: implement the commit algorithm and only reply after the backend has @@ -2369,29 +2410,6 @@ impl IndexedDBManager { let _ = sender.send(Err(BackendError::DbNotFound)); } }, - SyncOperation::CreateObjectStore( - sender, - origin, - db_name, - store_name, - key_paths, - auto_increment, - ) => { - if let Some(db) = self.get_database_mut(origin, db_name) { - let result = db.create_object_store(&store_name, key_paths, auto_increment); - let _ = sender.send(result.map_err(BackendError::from)); - } else { - let _ = sender.send(Err(BackendError::DbNotFound)); - } - }, - SyncOperation::DeleteObjectStore(sender, origin, db_name, store_name) => { - if let Some(db) = self.get_database_mut(origin, db_name) { - let result = db.delete_object_store(&store_name); - let _ = sender.send(result.map_err(BackendError::from)); - } else { - let _ = sender.send(Err(BackendError::DbNotFound)); - } - }, SyncOperation::Version(sender, origin, db_name) => { if let Some(db) = self.get_database(origin, db_name) { let _ = sender.send(db.version().map_err(backend_error_from_sqlite_error)); @@ -2413,12 +2431,9 @@ impl IndexedDBManager { } } - /// - /// - /// > When a transaction is aborted the implementation must undo (roll back) any changes that - /// > were made to the database during that transaction. - /// - /// TODO: implement the abort algorithm and rollback for the engine. + /// Handling for the `Abort` message which will call [`Self::abort`] if the transaction + /// being aborted is not ongoing. If the transaction is in process, abort is delayed until + /// the batch finishes. fn handle_abort( &mut self, abort_callback: GenericCallback, @@ -2433,26 +2448,32 @@ impl IndexedDBManager { result: Err(BackendError::Abort), }; - if let Some(database) = self.get_database_mut(origin.clone(), database_name.clone()) { - for callback in database.take_pending_commit_callbacks(transaction) { - if callback.send(message()).is_err() { - error!( - "Failed to send deferred abort completion for \ - database '{database_name}' transaction {transaction}.", - ); - } + let Some(database) = self.get_database_mut(origin.clone(), database_name.clone()) else { + // We didn't find the database, so just treat the transaction as aborted. + if abort_callback.send(message()).is_err() { + error!( + "Failed to send abort completion for database \ + '{database_name}' transaction {transaction}.", + ); } + return; + }; - database.abort_transaction(transaction); - database.schedule_transactions(origin.clone(), &database_name); - } + database + .pending_abort_callbacks + .entry(transaction) + .or_default() + .push(abort_callback); - if abort_callback.send(message()).is_err() { - error!( - "Failed to send deferred abort completion for \ - database '{database_name}' transaction {transaction}.", - ); + // If the transaction is running wait to abort until after it finishes to actually + // abort. + if database.running_readwrite == Some(transaction) || + database.running_readonly.contains(&transaction) + { + return; } + + database.abort(&origin, &database_name, transaction); } fn collect_memory_reports(&self) -> Vec { diff --git a/tests/wpt/meta/IndexedDB/idbdatabase_deleteObjectStore.any.js.ini b/tests/wpt/meta/IndexedDB/idbdatabase_deleteObjectStore.any.js.ini index 04064ff152d84..bad7a5f10dfa8 100644 --- a/tests/wpt/meta/IndexedDB/idbdatabase_deleteObjectStore.any.js.ini +++ b/tests/wpt/meta/IndexedDB/idbdatabase_deleteObjectStore.any.js.ini @@ -1,25 +1,2 @@ -[idbdatabase_deleteObjectStore.any.worker.html] - [Attempting to access an index that was deleted as part of object store deletion and then recreated using the same object store name should throw a NotFoundError] - expected: FAIL - - [Deleted object store's name should be removed from database's list. Attempting to use a deleted IDBObjectStore should throw an InvalidStateError] - expected: FAIL - - -[idbdatabase_deleteObjectStore.any.html] - [Deleted object store's name should be removed from database's list. Attempting to use a deleted IDBObjectStore should throw an InvalidStateError] - expected: FAIL - - [Attempting to access an index that was deleted as part of object store deletion and then recreated using the same object store name should throw a NotFoundError] - expected: FAIL - - [idbdatabase_deleteObjectStore.any.serviceworker.html] expected: ERROR - -[idbdatabase_deleteObjectStore.any.sharedworker.html] - [Deleted object store's name should be removed from database's list. Attempting to use a deleted IDBObjectStore should throw an InvalidStateError] - expected: FAIL - - [Attempting to access an index that was deleted as part of object store deletion and then recreated using the same object store name should throw a NotFoundError] - expected: FAIL diff --git a/tests/wpt/meta/IndexedDB/name-scopes.any.js.ini b/tests/wpt/meta/IndexedDB/name-scopes.any.js.ini index da5984c7f5187..c94406a6beaf1 100644 --- a/tests/wpt/meta/IndexedDB/name-scopes.any.js.ini +++ b/tests/wpt/meta/IndexedDB/name-scopes.any.js.ini @@ -2,7 +2,6 @@ expected: ERROR [name-scopes.any.worker.html] - expected: ERROR [Non-unique index keys] expected: FAIL @@ -11,7 +10,6 @@ [name-scopes.any.html] - expected: ERROR [Non-unique index keys] expected: FAIL @@ -20,7 +18,6 @@ [name-scopes.any.sharedworker.html] - expected: ERROR [Non-unique index keys] expected: FAIL