IDBObserver: Lifetime Management: Adding Observer

content/browser/indexed_db/indexed_db_connection.h

Index: content/browser/indexed_db/indexed_db_connection.h
diff --git a/content/browser/indexed_db/indexed_db_connection.h b/content/browser/indexed_db/indexed_db_connection.h
index 4d828e5e6ff5e28a138f19c79959f5a928ab165e..fd0629f63231e7290df20cf69f4b22ba43d99eb9 100644
--- a/content/browser/indexed_db/indexed_db_connection.h
+++ b/content/browser/indexed_db/indexed_db_connection.h
@@ -10,6 +10,7 @@
 #include "base/memory/weak_ptr.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"
 
 namespace content {
 class IndexedDBCallbacks;

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

This is unrelated to your change, but can you delete the unused forward
declaration of IndexedDBConnection?

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

Done.

@@ -28,8 +29,20 @@ class CONTENT_EXPORT IndexedDBConnection {
 
   void VersionChangeIgnored();
 
+  // The observers begin listening to changes only once they are activated.

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

Nit: I think this comment is better placed above active_observers_.

https://google.github.io/styleguide/cppguide.html#Function_Comments

"Declaration comments describe use of the function; comments at the definition
of a function describe operation."

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

Thanks for that nit. Change made.

+  virtual void ActivatePendingObservers(

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

Should pending_observers be a non-const reference - this makes a copy of that
vector right?

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

I am actually moving the argument vector not copying it       
connection_->ActivatePendingObservers(std::move(pending_observers_));
(indexed_db_transaction.cc:348)

Since, we need to move the elements of the vector, can't pass it as a const ref.
I can make it a pointer however, if that's preferred to moving.

+      std::vector<std::unique_ptr<IndexedDBObserver>> pending_observers);
+  virtual void RemoveObservers(const std::vector<int32_t>& remove_observer_ids);

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

Maybe add a comment above RemoveObservers stating that it will remove either a
pending or an active observer.

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

Done.

+
   IndexedDBDatabase* database() const { return database_.get(); }
   IndexedDBDatabaseCallbacks* callbacks() const { return callbacks_.get(); }
+  const std::vector<std::unique_ptr<IndexedDBObserver>>& active_observers()
+      const {
+    return active_observers_;
+  }
+  base::WeakPtr<IndexedDBConnection> GetWeakPtr() {
+    return weak_factory_.GetWeakPtr();
+  }
 
  private:
   // NULL in some unit tests, and after the connection is closed.
@@ -38,7 +51,7 @@ class CONTENT_EXPORT IndexedDBConnection {
   // The callbacks_ member is cleared when the connection is closed.
   // May be NULL in unit tests.
   scoped_refptr<IndexedDBDatabaseCallbacks> callbacks_;
-
+  std::vector<std::unique_ptr<IndexedDBObserver>> active_observers_;
   base::WeakPtrFactory<IndexedDBConnection> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(IndexedDBConnection);