[IndexedDB] Moving IDBDatabase's PendingConnection to use ScopesLockManager

This allows scopes to be used in the DeleteDatabase call, and cleans up
some code in the IndexedDBDatabase class.

The last part of the codebase that doesn't have locks before being used
is the IndexedDBBackingStore::Initialize call. Since this happens on the
opening of the database, this can probably use just the raw database +
a WriteBatch.

Bug: 862456
Change-Id: I89c79ec89a4450e21f1ee700b007ec867b3a4f48
Reviewed-on: https://chromium-review.googlesource.com/c/1478391
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Chase Phillips <cmp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#634358}

content/browser/indexed_db/indexed_db_database.cc

@@ -14,6 +14,7 @@
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/numerics/safe_conversions.h"
 #include "base/stl_util.h"
@@ -32,6 +33,8 @@
 #include "content/browser/indexed_db/indexed_db_tracing.h"
 #include "content/browser/indexed_db/indexed_db_transaction.h"
 #include "content/browser/indexed_db/indexed_db_value.h"
+#include "content/browser/indexed_db/scopes/scope_lock.h"
+#include "content/browser/indexed_db/scopes/scopes_lock_manager.h"
 #include "content/public/common/content_switches.h"
 #include "ipc/ipc_channel.h"
 #include "storage/browser/blob/blob_data_handle.h"
@@ -124,7 +127,9 @@ class IndexedDBDatabase::OpenRequest
  public:
   OpenRequest(scoped_refptr<IndexedDBDatabase> db,
               std::unique_ptr<IndexedDBPendingConnection> pending_connection)
-      : ConnectionRequest(db), pending_(std::move(pending_connection)) {}
+      : ConnectionRequest(db),
+        pending_(std::move(pending_connection)),
+        weak_factory_(this) {}
 
   void Perform() override {
     if (db_->metadata_.id == kInvalidId) {
@@ -200,7 +205,13 @@ class IndexedDBDatabase::OpenRequest
     DCHECK_GT(new_version, old_version);
 
     if (db_->connections_.empty()) {
-      StartUpgrade();
+      std::vector<ScopesLockManager::ScopeLockRequest> lock_requests = {
+          {kDatabaseRangeLockLevel, GetDatabaseLockRange(db_->metadata_.id),
+           ScopesLockManager::LockType::kExclusive}};
+      db_->lock_manager_->AcquireLocks(
+          std::move(lock_requests),
+          base::BindOnce(&IndexedDBDatabase::OpenRequest::StartUpgrade,
+                         weak_factory_.GetWeakPtr()));
       return;
     }
 
@@ -234,13 +245,19 @@ class IndexedDBDatabase::OpenRequest
     if (!db_->connections_.empty())
       return;
 
-    StartUpgrade();
+    std::vector<ScopesLockManager::ScopeLockRequest> lock_requests = {
+        {kDatabaseRangeLockLevel, GetDatabaseLockRange(db_->metadata_.id),
+         ScopesLockManager::LockType::kExclusive}};
+    db_->lock_manager_->AcquireLocks(
+        std::move(lock_requests),
+        base::BindOnce(&IndexedDBDatabase::OpenRequest::StartUpgrade,
+                       weak_factory_.GetWeakPtr()));
   }
 
   // Initiate the upgrade. The bulk of the work actually happens in
   // IndexedDBDatabase::VersionChangeOperation in order to kick the
   // transaction into the correct state.
-  void StartUpgrade() {
+  void StartUpgrade(std::vector<ScopeLock> locks) {
     connection_ = db_->CreateConnection(pending_->database_callbacks,
                                         pending_->child_process_id);
     DCHECK_EQ(db_->connections_.count(connection_.get()), 1UL);
@@ -252,11 +269,10 @@ class IndexedDBDatabase::OpenRequest
         std::set<int64_t>(object_store_ids.begin(), object_store_ids.end()),
         blink::mojom::IDBTransactionMode::VersionChange,
         new IndexedDBBackingStore::Transaction(db_->backing_store()));
-    db_->RegisterAndScheduleTransaction(transaction);
-
     transaction->ScheduleTask(
         base::BindOnce(&IndexedDBDatabase::VersionChangeOperation, db_,
                        pending_->version, pending_->callbacks));
+    transaction->Start(std::move(locks));
   }
 
   // Called when the upgrade transaction has started executing.
@@ -296,6 +312,7 @@ class IndexedDBDatabase::OpenRequest
   // transferred to the IndexedDBDispatcherHost via OnUpgradeNeeded.
   std::unique_ptr<IndexedDBConnection> connection_;
 
+  base::WeakPtrFactory<OpenRequest> weak_factory_;
   DISALLOW_COPY_AND_ASSIGN(OpenRequest);
 };
 
