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..b94d5e7677944e5f5bfed2308c7330cc15805c2d 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.

[Scott Hess - ex-Googler, 2016/06/21 22:23:25]

Did this comment float away from the code it refers to?

[vakh (use Gerrit instead), 2016/06/23 06:23:50]

This is a comment about what this method does, so I've kept it at the top of the
method definition. Would it be better to have it just before the method
definition?

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

I'd put it on the method itself, since inline like this is looks like it's
telling you what the next few lines do.

The comment out in the header file pretty much covers the same ground, so
another option is to just drop it entirely.  That way you don't have to keep the
code and two comments in sync.

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

Done.

+  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 (IO thread).

[Scott Hess - ex-Googler, 2016/06/21 22:23:25]

IO thread is implied two lines up.  I think "current thread" is more correct,
because it will be right even if this moves to a different thread.

[vakh (use Gerrit instead), 2016/06/23 06:23:50]

Done.

+  const scoped_refptr<base::SingleThreadTaskRunner>& current_task_runner =
+      base::MessageLoop::current()->task_runner();
+  for (const auto& 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>();
+  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