IDBObserver: Lifetime Management: Adding Observer

content/browser/indexed_db/indexed_db_dispatcher_host.cc

Index: content/browser/indexed_db/indexed_db_dispatcher_host.cc
diff --git a/content/browser/indexed_db/indexed_db_dispatcher_host.cc b/content/browser/indexed_db/indexed_db_dispatcher_host.cc
index 96cad054d68c0110179e1aeb1e3a8c516ae73e60..9a35614e4305d7c36e988677c987426f6c130a39 100644
--- a/content/browser/indexed_db/indexed_db_dispatcher_host.cc
+++ b/content/browser/indexed_db/indexed_db_dispatcher_host.cc
@@ -193,6 +193,26 @@ int32_t IndexedDBDispatcherHost::Add(IndexedDBConnection* connection,
   return ipc_database_id;
 }
 
+int64_t IndexedDBDispatcherHost::HostObserverId(int64_t observer_id) {
+  // Inject the renderer process id into the observer id, to
+  // uniquely identify this observer, and effectively bind it to
+  // the renderer that initiated it. The lower 32 bits of
+  // observer_id are guaranteed to be unique within that renderer.

[Marijn Kruisselbrink, 2016/06/15 13:14:57]

There is no such guarantee. There is a separate IndexedDBDispatcher for each
thread in the renderer (so documents and workers don't share the same dispatcher
instance). And at least as currently implemented there are no guarantees that
those two separate dispatchers don't create the same IDs.
Separately I'm not quite sure what these IDs are used for/why we need this
complicated ID scheme. It all seems rather fragile/complicated...

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

Oh, I was trying to imitate the way host transaction id are generated. Changed
now to djb2 hash suggested by dmurph.

+  base::ProcessId pid = peer_pid();
+  DCHECK(!(observer_id >> 32)) << "observer ids can only be 32 bits";
+  static_assert(sizeof(base::ProcessId) <= sizeof(int32_t),
+                "Process ID must fit in 32 bits");
+
+  return observer_id | (static_cast<uint64_t>(pid) << 32);
+}
+
+int64_t IndexedDBDispatcherHost::RendererObserverId(int64_t host_observer_id) {
+  DCHECK(host_observer_id >> 32 == peer_pid())
+      << "Invalid renderer target for observer id";
+  return host_observer_id & 0xffffffff;
+}
+
+// Do I need this for observer? origin?
 void IndexedDBDispatcherHost::RegisterTransactionId(int64_t host_transaction_id,
                                                     const url::Origin& origin) {
   if (!database_dispatcher_host_)
@@ -520,6 +540,7 @@ bool IndexedDBDispatcherHost::DatabaseDispatcherHost::OnMessageReceived(
     IPC_MESSAGE_HANDLER(IndexedDBHostMsg_DatabaseVersionChangeIgnored,
                         OnVersionChangeIgnored)
     IPC_MESSAGE_HANDLER(IndexedDBHostMsg_DatabaseDestroyed, OnDestroyed)
+    IPC_MESSAGE_HANDLER(IndexedDBHostMsg_DatabaseObserve, OnObserve)
     IPC_MESSAGE_HANDLER(IndexedDBHostMsg_DatabaseGet, OnGet)
     IPC_MESSAGE_HANDLER(IndexedDBHostMsg_DatabaseGetAll, OnGetAll)
     IPC_MESSAGE_HANDLER(IndexedDBHostMsg_DatabasePut, OnPutWrapper)
@@ -629,6 +650,20 @@ void IndexedDBDispatcherHost::DatabaseDispatcherHost::OnDestroyed(
   parent_->DestroyObject(&map_, ipc_object_id);
 }
 
+void IndexedDBDispatcherHost::DatabaseDispatcherHost::OnObserve(
+    const IndexedDBHostMsg_DatabaseObserve_Params& params) {
+  DCHECK(parent_->context()->TaskRunner()->RunsTasksOnCurrentThread());
+  IndexedDBConnection* connection =
+      parent_->GetOrTerminateProcess(&map_, params.ipc_database_id);
+  if (!connection || !connection->IsConnected())
+    return;
+
+  observers_.push_back(params.observer_id);
+  connection->database()->Observe(
+      parent_->HostTransactionId(params.transaction_id),
+      parent_->HostObserverId(params.observer_id));
+}
+
 void IndexedDBDispatcherHost::DatabaseDispatcherHost::OnGet(
     const IndexedDBHostMsg_DatabaseGet_Params& params) {
   DCHECK(parent_->context()->TaskRunner()->RunsTasksOnCurrentThread());