@@ -304,12 +321,18 @@ class IndexedDBDatabase::DeleteRequest
  public:
   DeleteRequest(scoped_refptr<IndexedDBDatabase> db,
                 scoped_refptr<IndexedDBCallbacks> callbacks)
-      : ConnectionRequest(db), callbacks_(callbacks) {}
+      : ConnectionRequest(db), callbacks_(callbacks), weak_factory_(this) {}
 
   void Perform() override {
     if (db_->connections_.empty()) {
       // No connections, so delete immediately.
-      DoDelete();
+      std::vector<ScopesLockManager::ScopeLockRequest> lock_requests = {
+          {kDatabaseRangeLockLevel, GetDatabaseLockRange(db_->metadata_.id),
+           ScopesLockManager::LockType::kExclusive}};
+      db_->lock_manager_->AcquireLocks(
+          std::move(lock_requests),
+          base::BindOnce(&IndexedDBDatabase::DeleteRequest::DoDelete,
+                         weak_factory_.GetWeakPtr()));
       return;
     }
 
@@ -328,10 +351,17 @@ class IndexedDBDatabase::DeleteRequest
   void OnConnectionClosed(IndexedDBConnection* connection) override {
     if (!db_->connections_.empty())
       return;
-    DoDelete();
+
+    std::vector<ScopesLockManager::ScopeLockRequest> lock_requests = {
+        {kDatabaseRangeLockLevel, GetDatabaseLockRange(db_->metadata_.id),
+         ScopesLockManager::LockType::kExclusive}};
+    db_->lock_manager_->AcquireLocks(
+        std::move(lock_requests),
+        base::BindOnce(&IndexedDBDatabase::DeleteRequest::DoDelete,
+                       weak_factory_.GetWeakPtr()));
   }
 
-  void DoDelete() {
+  void DoDelete(std::vector<ScopeLock> locks) {
     Status s;
     if (db_->backing_store_)
       s = db_->backing_store_->DeleteDatabase(db_->metadata_.name);
@@ -374,6 +404,7 @@ class IndexedDBDatabase::DeleteRequest
  private:
   scoped_refptr<IndexedDBCallbacks> callbacks_;
 
+  base::WeakPtrFactory<DeleteRequest> weak_factory_;
   DISALLOW_COPY_AND_ASSIGN(DeleteRequest);
 };
 
@@ -1806,18 +1837,28 @@ Status IndexedDBDatabase::VersionChangeOperation(
   return Status::OK();
 }
 
-void IndexedDBDatabase::TransactionFinished(IndexedDBTransaction* transaction,
-                                            bool committed) {
-  IDB_TRACE1("IndexedDBTransaction::TransactionFinished", "txn.id",
-             transaction->id());
+void IndexedDBDatabase::TransactionCreated() {
+  UMA_HISTOGRAM_COUNTS_1000(
+      "WebCore.IndexedDB.Database.OutstandingTransactionCount",
+      transaction_count_);
+  ++transaction_count_;
+}
+
+void IndexedDBDatabase::TransactionFinished(
+    blink::mojom::IDBTransactionMode mode,
+    bool committed) {
   --transaction_count_;
   DCHECK_GE(transaction_count_, 0);
 
+  // TODO(dmurph): To help remove this integration with IndexedDBDatabase, make
+  // a 'committed' listener closure on all transactions. Then the request can
+  // just listen for that.
+
   // This may be an unrelated transaction finishing while waiting for
   // connections to close, or the actual upgrade transaction from an active
   // request. Notify the active request if it's the latter.
   if (active_request_ &&
-      transaction->mode() == blink::mojom::IDBTransactionMode::VersionChange) {
+      mode == blink::mojom::IDBTransactionMode::VersionChange) {
     active_request_->UpgradeTransactionFinished(committed);
   }
 }
@@ -1861,11 +1902,6 @@ void IndexedDBDatabase::RegisterAndScheduleTransaction(
     IndexedDBTransaction* transaction) {
   IDB_TRACE1("IndexedDBDatabase::RegisterAndScheduleTransaction", "txn.id",
              transaction->id());
-
-  UMA_HISTOGRAM_COUNTS_1000(
-      "WebCore.IndexedDB.Database.OutstandingTransactionCount",
-      transaction_count_);
-  transaction_count_++;
   std::vector<ScopesLockManager::ScopeLockRequest> lock_requests;
   lock_requests.reserve(1 + transaction->scope().size());
   lock_requests.emplace_back(

content/browser/indexed_db/indexed_db_database.h

@@ -103,8 +103,8 @@ class CONTENT_EXPORT IndexedDBDatabase
                          int64_t object_store_id,
                          const base::string16& new_name);
 
-  // Returns a pointer to a newly created transaction. The object is owned
-  // by the connection.
+  // TODO(dmurph): Remove this method and have transactions be directly
+  // scheduled using the lock manager.
   void RegisterAndScheduleTransaction(IndexedDBTransaction* transaction);
   void Close(IndexedDBConnection* connection, bool forced);
   void ForceClose();
@@ -136,7 +136,9 @@ class CONTENT_EXPORT IndexedDBDatabase
     return lock_manager_;
   }
 
-  void TransactionFinished(IndexedDBTransaction* transaction, bool committed);
+  void TransactionCreated();
+  void TransactionFinished(blink::mojom::IDBTransactionMode mode,
+                           bool committed);
 
   void AbortAllTransactionsForConnections();
 

content/browser/indexed_db/indexed_db_transaction.cc

@@ -119,6 +119,8 @@ IndexedDBTransaction::IndexedDBTransaction(
   IDB_ASYNC_TRACE_BEGIN("IndexedDBTransaction::lifetime", this);
   callbacks_ = connection_->callbacks();
   database_ = connection_->database();
+  if (database_)
+    database_->TransactionCreated();
 
   diagnostics_.tasks_scheduled = 0;
   diagnostics_.tasks_completed = 0;
@@ -237,7 +239,7 @@ void IndexedDBTransaction::Abort(const IndexedDBDatabaseError& error) {
   if (callbacks_.get())
     callbacks_->OnAbort(*this, error);
 
-  database_->TransactionFinished(this, false);
+  database_->TransactionFinished(mode_, false);
 
   // RemoveTransaction will delete |this|.
   // Note: During force-close situations, the connection can be destroyed during
@@ -460,7 +462,7 @@ leveldb::Status IndexedDBTransaction::CommitPhaseTwo() {
     if (!pending_observers_.empty() && connection_)
       connection_->ActivatePendingObservers(std::move(pending_observers_));
 
-    database_->TransactionFinished(this, true);
+    database_->TransactionFinished(mode_, true);
     // RemoveTransaction will delete |this|.
     connection_->RemoveTransaction(id_);
     return s;
@@ -479,7 +481,7 @@ leveldb::Status IndexedDBTransaction::CommitPhaseTwo() {
                                  "Internal error committing transaction.");
     }
     callbacks_->OnAbort(*this, error);
-    database_->TransactionFinished(this, false);
+    database_->TransactionFinished(mode_, false);
   }
   return s;
 }