IDBObserver: Lifetime Management: Adding Observer

content/child/indexed_db/indexed_db_dispatcher.h

Index: content/child/indexed_db/indexed_db_dispatcher.h
diff --git a/content/child/indexed_db/indexed_db_dispatcher.h b/content/child/indexed_db/indexed_db_dispatcher.h
index 9e5068db7b6f12e14a3ef48bc722d0bcdf3e2093..2efdcbc23d45d5390ec72c6911d310749cbacd32 100644
--- a/content/child/indexed_db/indexed_db_dispatcher.h
+++ b/content/child/indexed_db/indexed_db_dispatcher.h
@@ -36,6 +36,7 @@ struct IndexedDBMsg_CallbacksSuccessValue_Params;
 struct IndexedDBMsg_CallbacksUpgradeNeeded_Params;
 
 namespace blink {
+class IDBObserver;
 class WebData;
 }
 
@@ -74,6 +75,10 @@ class CONTENT_EXPORT IndexedDBDispatcher : public WorkerThread::Observer {
   // This method is virtual so it can be overridden in unit tests.
   virtual bool Send(IPC::Message* msg);
 

[dmurph, 2016/06/15 12:49:44]

Also add RemoveIDBObservers, which accepts a vector of IDs, and is called by
webidbdatabase_impl in close()

+  void AddIDBObserver(int32_t ipc_database_id,

[dmurph, 2016/06/15 12:49:43]

Have this return the observe ID so you can store it in the webdatabase_impl

[palakj1, 2016/06/16 07:05:41]

Done

+                      int64_t transaction_id,
+                      blink::IDBObserver* observer);
+
   void RequestIDBFactoryGetDatabaseNames(blink::WebIDBCallbacks* callbacks,
                                          const url::Origin& origin);
 
@@ -251,6 +256,14 @@ class CONTENT_EXPORT IndexedDBDispatcher : public WorkerThread::Observer {
   void ResetCursorPrefetchCaches(int64_t transaction_id,
                                  int32_t ipc_exception_cursor_id);
 
+  // int64_t nextObserverId() {
+  //   // Only keep a 32-bit counter to allow ports to use the other 32
+  //   // bits of the id.
+  //   // overflow??
+  //   static int currentObserverId = 0;
+  //   return atomicIncrement(&currentObserverId);
+  // }
+
   scoped_refptr<ThreadSafeSender> thread_safe_sender_;
 
   // Maximum size (in bytes) of value/key pair allowed for put requests. Any
@@ -258,7 +271,7 @@ class CONTENT_EXPORT IndexedDBDispatcher : public WorkerThread::Observer {
   // Used by unit tests to exercise behavior without allocating huge chunks
   // of memory.
   size_t max_put_value_size_ = kMaxIDBMessageSizeInBytes;
-
+  int maxObserverId = 0;

[dmurph, 2016/06/15 12:49:44]

remove this since IDMap will handle id allocation.

   // Careful! WebIDBCallbacks wraps non-threadsafe data types. It must be
   // destroyed and used on the same thread it was created on.
   IDMap<blink::WebIDBCallbacks, IDMapOwnPointer> pending_callbacks_;
@@ -274,6 +287,7 @@ class CONTENT_EXPORT IndexedDBDispatcher : public WorkerThread::Observer {
   std::map<int32_t, WebIDBCursorImpl*> cursors_;
 
   std::map<int32_t, WebIDBDatabaseImpl*> databases_;
+  std::map<int64_t, blink::IDBObserver*> observers_;

[dmurph, 2016/06/15 12:49:44]

Use IDMap like above, but we need to make sure the lifetime story works. This
means we'll be using a RefPtr. So something like:
IDMap<RefPtr<blink::IDBObserver>, IDMapOwnPointer>>

The only quirk here is that we'll be allocating the RefPtr on the heap and not
the stack, as the map expects to be given a pointer.

[palakj1, 2016/06/16 07:05:40]

Do we use IDMap cause it provides some performance benefits (auto generating id)
over std::map? Or can we not use std::map in this context at all?
Also, I can't make IDMap work with IDBObserver, unable to include it in header
(relative path trouble) and IDMap has a concrete destructor, so forward
declaration doesn't work. 
Deferring it to another patch.

 
   DISALLOW_COPY_AND_ASSIGN(IndexedDBDispatcher);
 };