[IndexedDB] Propogating changes to observers : Renderer

content/browser/indexed_db/indexed_db_dispatcher_host.cc

 // Copyright (c) 2012 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.
 
 #include "content/browser/indexed_db/indexed_db_dispatcher_host.h"
 
 #include <stddef.h>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/guid.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/scoped_vector.h"
 #include "base/process/process.h"
 #include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "content/browser/bad_message.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/indexed_db/indexed_db_callbacks.h"
 #include "content/browser/indexed_db/indexed_db_connection.h"
 #include "content/browser/indexed_db/indexed_db_context_impl.h"
 #include "content/browser/indexed_db/indexed_db_cursor.h"
 #include "content/browser/indexed_db/indexed_db_database_callbacks.h"
 #include "content/browser/indexed_db/indexed_db_metadata.h"
+#include "content/browser/indexed_db/indexed_db_observation.h"
+#include "content/browser/indexed_db/indexed_db_observer_changes.h"
 #include "content/browser/indexed_db/indexed_db_pending_connection.h"
 #include "content/browser/indexed_db/indexed_db_value.h"
 #include "content/browser/renderer_host/render_message_filter.h"
 #include "content/common/indexed_db/indexed_db_messages.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/result_codes.h"
 #include "storage/browser/blob/blob_data_builder.h"
 #include "storage/browser/blob/blob_storage_context.h"
@@ skipping 145 matching lines @@
 
 int32_t IndexedDBDispatcherHost::Add(IndexedDBConnection* connection,
                                      int32_t ipc_thread_id,
                                      const url::Origin& origin) {
   if (!database_dispatcher_host_) {
     connection->Close();
     delete connection;
     return -1;
   }
   int32_t ipc_database_id = database_dispatcher_host_->map_.Add(connection);
+  connection->setId(ipc_database_id);
   context()->ConnectionOpened(origin, connection);
   database_dispatcher_host_->database_origin_map_[ipc_database_id] = origin;
   return ipc_database_id;
 }
 
 void IndexedDBDispatcherHost::RegisterTransactionId(int64_t host_transaction_id,
                                                     const url::Origin& origin) {
   if (!database_dispatcher_host_)
     return;
   database_dispatcher_host_->transaction_size_map_[host_transaction_id] = 0;
@@ skipping 109 matching lines @@
       idb_index_metadata.key_path = web_index_metadata.key_path;
       idb_index_metadata.unique = web_index_metadata.unique;
       idb_index_metadata.multi_entry = web_index_metadata.multi_entry;
       idb_store_metadata.indexes.push_back(idb_index_metadata);
     }
     metadata.object_stores.push_back(idb_store_metadata);
   }
   return metadata;
 }
 
+::IndexedDBMsg_ObserverChanges IndexedDBDispatcherHost::ConvertObserverChanges(

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

Is the leading :: needed here? Its only used in ConvertMetadata to distinguish
::IndexedDBDatabaseMetadata (an IPC-specific struct) from
content::IndexedDBDatabaseMetadata (the struct used in the non-IPC code)

[palakj1, 2016/07/11 22:25:40]

Done.

+    std::unique_ptr<content::IndexedDBObserverChanges> changes) {

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

Similarly, I don't think you need content:: here

[palakj1, 2016/07/11 22:25:40]

Done.

+  ::IndexedDBMsg_ObserverChanges idb_changes;
+  idb_changes.observation_index = changes->observation_index();
+  for (auto& observation : changes->observations()) {
+    idb_changes.observations.push_back(
+        ConvertObservation(std::move(observation)));

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

It's subtle that this is freeing the observations during iteration. Perhaps add
a comment, and (as noted below) make the ConvertObservation() take a const ptr
and do the removal explicitly here.

[palakj1, 2016/07/11 22:25:40]

done.

+  }
+  return idb_changes;
+}
+
+::IndexedDBMsg_Observation IndexedDBDispatcherHost::ConvertObservation(
+    std::unique_ptr<content::IndexedDBObservation> observation) {

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

Since this is not transferring ownership, can this be a const raw ptr rather
than a unique_ptr ?

[palakj1, 2016/07/11 22:25:40]

Done.

+  // TODO(palakj): Modify function for indexed_db_value.
+  ::IndexedDBMsg_Observation idb_observation;
+  idb_observation.object_store_id = observation->object_store_id();
+  idb_observation.type = observation->type();
+  idb_observation.key_range = observation->key_range();
+  return idb_observation;
+}
+
 void IndexedDBDispatcherHost::OnIDBFactoryGetDatabaseNames(
     const IndexedDBHostMsg_FactoryGetDatabaseNames_Params& params) {
   DCHECK(indexed_db_context_->TaskRunner()->RunsTasksOnCurrentThread());
 
   if (!IsValidOrigin(params.origin)) {
     bad_message::ReceivedBadMessage(this, bad_message::IDBDH_INVALID_ORIGIN);
     return;
   }
 
   base::FilePath indexed_db_path = indexed_db_context_->data_path();
@@ skipping 739 matching lines @@
     DLOG(ERROR) << "Unable to reset prefetch";
 }
 
 void IndexedDBDispatcherHost::CursorDispatcherHost::OnDestroyed(
     int32_t ipc_object_id) {
   DCHECK(parent_->context()->TaskRunner()->RunsTasksOnCurrentThread());
   parent_->DestroyObject(&map_, ipc_object_id);
 }
 
 }  // namespace content