Fetch incremental updates. Store new state in V4Store.

components/safe_browsing_db/v4_database.cc

Index: components/safe_browsing_db/v4_database.cc
diff --git a/components/safe_browsing_db/v4_database.cc b/components/safe_browsing_db/v4_database.cc
index 99a22468e0c1fd58896fe46f6ad483ac76d58b4e..0b5acf79d967421995c06e4cbe0092a5a334c1ed 100644
--- a/components/safe_browsing_db/v4_database.cc
+++ b/components/safe_browsing_db/v4_database.cc
@@ -23,16 +23,20 @@ void V4Database::Create(
     const scoped_refptr<base::SequencedTaskRunner>& db_task_runner,
     const base::FilePath& base_path,
     const StoreFileNameMap& store_file_name_map,
-    NewDatabaseReadyCallback callback) {
+    DatabaseUpdatedCallback db_updated_callback,
+    NewDatabaseReadyCallback new_db_callback) {
   // Create the database, which may be a lengthy operation, on the
   // db_task_runner, but once that is done, call the caller back on this
   // thread.
+  DCHECK(base_path.IsAbsolute());
+  DCHECK(!store_file_name_map.empty());
+
   const scoped_refptr<base::SingleThreadTaskRunner>& callback_task_runner =
       base::MessageLoop::current()->task_runner();
   db_task_runner->PostTask(
-      FROM_HERE,
-      base::Bind(&V4Database::CreateOnTaskRunner, db_task_runner, base_path,
-                 store_file_name_map, callback_task_runner, callback));
+      FROM_HERE, base::Bind(&V4Database::CreateOnTaskRunner, db_task_runner,
+                            base_path, store_file_name_map, db_updated_callback,
+                            callback_task_runner, new_db_callback));
 }
 
 // static
@@ -40,34 +44,40 @@ void V4Database::CreateOnTaskRunner(
     const scoped_refptr<base::SequencedTaskRunner>& db_task_runner,
     const base::FilePath& base_path,
     const StoreFileNameMap& store_file_name_map,
+    DatabaseUpdatedCallback db_updated_callback,
     const scoped_refptr<base::SingleThreadTaskRunner>& callback_task_runner,
-    NewDatabaseReadyCallback callback) {
+    NewDatabaseReadyCallback new_db_callback) {
   DCHECK(db_task_runner->RunsTasksOnCurrentThread());
-  DCHECK(base_path.IsAbsolute());
 
   if (!factory_) {
     factory_ = new V4StoreFactory();
     ANNOTATE_LEAKING_OBJECT_PTR(factory_);
   }
-  auto store_map = base::MakeUnique<StoreMap>();
+
+  std::unique_ptr<StoreMap> store_map = base::MakeUnique<StoreMap>();
   for (const auto& store_info : store_file_name_map) {
     const UpdateListIdentifier& update_list_identifier = store_info.first;
     const base::FilePath store_path = base_path.AppendASCII(store_info.second);
     (*store_map)[update_list_identifier].reset(
         factory_->CreateV4Store(db_task_runner, store_path));
   }
-  std::unique_ptr<V4Database> v4_database(
-      new V4Database(db_task_runner, std::move(store_map)));
+  std::unique_ptr<V4Database> v4_database(new V4Database(
+      db_task_runner, std::move(store_map), db_updated_callback));
 
-  // Database is done loading, pass it to the callback on the caller's thread.
+  // Database is done loading, pass it to the new_db_callback on the caller's
+  // thread.
   callback_task_runner->PostTask(
-      FROM_HERE, base::Bind(callback, base::Passed(&v4_database)));
+      FROM_HERE, base::Bind(new_db_callback, base::Passed(&v4_database)));
 }
 
 V4Database::V4Database(
     const scoped_refptr<base::SequencedTaskRunner>& db_task_runner,
-    std::unique_ptr<StoreMap> store_map)
-    : db_task_runner_(db_task_runner), store_map_(std::move(store_map)) {
+    std::unique_ptr<StoreMap> store_map,
+    DatabaseUpdatedCallback db_updated_callback)
+    : db_task_runner_(db_task_runner),
+      store_map_(std::move(store_map)),
+      db_updated_callback_(db_updated_callback),
+      pending_store_updates_(0) {
   DCHECK(db_task_runner->RunsTasksOnCurrentThread());
   // TODO(vakh): Implement skeleton
 }
