IndexedDB: Defer delete calls when there is a running upgrade

content/browser/indexed_db/indexed_db_database.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/indexed_db/indexed_db_database.h"
 
 #include <math.h>
 
 #include <limits>
 #include <memory>
@@ skipping 215 matching lines @@
                                            &metadata_.object_stores);
 
   return backing_store_->CreateIDBDatabaseMetaData(
       metadata_.name, metadata_.version, &metadata_.id);
 }
 
 IndexedDBDatabase::~IndexedDBDatabase() {
   DCHECK(transactions_.empty());
   DCHECK(pending_open_calls_.empty());
   DCHECK(pending_delete_calls_.empty());
+  DCHECK(blocked_delete_calls_.empty());
 }
 
 size_t IndexedDBDatabase::GetMaxMessageSizeInBytes() const {
   return kMaxIDBMessageSizeInBytes;
 }
 
 std::unique_ptr<IndexedDBConnection> IndexedDBDatabase::CreateConnection(
     scoped_refptr<IndexedDBDatabaseCallbacks> database_callbacks,
     int child_process_id) {
   std::unique_ptr<IndexedDBConnection> connection(
@@ skipping 1347 matching lines @@
 
 size_t IndexedDBDatabase::PendingUpgradeCount() const {
   return pending_run_version_change_transaction_call_ ? 1 : 0;
 }
 
 size_t IndexedDBDatabase::RunningUpgradeCount() const {
   return pending_second_half_open_ ? 1 : 0;
 }
 
 size_t IndexedDBDatabase::PendingDeleteCount() const {
-  return pending_delete_calls_.size();
+  return pending_delete_calls_.size() + blocked_delete_calls_.size();
 }
 
 void IndexedDBDatabase::ProcessPendingCalls() {
   if (pending_run_version_change_transaction_call_ && ConnectionCount() == 1) {
     DCHECK(pending_run_version_change_transaction_call_->version() >
            metadata_.version);
     std::unique_ptr<PendingUpgradeCall> pending_call =
         std::move(pending_run_version_change_transaction_call_);
     RunVersionChangeTransactionFinal(pending_call->callbacks(),
                                      pending_call->ReleaseConnection(),
                                      pending_call->transaction_id(),
                                      pending_call->version());
     DCHECK_EQ(1u, ConnectionCount());
     // Fall through would be a no-op, since transaction must complete
     // asynchronously.
+    DCHECK(IsUpgradeRunning());
     DCHECK(IsDeleteDatabaseBlocked());
     DCHECK(IsOpenConnectionBlocked());
     return;
   }
 
-  if (!IsDeleteDatabaseBlocked()) {
+  if (IsUpgradeRunning()) {
+    DCHECK_EQ(ConnectionCount(), 1UL);
+    return;
+  }
+
+  // These delete calls were blocked by a running upgrade, and did not even
+  // get as far as broadcasting OnVersionChange (if needed).
+  {
     PendingDeleteCallList pending_delete_calls;
     pending_delete_calls_.swap(pending_delete_calls);
     while (!pending_delete_calls.empty()) {
+      std::unique_ptr<PendingDeleteCall> pending_delete_call(
+          pending_delete_calls.front());
+      pending_delete_calls.pop_front();
+      DeleteDatabase(pending_delete_call->callbacks());
+    }
+    // DeleteDatabase should never re-queue these calls.
+    DCHECK(pending_delete_calls_.empty());
+    // Fall through when complete, as pending deletes/opens may be unblocked.
+  }
+
+  // These delete calls broadcast OnVersionChange (if needed) but were blocked
+  // by open connections.
+  if (!IsDeleteDatabaseBlocked()) {
+    PendingDeleteCallList blocked_delete_calls;
+    blocked_delete_calls_.swap(blocked_delete_calls);
+    while (!blocked_delete_calls.empty()) {
       // Only the first delete call will delete the database, but each must fire
       // callbacks.
       std::unique_ptr<PendingDeleteCall> pending_delete_call(
-          pending_delete_calls.front());
-      pending_delete_calls.pop_front();
+          blocked_delete_calls.front());
+      blocked_delete_calls.pop_front();
       DeleteDatabaseFinal(pending_delete_call->callbacks());
     }
-    // delete_database_final should never re-queue calls.
-    DCHECK(pending_delete_calls_.empty());
+    // DeleteDatabaseFinal should never re-queue these calls.
+    DCHECK(blocked_delete_calls_.empty());
     // Fall through when complete, as pending opens may be unblocked.
   }
 
+  // These open calls were blocked by a pending/running upgrade or a pending
+  // delete.
   if (!IsOpenConnectionBlocked()) {
     PendingOpenCallList pending_open_calls;
     pending_open_calls_.swap(pending_open_calls);
     while (!pending_open_calls.empty()) {
+      // This may re-queue open calls if an upgrade is necessary.
       OpenConnection(pending_open_calls.front());
       pending_open_calls.pop_front();
     }
   }
 }
 
 void IndexedDBDatabase::CreateTransaction(
     int64_t transaction_id,
     IndexedDBConnection* connection,
     const std::vector<int64_t>& object_store_ids,
@@ skipping 13 matching lines @@
   TransactionCreated(IndexedDBClassFactory::Get()->CreateIndexedDBTransaction(
       transaction_id, connection->callbacks(),
       std::set<int64_t>(object_store_ids.begin(), object_store_ids.end()), mode,
       this, new IndexedDBBackingStore::Transaction(backing_store_.get())));
 }
 
 void IndexedDBDatabase::TransactionCreated(IndexedDBTransaction* transaction) {
   transactions_[transaction->id()] = transaction;
 }
 
+bool IndexedDBDatabase::IsUpgradeRunning() const {
+  return transaction_coordinator_.IsRunningVersionChangeTransaction();
+}
+
+bool IndexedDBDatabase::IsUpgradePendingOrRunning() const {
+  return pending_run_version_change_transaction_call_ || IsUpgradeRunning();
+}
+
 bool IndexedDBDatabase::IsOpenConnectionBlocked() const {
-  return !pending_delete_calls_.empty() ||
-         transaction_coordinator_.IsRunningVersionChangeTransaction() ||
-         pending_run_version_change_transaction_call_;
+  return IsUpgradePendingOrRunning() || !blocked_delete_calls_.empty();

[cmumford, 2016/06/23 21:44:19]

nit: blocked_delete_calls_.empty() is constant time and faster - check it first?

[jsbell, 2016/06/23 22:26:50]

IsUpgradePendingOrRunning() should be constant time too - at most one
transaction needs to be checked for IsUpgradeRunning().

But yeah, I can flip it.

 }
 
 void IndexedDBDatabase::OpenConnection(
     const IndexedDBPendingConnection& connection) {
   DCHECK(backing_store_.get());
 
-  // TODO(jsbell): Should have a priority queue so that higher version
-  // requests are processed first. http://crbug.com/225850
   if (IsOpenConnectionBlocked()) {
     // The backing store only detects data loss when it is first opened. The
     // presence of existing connections means we didn't even check for data loss
     // so there'd better not be any.
     DCHECK_NE(blink::WebIDBDataLossTotal, connection.callbacks->data_loss());
     pending_open_calls_.push_back(connection);
     return;
   }
 
   if (metadata_.id == kInvalidId) {
@@ skipping 106 matching lines @@
     scoped_refptr<IndexedDBCallbacks> callbacks,
     std::unique_ptr<IndexedDBConnection> connection,
     int64_t transaction_id,
     int64_t requested_version) {
   std::vector<int64_t> object_store_ids;
   CreateTransaction(transaction_id,
                     connection.get(),
                     object_store_ids,
                     blink::WebIDBTransactionModeVersionChange);
 
+  DCHECK(transaction_coordinator_.IsRunningVersionChangeTransaction());
   transactions_[transaction_id]->ScheduleTask(
       base::Bind(&IndexedDBDatabase::VersionChangeOperation,
                  this,
                  requested_version,
                  callbacks,
                  base::Passed(&connection)));
   DCHECK(!pending_second_half_open_);
 }
 
 void IndexedDBDatabase::DeleteDatabase(
     scoped_refptr<IndexedDBCallbacks> callbacks) {
+  // If there's a running upgrade, the delete calls should be deferred so that
+  // "versionchange" is seen after "success" fires.
+  if (IsUpgradeRunning()) {
+    pending_delete_calls_.push_back(new PendingDeleteCall(callbacks));
+    return;
+  }
 
   if (IsDeleteDatabaseBlocked()) {
     for (const auto* connection : connections_) {
       // Front end ensures the event is not fired at connections that have
       // close_pending set.
       connection->callbacks()->OnVersionChange(
           metadata_.version, IndexedDBDatabaseMetadata::NO_VERSION);
     }
     // OnBlocked will be fired at the request when one of the other
     // connections acks that the OnVersionChange was ignored.
 
-    pending_delete_calls_.push_back(new PendingDeleteCall(callbacks));
+    blocked_delete_calls_.push_back(new PendingDeleteCall(callbacks));
     return;
   }
   DeleteDatabaseFinal(callbacks);
 }
 
 bool IndexedDBDatabase::IsDeleteDatabaseBlocked() const {
-  return !!ConnectionCount();
+  return IsUpgradePendingOrRunning() || !!ConnectionCount();
 }
 
 void IndexedDBDatabase::DeleteDatabaseFinal(
     scoped_refptr<IndexedDBCallbacks> callbacks) {
   DCHECK(!IsDeleteDatabaseBlocked());
   DCHECK(backing_store_.get());
   leveldb::Status s = backing_store_->DeleteDatabase(metadata_.name);
   if (!s.ok()) {
     IndexedDBDatabaseError error(blink::WebIDBDatabaseExceptionUnknownError,
                                  "Internal error deleting database.");
@@ skipping 18 matching lines @@
   scoped_refptr<IndexedDBDatabase> protect(this);
   ConnectionSet::const_iterator it = connections_.begin();
   while (it != connections_.end()) {
     IndexedDBConnection* connection = *it++;
     connection->ForceClose();
   }
   DCHECK(connections_.empty());
 }
 
 void IndexedDBDatabase::VersionChangeIgnored() {
-  if (pending_run_version_change_transaction_call_)
+  if (pending_run_version_change_transaction_call_) {
     pending_run_version_change_transaction_call_->callbacks()->OnBlocked(
         metadata_.version);
+  }
 
-  for (const auto& pending_delete_call : pending_delete_calls_)
+  for (const auto& pending_delete_call : blocked_delete_calls_)
     pending_delete_call->callbacks()->OnBlocked(metadata_.version);
 }
 
-
 void IndexedDBDatabase::Close(IndexedDBConnection* connection, bool forced) {
   DCHECK(connections_.count(connection));
   DCHECK(connection->IsConnected());
   DCHECK(connection->database() == this);
 
   IDB_TRACE("IndexedDBDatabase::Close");
 
   connections_.erase(connection);
 
   // Abort outstanding transactions from the closing connection. This
@@ skipping 15 matching lines @@
     pending_second_half_open_->callbacks()->OnError(
         IndexedDBDatabaseError(blink::WebIDBDatabaseExceptionAbortError,
                                "The connection was closed."));
     pending_second_half_open_.reset();
   }
 
   ProcessPendingCalls();
 
   // TODO(jsbell): Add a test for the pending_open_calls_ cases below.
   if (!ConnectionCount() && !pending_open_calls_.size() &&
-      !pending_delete_calls_.size()) {
+      !blocked_delete_calls_.size()) {

[cmumford, 2016/06/23 21:44:19]

Nit: I know you're sticking with existing pattern, but
blocked_delete_calls_.empty() is more readable, and std::list::size() might not
be constant time.

[jsbell, 2016/06/23 22:26:50]

Whoops, yeah, I'll use empty(). That was ported from webkit code. :P

(std::list::size() is apparently constant since C++11)

     DCHECK(transactions_.empty());
     backing_store_ = NULL;
     factory_->ReleaseDatabase(identifier_, forced);
   }
 }
 
 void IndexedDBDatabase::CreateObjectStoreAbortOperation(
     int64_t object_store_id,
     IndexedDBTransaction* transaction) {
   DCHECK(!transaction);
@@ skipping 12 matching lines @@
 
 void IndexedDBDatabase::VersionChangeAbortOperation(
     int64_t previous_version,
     IndexedDBTransaction* transaction) {
   DCHECK(!transaction);
   IDB_TRACE("IndexedDBDatabase::VersionChangeAbortOperation");
   metadata_.version = previous_version;
 }
 
 }  // namespace content