Fetch incremental updates. Store new state in V4Store.

components/safe_browsing_db/v4_database_unittest.cc

Index: components/safe_browsing_db/v4_database_unittest.cc
diff --git a/components/safe_browsing_db/v4_database_unittest.cc b/components/safe_browsing_db/v4_database_unittest.cc
index 83ea5f41224802a4e7b4991ec6283ef5f2ab21d5..9c57f9ed5a037da075a9fe1ef40d20fecac203c5 100644
--- a/components/safe_browsing_db/v4_database_unittest.cc
+++ b/components/safe_browsing_db/v4_database_unittest.cc
@@ -73,7 +73,7 @@ class SafeBrowsingV4DatabaseTest : public PlatformTest {
     update_list_identifier.platform_type = WINDOWS_PLATFORM;
     update_list_identifier.threat_entry_type = URL;
     update_list_identifier.threat_type = MALWARE_THREAT;
-    list_info_map_[update_list_identifier] = "win_url_malware";
+    store_file_name_map_[update_list_identifier] = "win_url_malware";
     expected_identifiers_.push_back(update_list_identifier);
     expected_store_paths_.push_back(
         database_dirname_.AppendASCII("win_url_malware.fake"));
@@ -81,12 +81,13 @@ class SafeBrowsingV4DatabaseTest : public PlatformTest {
     update_list_identifier.platform_type = LINUX_PLATFORM;
     update_list_identifier.threat_entry_type = URL;
     update_list_identifier.threat_type = MALWARE_THREAT;
-    list_info_map_[update_list_identifier] = "linux_url_malware";
+    store_file_name_map_[update_list_identifier] = "linux_url_malware";
     expected_identifiers_.push_back(update_list_identifier);
     expected_store_paths_.push_back(
         database_dirname_.AppendASCII("linux_url_malware.fake"));
   }
 
+  void DatabaseUpdated() {}
   void NewDatabaseReadyWithExpectedStorePathsAndIds(
       std::vector<base::FilePath> expected_store_paths,
       std::vector<UpdateListIdentifier> expected_identifiers,
@@ -112,6 +113,21 @@ class SafeBrowsingV4DatabaseTest : public PlatformTest {
 
     EXPECT_FALSE(created_and_called_back_);
     created_and_called_back_ = true;
+
+    v4_database_ = std::move(v4_database);
+  }
+
+  void PopulateFakeUpdateResponse(StoreStateMap store_state_map,
+                                  std::vector<ListUpdateResponse>* responses) {
+    for (const auto& store_state_iter : store_state_map) {
+      UpdateListIdentifier identifier = store_state_iter.first;
+      ListUpdateResponse list_update_response;
+      list_update_response.set_platform_type(identifier.platform_type);
+      list_update_response.set_threat_entry_type(identifier.threat_entry_type);
+      list_update_response.set_threat_type(identifier.threat_type);
+      list_update_response.set_new_client_state(store_state_iter.second);
+      responses->push_back(list_update_response);
+    }
   }
 
   scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
@@ -121,30 +137,30 @@ class SafeBrowsingV4DatabaseTest : public PlatformTest {
   content::TestBrowserThreadBundle thread_bundle_;
   bool created_but_not_called_back_;
   bool created_and_called_back_;
-  ListInfoMap list_info_map_;
+  StoreFileNameMap store_file_name_map_;
   std::vector<UpdateListIdentifier> expected_identifiers_;
   std::vector<base::FilePath> expected_store_paths_;
 };
 
-// Test to set up the database with no stores.
+// Test to set up the database with no stores, which returns false right away.
 TEST_F(SafeBrowsingV4DatabaseTest, TestSetupDatabaseWithNoStores) {
+  DatabaseUpdatedCallback callback_db_updated = base::Bind(
+      &SafeBrowsingV4DatabaseTest::DatabaseUpdated, base::Unretained(this));
   NewDatabaseReadyCallback callback_db_ready = base::Bind(
       &SafeBrowsingV4DatabaseTest::NewDatabaseReadyWithExpectedStorePathsAndIds,
       base::Unretained(this), expected_store_paths_, expected_identifiers_,
       true);
-  V4Database::Create(task_runner_, database_dirname_, list_info_map_,
-                     callback_db_ready);
-  created_but_not_called_back_ = true;
-  task_runner_->RunPendingTasks();
-
-  base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(true, created_and_called_back_);
+  EXPECT_FALSE(V4Database::Create(task_runner_, database_dirname_,
+                                  store_file_name_map_, callback_db_updated,
+                                  callback_db_ready));
 }
 
 // Test to set up the database with fake stores.
 TEST_F(SafeBrowsingV4DatabaseTest, TestSetupDatabaseWithFakeStores) {
   SetupInfoMapAndExpectedState();
 
+  DatabaseUpdatedCallback callback_db_updated = base::Bind(
+      &SafeBrowsingV4DatabaseTest::DatabaseUpdated, base::Unretained(this));
   NewDatabaseReadyCallback callback_db_ready = base::Bind(
       &SafeBrowsingV4DatabaseTest::NewDatabaseReadyWithExpectedStorePathsAndIds,
       base::Unretained(this), expected_store_paths_, expected_identifiers_,
@@ -153,8 +169,8 @@ TEST_F(SafeBrowsingV4DatabaseTest, TestSetupDatabaseWithFakeStores) {
   FakeV4StoreFactory* factory = new FakeV4StoreFactory(false);
   ANNOTATE_LEAKING_OBJECT_PTR(factory);
   V4Database::RegisterStoreFactoryForTest(factory);
-  V4Database::Create(task_runner_, database_dirname_, list_info_map_,
-                     callback_db_ready);
+  V4Database::Create(task_runner_, database_dirname_, store_file_name_map_,
+                     callback_db_updated, callback_db_ready);
   created_but_not_called_back_ = true;
   task_runner_->RunPendingTasks();
 
@@ -166,6 +182,8 @@ TEST_F(SafeBrowsingV4DatabaseTest, TestSetupDatabaseWithFakeStores) {
 TEST_F(SafeBrowsingV4DatabaseTest, TestSetupDatabaseWithFakeStoresFailsReset) {
   SetupInfoMapAndExpectedState();
 
+  DatabaseUpdatedCallback callback_db_updated = base::Bind(
+      &SafeBrowsingV4DatabaseTest::DatabaseUpdated, base::Unretained(this));
   NewDatabaseReadyCallback callback_db_ready = base::Bind(
       &SafeBrowsingV4DatabaseTest::NewDatabaseReadyWithExpectedStorePathsAndIds,
       base::Unretained(this), expected_store_paths_, expected_identifiers_,
@@ -174,8 +192,8 @@ TEST_F(SafeBrowsingV4DatabaseTest, TestSetupDatabaseWithFakeStoresFailsReset) {
   FakeV4StoreFactory* factory = new FakeV4StoreFactory(true);
   ANNOTATE_LEAKING_OBJECT_PTR(factory);
   V4Database::RegisterStoreFactoryForTest(factory);
-  V4Database::Create(task_runner_, database_dirname_, list_info_map_,
-                     callback_db_ready);
+  V4Database::Create(task_runner_, database_dirname_, store_file_name_map_,
+                     callback_db_updated, callback_db_ready);
   created_but_not_called_back_ = true;
   task_runner_->RunPendingTasks();
 
@@ -183,4 +201,177 @@ TEST_F(SafeBrowsingV4DatabaseTest, TestSetupDatabaseWithFakeStoresFailsReset) {
   EXPECT_EQ(true, created_and_called_back_);
 }
 
+// Test to check database updates as expected.
+TEST_F(SafeBrowsingV4DatabaseTest, TestApplyUpdateWithNewStates) {
+  SetupInfoMapAndExpectedState();
+
+  DatabaseUpdatedCallback callback_db_updated = base::Bind(
+      &SafeBrowsingV4DatabaseTest::DatabaseUpdated, base::Unretained(this));
+  NewDatabaseReadyCallback callback_db_ready = base::Bind(
+      &SafeBrowsingV4DatabaseTest::NewDatabaseReadyWithExpectedStorePathsAndIds,
+      base::Unretained(this), expected_store_paths_, expected_identifiers_,
+      true);
+
+  FakeV4StoreFactory* factory = new FakeV4StoreFactory(false);

[Nathan Parker, 2016/06/15 21:24:17]

Could this go in SetUp(), since you probably don't want to use real stores
anywhere in this file?

[vakh (use Gerrit instead), 2016/06/16 01:07:35]

Done.

+  ANNOTATE_LEAKING_OBJECT_PTR(factory);
+  V4Database::RegisterStoreFactoryForTest(factory);
+  V4Database::Create(task_runner_, database_dirname_, store_file_name_map_,
+                     callback_db_updated, callback_db_ready);
+  created_but_not_called_back_ = true;
+  task_runner_->RunPendingTasks();
+  base::RunLoop().RunUntilIdle();

[Nathan Parker, 2016/06/15 21:24:17]

A chunk of this code is repeated in tests below.  How about putting it in a
function?  Otherwise it's hard to tell if it's the same, and harder to change
reliably.

[vakh (use Gerrit instead), 2016/06/16 01:07:35]

Done.

+
+  // The database has now been created. Time to try to update it.
+  EXPECT_TRUE(v4_database_);
+  const StoreMap* db_stores = v4_database_->store_map_.get();
+  EXPECT_EQ(expected_store_paths_.size(), db_stores->size());
+  StoreStateMap expected_store_state_map;
+  base::hash_map<UpdateListIdentifier, V4Store*> old_stores_map;
+  for (const auto& store_iter : *db_stores) {
+    V4Store* store = store_iter.second.get();
+    expected_store_state_map[store_iter.first] = store->state() + "_fake";
+    old_stores_map[store_iter.first] = store;
+  }
+
+  std::vector<ListUpdateResponse> update_response;
+  PopulateFakeUpdateResponse(expected_store_state_map, &update_response);
+  v4_database_->ApplyUpdate(update_response);
+
+  task_runner_->RunPendingTasks();
+  base::RunLoop().RunUntilIdle();
+
+  const StoreMap* new_store_map = v4_database_->store_map_.get();
+  const StoreStateMap* new_store_state_map = v4_database_->store_state_map();
+  EXPECT_EQ(expected_store_state_map.size(), new_store_map->size());
+  EXPECT_EQ(expected_store_state_map.size(), new_store_state_map->size());
+  for (const auto& expected_iter : expected_store_state_map) {
+    const UpdateListIdentifier& identifier = expected_iter.first;
+    const std::string& state = expected_iter.second;
+    EXPECT_EQ(1ul, new_store_map->count(identifier));
+    EXPECT_EQ(1ul, new_store_state_map->count(identifier));
+
+    // Verify the expected state in the store map and the state map.
+    EXPECT_EQ(state, new_store_map->at(identifier)->state());
+    EXPECT_EQ(state, new_store_state_map->at(identifier));
+
+    // Verify that a new store was created.

[Nathan Parker, 2016/06/15 21:24:17]

Is new one created by the update? I thought the list was set at db-create-time.

[vakh (use Gerrit instead), 2016/06/16 01:07:35]

A new store is created but since the map holds unique_ptrs, as soon as we point
the DB to the new store, the old one becomes unonwned and marked for deletion.
This check just ensures that the address of the new store is different from that
of the old store.

+    EXPECT_NE(old_stores_map[identifier], new_store_map->at(identifier).get());
+  }
+}
+
+// Test to ensure no state updates leads to no store updates.
+TEST_F(SafeBrowsingV4DatabaseTest, TestApplyUpdateWithNoNewState) {
+  SetupInfoMapAndExpectedState();
+
+  DatabaseUpdatedCallback callback_db_updated = base::Bind(
+      &SafeBrowsingV4DatabaseTest::DatabaseUpdated, base::Unretained(this));
+  NewDatabaseReadyCallback callback_db_ready = base::Bind(
+      &SafeBrowsingV4DatabaseTest::NewDatabaseReadyWithExpectedStorePathsAndIds,
+      base::Unretained(this), expected_store_paths_, expected_identifiers_,
+      true);
+
+  FakeV4StoreFactory* factory = new FakeV4StoreFactory(false);
+  ANNOTATE_LEAKING_OBJECT_PTR(factory);
+  V4Database::RegisterStoreFactoryForTest(factory);
+  V4Database::Create(task_runner_, database_dirname_, store_file_name_map_,
+                     callback_db_updated, callback_db_ready);
+  created_but_not_called_back_ = true;
+  task_runner_->RunPendingTasks();
+  base::RunLoop().RunUntilIdle();
+
+  // The database has now been created. Time to try to update it.
+  EXPECT_TRUE(v4_database_);
+  const StoreMap* db_stores = v4_database_->store_map_.get();
+  EXPECT_EQ(expected_store_paths_.size(), db_stores->size());
+  StoreStateMap expected_store_state_map;
+  base::hash_map<UpdateListIdentifier, V4Store*> old_stores_map;
+  for (const auto& store_iter : *db_stores) {
+    V4Store* store = store_iter.second.get();
+    expected_store_state_map[store_iter.first] = store->state();
+    old_stores_map[store_iter.first] = store;
+  }
+
+  std::vector<ListUpdateResponse> update_response;
+  PopulateFakeUpdateResponse(expected_store_state_map, &update_response);
+  v4_database_->ApplyUpdate(update_response);
+
+  task_runner_->RunPendingTasks();
+  base::RunLoop().RunUntilIdle();
+
+  const StoreMap* new_store_map = v4_database_->store_map_.get();
+  const StoreStateMap* new_store_state_map = v4_database_->store_state_map();
+  EXPECT_EQ(expected_store_state_map.size(), new_store_map->size());
+  EXPECT_EQ(expected_store_state_map.size(), new_store_state_map->size());
+  for (const auto& expected_iter : expected_store_state_map) {
+    const UpdateListIdentifier& identifier = expected_iter.first;
+    const std::string& state = expected_iter.second;
+    EXPECT_EQ(1ul, new_store_map->count(identifier));
+    EXPECT_EQ(1ul, new_store_state_map->count(identifier));
+
+    // Verify the expected state in the store map and the state map.
+    EXPECT_EQ(state, new_store_map->at(identifier)->state());
+    EXPECT_EQ(state, new_store_state_map->at(identifier));
+
+    // Verify that NO new store was created.
+    EXPECT_EQ(old_stores_map[identifier], new_store_map->at(identifier).get());
+  }
+}
+
+// Test to ensure no updates leads to no store updates.
+TEST_F(SafeBrowsingV4DatabaseTest, TestApplyUpdateWithEmptyUpdate) {
+  SetupInfoMapAndExpectedState();
+
+  DatabaseUpdatedCallback callback_db_updated = base::Bind(
+      &SafeBrowsingV4DatabaseTest::DatabaseUpdated, base::Unretained(this));
+  NewDatabaseReadyCallback callback_db_ready = base::Bind(
+      &SafeBrowsingV4DatabaseTest::NewDatabaseReadyWithExpectedStorePathsAndIds,
+      base::Unretained(this), expected_store_paths_, expected_identifiers_,
+      true);
+
+  FakeV4StoreFactory* factory = new FakeV4StoreFactory(false);
+  ANNOTATE_LEAKING_OBJECT_PTR(factory);
+  V4Database::RegisterStoreFactoryForTest(factory);
+  V4Database::Create(task_runner_, database_dirname_, store_file_name_map_,
+                     callback_db_updated, callback_db_ready);
+  created_but_not_called_back_ = true;
+  task_runner_->RunPendingTasks();
+  base::RunLoop().RunUntilIdle();
+
+  // The database has now been created. Time to try to update it.
+  EXPECT_TRUE(v4_database_);
+  const StoreMap* db_stores = v4_database_->store_map_.get();
+  EXPECT_EQ(expected_store_paths_.size(), db_stores->size());
+  StoreStateMap expected_store_state_map;
+  base::hash_map<UpdateListIdentifier, V4Store*> old_stores_map;
+  for (const auto& store_iter : *db_stores) {
+    V4Store* store = store_iter.second.get();
+    expected_store_state_map[store_iter.first] = store->state();
+    old_stores_map[store_iter.first] = store;
+  }
+
+  std::vector<ListUpdateResponse> update_response;
+  v4_database_->ApplyUpdate(update_response);
+
+  task_runner_->RunPendingTasks();
+  base::RunLoop().RunUntilIdle();
+
+  const StoreMap* new_store_map = v4_database_->store_map_.get();
+  const StoreStateMap* new_store_state_map = v4_database_->store_state_map();
+  EXPECT_EQ(expected_store_state_map.size(), new_store_map->size());
+  EXPECT_EQ(expected_store_state_map.size(), new_store_state_map->size());
+  for (const auto& expected_iter : expected_store_state_map) {
+    const UpdateListIdentifier& identifier = expected_iter.first;
+    const std::string& state = expected_iter.second;
+    EXPECT_EQ(1ul, new_store_map->count(identifier));
+    EXPECT_EQ(1ul, new_store_state_map->count(identifier));
+
+    // Verify the expected state in the store map and the state map.
+    EXPECT_EQ(state, new_store_map->at(identifier)->state());
+    EXPECT_EQ(state, new_store_state_map->at(identifier));
+
+    // Verify that NO new store was created.
+    EXPECT_EQ(old_stores_map[identifier], new_store_map->at(identifier).get());
+  }
+}
+
 }  // namespace safe_browsing