From 12c3377275cca0293f62e69de943c2cf6e3ea644 Mon Sep 17 00:00:00 2001 From: "jsbell@chromium.org" Date: Thu, 22 May 2014 19:42:42 +0000 Subject: [PATCH] IndexedDB: Prevent store/index deletion from racing ahead of use In the Indexed DB spec, schema updates occur synchronously from the perspective of script, e.g. you can create a store then use it immediately without waiting for a request to complete. That model was carried through to the back end, but with a subtle issue that the deletion of a store or index could "race ahead" of a previously requested use of it. Change store/index deletion to take place in order with other requests, so do all the work (in-memory and backing store) during the scheduled operation, rather than on IPC receipt. Also change store/index creation to do all the work (in-memory and backing store) synchronously, since index population is already preemptively done. And since this requires shuffling when the "abort" operations should be queued, rework the API there slightly. BUG=370056,368271, 362723 R=cmumford@chromium.org,ericu@chromium.org Review URL: https://codereview.chromium.org/277583002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@272282 0039d316-1c4b-4281-b951-d872f2087c98 --- content/browser/indexed_db/indexed_db_database.cc | 133 +++++++------- content/browser/indexed_db/indexed_db_database.h | 10 +- .../indexed_db/indexed_db_database_unittest.cc | 199 ++++++++++++++++++++- .../indexed_db/indexed_db_fake_backing_store.cc | 23 ++- .../indexed_db/indexed_db_fake_backing_store.h | 9 + .../browser/indexed_db/indexed_db_transaction.cc | 18 +- .../browser/indexed_db/indexed_db_transaction.h | 2 +- .../indexed_db/indexed_db_transaction_unittest.cc | 41 +++-- .../indexed_db/mock_indexed_db_callbacks.cc | 2 + .../browser/indexed_db/mock_indexed_db_callbacks.h | 1 + 10 files changed, 319 insertions(+), 119 deletions(-) diff --git a/content/browser/indexed_db/indexed_db_database.cc b/content/browser/indexed_db/indexed_db_database.cc index 1d9ccbbc5f27..a7d5da06d2b0 100644 --- a/content/browser/indexed_db/indexed_db_database.cc +++ b/content/browser/indexed_db/indexed_db_database.cc @@ -277,6 +277,9 @@ void IndexedDBDatabase::CreateObjectStore(int64 transaction_id, return; } + // Store creation is done synchronously, as it may be followed by + // index creation (also sync) since preemptive OpenCursor/SetIndexKeys + // may follow. IndexedDBObjectStoreMetadata object_store_metadata( name, object_store_id, @@ -284,21 +287,6 @@ void IndexedDBDatabase::CreateObjectStore(int64 transaction_id, auto_increment, IndexedDBDatabase::kMinimumIndexId); - transaction->ScheduleTask( - base::Bind(&IndexedDBDatabase::CreateObjectStoreOperation, - this, - object_store_metadata), - base::Bind(&IndexedDBDatabase::CreateObjectStoreAbortOperation, - this, - object_store_id)); - - AddObjectStore(object_store_metadata, object_store_id); -} - -void IndexedDBDatabase::CreateObjectStoreOperation( - const IndexedDBObjectStoreMetadata& object_store_metadata, - IndexedDBTransaction* transaction) { - IDB_TRACE("IndexedDBDatabase::CreateObjectStoreOperation"); leveldb::Status s = backing_store_->CreateObjectStore(transaction->BackingStoreTransaction(), transaction->database()->id(), @@ -317,6 +305,12 @@ void IndexedDBDatabase::CreateObjectStoreOperation( error); return; } + + AddObjectStore(object_store_metadata, object_store_id); + transaction->ScheduleAbortTask( + base::Bind(&IndexedDBDatabase::CreateObjectStoreAbortOperation, + this, + object_store_id)); } void IndexedDBDatabase::DeleteObjectStore(int64 transaction_id, @@ -330,17 +324,10 @@ void IndexedDBDatabase::DeleteObjectStore(int64 transaction_id, if (!ValidateObjectStoreId(object_store_id)) return; - const IndexedDBObjectStoreMetadata& object_store_metadata = - metadata_.object_stores[object_store_id]; - transaction->ScheduleTask( base::Bind(&IndexedDBDatabase::DeleteObjectStoreOperation, this, - object_store_metadata), - base::Bind(&IndexedDBDatabase::DeleteObjectStoreAbortOperation, - this, - object_store_metadata)); - RemoveObjectStore(object_store_id); + object_store_id)); } void IndexedDBDatabase::CreateIndex(int64 transaction_id, @@ -358,27 +345,12 @@ void IndexedDBDatabase::CreateIndex(int64 transaction_id, if (!ValidateObjectStoreIdAndNewIndexId(object_store_id, index_id)) return; + + // Index creation is done synchronously since preemptive + // OpenCursor/SetIndexKeys may follow. const IndexedDBIndexMetadata index_metadata( name, index_id, key_path, unique, multi_entry); - transaction->ScheduleTask( - base::Bind(&IndexedDBDatabase::CreateIndexOperation, - this, - object_store_id, - index_metadata), - base::Bind(&IndexedDBDatabase::CreateIndexAbortOperation, - this, - object_store_id, - index_id)); - - AddIndex(object_store_id, index_metadata, index_id); -} - -void IndexedDBDatabase::CreateIndexOperation( - int64 object_store_id, - const IndexedDBIndexMetadata& index_metadata, - IndexedDBTransaction* transaction) { - IDB_TRACE("IndexedDBDatabase::CreateIndexOperation"); if (!backing_store_->CreateIndex(transaction->BackingStoreTransaction(), transaction->database()->id(), object_store_id, @@ -394,6 +366,13 @@ void IndexedDBDatabase::CreateIndexOperation( blink::WebIDBDatabaseExceptionUnknownError, error_string)); return; } + + AddIndex(object_store_id, index_metadata, index_id); + transaction->ScheduleAbortTask( + base::Bind(&IndexedDBDatabase::CreateIndexAbortOperation, + this, + object_store_id, + index_id)); } void IndexedDBDatabase::CreateIndexAbortOperation( @@ -416,32 +395,28 @@ void IndexedDBDatabase::DeleteIndex(int64 transaction_id, if (!ValidateObjectStoreIdAndIndexId(object_store_id, index_id)) return; - const IndexedDBIndexMetadata& index_metadata = - metadata_.object_stores[object_store_id].indexes[index_id]; transaction->ScheduleTask( base::Bind(&IndexedDBDatabase::DeleteIndexOperation, this, object_store_id, - index_metadata), - base::Bind(&IndexedDBDatabase::DeleteIndexAbortOperation, - this, - object_store_id, - index_metadata)); - - RemoveIndex(object_store_id, index_id); + index_id)); } void IndexedDBDatabase::DeleteIndexOperation( int64 object_store_id, - const IndexedDBIndexMetadata& index_metadata, + int64 index_id, IndexedDBTransaction* transaction) { IDB_TRACE("IndexedDBDatabase::DeleteIndexOperation"); + + const IndexedDBIndexMetadata index_metadata = + metadata_.object_stores[object_store_id].indexes[index_id]; + leveldb::Status s = backing_store_->DeleteIndex(transaction->BackingStoreTransaction(), transaction->database()->id(), object_store_id, - index_metadata.id); + index_id); if (!s.ok()) { base::string16 error_string = ASCIIToUTF16("Internal error deleting index '") + @@ -452,7 +427,15 @@ void IndexedDBDatabase::DeleteIndexOperation( if (s.IsCorruption()) factory_->HandleBackingStoreCorruption(backing_store_->origin_url(), error); + return; } + + RemoveIndex(object_store_id, index_id); + transaction->ScheduleAbortTask( + base::Bind(&IndexedDBDatabase::DeleteIndexAbortOperation, + this, + object_store_id, + index_metadata)); } void IndexedDBDatabase::DeleteIndexAbortOperation( @@ -1299,13 +1282,16 @@ void IndexedDBDatabase::ClearOperation( } void IndexedDBDatabase::DeleteObjectStoreOperation( - const IndexedDBObjectStoreMetadata& object_store_metadata, + int64 object_store_id, IndexedDBTransaction* transaction) { IDB_TRACE("IndexedDBDatabase::DeleteObjectStoreOperation"); + + const IndexedDBObjectStoreMetadata object_store_metadata = + metadata_.object_stores[object_store_id]; leveldb::Status s = backing_store_->DeleteObjectStore(transaction->BackingStoreTransaction(), transaction->database()->id(), - object_store_metadata.id); + object_store_id); if (!s.ok()) { base::string16 error_string = ASCIIToUTF16("Internal error deleting object store '") + @@ -1316,7 +1302,14 @@ void IndexedDBDatabase::DeleteObjectStoreOperation( if (s.IsCorruption()) factory_->HandleBackingStoreCorruption(backing_store_->origin_url(), error); + return; } + + RemoveObjectStore(object_store_id); + transaction->ScheduleAbortTask( + base::Bind(&IndexedDBDatabase::DeleteObjectStoreAbortOperation, + this, + object_store_metadata)); } void IndexedDBDatabase::VersionChangeOperation( @@ -1327,11 +1320,9 @@ void IndexedDBDatabase::VersionChangeOperation( IDB_TRACE("IndexedDBDatabase::VersionChangeOperation"); int64 old_version = metadata_.int_version; DCHECK_GT(version, old_version); - metadata_.int_version = version; + if (!backing_store_->UpdateIDBDatabaseIntVersion( - transaction->BackingStoreTransaction(), - id(), - metadata_.int_version)) { + transaction->BackingStoreTransaction(), id(), version)) { IndexedDBDatabaseError error( blink::WebIDBDatabaseExceptionUnknownError, ASCIIToUTF16( @@ -1341,6 +1332,15 @@ void IndexedDBDatabase::VersionChangeOperation( transaction->Abort(error); return; } + + transaction->ScheduleAbortTask( + base::Bind(&IndexedDBDatabase::VersionChangeAbortOperation, + this, + metadata_.version, + metadata_.int_version)); + metadata_.int_version = version; + metadata_.version = kNoStringVersion; + DCHECK(!pending_second_half_open_); pending_second_half_open_.reset( new PendingSuccessCall(callbacks, connection.get(), version)); @@ -1623,17 +1623,12 @@ void IndexedDBDatabase::RunVersionChangeTransactionFinal( object_store_ids, indexed_db::TRANSACTION_VERSION_CHANGE); - transactions_[transaction_id] - ->ScheduleTask(base::Bind(&IndexedDBDatabase::VersionChangeOperation, - this, - requested_version, - callbacks, - base::Passed(&connection)), - base::Bind(&IndexedDBDatabase::VersionChangeAbortOperation, - this, - metadata_.version, - metadata_.int_version)); - + transactions_[transaction_id]->ScheduleTask( + base::Bind(&IndexedDBDatabase::VersionChangeOperation, + this, + requested_version, + callbacks, + base::Passed(&connection))); DCHECK(!pending_second_half_open_); } diff --git a/content/browser/indexed_db/indexed_db_database.h b/content/browser/indexed_db/indexed_db_database.h index a6e30f366a74..c0bd78931764 100644 --- a/content/browser/indexed_db/indexed_db_database.h +++ b/content/browser/indexed_db/indexed_db_database.h @@ -175,13 +175,10 @@ class CONTENT_EXPORT IndexedDBDatabase size_t PendingDeleteCount() const; // Asynchronous tasks scheduled within transactions: - void CreateObjectStoreOperation( - const IndexedDBObjectStoreMetadata& object_store_metadata, - IndexedDBTransaction* transaction); void CreateObjectStoreAbortOperation(int64 object_store_id, IndexedDBTransaction* transaction); void DeleteObjectStoreOperation( - const IndexedDBObjectStoreMetadata& object_store_metadata, + int64 object_store_id, IndexedDBTransaction* transaction); void DeleteObjectStoreAbortOperation( const IndexedDBObjectStoreMetadata& object_store_metadata, @@ -193,11 +190,8 @@ class CONTENT_EXPORT IndexedDBDatabase void VersionChangeAbortOperation(const base::string16& previous_version, int64 previous_int_version, IndexedDBTransaction* transaction); - void CreateIndexOperation(int64 object_store_id, - const IndexedDBIndexMetadata& index_metadata, - IndexedDBTransaction* transaction); void DeleteIndexOperation(int64 object_store_id, - const IndexedDBIndexMetadata& index_metadata, + int64 index_id, IndexedDBTransaction* transaction); void CreateIndexAbortOperation(int64 object_store_id, int64 index_id, diff --git a/content/browser/indexed_db/indexed_db_database_unittest.cc b/content/browser/indexed_db/indexed_db_database_unittest.cc index 7feed8201d7a..c5f3a13aacee 100644 --- a/content/browser/indexed_db/indexed_db_database_unittest.cc +++ b/content/browser/indexed_db/indexed_db_database_unittest.cc @@ -6,6 +6,7 @@ #include "base/auto_reset.h" #include "base/logging.h" +#include "base/run_loop.h" #include "base/strings/string16.h" #include "base/strings/utf_string_conversions.h" #include "content/browser/indexed_db/indexed_db.h" @@ -17,6 +18,7 @@ #include "content/browser/indexed_db/indexed_db_factory.h" #include "content/browser/indexed_db/indexed_db_fake_backing_store.h" #include "content/browser/indexed_db/indexed_db_transaction.h" +#include "content/browser/indexed_db/indexed_db_value.h" #include "content/browser/indexed_db/mock_indexed_db_callbacks.h" #include "content/browser/indexed_db/mock_indexed_db_database_callbacks.h" #include "testing/gtest/include/gtest/gtest.h" @@ -24,7 +26,7 @@ using base::ASCIIToUTF16; namespace { -const int FAKE_CHILD_PROCESS_ID = 0; +const int kFakeChildProcessId = 0; } namespace content { @@ -71,7 +73,7 @@ TEST(IndexedDBDatabaseTest, ConnectionLifecycle) { IndexedDBPendingConnection connection1( request1, callbacks1, - FAKE_CHILD_PROCESS_ID, + kFakeChildProcessId, transaction_id1, IndexedDBDatabaseMetadata::DEFAULT_INT_VERSION); db->OpenConnection(connection1); @@ -85,7 +87,7 @@ TEST(IndexedDBDatabaseTest, ConnectionLifecycle) { IndexedDBPendingConnection connection2( request2, callbacks2, - FAKE_CHILD_PROCESS_ID, + kFakeChildProcessId, transaction_id2, IndexedDBDatabaseMetadata::DEFAULT_INT_VERSION); db->OpenConnection(connection2); @@ -129,7 +131,7 @@ TEST(IndexedDBDatabaseTest, ForcedClose) { IndexedDBPendingConnection connection( request, callbacks, - FAKE_CHILD_PROCESS_ID, + kFakeChildProcessId, upgrade_transaction_id, IndexedDBDatabaseMetadata::DEFAULT_INT_VERSION); database->OpenConnection(connection); @@ -193,7 +195,7 @@ TEST(IndexedDBDatabaseTest, PendingDelete) { IndexedDBPendingConnection connection( request1, callbacks1, - FAKE_CHILD_PROCESS_ID, + kFakeChildProcessId, transaction_id1, IndexedDBDatabaseMetadata::DEFAULT_INT_VERSION); db->OpenConnection(connection); @@ -213,4 +215,191 @@ TEST(IndexedDBDatabaseTest, PendingDelete) { EXPECT_TRUE(request2->success_called()); } +void DummyOperation(IndexedDBTransaction* transaction) { +} + +class IndexedDBDatabaseOperationTest : public testing::Test { + public: + IndexedDBDatabaseOperationTest() : commit_success_(true) {} + + virtual void SetUp() { + backing_store_ = new IndexedDBFakeBackingStore(); + leveldb::Status s; + db_ = IndexedDBDatabase::Create(ASCIIToUTF16("db"), + backing_store_, + NULL /*factory*/, + IndexedDBDatabase::Identifier(), + &s); + ASSERT_TRUE(s.ok()); + + request_ = new MockIndexedDBCallbacks(); + callbacks_ = new MockIndexedDBDatabaseCallbacks(); + const int64 transaction_id = 1; + db_->OpenConnection(IndexedDBPendingConnection( + request_, + callbacks_, + kFakeChildProcessId, + transaction_id, + IndexedDBDatabaseMetadata::DEFAULT_INT_VERSION)); + EXPECT_EQ(IndexedDBDatabaseMetadata::NO_INT_VERSION, + db_->metadata().int_version); + + transaction_ = new IndexedDBTransaction( + transaction_id, + callbacks_, + std::set() /*scope*/, + indexed_db::TRANSACTION_VERSION_CHANGE, + db_, + new IndexedDBFakeBackingStore::FakeTransaction(commit_success_)); + db_->TransactionCreated(transaction_); + + // Add a dummy task which takes the place of the VersionChangeOperation + // which kicks off the upgrade. This ensures that the transaction has + // processed at least one task before the CreateObjectStore call. + transaction_->ScheduleTask(base::Bind(&DummyOperation)); + } + + void RunPostedTasks() { base::RunLoop().RunUntilIdle(); } + + protected: + scoped_refptr backing_store_; + scoped_refptr db_; + scoped_refptr request_; + scoped_refptr callbacks_; + scoped_refptr transaction_; + + bool commit_success_; + + private: + base::MessageLoop message_loop_; + + DISALLOW_COPY_AND_ASSIGN(IndexedDBDatabaseOperationTest); +}; + +TEST_F(IndexedDBDatabaseOperationTest, CreateObjectStore) { + EXPECT_EQ(0ULL, db_->metadata().object_stores.size()); + const int64 store_id = 1001; + db_->CreateObjectStore(transaction_->id(), + store_id, + ASCIIToUTF16("store"), + IndexedDBKeyPath(), + false /*auto_increment*/); + EXPECT_EQ(1ULL, db_->metadata().object_stores.size()); + RunPostedTasks(); + transaction_->Commit(); + EXPECT_EQ(1ULL, db_->metadata().object_stores.size()); +} + +TEST_F(IndexedDBDatabaseOperationTest, CreateIndex) { + EXPECT_EQ(0ULL, db_->metadata().object_stores.size()); + const int64 store_id = 1001; + db_->CreateObjectStore(transaction_->id(), + store_id, + ASCIIToUTF16("store"), + IndexedDBKeyPath(), + false /*auto_increment*/); + EXPECT_EQ(1ULL, db_->metadata().object_stores.size()); + const int64 index_id = 2002; + db_->CreateIndex(transaction_->id(), + store_id, + index_id, + ASCIIToUTF16("index"), + IndexedDBKeyPath(), + false /*unique*/, + false /*multi_entry*/); + EXPECT_EQ( + 1ULL, + db_->metadata().object_stores.find(store_id)->second.indexes.size()); + RunPostedTasks(); + transaction_->Commit(); + EXPECT_EQ(1ULL, db_->metadata().object_stores.size()); + EXPECT_EQ( + 1ULL, + db_->metadata().object_stores.find(store_id)->second.indexes.size()); +} + +class IndexedDBDatabaseOperationAbortTest + : public IndexedDBDatabaseOperationTest { + public: + IndexedDBDatabaseOperationAbortTest() { commit_success_ = false; } +}; + +TEST_F(IndexedDBDatabaseOperationAbortTest, CreateObjectStore) { + EXPECT_EQ(0ULL, db_->metadata().object_stores.size()); + const int64 store_id = 1001; + db_->CreateObjectStore(transaction_->id(), + store_id, + ASCIIToUTF16("store"), + IndexedDBKeyPath(), + false /*auto_increment*/); + EXPECT_EQ(1ULL, db_->metadata().object_stores.size()); + RunPostedTasks(); + transaction_->Commit(); + EXPECT_EQ(0ULL, db_->metadata().object_stores.size()); +} + +TEST_F(IndexedDBDatabaseOperationAbortTest, CreateIndex) { + EXPECT_EQ(0ULL, db_->metadata().object_stores.size()); + const int64 store_id = 1001; + db_->CreateObjectStore(transaction_->id(), + store_id, + ASCIIToUTF16("store"), + IndexedDBKeyPath(), + false /*auto_increment*/); + EXPECT_EQ(1ULL, db_->metadata().object_stores.size()); + const int64 index_id = 2002; + db_->CreateIndex(transaction_->id(), + store_id, + index_id, + ASCIIToUTF16("index"), + IndexedDBKeyPath(), + false /*unique*/, + false /*multi_entry*/); + EXPECT_EQ( + 1ULL, + db_->metadata().object_stores.find(store_id)->second.indexes.size()); + RunPostedTasks(); + transaction_->Commit(); + EXPECT_EQ(0ULL, db_->metadata().object_stores.size()); +} + +TEST_F(IndexedDBDatabaseOperationTest, CreatePutDelete) { + EXPECT_EQ(0ULL, db_->metadata().object_stores.size()); + const int64 store_id = 1001; + + // Creation is synchronous. + db_->CreateObjectStore(transaction_->id(), + store_id, + ASCIIToUTF16("store"), + IndexedDBKeyPath(), + false /*auto_increment*/); + EXPECT_EQ(1ULL, db_->metadata().object_stores.size()); + + + // Put is asynchronous + IndexedDBValue value("value1", std::vector()); + ScopedVector handles; + scoped_ptr key(new IndexedDBKey("key")); + std::vector index_keys; + scoped_refptr request( + new MockIndexedDBCallbacks(false)); + db_->Put(transaction_->id(), + store_id, + &value, + &handles, + key.Pass(), + IndexedDBDatabase::ADD_ONLY, + request, + index_keys); + + // Deletion is asynchronous. + db_->DeleteObjectStore(transaction_->id(), + store_id); + EXPECT_EQ(1ULL, db_->metadata().object_stores.size()); + + // This will execute the Put then Delete. + RunPostedTasks(); + EXPECT_EQ(0ULL, db_->metadata().object_stores.size()); +} + } // namespace content diff --git a/content/browser/indexed_db/indexed_db_fake_backing_store.cc b/content/browser/indexed_db/indexed_db_fake_backing_store.cc index 35e3a50e4d82..6773d1bba5d6 100644 --- a/content/browser/indexed_db/indexed_db_fake_backing_store.cc +++ b/content/browser/indexed_db/indexed_db_fake_backing_store.cc @@ -67,21 +67,32 @@ leveldb::Status IndexedDBFakeBackingStore::CreateObjectStore( const base::string16& name, const IndexedDBKeyPath&, bool auto_increment) { - return leveldb::Status::IOError("test error"); + return leveldb::Status::OK(); +} + +leveldb::Status IndexedDBFakeBackingStore::PutRecord( + IndexedDBBackingStore::Transaction* transaction, + int64 database_id, + int64 object_store_id, + const IndexedDBKey& key, + IndexedDBValue& value, + ScopedVector* handles, + RecordIdentifier* record) { + return leveldb::Status::OK(); } leveldb::Status IndexedDBFakeBackingStore::ClearObjectStore( Transaction*, int64 database_id, int64 object_store_id) { - return leveldb::Status::IOError("test error"); + return leveldb::Status::OK(); } leveldb::Status IndexedDBFakeBackingStore::DeleteRecord( Transaction*, int64 database_id, int64 object_store_id, const RecordIdentifier&) { - return leveldb::Status::IOError("test error"); + return leveldb::Status::OK(); } leveldb::Status IndexedDBFakeBackingStore::GetKeyGeneratorCurrentNumber( Transaction*, @@ -117,14 +128,14 @@ leveldb::Status IndexedDBFakeBackingStore::CreateIndex( const IndexedDBKeyPath&, bool is_unique, bool is_multi_entry) { - return leveldb::Status::IOError("test error"); + return leveldb::Status::OK(); } leveldb::Status IndexedDBFakeBackingStore::DeleteIndex(Transaction*, int64 database_id, int64 object_store_id, int64 index_id) { - return leveldb::Status::IOError("test error"); + return leveldb::Status::OK(); } leveldb::Status IndexedDBFakeBackingStore::PutIndexDataForRecord( Transaction*, @@ -133,7 +144,7 @@ leveldb::Status IndexedDBFakeBackingStore::PutIndexDataForRecord( int64 index_id, const IndexedDBKey&, const RecordIdentifier&) { - return leveldb::Status::IOError("test error"); + return leveldb::Status::OK(); } void IndexedDBFakeBackingStore::ReportBlobUnused(int64 database_id, diff --git a/content/browser/indexed_db/indexed_db_fake_backing_store.h b/content/browser/indexed_db/indexed_db_fake_backing_store.h index 5c64370563f3..43d244a1af6b 100644 --- a/content/browser/indexed_db/indexed_db_fake_backing_store.h +++ b/content/browser/indexed_db/indexed_db_fake_backing_store.h @@ -44,6 +44,15 @@ class IndexedDBFakeBackingStore : public IndexedDBBackingStore { const IndexedDBKeyPath&, bool auto_increment) OVERRIDE; + virtual leveldb::Status PutRecord( + IndexedDBBackingStore::Transaction* transaction, + int64 database_id, + int64 object_store_id, + const IndexedDBKey& key, + IndexedDBValue& value, + ScopedVector* handles, + RecordIdentifier* record) OVERRIDE; + virtual leveldb::Status ClearObjectStore(Transaction*, int64 database_id, int64 object_store_id) OVERRIDE; diff --git a/content/browser/indexed_db/indexed_db_transaction.cc b/content/browser/indexed_db/indexed_db_transaction.cc index bb7fc03c5bf0..457a7807726c 100644 --- a/content/browser/indexed_db/indexed_db_transaction.cc +++ b/content/browser/indexed_db/indexed_db_transaction.cc @@ -86,18 +86,6 @@ IndexedDBTransaction::~IndexedDBTransaction() { DCHECK(abort_task_stack_.empty()); } -void IndexedDBTransaction::ScheduleTask(Operation task, Operation abort_task) { - if (state_ == FINISHED) - return; - - timeout_timer_.Stop(); - used_ = true; - task_queue_.push(task); - ++diagnostics_.tasks_scheduled; - abort_task_stack_.push(abort_task); - RunTasksIfStarted(); -} - void IndexedDBTransaction::ScheduleTask(IndexedDBDatabase::TaskType type, Operation task) { if (state_ == FINISHED) @@ -114,6 +102,12 @@ void IndexedDBTransaction::ScheduleTask(IndexedDBDatabase::TaskType type, RunTasksIfStarted(); } +void IndexedDBTransaction::ScheduleAbortTask(Operation abort_task) { + DCHECK_NE(FINISHED, state_); + DCHECK(used_); + abort_task_stack_.push(abort_task); +} + void IndexedDBTransaction::RunTasksIfStarted() { DCHECK(used_); diff --git a/content/browser/indexed_db/indexed_db_transaction.h b/content/browser/indexed_db/indexed_db_transaction.h index 6c696dbf8ed7..a898d85c7314 100644 --- a/content/browser/indexed_db/indexed_db_transaction.h +++ b/content/browser/indexed_db/indexed_db_transaction.h @@ -49,8 +49,8 @@ class CONTENT_EXPORT IndexedDBTransaction void ScheduleTask(Operation task) { ScheduleTask(IndexedDBDatabase::NORMAL_TASK, task); } - void ScheduleTask(Operation task, Operation abort_task); void ScheduleTask(IndexedDBDatabase::TaskType, Operation task); + void ScheduleAbortTask(Operation abort_task); void RegisterOpenCursor(IndexedDBCursor* cursor); void UnregisterOpenCursor(IndexedDBCursor* cursor); void AddPreemptiveEvent() { pending_preemptive_events_++; } diff --git a/content/browser/indexed_db/indexed_db_transaction_unittest.cc b/content/browser/indexed_db/indexed_db_transaction_unittest.cc index cf35852fa183..74f402919324 100644 --- a/content/browser/indexed_db/indexed_db_transaction_unittest.cc +++ b/content/browser/indexed_db/indexed_db_transaction_unittest.cc @@ -14,6 +14,21 @@ namespace content { +class AbortObserver { + public: + AbortObserver() : abort_task_called_(false) {} + + void AbortTask(IndexedDBTransaction* transaction) { + abort_task_called_ = true; + } + + bool abort_task_called() const { return abort_task_called_; } + + private: + bool abort_task_called_; + DISALLOW_COPY_AND_ASSIGN(AbortObserver); +}; + class IndexedDBTransactionTest : public testing::Test { public: IndexedDBTransactionTest() { @@ -37,6 +52,11 @@ class IndexedDBTransactionTest : public testing::Test { void RunPostedTasks() { message_loop_.RunUntilIdle(); } void DummyOperation(IndexedDBTransaction* transaction) {} + void AbortableOperation(AbortObserver* observer, + IndexedDBTransaction* transaction) { + transaction->ScheduleAbortTask( + base::Bind(&AbortObserver::AbortTask, base::Unretained(observer))); + } protected: scoped_refptr backing_store_; @@ -132,21 +152,6 @@ TEST_F(IndexedDBTransactionTest, NoTimeoutReadOnly) { EXPECT_FALSE(transaction->IsTimeoutTimerRunning()); } -class AbortObserver { - public: - AbortObserver() : abort_task_called_(false) {} - - void AbortTask(IndexedDBTransaction* transaction) { - abort_task_called_ = true; - } - - bool abort_task_called() const { return abort_task_called_; } - - private: - bool abort_task_called_; - DISALLOW_COPY_AND_ASSIGN(AbortObserver); -}; - TEST_P(IndexedDBTransactionTestMode, ScheduleNormalTask) { const int64 id = 0; const std::set scope; @@ -285,9 +290,9 @@ TEST_P(IndexedDBTransactionTestMode, AbortTasks) { AbortObserver observer; transaction->ScheduleTask( - base::Bind(&IndexedDBTransactionTest::DummyOperation, - base::Unretained(this)), - base::Bind(&AbortObserver::AbortTask, base::Unretained(&observer))); + base::Bind(&IndexedDBTransactionTest::AbortableOperation, + base::Unretained(this), + base::Unretained(&observer))); // Pump the message loop so that the transaction completes all pending tasks, // otherwise it will defer the commit. diff --git a/content/browser/indexed_db/mock_indexed_db_callbacks.cc b/content/browser/indexed_db/mock_indexed_db_callbacks.cc index 42346d6633fb..2ab3ca5b2269 100644 --- a/content/browser/indexed_db/mock_indexed_db_callbacks.cc +++ b/content/browser/indexed_db/mock_indexed_db_callbacks.cc @@ -23,6 +23,8 @@ void MockIndexedDBCallbacks::OnSuccess(int64) {} void MockIndexedDBCallbacks::OnSuccess(const std::vector&) {} +void MockIndexedDBCallbacks::OnSuccess(const IndexedDBKey& key) {} + void MockIndexedDBCallbacks::OnSuccess( scoped_ptr connection, const IndexedDBDatabaseMetadata& metadata) { diff --git a/content/browser/indexed_db/mock_indexed_db_callbacks.h b/content/browser/indexed_db/mock_indexed_db_callbacks.h index 495f9f4e4cf7..0b99c05ae8fb 100644 --- a/content/browser/indexed_db/mock_indexed_db_callbacks.h +++ b/content/browser/indexed_db/mock_indexed_db_callbacks.h @@ -18,6 +18,7 @@ class MockIndexedDBCallbacks : public IndexedDBCallbacks { virtual void OnSuccess() OVERRIDE; virtual void OnSuccess(int64) OVERRIDE; virtual void OnSuccess(const std::vector&) OVERRIDE; + virtual void OnSuccess(const IndexedDBKey& key) OVERRIDE; virtual void OnSuccess(scoped_ptr connection, const IndexedDBDatabaseMetadata& metadata) OVERRIDE; IndexedDBConnection* connection() { return connection_.get(); } -- 2.11.4.GIT