IDBObserver: Lifetime Management: Adding Observer

content/browser/indexed_db/indexed_db_transaction.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_transaction.h"
 
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/logging.h"
+#include "base/memory/ptr_util.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "content/browser/indexed_db/indexed_db_backing_store.h"
 #include "content/browser/indexed_db/indexed_db_cursor.h"
 #include "content/browser/indexed_db/indexed_db_database.h"
 #include "content/browser/indexed_db/indexed_db_database_callbacks.h"
+#include "content/browser/indexed_db/indexed_db_observer.h"
 #include "content/browser/indexed_db/indexed_db_tracing.h"
 #include "content/browser/indexed_db/indexed_db_transaction_coordinator.h"
 #include "third_party/WebKit/public/platform/modules/indexeddb/WebIDBDatabaseException.h"
 #include "third_party/leveldatabase/env_chromium.h"
 
 namespace content {
 
 namespace {
 
 const int64_t kInactivityTimeoutPeriodSeconds = 60;
@@ skipping 32 matching lines @@
 
 IndexedDBTransaction::Operation IndexedDBTransaction::TaskStack::pop() {
   DCHECK(!stack_.empty());
   Operation task(stack_.top());
   stack_.pop();
   return task;
 }
 
 IndexedDBTransaction::IndexedDBTransaction(
     int64_t id,
-    scoped_refptr<IndexedDBDatabaseCallbacks> callbacks,
+    base::WeakPtr<IndexedDBConnection> connection,
     const std::set<int64_t>& object_store_ids,
     blink::WebIDBTransactionMode mode,
-    IndexedDBDatabase* database,
     IndexedDBBackingStore::Transaction* backing_store_transaction)
     : id_(id),
       object_store_ids_(object_store_ids),
       mode_(mode),
       used_(false),
       state_(CREATED),
       commit_pending_(false),
-      callbacks_(callbacks),
-      database_(database),
+      connection_(std::move(connection)),
       transaction_(backing_store_transaction),
       backing_store_transaction_begun_(false),
       should_process_queue_(false),
       pending_preemptive_events_(0) {
+  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();
 }
 
 IndexedDBTransaction::~IndexedDBTransaction() {
   // It shouldn't be possible for this object to get deleted until it's either
   // complete or aborted.
@@ skipping 88 matching lines @@
 #ifndef NDEBUG
   DCHECK(!database_->transaction_coordinator().IsActive(this));
 #endif
 
   if (callbacks_.get())
     callbacks_->OnAbort(id_, error);
 
   database_->TransactionFinished(this, false);
 
   database_ = NULL;
+  connection_ = nullptr;

[cmumford, 2016/07/01 18:35:01]

What about also calling:

  pending_observers_.clear();

[palakj1, 2016/07/02 00:48:14]

Done. Also adding a check in transaction destructor
 DCHECK(pending_observers_.empty())

 }
 
 bool IndexedDBTransaction::IsTaskQueueEmpty() const {
   return preemptive_task_queue_.empty() && task_queue_.empty();
 }
 
 bool IndexedDBTransaction::HasPendingTasks() const {
   return pending_preemptive_events_ || !IsTaskQueueEmpty();
 }
 
@@ skipping 128 matching lines @@
   // released, and order is critical.
   CloseOpenCursors();
   transaction_->Reset();
 
   // Transactions must also be marked as completed before the
   // front-end is notified, as the transaction completion unblocks
   // operations like closing connections.
   database_->transaction_coordinator().DidFinishTransaction(this);
 
   if (committed) {
+    // TODO (palakj) : Send Observations to observers
+    if (!pending_observers_.empty() && connection_)
+      connection_->ActivatePendingObservers(std::move(pending_observers_));
     abort_task_stack_.clear();
     {
       IDB_TRACE1(
           "IndexedDBTransaction::CommitPhaseTwo.TransactionCompleteCallbacks",
           "txn.id", id());
       callbacks_->OnComplete(id_);
     }
     database_->TransactionFinished(this, true);
   } else {
     while (!abort_task_stack_.empty())
@@ skipping 86 matching lines @@
       base::ASCIIToUTF16("Transaction timed out due to inactivity.")));
 }
 
 void IndexedDBTransaction::CloseOpenCursors() {
   IDB_TRACE1("IndexedDBTransaction::CloseOpenCursors", "txn.id", id());
   for (auto* cursor : open_cursors_)
     cursor->Close();
   open_cursors_.clear();
 }
 
+void IndexedDBTransaction::AddPendingObserver(int32_t observer_id) {
+  pending_observers_.push_back(
+      base::WrapUnique(new IndexedDBObserver(observer_id)));
+}
+
+void IndexedDBTransaction::RemovePendingObservers(
+    const std::vector<int32_t>& pending_observer_ids) {
+  for (size_t i = 0; i < pending_observer_ids.size(); i++) {

[cmumford, 2016/07/01 18:35:01]

Nit: I don't believe we have a rule on using iterators vs indexes - I don't
believe we do. I generally avoid using "break" when possible, and iterators make
it easier to switch from one container type to another without modifying the
code - but may be less pleasing to the eye.

  auto it = pending_observers_.begin();
  while (it != pending_observers_.end()) {
    if (std::find(pending_observer_ids.begin(), pending_observer_ids.end(),
                  (*it)->id()) != pending_observer_ids.end())
      it = pending_observers_.erase(it);
    else
      ++it;
  }

Just something to consider - only change if you think this is cleaner.

[palakj1, 2016/07/02 00:48:14]

Changed. Used remove_if function instead to eliminate the loop.

+    for (size_t j = 0; j < pending_observers_.size(); j++) {
+      if (pending_observers_[j]->id() == pending_observer_ids[i]) {
+        pending_observers_.erase(pending_observers_.begin() + j);
+        break;
+      }
+    }
+  }
+}
+
 }  // namespace content