@@ -84,15 +94,69 @@ V4Database::~V4Database() {
   DCHECK(db_task_runner_->RunsTasksOnCurrentThread());
 }
 
+void V4Database::ApplyUpdate(const std::vector<ListUpdateResponse>& responses) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK(!pending_store_updates_);
+
+  // Post the V4Store update task on the task runner but get the callback on the
+  // current thread.
+  const scoped_refptr<base::SingleThreadTaskRunner>& current_task_runner =
+      base::MessageLoop::current()->task_runner();
+  for (const ListUpdateResponse& response : responses) {
+    UpdateListIdentifier identifier(response);
+    StoreMap::const_iterator iter = store_map_->find(identifier);
+    if (iter != store_map_->end()) {
+      const std::unique_ptr<V4Store>& old_store = iter->second;
+      if (old_store->state() != response.new_client_state()) {
+        // A different state implies there are updates to process.
+        pending_store_updates_++;
+        UpdatedStoreReadyCallback store_ready_callback = base::Bind(
+            &V4Database::UpdatedStoreReady, base::Unretained(this), identifier);
+        db_task_runner_->PostTask(
+            FROM_HERE,
+            base::Bind(&V4Store::ApplyUpdate, base::Unretained(old_store.get()),
+                       response, current_task_runner, store_ready_callback));
+      }
+    } else {
+      NOTREACHED() << "Got update for unexpected identifier: " << identifier;
+    }
+  }
+
+  if (!pending_store_updates_) {
+    current_task_runner->PostTask(FROM_HERE, db_updated_callback_);
+  }
+}
+
+void V4Database::UpdatedStoreReady(UpdateListIdentifier identifier,
+                                   std::unique_ptr<V4Store> new_store) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK(pending_store_updates_);
+  (*store_map_)[identifier] = std::move(new_store);
+
+  pending_store_updates_--;
+  if (!pending_store_updates_) {
+    db_updated_callback_.Run();
+  }
+}
+
 bool V4Database::ResetDatabase() {
   DCHECK(db_task_runner_->RunsTasksOnCurrentThread());
   bool reset_success = true;
-  for (const auto& store_id_and_store : *store_map_) {
-    if (!store_id_and_store.second->Reset()) {
+  for (const auto& store_map_iter : *store_map_) {
+    if (!store_map_iter.second->Reset()) {
       reset_success = false;
     }
   }
   return reset_success;
 }
 
+std::unique_ptr<StoreStateMap> V4Database::GetStoreStateMap() {
+  std::unique_ptr<StoreStateMap> store_state_map =
+      base::MakeUnique<StoreStateMap>();

[Scott Hess - ex-Googler, 2016/06/24 23:01:19]

Can this do the right thing as a StoreStateMap without the unique_ptr?  I think
so.  Just a modest preference, if you think it makes it less clear that it does
the right thing, unique_ptr is reasonable.

[vakh (use Gerrit instead), 2016/06/27 19:38:27]

Just trying to not create another map on return. It is possible that the
compiler will be smart and not create a copy but a unique_ptr just makes sure of
that. I don't feel too strongly either way but it is such a simple method that
either implementation is clear.
I'd be happy to change it if you prefer that.

[Scott Hess - ex-Googler, 2016/06/27 22:19:17]

Acknowledged.

I guess I want to trust the language to do RVO, and it will probably do RVO, and
the structure is probably small enough not to matter anyhow, but I think I agree
with you that with a unique_ptr<> it's pretty clear that it's not making a copy,
and as a reader I don't have to think about whether a copy is being made.

+  for (const auto& store_map_iter : *store_map_) {
+    (*store_state_map)[store_map_iter.first] = store_map_iter.second->state();
+  }
+  return store_state_map;
+}
+
 }  // namespace safe_browsing