Reland "[IndexedDB] Moving IDBDatabase's PendingConnection to use ScopesLockManager"

This is a reland of 1bc0f674

Original change's description:
> [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}

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

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,
+void IndexedDBDatabase::TransactionCreated() {
+  UMA_HISTOGRAM_COUNTS_1000(
+      "WebCore.IndexedDB.Database.OutstandingTransactionCount",
+      transaction_count_);
+  ++transaction_count_;
+}
+
+void IndexedDBDatabase::TransactionFinished(
+    blink::mojom::IDBTransactionMode mode,
     bool committed) {
-  IDB_TRACE1("IndexedDBTransaction::TransactionFinished", "txn.id",
-             transaction->id());
   --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;
@@ -246,7 +248,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
@@ -469,7 +471,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;
@@ -488,7 +490,7 @@ leveldb::Status IndexedDBTransaction::CommitPhaseTwo() {
                                  "Internal error committing transaction.");
     }
     callbacks_->OnAbort(*this, error);
-    database_->TransactionFinished(this, false);
+    database_->TransactionFinished(mode_, false);
   }
   return s;
 }