Add migration code to migrate the obsolete kContentSettingsPatterns pref.

BUG=86751
TEST=content_settings_pref_provider_unittest.cc,
          host_content_settings_map_unittest.cc


Review URL: http://codereview.chromium.org/7190034

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@90552 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 57 insertions, 71 deletions

diff --git a/chrome/browser/content_settings/content_settings_pref_provider.cc b/chrome/browser/content_settings/content_settings_pref_provider.cc
index 8f438ad..027718a 100644
--- a/chrome/browser/content_settings/content_settings_pref_provider.cc
+++ b/chrome/browser/content_settings/content_settings_pref_provider.cc
@@ -395,10 +395,12 @@ void PrefProvider::RegisterUserPrefs(PrefService* prefs) {
       prefs::kContentSettingsVersion,
       ContentSettingsPattern::kContentSettingsPatternVersion,
       PrefService::UNSYNCABLE_PREF);
-  prefs->RegisterDictionaryPref(prefs::kContentSettingsPatterns,
+  prefs->RegisterDictionaryPref(prefs::kContentSettingsPatternPairs,
                                 PrefService::SYNCABLE_PREF);
 
   // Obsolete prefs, for migration:
+  prefs->RegisterDictionaryPref(prefs::kContentSettingsPatterns,
+                                PrefService::UNSYNCABLE_PREF);
   prefs->RegisterListPref(prefs::kPopupWhitelistedHosts,
                           PrefService::UNSYNCABLE_PREF);
   prefs->RegisterDictionaryPref(prefs::kPerHostContentSettings,
@@ -419,7 +421,7 @@ void PrefProvider::Init() {
   // Migrate obsolete preferences.
   MigrateObsoletePerhostPref(prefs);
   MigrateObsoletePopupsPref(prefs);
-  MigrateSinglePatternSettings(prefs);
+  MigrateObsoleteContentSettingsPatternPref(prefs);
 
   // Verify preferences version.
   if (!prefs->HasPrefPath(prefs::kContentSettingsVersion)) {
@@ -441,7 +443,7 @@ void PrefProvider::Init() {
   }
 
   pref_change_registrar_.Init(prefs);
-  pref_change_registrar_.Add(prefs::kContentSettingsPatterns, this);
+  pref_change_registrar_.Add(prefs::kContentSettingsPatternPairs, this);
 
   notification_registrar_.Add(this, NotificationType::PROFILE_DESTROYED,
                               Source<Profile>(profile_));
@@ -515,7 +517,7 @@ void PrefProvider::SetContentSetting(
   updating_preferences_ = true;
   {
     DictionaryPrefUpdate update(profile_ ? profile_->GetPrefs() : NULL,
-                                prefs::kContentSettingsPatterns);
+                                prefs::kContentSettingsPatternPairs);
     DictionaryValue* all_settings_dictionary = NULL;
 
     OriginIdentifierValueMap* map_to_modify = &incognito_value_map_;
@@ -621,7 +623,7 @@ void PrefProvider::ResetToDefaults() {
   if (!is_incognito_) {
     PrefService* prefs = profile_->GetPrefs();
     updating_preferences_ = true;
-    prefs->ClearPref(prefs::kContentSettingsPatterns);
+    prefs->ClearPref(prefs::kContentSettingsPatternPairs);
     updating_preferences_ = false;
   }
 }
@@ -633,7 +635,7 @@ void PrefProvider::ClearAllContentSettingsRules(
   updating_preferences_ = true;
   {  // Begin scope of update.
     DictionaryPrefUpdate update(profile_->GetPrefs(),
-                                prefs::kContentSettingsPatterns);
+                                prefs::kContentSettingsPatternPairs);
 
     DictionaryValue* all_settings_dictionary = NULL;
     OriginIdentifierValueMap* map_to_modify = &incognito_value_map_;
@@ -697,7 +699,7 @@ void PrefProvider::Observe(
       return;
 
     std::string* name = Details<std::string>(details).ptr();
-    if (*name == prefs::kContentSettingsPatterns) {
+    if (*name == prefs::kContentSettingsPatternPairs) {
       ReadExceptions(true);
     } else {
       NOTREACHED() << "Unexpected preference observed";
@@ -731,7 +733,7 @@ void PrefProvider::ReadExceptions(bool overwrite) {
 
   PrefService* prefs = profile_->GetPrefs();
   const DictionaryValue* all_settings_dictionary =
-      prefs->GetDictionary(prefs::kContentSettingsPatterns);
+      prefs->GetDictionary(prefs::kContentSettingsPatternPairs);
 
   if (overwrite)
     value_map_.clear();
@@ -739,7 +741,7 @@ void PrefProvider::ReadExceptions(bool overwrite) {
   updating_preferences_ = true;
   // Careful: The returned value could be NULL if the pref has never been set.
   if (all_settings_dictionary != NULL) {
-    DictionaryPrefUpdate update(prefs, prefs::kContentSettingsPatterns);
+    DictionaryPrefUpdate update(prefs, prefs::kContentSettingsPatternPairs);
     DictionaryValue* mutable_settings;
     scoped_ptr<DictionaryValue> mutable_settings_scope;
 
@@ -968,65 +970,48 @@ void PrefProvider::MigrateObsoletePopupsPref(PrefService* prefs) {
   }
 }
 
-void PrefProvider::MigrateSinglePatternSettings(PrefService* prefs) {
-  const DictionaryValue* all_settings_dictionary =
+void PrefProvider::MigrateObsoleteContentSettingsPatternPref(
+    PrefService* prefs) {
+  if (prefs->HasPrefPath(prefs::kContentSettingsPatterns) &&
+      !is_incognito_) {
+    const DictionaryValue* all_settings_dictionary =
       prefs->GetDictionary(prefs::kContentSettingsPatterns);
-  // The all_settings_dictionary can be |NULL| if the preferences hasn't been
-  // set yet. In incognito mode we must not write to preferences.
-  if (all_settings_dictionary && !is_incognito_) {
-    DictionaryPrefUpdate update(prefs, prefs::kContentSettingsPatterns);
-    DictionaryValue* mutable_settings;
-    mutable_settings = update.Get();
 
-    // Create a list of items to migrate.
-    std::list<std::string> move_items;
-    for (DictionaryValue::key_iterator i(mutable_settings->begin_keys());
-         i != mutable_settings->end_keys();
+    DictionaryPrefUpdate update(prefs, prefs::kContentSettingsPatternPairs);
+    DictionaryValue* exceptions_dictionary;
+    exceptions_dictionary = update.Get();
+    for (DictionaryValue::key_iterator i(all_settings_dictionary->begin_keys());
+         i != all_settings_dictionary->end_keys();
          ++i) {
-      const std::string& pattern_str(*i);
-      std::pair<ContentSettingsPattern, ContentSettingsPattern> pattern_pair =
-          ParsePatternString(pattern_str);
+      const std::string& key(*i);
 
-      // If the pattern_str contains two valid patterns it must not be migrated,
-      // so we skip it.
-      if (pattern_pair.first.IsValid() &&
-          pattern_pair.second.IsValid()) {
-        continue;
-      }
-      if (!pattern_pair.first.IsValid()) {
-        // Skip over corrupted and malformed patterns.
-        LOG(DFATAL) << "Invalid pattern string: " << pattern_str;
+      // Validate pattern string and skip it if it is invalid.
+      std::pair<ContentSettingsPattern, ContentSettingsPattern> pattern_pair =
+          ParsePatternString(key);
+      ContentSettingsPattern primary_pattern = pattern_pair.first;
+      if (!primary_pattern.IsValid()) {
+        LOG(DFATAL) << "Invalid pattern string: " << key;
         continue;
       }
-
-      move_items.push_back(pattern_str);
-    }
-
-    // Migrate the items.
-    for (std::list<std::string>::iterator i = move_items.begin();
-         i != move_items.end();
-         ++i) {
-      const std::string& pattern_str(*i);
-      Value* value;
-      bool found = mutable_settings->RemoveWithoutPathExpansion(*i, &value);
+      // Get old settings.
+      DictionaryValue* dictionary = NULL;
+      bool found = all_settings_dictionary->GetDictionaryWithoutPathExpansion(
+          key, &dictionary);
       DCHECK(found);
 
-      // Migrate the content settings types that used to be applied based on the
-      // top level frame URL only.
+      // Create new dictionary key.
       std::string new_pattern_str = CreatePatternString(
-          ContentSettingsPattern::FromString(pattern_str),
-          ContentSettingsPattern::Wildcard());
-      // Check if there is already a entry for the new pattern string.
-      DictionaryValue* dictionary = NULL;
-      found = mutable_settings->GetDictionaryWithoutPathExpansion(
-          new_pattern_str, &dictionary);
-      if (!found) {
-        mutable_settings->SetWithoutPathExpansion(
-            new_pattern_str, value);
+          primary_pattern, ContentSettingsPattern::Wildcard());
+
+      if (!exceptions_dictionary->HasKey(new_pattern_str)) {
+        exceptions_dictionary->SetWithoutPathExpansion(
+            new_pattern_str, dictionary->DeepCopy());
       } else {
-        NOTREACHED();
+        LOG(DFATAL) << "Existing settings for: " << new_pattern_str;
       }
     }
+
+    prefs->ClearPref(prefs::kContentSettingsPatterns);
   }
 }
 
diff --git a/chrome/browser/content_settings/content_settings_pref_provider.h b/chrome/browser/content_settings/content_settings_pref_provider.h
index 266cc06..628b0e4 100644
--- a/chrome/browser/content_settings/content_settings_pref_provider.h
+++ b/chrome/browser/content_settings/content_settings_pref_provider.h
@@ -146,7 +146,7 @@ class PrefProvider : public ProviderInterface,
   // migrated to the new format).
   void MigrateObsoletePerhostPref(PrefService* prefs);
   void MigrateObsoletePopupsPref(PrefService* prefs);
-  void MigrateSinglePatternSettings(PrefService* prefs);
+  void MigrateObsoleteContentSettingsPatternPref(PrefService* prefs);
 
   void CanonicalizeContentSettingsExceptions(
       DictionaryValue* all_settings_dictionary);
diff --git a/chrome/browser/content_settings/content_settings_pref_provider_unittest.cc b/chrome/browser/content_settings/content_settings_pref_provider_unittest.cc
index 5383df0..7ef710f 100644
--- a/chrome/browser/content_settings/content_settings_pref_provider_unittest.cc
+++ b/chrome/browser/content_settings/content_settings_pref_provider_unittest.cc
@@ -350,20 +350,21 @@ TEST_F(PrefProviderTest, MigrateSinglePatternSettings) {
   all_settings_dictionary->SetWithoutPathExpansion(
       pattern.ToString(), settings_dictionary);
 
+  // Set Obsolete preference.
   prefs->Set(prefs::kContentSettingsPatterns, *all_settings_dictionary);
 
   // Test if single pattern settings are properly migrated.
   content_settings::PrefProvider provider(profile.GetOriginalProfile());
 
+  // Validate migrated preferences
   const DictionaryValue* const_all_settings_dictionary =
-      prefs->GetDictionary(prefs::kContentSettingsPatterns);
+      prefs->GetDictionary(prefs::kContentSettingsPatternPairs);
   EXPECT_EQ(1U, const_all_settings_dictionary->size());
   EXPECT_FALSE(const_all_settings_dictionary->HasKey(pattern.ToString()));
   EXPECT_TRUE(const_all_settings_dictionary->HasKey(
       pattern.ToString() + "," +
       ContentSettingsPattern::Wildcard().ToString()));
 
-  // TODO test provider
   EXPECT_EQ(CONTENT_SETTING_BLOCK,  provider.GetContentSetting(
       GURL("http://www.example.com"),
       GURL("http://www.example.com"),
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 54d0cb2..eb165a2 100644
--- a/chrome/browser/content_settings/host_content_settings_map_unittest.cc
+++ b/chrome/browser/content_settings/host_content_settings_map_unittest.cc
@@ -349,7 +349,7 @@ TEST_F(HostContentSettingsMapTest, ObserveExceptionPref) {
 
   // Make a copy of the default pref value so we can reset it later.
   scoped_ptr<Value> default_value(prefs->FindPreference(
-      prefs::kContentSettingsPatterns)->GetValue()->DeepCopy());
+      prefs::kContentSettingsPatternPairs)->GetValue()->DeepCopy());
 
   ContentSettingsPattern pattern =
        ContentSettingsPattern::FromString("[*.]example.com");
@@ -367,16 +367,16 @@ TEST_F(HostContentSettingsMapTest, ObserveExceptionPref) {
 
   // Make a copy of the pref's new value so we can reset it later.
   scoped_ptr<Value> new_value(prefs->FindPreference(
-      prefs::kContentSettingsPatterns)->GetValue()->DeepCopy());
+      prefs::kContentSettingsPatternPairs)->GetValue()->DeepCopy());
 
   // Clearing the backing pref should also clear the internal cache.
-  prefs->Set(prefs::kContentSettingsPatterns, *default_value);
+  prefs->Set(prefs::kContentSettingsPatternPairs, *default_value);
   EXPECT_EQ(CONTENT_SETTING_ALLOW,
             host_content_settings_map->GetCookieContentSetting(
                 host, host, true));
 
   // Reseting the pref to its previous value should update the cache.
-  prefs->Set(prefs::kContentSettingsPatterns, *new_value);
+  prefs->Set(prefs::kContentSettingsPatternPairs, *new_value);
   EXPECT_EQ(CONTENT_SETTING_BLOCK,
             host_content_settings_map->GetCookieContentSetting(
                 host, host, true));
@@ -717,7 +717,7 @@ TEST_F(HostContentSettingsMapTest, CanonicalizeExceptionsUnicodeOnly) {
 
   // Set utf-8 data.
   {
-    DictionaryPrefUpdate update(prefs, prefs::kContentSettingsPatterns);
+    DictionaryPrefUpdate update(prefs, prefs::kContentSettingsPatternPairs);
     DictionaryValue* all_settings_dictionary = update.Get();
     ASSERT_TRUE(NULL != all_settings_dictionary);
 
@@ -729,7 +729,7 @@ TEST_F(HostContentSettingsMapTest, CanonicalizeExceptionsUnicodeOnly) {
   profile.GetHostContentSettingsMap();
 
   const DictionaryValue* all_settings_dictionary =
-      prefs->GetDictionary(prefs::kContentSettingsPatterns);
+      prefs->GetDictionary(prefs::kContentSettingsPatternPairs);
   DictionaryValue* result = NULL;
   EXPECT_FALSE(all_settings_dictionary->GetDictionaryWithoutPathExpansion(
       "[*.]\xC4\x87ira.com,*", &result));
@@ -749,18 +749,18 @@ TEST_F(HostContentSettingsMapTest, CanonicalizeExceptionsUnicodeAndPunycode) {
 
   scoped_ptr<Value> value(base::JSONReader::Read(
       "{\"[*.]\\xC4\\x87ira.com,*\":{\"per_plugin\":{\"pluginx\":2}}}", false));
-  profile.GetPrefs()->Set(prefs::kContentSettingsPatterns, *value);
+  profile.GetPrefs()->Set(prefs::kContentSettingsPatternPairs, *value);
 
   // Set punycode equivalent, with different setting.
   scoped_ptr<Value> puny_value(base::JSONReader::Read(
       "{\"[*.]xn--ira-ppa.com,*\":{\"per_plugin\":{\"pluginy\":2}}}", false));
-  profile.GetPrefs()->Set(prefs::kContentSettingsPatterns, *puny_value);
+  profile.GetPrefs()->Set(prefs::kContentSettingsPatternPairs, *puny_value);
 
   // Initialize the content map.
   profile.GetHostContentSettingsMap();
 
   const DictionaryValue* content_setting_prefs =
-      profile.GetPrefs()->GetDictionary(prefs::kContentSettingsPatterns);
+      profile.GetPrefs()->GetDictionary(prefs::kContentSettingsPatternPairs);
   std::string prefs_as_json;
   base::JSONWriter::Write(content_setting_prefs, false, &prefs_as_json);
   EXPECT_STREQ("{\"[*.]xn--ira-ppa.com,*\":{\"per_plugin\":{\"pluginy\":2}}}",
@@ -845,7 +845,7 @@ TEST_F(HostContentSettingsMapTest, ResourceIdentifierPrefs) {
   TestingProfile profile;
   scoped_ptr<Value> value(base::JSONReader::Read(
       "{\"[*.]example.com,*\":{\"per_plugin\":{\"someplugin\":2}}}", false));
-  profile.GetPrefs()->Set(prefs::kContentSettingsPatterns, *value);
+  profile.GetPrefs()->Set(prefs::kContentSettingsPatternPairs, *value);
   HostContentSettingsMap* host_content_settings_map =
       profile.GetHostContentSettingsMap();
 
@@ -869,7 +869,7 @@ TEST_F(HostContentSettingsMapTest, ResourceIdentifierPrefs) {
       CONTENT_SETTING_DEFAULT);
 
   const DictionaryValue* content_setting_prefs =
-      profile.GetPrefs()->GetDictionary(prefs::kContentSettingsPatterns);
+      profile.GetPrefs()->GetDictionary(prefs::kContentSettingsPatternPairs);
   std::string prefs_as_json;
   base::JSONWriter::Write(content_setting_prefs, false, &prefs_as_json);
   EXPECT_STREQ("{}", prefs_as_json.c_str());
@@ -882,7 +882,7 @@ TEST_F(HostContentSettingsMapTest, ResourceIdentifierPrefs) {
       CONTENT_SETTING_BLOCK);
 
   content_setting_prefs =
-      profile.GetPrefs()->GetDictionary(prefs::kContentSettingsPatterns);
+      profile.GetPrefs()->GetDictionary(prefs::kContentSettingsPatternPairs);
   base::JSONWriter::Write(content_setting_prefs, false, &prefs_as_json);
   EXPECT_STREQ("{\"[*.]example.com,*\":{\"per_plugin\":{\"otherplugin\":2}}}",
                prefs_as_json.c_str());