Migrate geolocation settings to host content settings map and remove the geolocation settings map.

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


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

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

4 files changed, 355 insertions, 57 deletions

diff --git a/chrome/browser/content_settings/content_settings_pref_provider.cc b/chrome/browser/content_settings/content_settings_pref_provider.cc
index d088a88..e4faf955 100644
--- a/chrome/browser/content_settings/content_settings_pref_provider.cc
+++ b/chrome/browser/content_settings/content_settings_pref_provider.cc
@@ -42,7 +42,7 @@ const char* kResourceTypeNames[] = {
   NULL,
   "per_plugin",
   NULL,
-  NULL,  // Not used for Geolocation
+  NULL,
   NULL,  // Not used for Notifications
 };
 COMPILE_ASSERT(arraysize(kResourceTypeNames) == CONTENT_SETTINGS_NUM_TYPES,
@@ -68,9 +68,9 @@ const char* kTypeNames[] = {
   "javascript",
   "plugins",
   "popups",
+  "geolocation",
   // TODO(markusheintz): Refactoring in progress. Content settings exceptions
-  // for notifications and geolocation will be added next.
-  "geolocation",  // Only used for default Geolocation settings
+  // for notifications added next.
   "notifications",  // Only used for default Notifications settings.
 };
 COMPILE_ASSERT(arraysize(kTypeNames) == CONTENT_SETTINGS_NUM_TYPES,
@@ -361,6 +361,8 @@ void PrefProvider::RegisterUserPrefs(PrefService* prefs) {
                                 PrefService::SYNCABLE_PREF);
 
   // Obsolete prefs, for migration:
+  prefs->RegisterDictionaryPref(prefs::kGeolocationContentSettings,
+                                PrefService::SYNCABLE_PREF);
   prefs->RegisterDictionaryPref(prefs::kContentSettingsPatterns,
                                 PrefService::SYNCABLE_PREF);
   prefs->RegisterListPref(prefs::kPopupWhitelistedHosts,
@@ -380,6 +382,7 @@ PrefProvider::PrefProvider(PrefService* prefs,
     MigrateObsoletePerhostPref();
     MigrateObsoletePopupsPref();
     MigrateObsoleteContentSettingsPatternPref();
+    MigrateObsoleteGeolocationPref();
   }
 
   // Verify preferences version.
@@ -403,6 +406,7 @@ PrefProvider::PrefProvider(PrefService* prefs,
   pref_change_registrar_.Init(prefs_);
   pref_change_registrar_.Add(prefs::kContentSettingsPatterns, this);
   pref_change_registrar_.Add(prefs::kContentSettingsPatternPairs, this);
+  pref_change_registrar_.Add(prefs::kGeolocationContentSettings, this);
 }
 
 ContentSetting PrefProvider::GetContentSetting(
@@ -469,7 +473,7 @@ void PrefProvider::SetContentSetting(
     ContentSetting setting) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(prefs_);
-  DCHECK(kTypeNames[content_type] != NULL);  // Don't call this for Geolocation.
+  DCHECK(kTypeNames[content_type] != NULL);
 
   // Update in memory value map.
   OriginIdentifierValueMap* map_to_modify = &incognito_value_map_;
@@ -553,18 +557,20 @@ void PrefProvider::Observe(
     if (updating_preferences_)
       return;
 
+    AutoReset<bool> auto_reset(&updating_preferences_, true);
     std::string* name = Details<std::string>(details).ptr();
     if (*name == prefs::kContentSettingsPatternPairs) {
-      SyncObsoletePref();
-      ReadContentSettingsFromPref(true);
+      SyncObsoletePatternPref();
+      SyncObsoleteGeolocationPref();
     } else if (*name == prefs::kContentSettingsPatterns) {
-      AutoReset<bool> auto_reset(&updating_preferences_, true);
       MigrateObsoleteContentSettingsPatternPref();
-      ReadContentSettingsFromPref(true);
+    } else if (*name == prefs::kGeolocationContentSettings) {
+      MigrateObsoleteGeolocationPref();
     } else {
       NOTREACHED() << "Unexpected preference observed";
       return;
     }
+    ReadContentSettingsFromPref(true);
 
     NotifyObservers(ContentSettingsPattern(),
                     ContentSettingsPattern(),
@@ -594,11 +600,17 @@ void PrefProvider::UpdatePref(
                          content_type,
                          resource_identifier,
                          setting);
-  UpdatePatternsPref(primary_pattern,
-                     secondary_pattern,
-                     content_type,
-                     resource_identifier,
-                     setting);
+  if (content_type != CONTENT_SETTINGS_TYPE_GEOLOCATION &&
+      content_type != CONTENT_SETTINGS_TYPE_NOTIFICATIONS) {
+    UpdateObsoletePatternsPref(primary_pattern,
+                               secondary_pattern,
+                               content_type,
+                               resource_identifier,
+                               setting);
+  }
+  if (content_type == CONTENT_SETTINGS_TYPE_GEOLOCATION) {
+    UpdateObsoleteGeolocationPref(primary_pattern, secondary_pattern, setting);
+  }
 }
 
 void PrefProvider::ReadContentSettingsFromPref(bool overwrite) {
@@ -693,7 +705,7 @@ void PrefProvider::ReadContentSettingsFromPref(bool overwrite) {
   }
 }
 
-void PrefProvider::UpdatePatternsPref(
+void PrefProvider::UpdateObsoletePatternsPref(
       const ContentSettingsPattern& primary_pattern,
       const ContentSettingsPattern& secondary_pattern,
       ContentSettingsType content_type,
@@ -826,6 +838,43 @@ void PrefProvider::UpdatePatternPairsPref(
   }
 }
 
+void PrefProvider::UpdateObsoleteGeolocationPref(
+    const ContentSettingsPattern& primary_pattern,
+    const ContentSettingsPattern& secondary_pattern,
+    ContentSetting setting) {
+  if (!prefs_)
+    return;
+
+  const GURL requesting_origin(primary_pattern.ToString());
+  const GURL embedding_origin(secondary_pattern.ToString());
+  DCHECK(requesting_origin.is_valid() && embedding_origin.is_valid());
+
+  DictionaryPrefUpdate update(prefs_, prefs::kGeolocationContentSettings);
+  DictionaryValue* obsolete_geolocation_settings = update.Get();
+  DictionaryValue* requesting_origin_settings_dictionary = NULL;
+  obsolete_geolocation_settings->GetDictionaryWithoutPathExpansion(
+      requesting_origin.spec(), &requesting_origin_settings_dictionary);
+  if (setting == CONTENT_SETTING_DEFAULT) {
+    if (requesting_origin_settings_dictionary) {
+      requesting_origin_settings_dictionary->RemoveWithoutPathExpansion(
+          embedding_origin.spec(), NULL);
+      if (requesting_origin_settings_dictionary->empty()) {
+        obsolete_geolocation_settings->RemoveWithoutPathExpansion(
+            requesting_origin.spec(), NULL);
+      }
+    }
+  } else {
+    if (!requesting_origin_settings_dictionary) {
+      requesting_origin_settings_dictionary = new DictionaryValue;
+      obsolete_geolocation_settings->SetWithoutPathExpansion(
+          requesting_origin.spec(), requesting_origin_settings_dictionary);
+    }
+    DCHECK(requesting_origin_settings_dictionary);
+    requesting_origin_settings_dictionary->SetWithoutPathExpansion(
+        embedding_origin.spec(), Value::CreateIntegerValue(setting));
+  }
+}
+
 // static
 void PrefProvider::CanonicalizeContentSettingsExceptions(
     DictionaryValue* all_settings_dictionary) {
@@ -969,12 +1018,11 @@ void PrefProvider::MigrateObsoletePopupsPref() {
 void PrefProvider::MigrateObsoleteContentSettingsPatternPref() {
   if (prefs_->HasPrefPath(prefs::kContentSettingsPatterns) && !is_incognito_) {
     const DictionaryValue* patterns_dictionary =
-      prefs_->GetDictionary(prefs::kContentSettingsPatterns);
+        prefs_->GetDictionary(prefs::kContentSettingsPatterns);
 
     // A map with an old key, new key mapping. If the new key is empty then the
     // value for the old key will be removed.
     StringMap keys_to_change;
-
     {
       DictionaryPrefUpdate update(prefs_, prefs::kContentSettingsPatternPairs);
       DictionaryValue* pattern_pairs_dictionary = update.Get();
@@ -1046,8 +1094,7 @@ void PrefProvider::MigrateObsoleteContentSettingsPatternPref() {
   }
 }
 
-void PrefProvider::SyncObsoletePref() {
-  AutoReset<bool> auto_reset(&updating_preferences_, true);
+void PrefProvider::SyncObsoletePatternPref() {
   if (prefs_->HasPrefPath(prefs::kContentSettingsPatternPairs) &&
       !is_incognito_) {
     const DictionaryValue* pattern_pairs_dictionary =
@@ -1069,15 +1116,117 @@ void PrefProvider::SyncObsoletePref() {
         continue;
       }
 
-      // Copy dictionary
-      DictionaryValue* dictionary = NULL;
+      DictionaryValue* settings_dictionary = NULL;
       bool found = pattern_pairs_dictionary->GetDictionaryWithoutPathExpansion(
-          key, &dictionary);
+          key, &settings_dictionary);
       DCHECK(found);
-      std::string new_key = pattern_pair.first.ToString();
-      // Existing values are overwritten.
-      obsolete_settings_dictionary->SetWithoutPathExpansion(
-          new_key, dictionary->DeepCopy());
+      scoped_ptr<DictionaryValue> settings_dictionary_copy(
+          new DictionaryValue());
+      for (size_t i = CONTENT_SETTINGS_TYPE_COOKIES;
+           i <= CONTENT_SETTINGS_TYPE_POPUPS;
+           ++i) {
+        DCHECK(kTypeNames[i]);
+        std::string type_name(kTypeNames[i]);
+        if (settings_dictionary->HasKey(type_name)) {
+          Value* value = NULL;
+          bool found = settings_dictionary->GetWithoutPathExpansion(
+              type_name, &value);
+          DCHECK(found);
+          settings_dictionary_copy->SetWithoutPathExpansion(
+              type_name, value->DeepCopy());
+        }
+      }
+
+      // Ignore empty dictionaryies.
+      if (!settings_dictionary_copy->empty()) {
+        std::string new_key = pattern_pair.first.ToString();
+        // Existing values are overwritten.
+        obsolete_settings_dictionary->SetWithoutPathExpansion(
+            new_key, settings_dictionary_copy.release());
+      }
+    }
+  }
+}
+
+void PrefProvider::MigrateObsoleteGeolocationPref() {
+  if (!prefs_->HasPrefPath(prefs::kGeolocationContentSettings))
+    return;
+
+  const DictionaryValue* geolocation_settings =
+      prefs_->GetDictionary(prefs::kGeolocationContentSettings);
+  for (DictionaryValue::key_iterator i =
+           geolocation_settings->begin_keys();
+       i != geolocation_settings->end_keys();
+       ++i) {
+    const std::string& primary_key(*i);
+    GURL primary_url(primary_key);
+    DCHECK(primary_url.is_valid());
+
+    DictionaryValue* requesting_origin_settings = NULL;
+    bool found = geolocation_settings->GetDictionaryWithoutPathExpansion(
+        primary_key, &requesting_origin_settings);
+    DCHECK(found);
+
+    for (DictionaryValue::key_iterator j =
+             requesting_origin_settings->begin_keys();
+         j != requesting_origin_settings->end_keys();
+         ++j) {
+      const std::string& secondary_key(*j);
+      GURL secondary_url(secondary_key);
+      DCHECK(secondary_url.is_valid());
+
+      int setting_value;
+      found = requesting_origin_settings->GetIntegerWithoutPathExpansion(
+          secondary_key, &setting_value);
+      DCHECK(found);
+
+      ContentSettingsPattern primary_pattern =
+          ContentSettingsPattern::FromURLNoWildcard(primary_url);
+      ContentSettingsPattern secondary_pattern =
+          ContentSettingsPattern::FromURLNoWildcard(secondary_url);
+      DCHECK(primary_pattern.IsValid() && secondary_pattern.IsValid());
+
+      SetContentSetting(primary_pattern,
+                        secondary_pattern,
+                        CONTENT_SETTINGS_TYPE_GEOLOCATION,
+                        std::string(),
+                        IntToContentSetting(setting_value));
+    }
+  }
+}
+
+void PrefProvider::SyncObsoleteGeolocationPref() {
+  DCHECK(prefs_);
+  DCHECK(prefs_->HasPrefPath(prefs::kContentSettingsPatternPairs));
+
+  // Clear the obsolete preference for geolocation settings. Then copy all
+  // geolocation settings from the new preference to the obsolete one.
+  prefs_->ClearPref(prefs::kGeolocationContentSettings);
+  const DictionaryValue* pattern_pairs_dictionary =
+      prefs_->GetDictionary(prefs::kContentSettingsPatternPairs);
+  for (DictionaryValue::key_iterator i =
+           pattern_pairs_dictionary->begin_keys();
+       i != pattern_pairs_dictionary->end_keys();
+       ++i) {
+    const std::string& key(*i);
+    std::pair<ContentSettingsPattern, ContentSettingsPattern> pattern_pair =
+        ParsePatternString(key);
+    DCHECK(pattern_pair.first.IsValid() && pattern_pair.second.IsValid());
+
+    DictionaryValue* settings_dictionary = NULL;
+    bool found = pattern_pairs_dictionary->GetDictionaryWithoutPathExpansion(
+        key, &settings_dictionary);
+    DCHECK(found);
+
+    if (settings_dictionary->HasKey(
+            kTypeNames[CONTENT_SETTINGS_TYPE_GEOLOCATION])) {
+      int setting_value;
+      settings_dictionary->GetInteger(
+          kTypeNames[CONTENT_SETTINGS_TYPE_GEOLOCATION], &setting_value);
+
+      UpdateObsoleteGeolocationPref(pattern_pair.first,
+                                    pattern_pair.second,
+                                    ContentSetting(setting_value));
     }
   }
 }
diff --git a/chrome/browser/content_settings/content_settings_pref_provider.h b/chrome/browser/content_settings/content_settings_pref_provider.h
index 9c717a2..3cb3b64 100644
--- a/chrome/browser/content_settings/content_settings_pref_provider.h
+++ b/chrome/browser/content_settings/content_settings_pref_provider.h
@@ -134,8 +134,6 @@ class PrefProvider : public ObservableProvider,
                        const NotificationDetails& details);
 
  private:
-  void Init();
-
   // Reads all content settings exceptions from the preference and load them
   // into the |value_map_|. The |value_map_| is cleared first if |overwrite| is
   // true.
@@ -151,8 +149,8 @@ class PrefProvider : public ObservableProvider,
       ContentSetting setting);
 
   // Update the preference prefs::kContentSettingsPatternPairs, which is used to
-  // persist content settigns exceptions and supposed to replace the preferences
-  // prefs::kContentSettingsPatterns.
+  // persist content settings exceptions and supposed to replace the preferences
+  // prefs::kContentSettingsPatterns and prefs::kGeolocationContentSettings.
   void UpdatePatternPairsPref(
       const ContentSettingsPattern& primary_pattern,
       const ContentSettingsPattern& secondary_pattern,
@@ -162,24 +160,40 @@ class PrefProvider : public ObservableProvider,
 
   // Updates the preferences prefs::kContentSettingsPatterns. This preferences
   // is obsolete and only used for compatibility reasons.
-  void UpdatePatternsPref(
+  void UpdateObsoletePatternsPref(
       const ContentSettingsPattern& primary_pattern,
       const ContentSettingsPattern& secondary_pattern,
       ContentSettingsType content_type,
       const ResourceIdentifier& resource_identifier,
       ContentSetting setting);
 
+  // Updates the preferences prefs::kGeolocationContentSettings. This preference
+  // is obsolete and only used to keep sync working with older chrome versions
+  // that do not know about the new preference.
+  void UpdateObsoleteGeolocationPref(
+      const ContentSettingsPattern& primary_pattern,
+      const ContentSettingsPattern& secondary_pattern,
+      ContentSetting setting);
+
   // Various migration methods (old cookie, popup and per-host data gets
   // migrated to the new format).
   void MigrateObsoletePerhostPref();
   void MigrateObsoletePopupsPref();
   void MigrateObsoleteContentSettingsPatternPref();
+  void MigrateObsoleteGeolocationPref();
 
   // Copies the value of the preference that stores the content settings
   // exceptions to the obsolete preference for content settings exceptions. This
   // is necessary to allow content settings exceptions beeing synced to older
-  // versions of chrome that only use the obsolete.
-  void SyncObsoletePref();
+  // versions of chrome that only use the obsolete preference.
+  void SyncObsoletePatternPref();
+
+  // Copies the geolocation content settings exceptions from the preference that
+  // stores the content settings exceptions to the obsolete preference for
+  // geolocation content settings exceptions. This is necessary to allow
+  // geolocation content settings exceptions being synced to older versions of
+  // chrome that only use the obsolete preference.
+  void SyncObsoleteGeolocationPref();
 
   static void CanonicalizeContentSettingsExceptions(
       base::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 587f8c8..033b69b 100644
--- a/chrome/browser/content_settings/content_settings_pref_provider_unittest.cc
+++ b/chrome/browser/content_settings/content_settings_pref_provider_unittest.cc
@@ -26,6 +26,25 @@
 
 using ::testing::_;
 
+namespace {
+
+void ExpectObsoleteGeolocationSetting(
+    const DictionaryValue& geo_settings_dictionary,
+    const GURL& primary_origin,
+    const GURL& secondary_origin,
+    ContentSetting expected_setting) {
+
+  DictionaryValue* one_origin_settings;
+  ASSERT_TRUE(geo_settings_dictionary.GetDictionaryWithoutPathExpansion(
+      std::string(primary_origin.spec()), &one_origin_settings));
+  int setting_value;
+  ASSERT_TRUE(one_origin_settings->GetIntegerWithoutPathExpansion(
+      std::string(secondary_origin.spec()), &setting_value));
+  EXPECT_EQ(expected_setting, setting_value);
+}
+
+}  // namespace
+
 namespace content_settings {
 
 class PrefDefaultProviderTest : public TestingBrowserProcessTest {
@@ -401,25 +420,14 @@ TEST_F(PrefProviderTest, MigrateObsoleteContentSettingsPatternPref) {
       GURL("http://www.example.com"),
       CONTENT_SETTINGS_TYPE_POPUPS,
       ""));
-
-  // Change obsolete preference. This can happen if a user has enabled sync
-  // while using an old version of chrome.
-  {
-    DictionaryPrefUpdate update(prefs, prefs::kContentSettingsPatterns);
-    DictionaryValue* mutable_patterns = update.Get();
-    DictionaryValue* mutable_settings = NULL;
-    std::string key = pattern.ToString();
-    mutable_patterns->GetDictionaryWithoutPathExpansion(key, &mutable_settings);
-    ASSERT_TRUE(mutable_settings != NULL) << "Dictionary has no key: " << key;
-    mutable_settings->SetInteger("javascript", CONTENT_SETTING_BLOCK);
-  }
-
-  // Test if the changed single pattern setting was migrated correctly.
-  EXPECT_EQ(CONTENT_SETTING_BLOCK,  provider.GetContentSetting(
-      GURL("http://www.example.com"),
-      GURL("http://www.foo.com"),
-      CONTENT_SETTINGS_TYPE_JAVASCRIPT,
-      ""));
+  // Test if single pattern settings are properly migrated.
+  const_all_settings_dictionary = 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()));
 
   provider.ShutdownOnUIThread();
 }
@@ -555,4 +563,129 @@ TEST_F(PrefProviderTest, FixOrRemoveMalformedPatternKeysFromObsoletePref) {
   provider.ShutdownOnUIThread();
 }
 
+TEST_F(PrefProviderTest, MigrateObsoleteGeolocationPref) {
+  TestingProfile profile;
+  PrefService* prefs = profile.GetPrefs();
+  GURL secondary_url("http://www.foo.com");
+  GURL primary_url("http://www.bar.com");
+
+  // Set obsolete preference.
+  DictionaryValue* secondary_patterns_dictionary = new DictionaryValue();
+  secondary_patterns_dictionary->SetWithoutPathExpansion(
+      secondary_url.spec(),
+      Value::CreateIntegerValue(CONTENT_SETTING_BLOCK));
+  scoped_ptr<DictionaryValue> geolocation_settings_dictionary(
+      new DictionaryValue());
+  geolocation_settings_dictionary->SetWithoutPathExpansion(
+      primary_url.spec(), secondary_patterns_dictionary);
+  prefs->Set(prefs::kGeolocationContentSettings,
+             *geolocation_settings_dictionary);
+
+
+  content_settings::PrefProvider provider(prefs, false);
+
+  // Test if the migrated settings are loaded and available.
+  EXPECT_EQ(CONTENT_SETTING_BLOCK, provider.GetContentSetting(
+      primary_url,
+      secondary_url,
+      CONTENT_SETTINGS_TYPE_GEOLOCATION,
+      ""));
+  EXPECT_EQ(CONTENT_SETTING_DEFAULT, provider.GetContentSetting(
+      GURL("http://www.example.com"),
+      secondary_url,
+      CONTENT_SETTINGS_TYPE_GEOLOCATION,
+      ""));
+  // Check if the settings where migrated correctly.
+  const DictionaryValue* const_all_settings_dictionary =
+      prefs->GetDictionary(prefs::kContentSettingsPatternPairs);
+  EXPECT_EQ(1U, const_all_settings_dictionary->size());
+  EXPECT_TRUE(const_all_settings_dictionary->HasKey(
+      ContentSettingsPattern::FromURLNoWildcard(primary_url).ToString() + "," +
+      ContentSettingsPattern::FromURLNoWildcard(secondary_url).ToString()));
+  // Check that geolocation settings were not synced to the obsolete content
+  // settings pattern preference.
+  const DictionaryValue* const_obsolete_patterns_dictionary =
+      prefs->GetDictionary(prefs::kContentSettingsPatterns);
+  EXPECT_TRUE(const_obsolete_patterns_dictionary->empty());
+
+  // Change obsolete preference. This could be triggered by sync if sync is used
+  // with an old version of chrome.
+  secondary_patterns_dictionary = new DictionaryValue();
+  secondary_patterns_dictionary->SetWithoutPathExpansion(
+      secondary_url.spec(),
+      Value::CreateIntegerValue(CONTENT_SETTING_ALLOW));
+  geolocation_settings_dictionary.reset(new DictionaryValue());
+  geolocation_settings_dictionary->SetWithoutPathExpansion(
+      primary_url.spec(), secondary_patterns_dictionary);
+  prefs->Set(prefs::kGeolocationContentSettings,
+             *geolocation_settings_dictionary);
+
+  // Test if the changed obsolete preference was migrated correctly.
+  EXPECT_EQ(CONTENT_SETTING_ALLOW, provider.GetContentSetting(
+      primary_url,
+      secondary_url,
+      CONTENT_SETTINGS_TYPE_GEOLOCATION,
+      ""));
+  // Check that geolocation settings were not synced to the obsolete content
+  // settings pattern preference.
+  const_obsolete_patterns_dictionary =
+      prefs->GetDictionary(prefs::kContentSettingsPatterns);
+  EXPECT_TRUE(const_obsolete_patterns_dictionary->empty());
+
+  provider.ShutdownOnUIThread();
+}
+
+TEST_F(PrefProviderTest, SyncObsoleteGeolocationPref) {
+  TestingProfile profile;
+  PrefService* prefs = profile.GetPrefs();
+
+  content_settings::PrefProvider provider(prefs, false);
+
+  // Changing the preferences prefs::kContentSettingsPatternPairs.
+  ContentSettingsPattern primary_pattern=
+      ContentSettingsPattern::FromString("http://www.bar.com");
+  ContentSettingsPattern primary_pattern_2 =
+      ContentSettingsPattern::FromString("http://www.example.com");
+  ContentSettingsPattern secondary_pattern =
+      ContentSettingsPattern::FromString("http://www.foo.com");
+  scoped_ptr<DictionaryValue> settings_dictionary(new DictionaryValue());
+  settings_dictionary->SetInteger("geolocation", CONTENT_SETTING_BLOCK);
+  {
+    DictionaryPrefUpdate update(prefs,
+                                prefs::kContentSettingsPatternPairs);
+    DictionaryValue* all_settings_dictionary = update.Get();
+    std::string key(
+        primary_pattern.ToString()+ "," +
+        secondary_pattern.ToString());
+    all_settings_dictionary->SetWithoutPathExpansion(
+        key, settings_dictionary->DeepCopy());
+
+    key = std::string(
+        primary_pattern_2.ToString()+ "," +
+        secondary_pattern.ToString());
+    all_settings_dictionary->SetWithoutPathExpansion(
+        key, settings_dictionary->DeepCopy());
+  }
+
+  // Test if the obsolete geolocation preference is kept in sync if the new
+  // preference is changed by a sync.
+  GURL primary_url("http://www.bar.com");
+  GURL primary_url_2("http://www.example.com");
+  GURL secondary_url("http://www.foo.com");
+
+  const DictionaryValue* geo_settings_dictionary =
+      prefs->GetDictionary(prefs::kGeolocationContentSettings);
+  EXPECT_EQ(2U, geo_settings_dictionary->size());
+  ExpectObsoleteGeolocationSetting(*geo_settings_dictionary,
+                                   primary_url,
+                                   secondary_url,
+                                   CONTENT_SETTING_BLOCK);
+  ExpectObsoleteGeolocationSetting(*geo_settings_dictionary,
+                                   primary_url_2,
+                                   secondary_url,
+                                   CONTENT_SETTING_BLOCK);
+
+  provider.ShutdownOnUIThread();
+}
+
 }  // namespace content_settings
diff --git a/chrome/browser/content_settings/host_content_settings_map.cc b/chrome/browser/content_settings/host_content_settings_map.cc
index b0e4dae..4bdb243 100644
--- a/chrome/browser/content_settings/host_content_settings_map.cc
+++ b/chrome/browser/content_settings/host_content_settings_map.cc
@@ -58,6 +58,8 @@ typedef std::vector<ProviderPtr>::const_iterator ConstProviderIterator;
 
 typedef content_settings::ProviderInterface::Rules Rules;
 
+typedef std::pair<std::string, std::string> StringPair;
+
 const char* kProviderNames[] = {
   "policy",
   "extension",
@@ -311,14 +313,14 @@ void HostContentSettingsMap::GetSettingsForOneType(
     content_settings_providers_[i]->GetAllContentSettingsRules(
         content_type, resource_identifier, &rules);
 
-    // Sort rules according to their primary pattern string using a map.
-    std::map<std::string, PatternSettingSourceTuple> settings_map;
+    // Sort rules according to their primary-secondary pattern string pairs
+    // using a map.
+    std::map<StringPair, PatternSettingSourceTuple> settings_map;
     for (Rules::iterator rule = rules.begin();
          rule != rules.end();
          ++rule) {
-      // We do not support pattern pairs in the UI, so we only display the
-      // primary pattern.
-      std::string sort_key = rule->primary_pattern.ToString();
+      StringPair sort_key(rule->primary_pattern.ToString(),
+                          rule->secondary_pattern.ToString());
       settings_map[sort_key] = PatternSettingSourceTuple(
           rule->primary_pattern,
           rule->secondary_pattern,
@@ -327,7 +329,7 @@ void HostContentSettingsMap::GetSettingsForOneType(
     }
 
     // TODO(markusheintz): Only the rules that are applied should be added.
-    for (std::map<std::string, PatternSettingSourceTuple>::iterator i(
+    for (std::map<StringPair, PatternSettingSourceTuple>::iterator i(
              settings_map.begin());
          i != settings_map.end();
          ++i) {