From 91b2ab545819f0d8abe8e2940f24d00763fa2d2c Mon Sep 17 00:00:00 2001 From: Ashwin Naren Date: Sat, 30 Aug 2025 20:54:18 -0700 Subject: [PATCH] indexeddb: Implement autoincremented keys and report autoincrementedness properly through DOM interface (#38723) Autoincrementedness was previously being reported as always false. This PR makes the state become queried from the backend, as the spec specifies. Additionally this PR ensures the backend correctly handles an object store which autoincrements. Testing: WPT Fixes: None --------- Signed-off-by: Ashwin Naren --- components/net/indexeddb/engines/sqlite.rs | 34 +++++++++++-- .../net/indexeddb/engines/sqlite/create.rs | 2 +- .../engines/sqlite/object_store_model.rs | 2 +- components/net/tests/sqlite.rs | 2 +- components/script/dom/idbobjectstore.rs | 16 ++----- components/shared/net/indexeddb_thread.rs | 2 +- .../clone-before-keypath-eval.any.js.ini | 6 --- .../crashtests/create-index.any.js.ini | 4 +- .../idbobjectstore-request-source.any.js.ini | 12 ----- .../idbobjectstore_getKey.any.js.ini | 48 +++++++++---------- .../IndexedDB/idbtransaction_abort.any.js.ini | 6 ++- .../interleaved-cursors-large.any.js.ini | 6 ++- .../interleaved-cursors-small.any.js.ini | 14 +++--- 13 files changed, 80 insertions(+), 74 deletions(-) diff --git a/components/net/indexeddb/engines/sqlite.rs b/components/net/indexeddb/engines/sqlite.rs index b26feacfa15..3086b848a6e 100644 --- a/components/net/indexeddb/engines/sqlite.rs +++ b/components/net/indexeddb/engines/sqlite.rs @@ -8,7 +8,8 @@ use ipc_channel::ipc::IpcSender; use log::{error, info}; use net_traits::indexeddb_thread::{ AsyncOperation, AsyncReadOnlyOperation, AsyncReadWriteOperation, BackendError, BackendResult, - CreateObjectResult, IndexedDBKeyRange, IndexedDBTxnMode, KeyPath, PutItemResult, + CreateObjectResult, IndexedDBKeyRange, IndexedDBKeyType, IndexedDBTxnMode, KeyPath, + PutItemResult, }; use rusqlite::{Connection, Error, OptionalExtension, params}; use sea_query::{Condition, Expr, ExprTrait, IntoCondition, SqliteQueryBuilder}; @@ -237,6 +238,22 @@ impl SqliteEngine { .query_row(&*values.as_params(), |row| row.get(0)) .map(|count: i64| count as usize) } + + fn generate_key( + connection: &Connection, + store: &object_store_model::Model, + ) -> Result { + if store.auto_increment == 0 { + unreachable!("Should be caught in the script thread"); + } + // TODO: handle overflows, this also needs to be able to handle 2^53 as per spec + let new_key = store.auto_increment + 1; + connection.execute( + "UPDATE object_store SET auto_increment = ? WHERE id = ?", + params![new_key, store.id], + )?; + Ok(IndexedDBKeyType::Number(new_key as f64)) + } } impl KvsEngine for SqliteEngine { @@ -260,7 +277,7 @@ impl KvsEngine for SqliteEngine { params![ store_name.to_string(), key_path.map(|v| bincode::serialize(&v).unwrap()), - auto_increment + auto_increment as i32 ], )?; @@ -350,6 +367,16 @@ impl KvsEngine for SqliteEngine { let Ok(object_store) = process_object_store(object_store, &sender) else { continue; }; + let key = match key + .map(Ok) + .unwrap_or_else(|| Self::generate_key(&connection, &object_store)) + { + Ok(key) => key, + Err(e) => { + let _ = sender.send(Err(BackendError::DbErr(format!("{:?}", e)))); + continue; + }, + }; let serialized_key: Vec = bincode::serialize(&key).unwrap(); let _ = sender.send( Self::put_item( @@ -442,7 +469,8 @@ impl KvsEngine for SqliteEngine { .optional() .unwrap() // TODO: Wrong (change trait definition for this function) - .unwrap_or_default() + .unwrap_or_default() != + 0 } fn key_path(&self, store_name: &str) -> Option { diff --git a/components/net/indexeddb/engines/sqlite/create.rs b/components/net/indexeddb/engines/sqlite/create.rs index 44bf7f23e2c..c841b98b149 100644 --- a/components/net/indexeddb/engines/sqlite/create.rs +++ b/components/net/indexeddb/engines/sqlite/create.rs @@ -22,7 +22,7 @@ create table object_store ( name varchar not null unique, key_path varbinary_blob, - auto_increment boolean default FALSE not null + auto_increment integer default FALSE not null );"#; conn.execute(OBJECT_STORE, [])?; diff --git a/components/net/indexeddb/engines/sqlite/object_store_model.rs b/components/net/indexeddb/engines/sqlite/object_store_model.rs index a41b0cf1577..ae694e72b7d 100644 --- a/components/net/indexeddb/engines/sqlite/object_store_model.rs +++ b/components/net/indexeddb/engines/sqlite/object_store_model.rs @@ -20,7 +20,7 @@ pub struct Model { pub id: i32, pub name: String, pub key_path: Option>, - pub auto_increment: bool, + pub auto_increment: i32, } impl TryFrom<&Row<'_>> for Model { diff --git a/components/net/tests/sqlite.rs b/components/net/tests/sqlite.rs index d20b60a6a29..1e0da5f71cf 100644 --- a/components/net/tests/sqlite.rs +++ b/components/net/tests/sqlite.rs @@ -210,7 +210,7 @@ fn test_async_operations() { store_name: store_name.to_owned(), operation: AsyncOperation::ReadWrite(AsyncReadWriteOperation::PutItem { sender: channel.0, - key: IndexedDBKeyType::Number(1.0), + key: Some(IndexedDBKeyType::Number(1.0)), value: vec![1, 2, 3], should_overwrite: false, }), diff --git a/components/script/dom/idbobjectstore.rs b/components/script/dom/idbobjectstore.rs index 618dc644c17..cc6566399d2 100644 --- a/components/script/dom/idbobjectstore.rs +++ b/components/script/dom/idbobjectstore.rs @@ -46,7 +46,6 @@ pub struct IDBObjectStore { key_path: Option, index_names: DomRoot, transaction: Dom, - auto_increment: bool, // We store the db name in the object store to be able to find the correct // store in the idb thread when checking if we have a key generator @@ -79,9 +78,6 @@ impl IDBObjectStore { index_names: DOMStringList::new(global, Vec::new(), can_gc), transaction: Dom::from_ref(transaction), - // FIXME:(arihant2math) - auto_increment: false, - db_name, } } @@ -240,10 +236,10 @@ impl IDBObjectStore { } // Step 8: If key was given, then: convert a value to a key with key - let serialized_key: IndexedDBKeyType; + let serialized_key: Option; if !key.is_undefined() { - serialized_key = convert_value_to_key(cx, key, None)?; + serialized_key = Some(convert_value_to_key(cx, key, None)?); } else { // Step 11: We should use in-line keys instead if let Some(Ok(kpk)) = self @@ -251,13 +247,12 @@ impl IDBObjectStore { .as_ref() .map(|p| extract_key(cx, value, p, None)) { - serialized_key = kpk; + serialized_key = Some(kpk); } else { if !self.has_key_generator() { return Err(Error::Data); } - // FIXME:(arihant2math) - return Err(Error::NotSupported); + serialized_key = None; } } @@ -509,7 +504,6 @@ impl IDBObjectStoreMethods for IDBObjectStore { // https://www.w3.org/TR/IndexedDB-2/#dom-idbobjectstore-autoincrement fn AutoIncrement(&self) -> bool { - // FIXME(arihant2math): This is wrong - self.auto_increment + self.has_key_generator() } } diff --git a/components/shared/net/indexeddb_thread.rs b/components/shared/net/indexeddb_thread.rs index 67838356bd1..5b05ccf8ab4 100644 --- a/components/shared/net/indexeddb_thread.rs +++ b/components/shared/net/indexeddb_thread.rs @@ -277,7 +277,7 @@ pub enum AsyncReadWriteOperation { /// Sets the value of the given key in the associated idb data PutItem { sender: IpcSender>, - key: IndexedDBKeyType, + key: Option, value: Vec, should_overwrite: bool, }, diff --git a/tests/wpt/meta/IndexedDB/clone-before-keypath-eval.any.js.ini b/tests/wpt/meta/IndexedDB/clone-before-keypath-eval.any.js.ini index 0f1bd1bd993..f03ebef5542 100644 --- a/tests/wpt/meta/IndexedDB/clone-before-keypath-eval.any.js.ini +++ b/tests/wpt/meta/IndexedDB/clone-before-keypath-eval.any.js.ini @@ -1,7 +1,4 @@ [clone-before-keypath-eval.any.worker.html] - [Key generator and key path validity check operates on a clone] - expected: FAIL - [Failing key path validity check operates on a clone] expected: FAIL @@ -19,9 +16,6 @@ expected: ERROR [clone-before-keypath-eval.any.html] - [Key generator and key path validity check operates on a clone] - expected: FAIL - [Failing key path validity check operates on a clone] expected: FAIL diff --git a/tests/wpt/meta/IndexedDB/crashtests/create-index.any.js.ini b/tests/wpt/meta/IndexedDB/crashtests/create-index.any.js.ini index a3a4aeff32f..ffe92f91853 100644 --- a/tests/wpt/meta/IndexedDB/crashtests/create-index.any.js.ini +++ b/tests/wpt/meta/IndexedDB/crashtests/create-index.any.js.ini @@ -1,11 +1,11 @@ [create-index.any.html] - expected: TIMEOUT + expected: CRASH [Assure no crash when populating index] expected: TIMEOUT [create-index.any.worker.html] - expected: CRASH + expected: TIMEOUT [Assure no crash when populating index] expected: TIMEOUT diff --git a/tests/wpt/meta/IndexedDB/idbobjectstore-request-source.any.js.ini b/tests/wpt/meta/IndexedDB/idbobjectstore-request-source.any.js.ini index 57237c72b4c..49750021560 100644 --- a/tests/wpt/meta/IndexedDB/idbobjectstore-request-source.any.js.ini +++ b/tests/wpt/meta/IndexedDB/idbobjectstore-request-source.any.js.ini @@ -2,12 +2,6 @@ expected: ERROR [idbobjectstore-request-source.any.worker.html] - [The source of the request from store => store.put(0) is the object store itself] - expected: FAIL - - [The source of the request from store => store.add(0) is the object store itself] - expected: FAIL - [The source of the request from store => store.getAll() is the object store itself] expected: FAIL @@ -28,12 +22,6 @@ expected: ERROR [idbobjectstore-request-source.any.html] - [The source of the request from store => store.put(0) is the object store itself] - expected: FAIL - - [The source of the request from store => store.add(0) is the object store itself] - expected: FAIL - [The source of the request from store => store.getAll() is the object store itself] expected: FAIL diff --git a/tests/wpt/meta/IndexedDB/idbobjectstore_getKey.any.js.ini b/tests/wpt/meta/IndexedDB/idbobjectstore_getKey.any.js.ini index 0a350f621d6..d4a7ff198d9 100644 --- a/tests/wpt/meta/IndexedDB/idbobjectstore_getKey.any.js.ini +++ b/tests/wpt/meta/IndexedDB/idbobjectstore_getKey.any.js.ini @@ -1,33 +1,34 @@ [idbobjectstore_getKey.any.worker.html] + expected: TIMEOUT [IDBObjectStore.getKey() - basic - key] - expected: FAIL + expected: TIMEOUT [IDBObjectStore.getKey() - basic - range] - expected: FAIL + expected: TIMEOUT [IDBObjectStore.getKey() - basic - key - no match] - expected: FAIL + expected: TIMEOUT [IDBObjectStore.getKey() - basic - range - no match] expected: FAIL [IDBObjectStore.getKey() - key path - key] - expected: FAIL + expected: TIMEOUT [IDBObjectStore.getKey() - key path - range] - expected: FAIL + expected: TIMEOUT [IDBObjectStore.getKey() - key path - key - no match] - expected: FAIL + expected: TIMEOUT [IDBObjectStore.getKey() - key path - range - no match] expected: FAIL [IDBObjectStore.getKey() - key generator - key] - expected: FAIL + expected: TIMEOUT [IDBObjectStore.getKey() - key generator - range] - expected: FAIL + expected: TIMEOUT [IDBObjectStore.getKey() - key generator - key - no match] expected: FAIL @@ -36,10 +37,10 @@ expected: FAIL [IDBObjectStore.getKey() - key generator and key path - key] - expected: FAIL + expected: TIMEOUT [IDBObjectStore.getKey() - key generator and key path - range] - expected: FAIL + expected: TIMEOUT [IDBObjectStore.getKey() - key generator and key path - key - no match] expected: FAIL @@ -47,40 +48,38 @@ [IDBObjectStore.getKey() - key generator and key path - range - no match] expected: FAIL - [IDBObjectStore.getKey() - invalid parameters] - expected: FAIL - [idbobjectstore_getKey.any.html] + expected: TIMEOUT [IDBObjectStore.getKey() - basic - key] - expected: FAIL + expected: TIMEOUT [IDBObjectStore.getKey() - basic - range] - expected: FAIL + expected: TIMEOUT [IDBObjectStore.getKey() - basic - key - no match] - expected: FAIL + expected: TIMEOUT [IDBObjectStore.getKey() - basic - range - no match] expected: FAIL [IDBObjectStore.getKey() - key path - key] - expected: FAIL + expected: TIMEOUT [IDBObjectStore.getKey() - key path - range] - expected: FAIL + expected: TIMEOUT [IDBObjectStore.getKey() - key path - key - no match] - expected: FAIL + expected: TIMEOUT [IDBObjectStore.getKey() - key path - range - no match] expected: FAIL [IDBObjectStore.getKey() - key generator - key] - expected: FAIL + expected: TIMEOUT [IDBObjectStore.getKey() - key generator - range] - expected: FAIL + expected: TIMEOUT [IDBObjectStore.getKey() - key generator - key - no match] expected: FAIL @@ -89,16 +88,13 @@ expected: FAIL [IDBObjectStore.getKey() - key generator and key path - key] - expected: FAIL + expected: TIMEOUT [IDBObjectStore.getKey() - key generator and key path - range] - expected: FAIL + expected: TIMEOUT [IDBObjectStore.getKey() - key generator and key path - key - no match] expected: FAIL [IDBObjectStore.getKey() - key generator and key path - range - no match] expected: FAIL - - [IDBObjectStore.getKey() - invalid parameters] - expected: FAIL diff --git a/tests/wpt/meta/IndexedDB/idbtransaction_abort.any.js.ini b/tests/wpt/meta/IndexedDB/idbtransaction_abort.any.js.ini index f18640ba38b..af19f68a153 100644 --- a/tests/wpt/meta/IndexedDB/idbtransaction_abort.any.js.ini +++ b/tests/wpt/meta/IndexedDB/idbtransaction_abort.any.js.ini @@ -1,4 +1,5 @@ [idbtransaction_abort.any.worker.html] + expected: TIMEOUT [Abort event should fire during transaction] expected: FAIL @@ -6,7 +7,7 @@ expected: FAIL [Abort on completed transaction should throw InvalidStateError.] - expected: FAIL + expected: TIMEOUT [idbtransaction_abort.any.sharedworker.html] @@ -16,6 +17,7 @@ expected: ERROR [idbtransaction_abort.any.html] + expected: TIMEOUT [Abort event should fire during transaction] expected: FAIL @@ -23,4 +25,4 @@ expected: FAIL [Abort on completed transaction should throw InvalidStateError.] - expected: FAIL + expected: TIMEOUT diff --git a/tests/wpt/meta/IndexedDB/interleaved-cursors-large.any.js.ini b/tests/wpt/meta/IndexedDB/interleaved-cursors-large.any.js.ini index 09c6c778b33..67e6069f370 100644 --- a/tests/wpt/meta/IndexedDB/interleaved-cursors-large.any.js.ini +++ b/tests/wpt/meta/IndexedDB/interleaved-cursors-large.any.js.ini @@ -1,6 +1,7 @@ [interleaved-cursors-large.any.html] + expected: TIMEOUT [250 cursors] - expected: FAIL + expected: TIMEOUT [interleaved-cursors-large.any.serviceworker.html] @@ -10,5 +11,6 @@ expected: ERROR [interleaved-cursors-large.any.worker.html] + expected: TIMEOUT [250 cursors] - expected: FAIL + expected: TIMEOUT diff --git a/tests/wpt/meta/IndexedDB/interleaved-cursors-small.any.js.ini b/tests/wpt/meta/IndexedDB/interleaved-cursors-small.any.js.ini index 83d531b8c79..2e49b90c834 100644 --- a/tests/wpt/meta/IndexedDB/interleaved-cursors-small.any.js.ini +++ b/tests/wpt/meta/IndexedDB/interleaved-cursors-small.any.js.ini @@ -2,25 +2,27 @@ expected: ERROR [interleaved-cursors-small.any.worker.html] + expected: TIMEOUT [1 cursors] - expected: FAIL + expected: TIMEOUT [10 cursors] - expected: FAIL + expected: NOTRUN [100 cursors] - expected: FAIL + expected: NOTRUN [interleaved-cursors-small.any.sharedworker.html] expected: ERROR [interleaved-cursors-small.any.html] + expected: TIMEOUT [1 cursors] - expected: FAIL + expected: TIMEOUT [10 cursors] - expected: FAIL + expected: NOTRUN [100 cursors] - expected: FAIL + expected: NOTRUN