Initialize and reset V4LocalDBManager. Instantiate V4Stores.

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 328bedcaf652852b7ae5b41836c4567cb66f11e4..ccf05e6b7b7a2e8875480174a98be47a3cee5c27 100644
--- a/components/safe_browsing_db/v4_database.cc
+++ b/components/safe_browsing_db/v4_database.cc
@@ -2,41 +2,101 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <memory>
+
+#include "base/callback.h"
 #include "components/safe_browsing_db/v4_database.h"
+#include "content/public/browser/browser_thread.h"
+
+using content::BrowserThread;
 
 namespace safe_browsing {
 
+namespace {
+
+V4Store* CreateStore(
+    const scoped_refptr<base::SequencedTaskRunner>& task_runner,
+    const base::FilePath& store_path) {
+  return new V4Store(task_runner, store_path);
+}
+
+}  // namespace
+
 // static
 V4DatabaseFactory* V4Database::factory_ = NULL;
 
 // static
-// Factory method, should be called on the Safe Browsing sequenced task runner,
-// which is also passed to the function as |db_task_runner|.
-V4Database* V4Database::Create(
+// Factory method, called on the task runner, which is also passed to the
+// function as |db_task_runner|.
+void V4Database::Create(
     const scoped_refptr<base::SequencedTaskRunner>& db_task_runner,
     const base::FilePath& base_path,
-    ListInfoMap list_info_map) {
+    ListInfoMap list_info_map,
+    NewDatabaseReadyCallback callback) {
   DCHECK(db_task_runner->RunsTasksOnCurrentThread());
+  db_task_runner->PostTask(
+      FROM_HERE, base::Bind(&V4Database::CreateOnTaskRunner, db_task_runner,
+                            base_path, list_info_map, callback));
+}
+
+// static
+// Factory method, called on the task runner, which is also passed to the
+// function as |db_task_runner|.
+void V4Database::CreateOnTaskRunner(
+    const scoped_refptr<base::SequencedTaskRunner>& db_task_runner,
+    const base::FilePath& base_path,
+    ListInfoMap list_info_map,
+    NewDatabaseReadyCallback callback) {
+  DCHECK(db_task_runner->RunsTasksOnCurrentThread());
+  DCHECK(!base_path.empty());
+
   if (!factory_) {
     StoreMap store_map;
-    // TODO(vakh): Populate the store_map using list_suffix_map.
-    return new V4Database(db_task_runner, std::move(store_map));
+
+    for (const auto& list_info : list_info_map) {
+      UpdateListIdentifier update_list_identifier = list_info.first;
+      const base::FilePath::CharType suffix = list_info.second;
+
+      const base::FilePath store_path =
+          base::FilePath(base_path.value() + suffix);
+      (*store_map)[update_list_identifier].reset(
+          CreateStore(db_task_runner, store_path));
+    }
+
+    std::unique_ptr<V4Database> v4_database(
+        new V4Database(db_task_runner, std::move(store_map)));
+    BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                            base::Bind(callback, base::Passed(&v4_database)));
   } else {
-    return factory_->CreateV4Database(db_task_runner, base_path, list_info_map);
+    std::unique_ptr<V4Database> v4_database(
+        factory_->CreateV4Database(db_task_runner, base_path, list_info_map));
+
+    // For tests, schedule the callback on the same thread (task runner) instead
+    // of the IO thread.
+    db_task_runner->PostTask(FROM_HERE,
+                             base::Bind(callback, base::Passed(&v4_database)));

[Scott Hess - ex-Googler, 2016/05/12 23:18:00]

This seems odd to me.  In some cases, tests setup a fake task runner where
everything actually runs on the same thread, in which case this works.  But if
the test is actually setting up a real thread for db_task_runner, then this is
posting the callback to that thread, which might differ from the IO thread in
that case.  If the test is directly testing this code, that is probably alright,
but if the test is sitting above a client of this code, then it could easily be
breaking threading assumptions made by that client.

WRT my suggestion about posting back to the task-runner that the original
Create() came from, I think this is all fine (the test case and the production
case will get the callback in the right logical place).  If you really don't
want to go that way, though, I think you need the Create() to accept a
callback_task_runner, so that where the callback runs is under control of the
client.

[Of course, if this ends up with a callback_task_runner parameter, then it's
going to seem like the PostTask() and v4_database declaration should be outside
the if() block.]

[vakh (use Gerrit instead), 2016/05/13 00:07:32]

Done.

   }
 }
 
 V4Database::V4Database(
     const scoped_refptr<base::SequencedTaskRunner>& db_task_runner,
-    StoreMap store_map) {
+    StoreMap store_map)
+    : db_task_runner_(db_task_runner), store_map_(std::move(store_map)) {
+  DCHECK(db_task_runner->RunsTasksOnCurrentThread());
   // TODO(vakh): Implement skeleton
 }
 
 V4Database::~V4Database() {}
 
 bool V4Database::ResetDatabase() {
-  // TODO(vakh): Delete the stores. Delete the backing files.
-  return true;
+  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()) {
+      reset_success = false;
+    }
+  }
+  return reset_success;
 }
 
 }  // namespace safe_browsing