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

content/browser/indexed_db/indexed_db_backing_store.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_backing_store.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "base/files/file_path.h"
@@ skipping 2250 matching lines @@
     if (aborted_) {
       self_ref_ = NULL;
       return;
     }
     if (iter_->size() != -1 && iter_->size() != bytes_written)
       succeeded = false;
     if (succeeded) {
       ++iter_;
       WriteNextFile();
     } else {
-      callback_->Run(false);
+      callback_->Run(false, false);

[jsbell, 2016/11/30 21:30:13]

Comment these args, e.g. /* name */

Or maybe consider adding enums. callback_->Run(BlobWriteCallback::kFailed,
BlobWriteCallback::kAsynchronous) would be more readable.

(I don't feel strongly re: comment vs. enum)

[dmurph, 2016/11/30 23:13:07]

Done.

     }
   }
 
   void Abort() override {
     aborted_ = true;
     if (!waiting_for_callback_)
       return;
     self_ref_ = this;
   }
 
  private:
   ~ChainedBlobWriterImpl() override {}
 
   void WriteNextFile() {
     DCHECK(!waiting_for_callback_);
     if (aborted_) {
       self_ref_ = nullptr;
       return;
     }
     if (iter_ == blobs_.end()) {
       DCHECK(!self_ref_.get());
-      callback_->Run(true);
+      callback_->Run(true, false);

[jsbell, 2016/11/30 21:30:13]

ditto

[dmurph, 2016/11/30 23:13:07]

Done.

       return;
     } else {
       if (!backing_store_->WriteBlobFile(database_id_, *iter_, this)) {
-        callback_->Run(false);
+        callback_->Run(false, false);

[jsbell, 2016/11/30 21:30:13]

ditto

[dmurph, 2016/11/30 23:13:06]

Done.

         return;
       }
       waiting_for_callback_ = true;
     }
   }
 
   bool waiting_for_callback_;
   scoped_refptr<ChainedBlobWriterImpl> self_ref_;
   WriteDescriptorVec blobs_;
   WriteDescriptorVec::const_iterator iter_;
   int64_t database_id_;
   IndexedDBBackingStore* backing_store_;
+  // Callback result is useless as call stack is no longer transaction's
+  // operations queue. Errors are instead handled in
+  // IndexedDBTransaction::BlobWriteComplete.
   scoped_refptr<IndexedDBBackingStore::BlobWriteCallback> callback_;
   std::unique_ptr<FileWriterDelegate> delegate_;
   bool aborted_;
 
   DISALLOW_COPY_AND_ASSIGN(ChainedBlobWriterImpl);
 };
 
 class LocalWriteClosure : public FileWriterDelegate::DelegateWriteCallback,
                           public base::RefCountedThreadSafe<LocalWriteClosure> {
  public:
@@ skipping 1718 matching lines @@
   std::unique_ptr<IndexCursorImpl> cursor(
       new IndexCursorImpl(this, transaction, database_id, cursor_options));
   if (!cursor->FirstSeek(s))
     return std::unique_ptr<IndexedDBBackingStore::Cursor>();
 
   return std::move(cursor);
 }
 
 IndexedDBBackingStore::Transaction::Transaction(
     IndexedDBBackingStore* backing_store)
