[IndexedDB] Refactoring to remove ref ptrs and host transaction ids.

content/browser/indexed_db/indexed_db_transaction.cc

Index: content/browser/indexed_db/indexed_db_transaction.cc
diff --git a/content/browser/indexed_db/indexed_db_transaction.cc b/content/browser/indexed_db/indexed_db_transaction.cc
index 5040dddf49fdcabdb0bc06ff18177f95b312c7a2..e843b90c01817e12b55da2eb51a374d858b1b1e2 100644
--- a/content/browser/indexed_db/indexed_db_transaction.cc
+++ b/content/browser/indexed_db/indexed_db_transaction.cc
@@ -31,7 +31,9 @@ const int64_t kInactivityTimeoutPeriodSeconds = 60;
 
 // Helper for posting a task to call IndexedDBTransaction::Commit when we know
 // the transaction had no requests and therefore the commit must succeed.
-void CommitUnused(scoped_refptr<IndexedDBTransaction> transaction) {
+void CommitUnused(base::WeakPtr<IndexedDBTransaction> transaction) {
+  if (!transaction)
+    return;
   leveldb::Status status = transaction->Commit();
   DCHECK(status.ok());
 }
@@ -70,7 +72,7 @@ IndexedDBTransaction::Operation IndexedDBTransaction::TaskStack::pop() {
 
 IndexedDBTransaction::IndexedDBTransaction(
     int64_t id,
-    base::WeakPtr<IndexedDBConnection> connection,
+    IndexedDBConnection* connection,
     const std::set<int64_t>& object_store_ids,
     blink::WebIDBTransactionMode mode,
     IndexedDBBackingStore::Transaction* backing_store_transaction)
@@ -81,15 +83,15 @@ IndexedDBTransaction::IndexedDBTransaction(
       state_(CREATED),
       commit_pending_(false),
       connection_(std::move(connection)),
+      size_(0),
       transaction_(backing_store_transaction),
       backing_store_transaction_begun_(false),
       should_process_queue_(false),
-      pending_preemptive_events_(0) {
+      pending_preemptive_events_(0),
+      ptr_factory_(this) {
   callbacks_ = connection_->callbacks();
   database_ = connection_->database();
 
-  database_->transaction_coordinator().DidCreateTransaction(this);
-
   diagnostics_.tasks_scheduled = 0;
   diagnostics_.tasks_completed = 0;
   diagnostics_.creation_time = base::Time::Now();
@@ -142,7 +144,8 @@ void IndexedDBTransaction::RunTasksIfStarted() {
 
   should_process_queue_ = true;
   base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, base::Bind(&IndexedDBTransaction::ProcessTaskQueue, this));
+      FROM_HERE, base::Bind(&IndexedDBTransaction::ProcessTaskQueue,
+                            ptr_factory_.GetWeakPtr()));
 }
 
 void IndexedDBTransaction::Abort() {
@@ -155,11 +158,6 @@ void IndexedDBTransaction::Abort(const IndexedDBDatabaseError& error) {
   if (state_ == FINISHED)
     return;
 
-  // The last reference to this object may be released while performing the
-  // abort steps below. We therefore take a self reference to keep ourselves
-  // alive while executing this method.
-  scoped_refptr<IndexedDBTransaction> protect(this);
-
   timeout_timer_.Stop();
 
   state_ = FINISHED;
@@ -192,13 +190,13 @@ void IndexedDBTransaction::Abort(const IndexedDBDatabaseError& error) {
 #endif
 
   if (callbacks_.get())
-    callbacks_->OnAbort(id_, error);
+    callbacks_->OnAbort(*this, error);
 
   database_->TransactionFinished(this, false);
 
-  database_ = NULL;
-  connection_ = nullptr;
   pending_observers_.clear();
+  // This destroys our object.

[cmumford, 2016/11/04 23:33:13]

Nit: avoid first person language. Maybe something like "EraseTransaction will
delete |this|".

[dmurph, 2016/11/07 20:05:23]

Done.

+  connection_->EraseTransaction(id_);
 }
 
 bool IndexedDBTransaction::IsTaskQueueEmpty() const {
@@ -229,7 +227,7 @@ void IndexedDBTransaction::Start() {
       // front-end previously requested a commit; do the commit now, but not
       // re-entrantly as that may renter the coordinator.
       base::ThreadTaskRunnerHandle::Get()->PostTask(
-          FROM_HERE, base::Bind(&CommitUnused, make_scoped_refptr(this)));
+          FROM_HERE, base::Bind(&CommitUnused, ptr_factory_.GetWeakPtr()));
     }
     return;
   }
@@ -240,9 +238,11 @@ void IndexedDBTransaction::Start() {
 class BlobWriteCallbackImpl : public IndexedDBBackingStore::BlobWriteCallback {
  public:
   explicit BlobWriteCallbackImpl(
-      scoped_refptr<IndexedDBTransaction> transaction)
-      : transaction_(transaction) {}
+      base::WeakPtr<IndexedDBTransaction> transaction)
+      : transaction_(std::move(transaction)) {}
   void Run(bool succeeded) override {
+    if (!transaction_)
+      return;
     transaction_->BlobWriteComplete(succeeded);
   }
 
@@ -250,7 +250,7 @@ class BlobWriteCallbackImpl : public IndexedDBBackingStore::BlobWriteCallback {
   ~BlobWriteCallbackImpl() override {}
 
  private:
-  scoped_refptr<IndexedDBTransaction> transaction_;
+  base::WeakPtr<IndexedDBTransaction> transaction_;
 };
 
 void IndexedDBTransaction::BlobWriteComplete(bool success) {
@@ -299,7 +299,7 @@ leveldb::Status IndexedDBTransaction::Commit() {
     s = CommitPhaseTwo();
   } else {
     scoped_refptr<IndexedDBBackingStore::BlobWriteCallback> callback(
-        new BlobWriteCallbackImpl(this));
+        new BlobWriteCallbackImpl(ptr_factory_.GetWeakPtr()));
     // CommitPhaseOne will call the callback synchronously if there are no blobs
     // to write.
     s = transaction_->CommitPhaseOne(callback);
@@ -318,11 +318,6 @@ leveldb::Status IndexedDBTransaction::CommitPhaseTwo() {
 
   DCHECK_EQ(state_, COMMITTING);
 
-  // The last reference to this object may be released while performing the
-  // commit steps below. We therefore take a self reference to keep ourselves
-  // alive while executing this method.
-  scoped_refptr<IndexedDBTransaction> protect(this);
-
   state_ = FINISHED;
 
   leveldb::Status s;
@@ -359,7 +354,7 @@ leveldb::Status IndexedDBTransaction::CommitPhaseTwo() {
       IDB_TRACE1(
           "IndexedDBTransaction::CommitPhaseTwo.TransactionCompleteCallbacks",
           "txn.id", id());
-      callbacks_->OnComplete(id_);
+      callbacks_->OnComplete(*this);
     }
     if (!pending_observers_.empty() && connection_) {
       connection_->ActivatePendingObservers(std::move(pending_observers_));
@@ -380,13 +375,15 @@ leveldb::Status IndexedDBTransaction::CommitPhaseTwo() {
       error = IndexedDBDatabaseError(blink::WebIDBDatabaseExceptionUnknownError,
                                      "Internal error committing transaction.");
     }
-    callbacks_->OnAbort(id_, error);
+    callbacks_->OnAbort(*this, error);
 
     database_->TransactionFinished(this, false);
     database_->TransactionCommitFailed(s);
   }
 
-  database_ = NULL;
+  pending_observers_.clear();
+  // This destroys our object.
+  connection_->EraseTransaction(id_);
   return s;
 }
 
@@ -405,11 +402,6 @@ void IndexedDBTransaction::ProcessTaskQueue() {
     backing_store_transaction_begun_ = true;
   }
 
-  // The last reference to this object may be released while performing the
-  // tasks. Take take a self reference to keep this object alive so that
-  // the loop termination conditions can be checked.
-  scoped_refptr<IndexedDBTransaction> protect(this);
-
   TaskQueue* task_queue =
       pending_preemptive_events_ ? &preemptive_task_queue_ : &task_queue_;
   while (!task_queue->empty() && state_ != FINISHED) {
@@ -443,8 +435,9 @@ void IndexedDBTransaction::ProcessTaskQueue() {
   // never requests further activity. Read-only transactions don't
   // block other transactions, so don't time those out.
   if (mode_ != blink::WebIDBTransactionModeReadOnly) {
-    timeout_timer_.Start(FROM_HERE, GetInactivityTimeout(),
-                         base::Bind(&IndexedDBTransaction::Timeout, this));
+    timeout_timer_.Start(
+        FROM_HERE, GetInactivityTimeout(),
+        base::Bind(&IndexedDBTransaction::Timeout, ptr_factory_.GetWeakPtr()));
   }
 }