IDBObserver: Lifetime Management: Adding Observer

content/browser/indexed_db/indexed_db_transaction_unittest.cc

Index: content/browser/indexed_db/indexed_db_transaction_unittest.cc
diff --git a/content/browser/indexed_db/indexed_db_transaction_unittest.cc b/content/browser/indexed_db/indexed_db_transaction_unittest.cc
index 170ccd23a3b2abde4d3376f139615a0107827ec8..8da736cafd983097905bfad03c85bbdb5fe0e8b2 100644
--- a/content/browser/indexed_db/indexed_db_transaction_unittest.cc
+++ b/content/browser/indexed_db/indexed_db_transaction_unittest.cc
@@ -12,6 +12,7 @@
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
+#include "content/browser/indexed_db/indexed_db_connection.h"
 #include "content/browser/indexed_db/indexed_db_fake_backing_store.h"
 #include "content/browser/indexed_db/mock_indexed_db_database_callbacks.h"
 #include "content/browser/indexed_db/mock_indexed_db_factory.h"
@@ -87,11 +88,8 @@ TEST_F(IndexedDBTransactionTest, Timeout) {
   const std::set<int64_t> scope;
   const leveldb::Status commit_success = leveldb::Status::OK();
   scoped_refptr<IndexedDBTransaction> transaction = new IndexedDBTransaction(
-      id,
-      new MockIndexedDBDatabaseCallbacks(),
-      scope,
-      blink::WebIDBTransactionModeReadWrite,
-      db_.get(),
+      id, new IndexedDBConnection(db_, new MockIndexedDBDatabaseCallbacks()),

[Marijn Kruisselbrink, 2016/06/30 00:17:25]

here too I believe you're leaking IndexedDBConnection instances. You need to
make sure that every IndexedDBConnection you create is also actually destroyed
at some point.

[palakj1, 2016/06/30 01:07:04]

Done.

+      scope, blink::WebIDBTransactionModeReadWrite,
       new IndexedDBFakeBackingStore::FakeTransaction(commit_success));
   db_->TransactionCreated(transaction.get());
 
@@ -131,11 +129,8 @@ TEST_F(IndexedDBTransactionTest, NoTimeoutReadOnly) {
   const std::set<int64_t> scope;
   const leveldb::Status commit_success = leveldb::Status::OK();
   scoped_refptr<IndexedDBTransaction> transaction = new IndexedDBTransaction(
-      id,
-      new MockIndexedDBDatabaseCallbacks(),
-      scope,
-      blink::WebIDBTransactionModeReadOnly,
-      db_.get(),
+      id, new IndexedDBConnection(db_, new MockIndexedDBDatabaseCallbacks()),
+      scope, blink::WebIDBTransactionModeReadOnly,
       new IndexedDBFakeBackingStore::FakeTransaction(commit_success));
   db_->TransactionCreated(transaction.get());
 
@@ -163,11 +158,8 @@ TEST_P(IndexedDBTransactionTestMode, ScheduleNormalTask) {
   const std::set<int64_t> scope;
   const leveldb::Status commit_success = leveldb::Status::OK();
   scoped_refptr<IndexedDBTransaction> transaction = new IndexedDBTransaction(
-      id,
-      new MockIndexedDBDatabaseCallbacks(),
-      scope,
-      GetParam(),
-      db_.get(),
+      id, new IndexedDBConnection(db_, new MockIndexedDBDatabaseCallbacks()),
+      scope, GetParam(),
       new IndexedDBFakeBackingStore::FakeTransaction(commit_success));
 
   EXPECT_FALSE(transaction->HasPendingTasks());
@@ -225,11 +217,8 @@ TEST_F(IndexedDBTransactionTest, SchedulePreemptiveTask) {
   const std::set<int64_t> scope;
   const leveldb::Status commit_failure = leveldb::Status::Corruption("Ouch.");
   scoped_refptr<IndexedDBTransaction> transaction = new IndexedDBTransaction(
-      id,
-      new MockIndexedDBDatabaseCallbacks(),
-      scope,
-      blink::WebIDBTransactionModeVersionChange,
-      db_.get(),
+      id, new IndexedDBConnection(db_, new MockIndexedDBDatabaseCallbacks()),
+      scope, blink::WebIDBTransactionModeVersionChange,
       new IndexedDBFakeBackingStore::FakeTransaction(commit_failure));
 
   EXPECT_FALSE(transaction->HasPendingTasks());
@@ -286,11 +275,8 @@ TEST_P(IndexedDBTransactionTestMode, AbortTasks) {
   const std::set<int64_t> scope;
   const leveldb::Status commit_failure = leveldb::Status::Corruption("Ouch.");
   scoped_refptr<IndexedDBTransaction> transaction = new IndexedDBTransaction(
-      id,
-      new MockIndexedDBDatabaseCallbacks(),
-      scope,
-      GetParam(),
-      db_.get(),
+      id, new IndexedDBConnection(db_, new MockIndexedDBDatabaseCallbacks()),
+      scope, GetParam(),
       new IndexedDBFakeBackingStore::FakeTransaction(commit_failure));
   db_->TransactionCreated(transaction.get());
 
@@ -316,11 +302,8 @@ TEST_P(IndexedDBTransactionTestMode, AbortPreemptive) {
   const std::set<int64_t> scope;
   const leveldb::Status commit_success = leveldb::Status::OK();
   scoped_refptr<IndexedDBTransaction> transaction = new IndexedDBTransaction(
-      id,
-      new MockIndexedDBDatabaseCallbacks(),
-      scope,
-      GetParam(),
-      db_.get(),
+      id, new IndexedDBConnection(db_, new MockIndexedDBDatabaseCallbacks()),
+      scope, GetParam(),
       new IndexedDBFakeBackingStore::FakeTransaction(commit_success));
   db_->TransactionCreated(transaction.get());
 
@@ -362,6 +345,49 @@ TEST_P(IndexedDBTransactionTestMode, AbortPreemptive) {
             transaction->diagnostics().tasks_scheduled);
 }
 
+TEST_F(IndexedDBTransactionTest, IndexedDBObserver) {
+  const int64_t id = 0;
+  const std::set<int64_t> scope;
+  const leveldb::Status commit_success = leveldb::Status::OK();
+  IndexedDBConnection* connection =

[Marijn Kruisselbrink, 2016/06/30 00:17:25]

probably make this a std::unique_ptr<IndexedDBConnection> to ensure it doesn't
leak.

[palakj1, 2016/06/30 01:07:04]

Done.

+      new IndexedDBConnection(db_, new MockIndexedDBDatabaseCallbacks());
+
+  scoped_refptr<IndexedDBTransaction> transaction = new IndexedDBTransaction(
+      id, connection, scope, blink::WebIDBTransactionModeReadWrite,
+      new IndexedDBFakeBackingStore::FakeTransaction(commit_success));
+  db_->TransactionCreated(transaction.get());
+
+  EXPECT_EQ(0UL, transaction->pending_observers_.size());
+  EXPECT_EQ(0UL, connection->active_observers().size());
+
+  // Add observers to pending observer list.
+  const int32_t observer_id1 = 1, observer_id2 = 2;
+  transaction->AddPendingObserver(observer_id1);
+  transaction->AddPendingObserver(observer_id2);
+  EXPECT_EQ(2UL, transaction->pending_observers_.size());
+  EXPECT_EQ(0UL, connection->active_observers().size());
+
+  // Before commit, observer would be in pending list of transaction.
+  std::vector<int32_t> observer_to_remove1 = {observer_id1};
+  connection->RemoveObservers(observer_to_remove1);
+  EXPECT_EQ(1UL, transaction->pending_observers_.size());
+  EXPECT_EQ(0UL, connection->active_observers().size());
+
+  // After commit, observer moved to connection's active observer.
+  transaction->Commit();
+  EXPECT_EQ(0UL, transaction->pending_observers_.size());
+  EXPECT_EQ(1UL, connection->active_observers().size());
+
+  // Observer does not exist, so no change to active_observers.
+  connection->RemoveObservers(observer_to_remove1);
+  EXPECT_EQ(1UL, connection->active_observers().size());
+
+  // Observer removed from connection's active observer.
+  std::vector<int32_t> observer_to_remove2 = {observer_id2};
+  connection->RemoveObservers(observer_to_remove2);
+  EXPECT_EQ(0UL, connection->active_observers().size());
+}
+
 static const blink::WebIDBTransactionMode kTestModes[] = {
     blink::WebIDBTransactionModeReadOnly, blink::WebIDBTransactionModeReadWrite,
     blink::WebIDBTransactionModeVersionChange};