-    : backing_store_(backing_store), database_id_(-1), committing_(false) {
-}
+    : backing_store_(backing_store),
+      database_id_(-1),
+      committing_(false),
+      ptr_factory_(this) {}
 
 IndexedDBBackingStore::Transaction::~Transaction() {
   DCHECK(!committing_);
 }
 
 void IndexedDBBackingStore::Transaction::Begin() {
   IDB_TRACE("IndexedDBBackingStore::Transaction::Begin");
   DCHECK(!transaction_.get());
   transaction_ = IndexedDBClassFactory::Get()->CreateLevelDBTransaction(
       backing_store_->db_.get());
@@ skipping 148 matching lines @@
   }
 
   committing_ = true;
   ++backing_store_->committing_transaction_count_;
 
   if (!new_files_to_write.empty()) {
     // This kicks off the writes of the new blobs, if any.
     // This call will zero out new_blob_entries and new_files_to_write.
     WriteNewBlobs(&new_blob_entries, &new_files_to_write, callback);
   } else {
-    callback->Run(true);
+    return callback->Run(true, true);
   }
 
   return leveldb::Status::OK();
 }
 
 leveldb::Status IndexedDBBackingStore::Transaction::CommitPhaseTwo() {
   IDB_TRACE("IndexedDBBackingStore::Transaction::CommitPhaseTwo");
   leveldb::Status s;
 
   DCHECK(committing_);
@@ skipping 84 matching lines @@
   UpdatePrimaryBlobJournal(update_journal_transaction.get(),
                            saved_primary_journal);
   s = update_journal_transaction->Commit();
   return s;
 }
 
 
 class IndexedDBBackingStore::Transaction::BlobWriteCallbackWrapper
     : public IndexedDBBackingStore::BlobWriteCallback {
  public:
-  BlobWriteCallbackWrapper(IndexedDBBackingStore::Transaction* transaction,
-                           scoped_refptr<BlobWriteCallback> callback)
-      : transaction_(transaction), callback_(callback) {}
-  void Run(bool succeeded) override {
+  BlobWriteCallbackWrapper(
+      base::WeakPtr<IndexedDBBackingStore::Transaction> transaction,
+      void* tracing_end_ptr,
+      scoped_refptr<BlobWriteCallback> callback)
+      : transaction_(std::move(transaction)),
+        tracing_end_ptr_(tracing_end_ptr),
+        callback_(callback) {}
+  leveldb::Status Run(bool succeeded, bool sync_call) override {
+    DCHECK(!sync_call);
     IDB_ASYNC_TRACE_END("IndexedDBBackingStore::Transaction::WriteNewBlobs",
-                        transaction_);
-    callback_->Run(succeeded);
-    if (succeeded)  // Else it's already been deleted during rollback.
-      transaction_->chained_blob_writer_ = NULL;
+                        tracing_end_ptr_);
+    leveldb::Status result = callback_->Run(succeeded, false);

[jsbell, 2016/11/30 21:30:13]

sync_call ?

[dmurph, 2016/11/30 23:13:06]

Done.

+    if (transaction_ && succeeded)
+      transaction_->chained_blob_writer_ = nullptr;
+    return result;
   }
 
  private:
   ~BlobWriteCallbackWrapper() override {}
   friend class base::RefCounted<IndexedDBBackingStore::BlobWriteCallback>;
 
-  IndexedDBBackingStore::Transaction* transaction_;
+  base::WeakPtr<IndexedDBBackingStore::Transaction> transaction_;
+  const void* const tracing_end_ptr_;
   scoped_refptr<BlobWriteCallback> callback_;
 
   DISALLOW_COPY_AND_ASSIGN(BlobWriteCallbackWrapper);
 };
 
 void IndexedDBBackingStore::Transaction::WriteNewBlobs(
     BlobEntryKeyValuePairVec* new_blob_entries,
     WriteDescriptorVec* new_files_to_write,
     scoped_refptr<BlobWriteCallback> callback) {
   IDB_ASYNC_TRACE_BEGIN("IndexedDBBackingStore::Transaction::WriteNewBlobs",
                         this);
   DCHECK(!new_files_to_write->empty());
   DCHECK_GT(database_id_, 0);
   for (auto& blob_entry_iter : *new_blob_entries) {
     // Add the new blob-table entry for each blob to the main transaction, or
     // remove any entry that may exist if there's no new one.
     if (blob_entry_iter.second.empty())
       transaction_->Remove(blob_entry_iter.first.Encode());
     else
       transaction_->Put(blob_entry_iter.first.Encode(),
                         &blob_entry_iter.second);
   }
-  // Creating the writer will start it going asynchronously.
-  chained_blob_writer_ =
-      new ChainedBlobWriterImpl(database_id_,
-                                backing_store_,
-                                new_files_to_write,
-                                new BlobWriteCallbackWrapper(this, callback));
+  // Creating the writer will start it going asynchronously. The transaction
+  // can be destructed before the callback is triggered.
+  chained_blob_writer_ = new ChainedBlobWriterImpl(
+      database_id_, backing_store_, new_files_to_write,
+      new BlobWriteCallbackWrapper(ptr_factory_.GetWeakPtr(), this, callback));
 }
 
 void IndexedDBBackingStore::Transaction::Rollback() {
   IDB_TRACE("IndexedDBBackingStore::Transaction::Rollback");
   if (committing_) {
     committing_ = false;
     DCHECK_GT(backing_store_->committing_transaction_count_, 0UL);
     --backing_store_->committing_transaction_count_;
   }
 
   if (chained_blob_writer_.get()) {
     chained_blob_writer_->Abort();
     chained_blob_writer_ = NULL;
   }
-  if (transaction_.get() == NULL)
+  if (!transaction_)
     return;
   transaction_->Rollback();
   transaction_ = NULL;
 }
 
 IndexedDBBackingStore::BlobChangeRecord::BlobChangeRecord(
     const std::string& key,
     int64_t object_store_id)
     : key_(key), object_store_id_(object_store_id) {}
 
@@ skipping 116 matching lines @@
 
 IndexedDBBackingStore::Transaction::WriteDescriptor::WriteDescriptor(
     const WriteDescriptor& other) = default;
 IndexedDBBackingStore::Transaction::WriteDescriptor::~WriteDescriptor() =
     default;
 IndexedDBBackingStore::Transaction::WriteDescriptor&
     IndexedDBBackingStore::Transaction::WriteDescriptor::
     operator=(const WriteDescriptor& other) = default;
 
 }  // namespace content