PVer4: Test checksum on startup outside the hotpath of DB load

components/safe_browsing_db/v4_local_database_manager_unittest.cc

Index: components/safe_browsing_db/v4_local_database_manager_unittest.cc
diff --git a/components/safe_browsing_db/v4_local_database_manager_unittest.cc b/components/safe_browsing_db/v4_local_database_manager_unittest.cc
index d3e75cb3b46b559a79ea2fead54e8768ee5899d0..8e821ee4ad104ff7bdf097fd4bc9aefe24ce7a3a 100644
--- a/components/safe_browsing_db/v4_local_database_manager_unittest.cc
+++ b/components/safe_browsing_db/v4_local_database_manager_unittest.cc
@@ -17,11 +17,20 @@ namespace safe_browsing {
 
 class FakeV4Database : public V4Database {
  public:
-  FakeV4Database(const scoped_refptr<base::SequencedTaskRunner>& db_task_runner,
-                 std::unique_ptr<StoreMap> store_map,
-                 const StoreAndHashPrefixes& store_and_hash_prefixes)
-      : V4Database(db_task_runner, std::move(store_map)),
-        store_and_hash_prefixes_(store_and_hash_prefixes) {}
+  static void Create(
+      const scoped_refptr<base::SequencedTaskRunner>& db_task_runner,
+      std::unique_ptr<StoreMap> store_map,
+      const StoreAndHashPrefixes& store_and_hash_prefixes,
+      NewDatabaseReadyCallback new_db_callback) {
+    // Mimics V4Database::Create
+    const scoped_refptr<base::SingleThreadTaskRunner>& callback_task_runner =
+        base::MessageLoop::current()->task_runner();
+    db_task_runner->PostTask(
+        FROM_HERE,
+        base::Bind(&FakeV4Database::CreateOnTaskRunner, db_task_runner,
+                   base::Passed(&store_map), store_and_hash_prefixes,
+                   callback_task_runner, new_db_callback));
+  }
 
   void GetStoresMatchingFullHash(
       const FullHash& full_hash,
@@ -31,6 +40,26 @@ class FakeV4Database : public V4Database {
   }
 
  private:
+  static void CreateOnTaskRunner(
+      const scoped_refptr<base::SequencedTaskRunner>& db_task_runner,
+      std::unique_ptr<StoreMap> store_map,
+      const StoreAndHashPrefixes& store_and_hash_prefixes,
+      const scoped_refptr<base::SingleThreadTaskRunner>& callback_task_runner,
+      NewDatabaseReadyCallback new_db_callback) {
+    // Mimics the semantics of V4Database::CreateOnTaskRunner
+    std::unique_ptr<FakeV4Database> fake_v4_database(new FakeV4Database(
+        db_task_runner, std::move(store_map), store_and_hash_prefixes));
+    callback_task_runner->PostTask(
+        FROM_HERE,
+        base::Bind(new_db_callback, base::Passed(&fake_v4_database)));
+  }
+
+  FakeV4Database(const scoped_refptr<base::SequencedTaskRunner>& db_task_runner,
+                 std::unique_ptr<StoreMap> store_map,
+                 const StoreAndHashPrefixes& store_and_hash_prefixes)
+      : V4Database(db_task_runner, std::move(store_map)),
+        store_and_hash_prefixes_(store_and_hash_prefixes) {}
+
   const StoreAndHashPrefixes& store_and_hash_prefixes_;
 };
 
@@ -77,22 +106,32 @@ class V4LocalDatabaseManagerTest : public PlatformTest {
     v4_local_database_manager_->enabled_ = false;
   }
 
+  void ForceEnableLocalDatabaseManager() {
+    v4_local_database_manager_->enabled_ = true;
+  }
+
   const V4LocalDatabaseManager::QueuedChecks& GetQueuedChecks() {
     return v4_local_database_manager_->queued_checks_;
   }
 
   void ReplaceV4Database(const StoreAndHashPrefixes& store_and_hash_prefixes) {
-    v4_local_database_manager_->v4_database_.reset(new FakeV4Database(
-        task_runner_, base::MakeUnique<StoreMap>(), store_and_hash_prefixes));
+    ForceDisableLocalDatabaseManager();

[Nathan Parker, 2016/10/07 23:24:29]

Are the disable/enables necessary? I guess I'm not clear what's stuff is running
here.  Maybe it just needs some comments.

[vakh (use Gerrit instead), 2016/10/10 17:42:33]

Done.

+    ResetV4Database();
+
+    ForceEnableLocalDatabaseManager();
+    NewDatabaseReadyCallback db_ready_callback =
+        base::Bind(&V4LocalDatabaseManager::DatabaseReadyForChecks,
+                   base::Unretained(v4_local_database_manager_.get()));
+    FakeV4Database::Create(task_runner_, base::MakeUnique<StoreMap>(),
+                           store_and_hash_prefixes, db_ready_callback);
   }
 
-  void ResetV4Database() { v4_local_database_manager_->v4_database_.reset(); }
+  void ResetV4Database() {
+    V4Database::Destroy(std::move(v4_local_database_manager_->v4_database_));
+  }
 
   void StartLocalDatabaseManager() {
     v4_local_database_manager_->StartOnIOThread(NULL, V4ProtocolConfig());
-
-    task_runner_->RunPendingTasks();
-    base::RunLoop().RunUntilIdle();
   }
 
   void StopLocalDatabaseManager() {
@@ -103,6 +142,11 @@ class V4LocalDatabaseManagerTest : public PlatformTest {
     base::RunLoop().RunUntilIdle();
   }
 
+  void WaitForLocalDatabaseManagerToBeSetup() {
+    task_runner_->RunPendingTasks();
+    base::RunLoop().RunUntilIdle();
+  }
+
   base::ScopedTempDir base_dir_;
   scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
   content::TestBrowserThreadBundle thread_bundle_;
@@ -131,6 +175,7 @@ TEST_F(V4LocalDatabaseManagerTest, TestCanCheckUrl) {
 
 TEST_F(V4LocalDatabaseManagerTest,
        TestCheckBrowseUrlWithEmptyStoresReturnsNoMatch) {
+  WaitForLocalDatabaseManagerToBeSetup();
   // Both the stores are empty right now so CheckBrowseUrl should return true.
   EXPECT_TRUE(v4_local_database_manager_->CheckBrowseUrl(
       GURL("http://example.com/a/"), nullptr));
@@ -193,17 +238,11 @@ TEST_F(V4LocalDatabaseManagerTest, TestChecksAreQueued) {
   TestClient client(SB_THREAT_TYPE_SAFE, url);
   EXPECT_TRUE(GetQueuedChecks().empty());
   v4_local_database_manager_->CheckBrowseUrl(url, &client);
-  // The database is available so the check shouldn't get queued.
-  EXPECT_TRUE(GetQueuedChecks().empty());
-
-  ResetV4Database();
-  v4_local_database_manager_->CheckBrowseUrl(url, &client);
   // The database is unavailable so the check should get queued.
   EXPECT_EQ(1ul, GetQueuedChecks().size());
 
-  // The following function calls StartOnIOThread which should load the
-  // database from disk and cause the queued check to be performed.
-  StartLocalDatabaseManager();
+  // The following function waits for the DB to load.
+  WaitForLocalDatabaseManagerToBeSetup();
   EXPECT_TRUE(GetQueuedChecks().empty());
 
   ResetV4Database();