IDBObserver: Lifetime Management: Adding Observer

third_party/WebKit/Source/modules/indexeddb/IDBObserver.h

Index: third_party/WebKit/Source/modules/indexeddb/IDBObserver.h
diff --git a/third_party/WebKit/Source/modules/indexeddb/IDBObserver.h b/third_party/WebKit/Source/modules/indexeddb/IDBObserver.h
index 538995bc8b907504792f182bd8b46a0514bd1308..106c3e3eecc17db4c9427a991b252c50e18e9fe1 100644
--- a/third_party/WebKit/Source/modules/indexeddb/IDBObserver.h
+++ b/third_party/WebKit/Source/modules/indexeddb/IDBObserver.h
@@ -6,23 +6,29 @@
 #define IDBObserver_h
 
 #include "bindings/core/v8/ScriptWrappable.h"
-#include "modules/indexeddb/IDBDatabase.h"
-#include "modules/indexeddb/IDBTransaction.h"
+#include "modules/ModulesExport.h"
 #include "platform/heap/Handle.h"
+#include <set>
 
 namespace blink {
 
+class ExceptionState;
 class IDBObserverCallback;
 class IDBObserverInit;
+class IDBDatabase;

[cmumford, 2016/07/06 22:57:56]

Put forward declarations in alphabetical order.

[palakj1, 2016/07/07 00:19:53]

Done.

+class IDBTransaction;
 
-class IDBObserver final : public GarbageCollected<IDBObserver>, public ScriptWrappable {
+class MODULES_EXPORT IDBObserver final : public GarbageCollectedFinalized<IDBObserver>, public ScriptWrappable {
     DEFINE_WRAPPERTYPEINFO();
 
 public:
     static IDBObserver* create(IDBObserverCallback&, const IDBObserverInit&);
 
+    ~IDBObserver();
     // API methods
     void observe(IDBDatabase*, IDBTransaction*, ExceptionState&);
+    void unobserve(IDBDatabase*, ExceptionState&);
+    void removeObserver(int32_t id);
 
     DECLARE_TRACE();
 
@@ -33,6 +39,7 @@ private:
     bool m_transaction;
     bool m_values;
     bool m_noRecords;
+    std::set<int32_t> m_observerIds;

[cmumford, 2016/07/06 22:57:57]

So both IDBObserver and WebIDBDatabaseImpl have a set of observer ID's, both are
in the same thread, and both are identical (ignoring short time during add and
erase) right? Is there any reason why we can't just use the one std::set() owned
by WebIDBDatabaseImpl?

[dmurph, 2016/07/06 23:34:24]

They're not identical. Here's the overview:
1. Every observer can have multiple 'observe call' ids. This can belong to
multiple databases, as we can observe multiple databases. There is one id per
call to 'observe'.

2. Every database has a set of 'observe call' ids that all of the
observervations registered for that database. These can be from different
observers.

operations:
1. observer.unobserve(db) stops observation on the given database. This means we
need to remove all of the ids we have in that observer that apply to the given
database, as well as remove those from the database, and then tell the backend
this set of ids.
2. db.close() needs to make all observers that are observing it remove any
'observe call' ids that apply to that database. We do this through the
WebIDBObserver destructor (which we have one registered per observe call).

Does that make more sense?

[palakj1, 2016/07/07 00:19:54]

They are in the same thread, but not the same. Consider,
obs1 = new IDBObserver();
obs2 = new IDBObserver();
obs1.observe(db1);   // id 1
obs1.observe(db2);   // id 2
obs2.observe(db1);   // id 3

obs1.observer_id = { 1, 2 }
obs2.observer_id = { 3 }
db1.observer_id = { 1, 3 }
db2.observer_id = { 2 }

obs1.unobserve(db2)  // removes id 2
db1.close()          // removes id 1, 3

We need both the lists because there are 2 ways to remove the observe calls
i. obs.unobserve(db) : This removes all observe calls associated with the
observer object 'obs' on the database 'db'.
ii. connection close : This removes all observe calls of all the observers on
the database 'db'. 

Thus, we need to bookkeep observe calls on an intersection of observer and
database to deal with unobserve functionality.

[cmumford, 2016/07/07 19:48:51]

yep - thx.

 };
 
 } // namespace blink