[IndexedDB] Propogating changes to observers : Renderer

content/child/indexed_db/indexed_db_dispatcher.cc

Index: content/child/indexed_db/indexed_db_dispatcher.cc
diff --git a/content/child/indexed_db/indexed_db_dispatcher.cc b/content/child/indexed_db/indexed_db_dispatcher.cc
index 7d6196c637d58a625027c509e93055c00d1588d4..6663cd8e1753084b2430f409b0383b0e0ce9b9dd 100644
--- a/content/child/indexed_db/indexed_db_dispatcher.cc
+++ b/content/child/indexed_db/indexed_db_dispatcher.cc
@@ -19,6 +19,7 @@
 #include "third_party/WebKit/public/platform/modules/indexeddb/WebIDBDatabaseCallbacks.h"
 #include "third_party/WebKit/public/platform/modules/indexeddb/WebIDBDatabaseError.h"
 #include "third_party/WebKit/public/platform/modules/indexeddb/WebIDBDatabaseException.h"
+#include "third_party/WebKit/public/platform/modules/indexeddb/WebIDBObservation.h"
 #include "third_party/WebKit/public/platform/modules/indexeddb/WebIDBValue.h"
 
 using blink::WebBlobInfo;
@@ -30,6 +31,7 @@ using blink::WebIDBDatabaseCallbacks;
 using blink::WebIDBDatabaseError;
 using blink::WebIDBKey;
 using blink::WebIDBMetadata;
+using blink::WebIDBObservation;
 using blink::WebIDBObserver;
 using blink::WebIDBValue;
 using blink::WebString;
@@ -125,6 +127,20 @@ WebIDBMetadata IndexedDBDispatcher::ConvertMetadata(
   return web_metadata;
 }
 
+std::vector<WebIDBObservation> IndexedDBDispatcher::ConvertObservations(
+    const std::vector<IndexedDBMsg_Observation>& idb_observations) {
+  std::vector<WebIDBObservation> web_observations;
+  for (const auto& idb_observation : idb_observations) {
+    WebIDBObservation web_observation;
+    web_observation.objectStoreId = idb_observation.object_store_id;
+    web_observation.type = idb_observation.type;
+    web_observation.keyRange =
+        WebIDBKeyRangeBuilder::Build(idb_observation.key_range);
+    web_observations.push_back(std::move(web_observation));
+  }
+  return web_observations;
+}
+
 void IndexedDBDispatcher::OnMessageReceived(const IPC::Message& msg) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(IndexedDBDispatcher, msg)
@@ -156,6 +172,7 @@ void IndexedDBDispatcher::OnMessageReceived(const IPC::Message& msg) {
                         OnVersionChange)
     IPC_MESSAGE_HANDLER(IndexedDBMsg_DatabaseCallbacksAbort, OnAbort)
     IPC_MESSAGE_HANDLER(IndexedDBMsg_DatabaseCallbacksComplete, OnComplete)
+    IPC_MESSAGE_HANDLER(IndexedDBMsg_DatabaseCallbacksChanges, OnDatabaseChange)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   // If a message gets here, IndexedDBMessageFilter already determined that it
@@ -797,6 +814,20 @@ void IndexedDBDispatcher::OnComplete(int32_t ipc_thread_id,
   callbacks->onComplete(transaction_id);
 }
 
+void IndexedDBDispatcher::OnDatabaseChange(
+    int32_t ipc_thread_id,
+    int32_t ipc_database_id,
+    const IndexedDBMsg_ObserverChanges& changes) {
+  DCHECK_EQ(ipc_thread_id, CurrentWorkerId());
+  std::vector<WebIDBObservation> observations(
+      ConvertObservations(changes.observations));
+  for (auto& obs : changes.observation_index) {
+    WebIDBObserver* observer = observers_.Lookup(obs.first);
+    // TODO(palakj): Get IDBDatabase object from ipc_database_id.

[jsbell, 2016/07/11 18:25:33]

Communication from the back end to an IDBDatabase is done via
WebIDBDatabaseCallbacks.

Starting in IndexedDBDatabaseCallbacks::OnDatabaseChange you can pass the
ipc_database_callbacks_id_ through in the message instead of ipc_database_id.
That lets you look up a WebIDBDatabaseCallbacks instance in the
IndexedDBDispatcher; you'd add onChange() there and in
WebIDBDatabaseCallbacksImpl and in IDBDatabaseCallbacks which has an IDBDatabase
weakptr.

Look at how OnAbort is propagated back for an example.

(Converting to Mojo will let us eliminate these indirections!)

[palakj1, 2016/07/11 19:37:54]

Thanks for the insight. This seems a good option. 
Our other alternative is to have IDBObserver hold a map of observer_id to
IDBDatabase weakptr corresponding to each observe call 'observer.observe(db)' 
(we already have a set of observer_id here, need to turn it to a map). 
 A benefit with this apporach is that the unobserve logic gets cleaner.
Currently, for observer.unobserve(db) , IDBObserver needs to check with
WebIDBDatabase if the latter holds the observer_id. 
Having a map of observer_id to IDBDatabase, we can complete the entire logic
within IDBObserver. Moving a bit of logic from webidb to blink.
The downside is creating a new weak ptr.

What would you prefer?

[jsbell, 2016/07/11 21:23:04]

It sounds like the weakptr in IDBObserver would be cleaner, so starting with
that approach sgtm.

+    observer->onChange(observations, std::move(obs.second));
+  }
+}
+
 void IndexedDBDispatcher::OnForcedClose(int32_t ipc_thread_id,
                                         int32_t ipc_database_callbacks_id) {
   DCHECK_EQ(ipc_thread_id, CurrentWorkerId());