IDBObserver: Lifetime Management: Adding Observer

content/browser/indexed_db/indexed_db_connection.h

 // Copyright 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.
 
 #ifndef CONTENT_BROWSER_INDEXED_DB_INDEXED_DB_CONNECTION_H_
 #define CONTENT_BROWSER_INDEXED_DB_INDEXED_DB_CONNECTION_H_
 
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #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.

 class IndexedDBDatabaseError;
 
 class CONTENT_EXPORT IndexedDBConnection {
  public:
   IndexedDBConnection(scoped_refptr<IndexedDBDatabase> db,
                       scoped_refptr<IndexedDBDatabaseCallbacks> callbacks);
   virtual ~IndexedDBConnection();
 
   // These methods are virtual to allow subclassing in unit tests.
   virtual void ForceClose();
   virtual void Close();
   virtual bool IsConnected();
 
   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.
   scoped_refptr<IndexedDBDatabase> database_;
 
   // 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);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_INDEXED_DB_INDEXED_DB_CONNECTION_H_