net: Split read-only and read-write IndexedDB operations into separate enums (#37575)

This change allows the compiler to recognize if any read-only operations
are missing an implementation when processing a readonly transaction.

Testing: The existing behaviour is unchanged, so current tests suffice.
The new code is unused and cannot be tested.
Fixes: part of #6963

---------

Signed-off-by: Josh Matthews <josh@joshmatthews.net>
This commit is contained in:
Josh Matthews 2025-07-12 07:29:36 -04:00 committed by GitHub
parent 2c116f4011
commit 6dbd64e72d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 67 additions and 27 deletions

View file

@ -8,7 +8,9 @@ use std::sync::{Arc, RwLock};
use heed::types::*; use heed::types::*;
use heed::{Database, Env, EnvOpenOptions}; use heed::{Database, Env, EnvOpenOptions};
use log::warn; use log::warn;
use net_traits::indexeddb_thread::{AsyncOperation, IndexedDBTxnMode}; use net_traits::indexeddb_thread::{
AsyncOperation, AsyncReadOnlyOperation, AsyncReadWriteOperation, IndexedDBTxnMode,
};
use tokio::sync::oneshot; use tokio::sync::oneshot;
use super::{KvsEngine, KvsTransaction, SanitizedName}; use super::{KvsEngine, KvsTransaction, SanitizedName};
@ -130,7 +132,7 @@ impl KvsEngine for HeedEngine {
let rtxn = env.read_txn().expect("Could not create idb store reader"); let rtxn = env.read_txn().expect("Could not create idb store reader");
for request in transaction.requests { for request in transaction.requests {
match request.operation { match request.operation {
AsyncOperation::GetItem(key) => { AsyncOperation::ReadOnly(AsyncReadOnlyOperation::GetItem(key)) => {
let key: Vec<u8> = bincode::serialize(&key).unwrap(); let key: Vec<u8> = bincode::serialize(&key).unwrap();
let stores = stores let stores = stores
.read() .read()
@ -146,7 +148,17 @@ impl KvsEngine for HeedEngine {
let _ = request.sender.send(None); let _ = request.sender.send(None);
} }
}, },
_ => { AsyncOperation::ReadOnly(AsyncReadOnlyOperation::Count(key)) => {
let _key: Vec<u8> = bincode::serialize(&key).unwrap();
let stores = stores
.read()
.expect("Could not acquire read lock on stores");
let _store = stores
.get(&request.store_name)
.expect("Could not get store");
// FIXME:(arihant2math) Return count with sender
},
AsyncOperation::ReadWrite(..) => {
// We cannot reach this, as checks are made earlier so that // We cannot reach this, as checks are made earlier so that
// no modifying requests are executed on readonly transactions // no modifying requests are executed on readonly transactions
unreachable!( unreachable!(
@ -169,7 +181,11 @@ impl KvsEngine for HeedEngine {
let mut wtxn = env.write_txn().expect("Could not create idb store writer"); let mut wtxn = env.write_txn().expect("Could not create idb store writer");
for request in transaction.requests { for request in transaction.requests {
match request.operation { match request.operation {
AsyncOperation::PutItem(key, value, overwrite) => { AsyncOperation::ReadWrite(AsyncReadWriteOperation::PutItem(
key,
value,
overwrite,
)) => {
let key: Vec<u8> = bincode::serialize(&key).unwrap(); let key: Vec<u8> = bincode::serialize(&key).unwrap();
let stores = stores let stores = stores
.write() .write()
@ -194,7 +210,7 @@ impl KvsEngine for HeedEngine {
let _ = request.sender.send(None); let _ = request.sender.send(None);
} }
}, },
AsyncOperation::GetItem(key) => { AsyncOperation::ReadOnly(AsyncReadOnlyOperation::GetItem(key)) => {
let key: Vec<u8> = bincode::serialize(&key).unwrap(); let key: Vec<u8> = bincode::serialize(&key).unwrap();
let stores = stores let stores = stores
.read() .read()
@ -210,7 +226,7 @@ impl KvsEngine for HeedEngine {
let _ = request.sender.send(None); let _ = request.sender.send(None);
} }
}, },
AsyncOperation::RemoveItem(key) => { AsyncOperation::ReadWrite(AsyncReadWriteOperation::RemoveItem(key)) => {
let key: Vec<u8> = bincode::serialize(&key).unwrap(); let key: Vec<u8> = bincode::serialize(&key).unwrap();
let stores = stores let stores = stores
.write() .write()
@ -221,7 +237,7 @@ impl KvsEngine for HeedEngine {
let result = store.inner.delete(&mut wtxn, &key).ok().and(Some(key)); let result = store.inner.delete(&mut wtxn, &key).ok().and(Some(key));
let _ = request.sender.send(result); let _ = request.sender.send(result);
}, },
AsyncOperation::Count(key) => { AsyncOperation::ReadOnly(AsyncReadOnlyOperation::Count(key)) => {
let _key: Vec<u8> = bincode::serialize(&key).unwrap(); let _key: Vec<u8> = bincode::serialize(&key).unwrap();
let stores = stores let stores = stores
.read() .read()

View file

@ -16,7 +16,8 @@ use js::rust::{HandleValue, MutableHandleValue};
use log::error; use log::error;
use net_traits::IpcSend; use net_traits::IpcSend;
use net_traits::indexeddb_thread::{ use net_traits::indexeddb_thread::{
AsyncOperation, IndexedDBKeyType, IndexedDBThreadMsg, SyncOperation, AsyncOperation, AsyncReadOnlyOperation, AsyncReadWriteOperation, IndexedDBKeyType,
IndexedDBThreadMsg, SyncOperation,
}; };
use profile_traits::ipc; use profile_traits::ipc;
@ -454,7 +455,11 @@ impl IDBObjectStore {
IDBRequest::execute_async( IDBRequest::execute_async(
self, self,
AsyncOperation::PutItem(serialized_key, serialized_value.serialized, overwrite), AsyncOperation::ReadWrite(AsyncReadWriteOperation::PutItem(
serialized_key,
serialized_value.serialized,
overwrite,
)),
None, None,
can_gc, can_gc,
) )
@ -486,7 +491,12 @@ impl IDBObjectStoreMethods<crate::DomTypeHolder> for IDBObjectStore {
fn Delete(&self, cx: SafeJSContext, query: HandleValue) -> Fallible<DomRoot<IDBRequest>> { fn Delete(&self, cx: SafeJSContext, query: HandleValue) -> Fallible<DomRoot<IDBRequest>> {
let serialized_query = IDBObjectStore::convert_value_to_key(cx, query, None); let serialized_query = IDBObjectStore::convert_value_to_key(cx, query, None);
serialized_query.and_then(|q| { serialized_query.and_then(|q| {
IDBRequest::execute_async(self, AsyncOperation::RemoveItem(q), None, CanGc::note()) IDBRequest::execute_async(
self,
AsyncOperation::ReadWrite(AsyncReadWriteOperation::RemoveItem(q)),
None,
CanGc::note(),
)
}) })
} }
@ -499,7 +509,12 @@ impl IDBObjectStoreMethods<crate::DomTypeHolder> for IDBObjectStore {
fn Get(&self, cx: SafeJSContext, query: HandleValue) -> Fallible<DomRoot<IDBRequest>> { fn Get(&self, cx: SafeJSContext, query: HandleValue) -> Fallible<DomRoot<IDBRequest>> {
let serialized_query = IDBObjectStore::convert_value_to_key(cx, query, None); let serialized_query = IDBObjectStore::convert_value_to_key(cx, query, None);
serialized_query.and_then(|q| { serialized_query.and_then(|q| {
IDBRequest::execute_async(self, AsyncOperation::GetItem(q), None, CanGc::note()) IDBRequest::execute_async(
self,
AsyncOperation::ReadOnly(AsyncReadOnlyOperation::GetItem(q)),
None,
CanGc::note(),
)
}) })
} }

View file

@ -67,15 +67,20 @@ impl IndexedDBKeyRange {
} }
} }
// Operations that are not executed instantly, but rather added to a
// queue that is eventually run.
#[derive(Debug, Deserialize, Serialize)] #[derive(Debug, Deserialize, Serialize)]
pub enum AsyncOperation { pub enum AsyncReadOnlyOperation {
/// Gets the value associated with the given key in the associated idb data /// Gets the value associated with the given key in the associated idb data
GetItem( GetItem(
IndexedDBKeyType, // Key IndexedDBKeyType, // Key
), ),
Count(
IndexedDBKeyType, // Key
),
}
#[derive(Debug, Deserialize, Serialize)]
pub enum AsyncReadWriteOperation {
/// Sets the value of the given key in the associated idb data /// Sets the value of the given key in the associated idb data
PutItem( PutItem(
IndexedDBKeyType, // Key IndexedDBKeyType, // Key
@ -87,15 +92,19 @@ pub enum AsyncOperation {
RemoveItem( RemoveItem(
IndexedDBKeyType, // Key IndexedDBKeyType, // Key
), ),
}
Count( /// Operations that are not executed instantly, but rather added to a
IndexedDBKeyType, // Key /// queue that is eventually run.
), #[derive(Debug, Deserialize, Serialize)]
pub enum AsyncOperation {
ReadOnly(AsyncReadOnlyOperation),
ReadWrite(AsyncReadWriteOperation),
} }
#[derive(Debug, Deserialize, Serialize)] #[derive(Debug, Deserialize, Serialize)]
pub enum SyncOperation { pub enum SyncOperation {
// Upgrades the version of the database /// Upgrades the version of the database
UpgradeVersion( UpgradeVersion(
IpcSender<Result<u64, ()>>, IpcSender<Result<u64, ()>>,
ImmutableOrigin, ImmutableOrigin,
@ -103,7 +112,7 @@ pub enum SyncOperation {
u64, // Serial number for the transaction u64, // Serial number for the transaction
u64, // Version to upgrade to u64, // Version to upgrade to
), ),
// Checks if an object store has a key generator, used in e.g. Put /// Checks if an object store has a key generator, used in e.g. Put
HasKeyGenerator( HasKeyGenerator(
IpcSender<bool>, IpcSender<bool>,
ImmutableOrigin, ImmutableOrigin,
@ -111,7 +120,7 @@ pub enum SyncOperation {
String, // Store String, // Store
), ),
// Commits changes of a transaction to the database /// Commits changes of a transaction to the database
Commit( Commit(
IpcSender<Result<(), ()>>, IpcSender<Result<(), ()>>,
ImmutableOrigin, ImmutableOrigin,
@ -119,7 +128,7 @@ pub enum SyncOperation {
u64, // Transaction serial number u64, // Transaction serial number
), ),
// Creates a new store for the database /// Creates a new store for the database
CreateObjectStore( CreateObjectStore(
IpcSender<Result<(), ()>>, IpcSender<Result<(), ()>>,
ImmutableOrigin, ImmutableOrigin,
@ -148,23 +157,23 @@ pub enum SyncOperation {
Option<u64>, // Eventual version Option<u64>, // Eventual version
), ),
// Deletes the database /// Deletes the database
DeleteDatabase( DeleteDatabase(
IpcSender<Result<(), ()>>, IpcSender<Result<(), ()>>,
ImmutableOrigin, ImmutableOrigin,
String, // Database String, // Database
), ),
// Returns an unique identifier that is used to be able to /// Returns an unique identifier that is used to be able to
// commit/abort transactions. /// commit/abort transactions.
RegisterNewTxn( RegisterNewTxn(
IpcSender<u64>, IpcSender<u64>,
ImmutableOrigin, ImmutableOrigin,
String, // Database String, // Database
), ),
// Starts executing the requests of a transaction /// Starts executing the requests of a transaction
// https://www.w3.org/TR/IndexedDB-2/#transaction-start /// <https://www.w3.org/TR/IndexedDB-2/#transaction-start>
StartTransaction( StartTransaction(
IpcSender<Result<(), ()>>, IpcSender<Result<(), ()>>,
ImmutableOrigin, ImmutableOrigin,
@ -172,7 +181,7 @@ pub enum SyncOperation {
u64, // The serial number of the mutating transaction u64, // The serial number of the mutating transaction
), ),
// Returns the version of the database /// Returns the version of the database
Version( Version(
IpcSender<u64>, IpcSender<u64>,
ImmutableOrigin, ImmutableOrigin,