Migrate old settings for ContentSettingTypes with wildcard as secondary_pattern

chrome/browser/content_settings/host_content_settings_map_unittest.cc

Index: chrome/browser/content_settings/host_content_settings_map_unittest.cc
diff --git a/chrome/browser/content_settings/host_content_settings_map_unittest.cc b/chrome/browser/content_settings/host_content_settings_map_unittest.cc
index a530604fa391bb10f4a810f8d0d1ca8218e66834..ca8c60066f105aafd47a49972df2773da7f6510f 100644
--- a/chrome/browser/content_settings/host_content_settings_map_unittest.cc
+++ b/chrome/browser/content_settings/host_content_settings_map_unittest.cc
@@ -1141,3 +1141,61 @@ TEST_F(HostContentSettingsMapTest, GuestProfileMigration) {
           GetPrefName(CONTENT_SETTINGS_TYPE_IMAGES));
   EXPECT_TRUE(all_settings_dictionary->empty());
 }
+
+TEST_F(HostContentSettingsMapTest, MigrateOldSettings) {
+  TestingProfile profile;
+  HostContentSettingsMap* host_content_settings_map =
+      HostContentSettingsMapFactory::GetForProfile(&profile);
+
+  // Set old formatted settings.
+  GURL host("http://example.com/");
+  ContentSettingsPattern pattern =
+      ContentSettingsPattern::FromURLNoWildcard(host);
+  EXPECT_EQ(
+      CONTENT_SETTING_ASK,
+      host_content_settings_map->GetContentSetting(
+          host, host, CONTENT_SETTINGS_TYPE_NOTIFICATIONS, std::string()));
+  host_content_settings_map->SetContentSettingDefaultScope(
+      host, GURL(), CONTENT_SETTINGS_TYPE_NOTIFICATIONS, std::string(),
+      CONTENT_SETTING_ALLOW);
+  host_content_settings_map->SetContentSetting(
+      pattern, pattern, CONTENT_SETTINGS_TYPE_NOTIFICATIONS, std::string(),
+      CONTENT_SETTING_BLOCK);
+  // Because of the old formatted setting entry which has two same patterns,
+  // SetContentSetting() to (host, GURL()) will be ignored.
+  host_content_settings_map->SetContentSettingDefaultScope(
+      host, GURL(), CONTENT_SETTINGS_TYPE_NOTIFICATIONS, std::string(),
+      CONTENT_SETTING_ASK);
+  EXPECT_EQ(
+      CONTENT_SETTING_BLOCK,
+      host_content_settings_map->GetContentSetting(
+          host, host, CONTENT_SETTINGS_TYPE_NOTIFICATIONS, std::string()));

[raymes, 2016/03/07 01:47:55]

Hey Liu, I think maybe we should just test the case where we have old-format
settings and then we call migrate and then check that they are all in the new
format and that getting/setting works after migration. This is because it is the
codepath we expect to exercise in practice (as opposed to having a mixture of
old/new before migration).

+
+  ContentSettingsForOneType host_settings;
+  host_content_settings_map->GetSettingsForOneType(
+      CONTENT_SETTINGS_TYPE_NOTIFICATIONS, std::string(), &host_settings);
+  // |host_settings| contains the default setting and two custom settings.
+  EXPECT_EQ(3U, host_settings.size());

[raymes, 2016/03/07 01:47:55]

I'm a bit confused about why there are 3 here? Shouldn't calling
SetContentSettingDefaultScope twice change the same entry?

[lshang, 2016/03/23 13:58:36]

Yeah, SetContentSettingDefaultScope() twice change the <host, GURL()> entry, and
the other two are <host, host> entry and <*, *> entry.

+
+  host_content_settings_map->MigrateOldSettings();
+
+  host_content_settings_map->GetSettingsForOneType(
+      CONTENT_SETTINGS_TYPE_NOTIFICATIONS, std::string(), &host_settings);
+
+  // MigrateOldSettings() removed setting entry with the two same patterns.
+  EXPECT_EQ(2U, host_settings.size());
+
+  EXPECT_EQ(
+      CONTENT_SETTING_BLOCK,
+      host_content_settings_map->GetContentSetting(
+          host, host, CONTENT_SETTINGS_TYPE_NOTIFICATIONS, std::string()));
+
+  // After migrating old settings, changes to the setting works.
+  host_content_settings_map->SetContentSettingDefaultScope(
+      host, GURL(), CONTENT_SETTINGS_TYPE_NOTIFICATIONS, std::string(),
+      CONTENT_SETTING_ASK);
+  EXPECT_EQ(
+      CONTENT_SETTING_ASK,
+      host_content_settings_map->GetContentSetting(
+          host, host, CONTENT_SETTINGS_TYPE_NOTIFICATIONS, std::string()));
+}