Migrate Obsolete NotificationsSettings and remove content_settings::NotificationsProvider.

chrome/browser/content_settings/content_settings_pref_provider.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/content_settings/content_settings_pref_provider.h"
 
 #include <map>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 25 matching lines @@
 
 // The preference keys where resource identifiers are stored for
 // ContentSettingsType values that support resource identifiers.
 const char* kResourceTypeNames[] = {
   NULL,
   NULL,
   NULL,
   "per_plugin",
   NULL,
   NULL,
-  NULL,  // Not used for Notifications
-  NULL,  // Not used for Intents.
+  NULL,
+  NULL,
 };
 COMPILE_ASSERT(arraysize(kResourceTypeNames) == CONTENT_SETTINGS_NUM_TYPES,
                resource_type_names_incorrect_size);
 
 // The default setting for each content type.
 const ContentSetting kDefaultSettings[] = {
   CONTENT_SETTING_ALLOW,  // CONTENT_SETTINGS_TYPE_COOKIES
   CONTENT_SETTING_ALLOW,  // CONTENT_SETTINGS_TYPE_IMAGES
   CONTENT_SETTING_ALLOW,  // CONTENT_SETTINGS_TYPE_JAVASCRIPT
   CONTENT_SETTING_ALLOW,  // CONTENT_SETTINGS_TYPE_PLUGINS
   CONTENT_SETTING_BLOCK,  // CONTENT_SETTINGS_TYPE_POPUPS
   CONTENT_SETTING_ASK,    // CONTENT_SETTINGS_TYPE_GEOLOCATION
   CONTENT_SETTING_ASK,    // CONTENT_SETTINGS_TYPE_NOTIFICATIONS
   CONTENT_SETTING_ASK,    // CONTENT_SETTINGS_TYPE_INTENTS
 };
 COMPILE_ASSERT(arraysize(kDefaultSettings) == CONTENT_SETTINGS_NUM_TYPES,
                default_settings_incorrect_size);
 
 // The names of the ContentSettingsType values, for use with dictionary prefs.
 const char* kTypeNames[] = {
   "cookies",
   "images",
   "javascript",
   "plugins",
   "popups",
   "geolocation",
-  // TODO(markusheintz): Refactoring in progress. Content settings exceptions
-  // for notifications added next.
-  "notifications",  // Only used for default Notifications settings.
+  "notifications",
   "intents",
 };
 COMPILE_ASSERT(arraysize(kTypeNames) == CONTENT_SETTINGS_NUM_TYPES,
                type_names_incorrect_size);
 
 void SetDefaultContentSettings(DictionaryValue* default_settings) {
   default_settings->Clear();
 
   for (int i = 0; i < CONTENT_SETTINGS_NUM_TYPES; ++i) {
     if (kTypeNames[i] != NULL) {
@@ skipping 26 matching lines @@
 
 ContentSetting FixObsoleteCookiePromptMode(ContentSettingsType content_type,
                                            ContentSetting setting) {
   if (content_type == CONTENT_SETTINGS_TYPE_COOKIES &&
       setting == CONTENT_SETTING_ASK) {
     return CONTENT_SETTING_BLOCK;
   }
   return setting;
 }
 
+// Clears all settings for the given |type| in the given |pattern_pairs|
+// dictionary.
+void ClearSettings(ContentSettingsType type,
+                   DictionaryValue* pattern_pairs) {
+  std::string type_name(kTypeNames[type]);
+  for (DictionaryValue::key_iterator i = pattern_pairs->begin_keys();
+       i != pattern_pairs->end_keys();
+       ++i) {
+    const std::string& pattern_pair(*i);
+
+    DictionaryValue* settings = NULL;
+    bool found = pattern_pairs->GetDictionaryWithoutPathExpansion(
+          pattern_pair, &settings);

[battre, 2011/08/22 15:00:57]

nit: indentation

[markusheintz_, 2011/08/24 00:47:12]

Done.

+    DCHECK(found);

[battre, 2011/08/22 15:00:57]

I thought this would cause a compiler error on the build bots because |found| is
never used if DCHECK is evaluated to nothing in release mode.

[markusheintz_, 2011/08/24 00:47:12]

You might be right. I removed the DCHECK since it is not really useful anyway.

+
+    settings->RemoveWithoutPathExpansion(type_name, NULL);
+  }
+}
+
 }  // namespace
 
 namespace content_settings {
 
 PrefDefaultProvider::PrefDefaultProvider(PrefService* prefs,
                                          bool incognito)
     : prefs_(prefs),
       is_incognito_(incognito),
       updating_preferences_(false) {
   DCHECK(prefs_);
@@ skipping 228 matching lines @@
       ContentSettingsPattern::kContentSettingsPatternVersion,
       PrefService::UNSYNCABLE_PREF);
   prefs->RegisterDictionaryPref(prefs::kContentSettingsPatternPairs,
                                 PrefService::SYNCABLE_PREF);
 
   // Obsolete prefs, for migration:
   prefs->RegisterDictionaryPref(prefs::kGeolocationContentSettings,
                                 PrefService::SYNCABLE_PREF);
   prefs->RegisterDictionaryPref(prefs::kContentSettingsPatterns,
                                 PrefService::SYNCABLE_PREF);
+  prefs->RegisterListPref(prefs::kDesktopNotificationAllowedOrigins,
+                          PrefService::SYNCABLE_PREF);

[battre, 2011/08/22 15:00:57]

please update pref_names.* to indicate that these preferences are obsolete and
what they have been replaced with. I would even consider renaming the constant
names.

[markusheintz_, 2011/08/24 00:47:12]

I marked the pref as obsolete in pref_names.h.

Why should I rename the constants?

+  prefs->RegisterListPref(prefs::kDesktopNotificationDeniedOrigins,
+                          PrefService::SYNCABLE_PREF);
   prefs->RegisterListPref(prefs::kPopupWhitelistedHosts,
                           PrefService::UNSYNCABLE_PREF);
   prefs->RegisterDictionaryPref(prefs::kPerHostContentSettings,
                                 PrefService::UNSYNCABLE_PREF);
 }
 
 PrefProvider::PrefProvider(PrefService* prefs,
                            bool incognito)
   : prefs_(prefs),
     is_incognito_(incognito),
     updating_preferences_(false) {
   DCHECK(prefs_);
   if (!is_incognito_) {
     // Migrate obsolete preferences.
     MigrateObsoletePerhostPref();
     MigrateObsoletePopupsPref();
     MigrateObsoleteContentSettingsPatternPref();
     MigrateObsoleteGeolocationPref();
+    MigrateObsoleteNotificationsPrefs();
   }
 
   // Verify preferences version.
   if (!prefs_->HasPrefPath(prefs::kContentSettingsVersion)) {
     prefs_->SetInteger(prefs::kContentSettingsVersion,
                       ContentSettingsPattern::kContentSettingsPatternVersion);
   }
   if (prefs_->GetInteger(prefs::kContentSettingsVersion) >
       ContentSettingsPattern::kContentSettingsPatternVersion) {
     return;
   }
 
   // Read content settings exceptions.
   ReadContentSettingsFromPref(false);
 
   if (!is_incognito_) {
     UMA_HISTOGRAM_COUNTS("ContentSettings.NumberOfExceptions",
                          value_map_.size());
   }
 
   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);
+  pref_change_registrar_.Add(prefs::kDesktopNotificationAllowedOrigins, this);
+  pref_change_registrar_.Add(prefs::kDesktopNotificationDeniedOrigins, this);
 }
 
 ContentSetting PrefProvider::GetContentSetting(
     const GURL& primary_url,
     const GURL& secondary_url,
     ContentSettingsType content_type,
     const ResourceIdentifier& resource_identifier) const {
   // For a |PrefProvider| used in a |HostContentSettingsMap| of a non incognito
   // profile, this will always return NULL.
   // TODO(markusheintz): I don't like this. I'd like to have an
@@ skipping 78 matching lines @@
     }
   }
 
   // Update the content settings preference.
   if (!is_incognito_) {
     UpdatePref(primary_pattern,
                secondary_pattern,
                content_type,
                resource_identifier,
                setting);
+    prefs_->ScheduleSavePersistentPrefs();
   }
 
   NotifyObservers(
       primary_pattern, secondary_pattern, content_type, resource_identifier);
 }
 
 void PrefProvider::ClearAllContentSettingsRules(
     ContentSettingsType content_type) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(kTypeNames[content_type] != NULL);  // Don't call this for Geolocation.
@@ skipping 14 matching lines @@
             entry->content_type,
             entry->identifier,
             CONTENT_SETTING_DEFAULT);
         // Delete current |entry| and set |entry| to the next value map entry.
         entry = map_to_modify->erase(entry);
       } else {
         ++entry;
       }
     }
   }
-
+  prefs_->ScheduleSavePersistentPrefs();
   NotifyObservers(ContentSettingsPattern(),
                   ContentSettingsPattern(),
                   content_type,
                   std::string());
 }
 
 void PrefProvider::Observe(
     int type,
     const NotificationSource& source,
     const NotificationDetails& details) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   if (type == chrome::NOTIFICATION_PREF_CHANGED) {
     DCHECK_EQ(prefs_, Source<PrefService>(source).ptr());
     if (updating_preferences_)
       return;
 
     AutoReset<bool> auto_reset(&updating_preferences_, true);
     std::string* name = Details<std::string>(details).ptr();
     if (*name == prefs::kContentSettingsPatternPairs) {
       SyncObsoletePatternPref();
-      SyncObsoleteGeolocationPref();
+      SyncObsoletePrefs();
     } else if (*name == prefs::kContentSettingsPatterns) {
       MigrateObsoleteContentSettingsPatternPref();
     } else if (*name == prefs::kGeolocationContentSettings) {
       MigrateObsoleteGeolocationPref();
+    } else if (*name == prefs::kDesktopNotificationAllowedOrigins ||
+               *name == prefs::kDesktopNotificationDeniedOrigins) {
+      MigrateObsoleteNotificationsPrefs();
     } else {
       NOTREACHED() << "Unexpected preference observed";
       return;
     }
+    prefs_->ScheduleSavePersistentPrefs();
     ReadContentSettingsFromPref(true);
 
     NotifyObservers(ContentSettingsPattern(),
                     ContentSettingsPattern(),
                     CONTENT_SETTINGS_TYPE_DEFAULT,
                     std::string());
   } else {
     NOTREACHED() << "Unexpected notification";
   }
 }
 
 PrefProvider::~PrefProvider() {
   DCHECK(!prefs_);
 }
 
 // ////////////////////////////////////////////////////////////////////////////
 // Private
 
 void PrefProvider::UpdatePref(
     const ContentSettingsPattern& primary_pattern,
     const ContentSettingsPattern& secondary_pattern,
     ContentSettingsType content_type,
     const ResourceIdentifier& resource_identifier,
     ContentSetting setting) {
   AutoReset<bool> auto_reset(&updating_preferences_, true);
-  UpdatePatternPairsPref(primary_pattern,
-                         secondary_pattern,
-                         content_type,
-                         resource_identifier,
-                         setting);
+  {
+    DictionaryPrefUpdate update(prefs_,
+                                prefs::kContentSettingsPatternPairs);
+    DictionaryValue* pattern_pairs_settings = update.Get();
+    UpdatePatternPairsSettings(primary_pattern,
+                               secondary_pattern,
+                               content_type,
+                               resource_identifier,
+                               setting,
+                               pattern_pairs_settings);
+  }
   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) {
+  } else if (content_type == CONTENT_SETTINGS_TYPE_GEOLOCATION) {
     UpdateObsoleteGeolocationPref(primary_pattern, secondary_pattern, setting);
+  } else if (content_type == CONTENT_SETTINGS_TYPE_NOTIFICATIONS) {
+    ListPrefUpdate update_allowed_sites(
+        prefs_, prefs::kDesktopNotificationAllowedOrigins);
+    ListPrefUpdate update_denied_sites(
+        prefs_, prefs::kDesktopNotificationDeniedOrigins);
+    UpdateObsoleteNotificationsSettings(primary_pattern,
+                                        secondary_pattern,
+                                        setting,
+                                        update_allowed_sites.Get(),
+                                        update_denied_sites.Get());
   }
 }
 
 void PrefProvider::ReadContentSettingsFromPref(bool overwrite) {
   base::AutoLock auto_lock(lock_);
 
   const DictionaryValue* all_settings_dictionary =
       prefs_->GetDictionary(prefs::kContentSettingsPatternPairs);
 
   if (overwrite)
@@ skipping 141 matching lines @@
       }
     }
     // Remove the settings dictionary if it is empty.
     if (settings_dictionary->empty()) {
       all_settings_dictionary->RemoveWithoutPathExpansion(
           pattern_str, NULL);
     }
   }
 }
 
-void PrefProvider::UpdatePatternPairsPref(
+void PrefProvider::UpdatePatternPairsSettings(
       const ContentSettingsPattern& primary_pattern,
       const ContentSettingsPattern& secondary_pattern,
       ContentSettingsType content_type,
       const ResourceIdentifier& resource_identifier,
-      ContentSetting setting) {
-  DictionaryPrefUpdate update(prefs_,
-                              prefs::kContentSettingsPatternPairs);
-  DictionaryValue* all_settings_dictionary = update.Get();
-
+      ContentSetting setting,
+      DictionaryValue* pattern_pairs_settings) {
   // Get settings dictionary for the given patterns.
   std::string pattern_str(CreatePatternString(primary_pattern,
                                               secondary_pattern));
   DictionaryValue* settings_dictionary = NULL;
-  bool found = all_settings_dictionary->GetDictionaryWithoutPathExpansion(
+  bool found = pattern_pairs_settings->GetDictionaryWithoutPathExpansion(
       pattern_str, &settings_dictionary);
 
   if (!found && (setting != CONTENT_SETTING_DEFAULT)) {
     settings_dictionary = new DictionaryValue;
-    all_settings_dictionary->SetWithoutPathExpansion(
+    pattern_pairs_settings->SetWithoutPathExpansion(
         pattern_str, settings_dictionary);
   }
 
   if (settings_dictionary) {
     if (RequiresResourceIdentifier(content_type)) {
       // Get resource dictionary.
       std::string res_dictionary_path(kResourceTypeNames[content_type]);
       DictionaryValue* resource_dictionary = NULL;
       found = settings_dictionary->GetDictionary(
           res_dictionary_path, &resource_dictionary);
@@ skipping 21 matching lines @@
       if (setting == CONTENT_SETTING_DEFAULT) {
         settings_dictionary->RemoveWithoutPathExpansion(setting_path,
                                                         NULL);
       } else {
         settings_dictionary->SetWithoutPathExpansion(
             setting_path, Value::CreateIntegerValue(setting));
       }
     }
     // Remove the settings dictionary if it is empty.
     if (settings_dictionary->empty()) {
-      all_settings_dictionary->RemoveWithoutPathExpansion(
+      pattern_pairs_settings->RemoveWithoutPathExpansion(
           pattern_str, NULL);
     }
   }
 }
 
 void PrefProvider::UpdateObsoleteGeolocationPref(
     const ContentSettingsPattern& primary_pattern,
     const ContentSettingsPattern& secondary_pattern,
     ContentSetting setting) {
   if (!prefs_)
@@ skipping 22 matching lines @@
       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));
   }
 }
 
+void PrefProvider::UpdateObsoleteNotificationsSettings(
+    const ContentSettingsPattern& primary_pattern,
+    const ContentSettingsPattern& secondary_pattern,
+    ContentSetting setting,
+    ListValue* allowed_sites,
+    ListValue* denied_sites) {
+  DCHECK_EQ(secondary_pattern, ContentSettingsPattern::Wildcard());
+  GURL origin(primary_pattern.ToString());
+  DCHECK(origin.is_valid());
+  StringValue* value = new StringValue(origin.spec());
+  if (setting == CONTENT_SETTING_ALLOW) {
+    denied_sites->Remove(*value, NULL);
+    allowed_sites->AppendIfNotPresent(value);
+  } else if (setting == CONTENT_SETTING_BLOCK) {
+    allowed_sites->Remove(*value, NULL);
+    denied_sites->AppendIfNotPresent(value);
+  } else if (setting == CONTENT_SETTING_DEFAULT) {
+    denied_sites->Remove(*value, NULL);
+    allowed_sites->Remove(*value, NULL);

[battre, 2011/08/22 15:00:57]

here you leak |value|

[markusheintz_, 2011/08/24 00:47:12]

Changed value to a scoped_ptr<StringValue>

+  } else {
+    NOTREACHED() << "Setting value: " << setting
+                 << " is not supported for notifications";
+  }
+}
+
 // static
 void PrefProvider::CanonicalizeContentSettingsExceptions(
     DictionaryValue* all_settings_dictionary) {
   DCHECK(all_settings_dictionary);
 
   std::vector<std::string> remove_items;
   std::vector<std::pair<std::string, std::string> > move_items;
   for (DictionaryValue::key_iterator i(all_settings_dictionary->begin_keys());
        i != all_settings_dictionary->end_keys(); ++i) {
     const std::string& pattern_str(*i);
@@ skipping 257 matching lines @@
             new_key, settings_dictionary_copy.release());
       }
     }
   }
 }
 
 void PrefProvider::MigrateObsoleteGeolocationPref() {
   if (!prefs_->HasPrefPath(prefs::kGeolocationContentSettings))
     return;
 
+  DictionaryPrefUpdate update(prefs_,
+                              prefs::kContentSettingsPatternPairs);
+  DictionaryValue* pattern_pairs_settings = update.Get();
+
   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());
 
@@ skipping 14 matching lines @@
       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());
 
-      UpdatePatternPairsPref(primary_pattern,
-                             secondary_pattern,
-                             CONTENT_SETTINGS_TYPE_GEOLOCATION,
-                             std::string(),
-                             IntToContentSetting(setting_value));
+      UpdatePatternPairsSettings(primary_pattern,
+                                 secondary_pattern,
+                                 CONTENT_SETTINGS_TYPE_GEOLOCATION,
+                                 std::string(),
+                                 IntToContentSetting(setting_value),
+                                 pattern_pairs_settings);
     }
   }
 }
 
-void PrefProvider::SyncObsoleteGeolocationPref() {
+void PrefProvider::MigrateObsoleteNotificationsPrefs() {
+  // The notifications settings in the preferences
+  // prefs::kContentSettingsPatternPairs do not contain the latest
+  // notifications settings. So all notification settings are cleared and
+  // migrated from the obsolete preferences for notifications settings that
+  // contain the lattest settings.

[battre, 2011/08/22 15:00:57]

nit: typo "latest"

[markusheintz_, 2011/08/24 00:47:12]

Done.

+  DictionaryPrefUpdate update(prefs_, prefs::kContentSettingsPatternPairs);
+  DictionaryValue* pattern_pairs_settings = update.Get();
+  ClearSettings(CONTENT_SETTINGS_TYPE_NOTIFICATIONS, pattern_pairs_settings);
+
+  const ListValue* allow_sites =
+      prefs_->GetList(prefs::kDesktopNotificationAllowedOrigins);
+  for (size_t i = 0; i < allow_sites->GetSize(); ++i) {
+    std::string url_string;
+    bool status = allow_sites->GetString(i, &url_string);
+    DCHECK(status);
+    ContentSettingsPattern primary_pattern =
+        ContentSettingsPattern::FromURLNoWildcard(GURL(url_string));
+    DCHECK(primary_pattern.IsValid());
+    UpdatePatternPairsSettings(primary_pattern,
+                               ContentSettingsPattern::Wildcard(),
+                               CONTENT_SETTINGS_TYPE_NOTIFICATIONS,
+                               std::string(),
+                               CONTENT_SETTING_ALLOW,
+                               pattern_pairs_settings);
+  }
+
+  const ListValue* denied_sites =
+      prefs_->GetList(prefs::kDesktopNotificationDeniedOrigins);
+  for (size_t i = 0; i < denied_sites->GetSize(); ++i) {
+    std::string url_string;
+    bool status = denied_sites->GetString(i, &url_string);
+    DCHECK(status);
+    ContentSettingsPattern primary_pattern =
+        ContentSettingsPattern::FromURLNoWildcard(GURL(url_string));
+    DCHECK(primary_pattern.IsValid());
+    UpdatePatternPairsSettings(primary_pattern,
+                               ContentSettingsPattern::Wildcard(),
+                               CONTENT_SETTINGS_TYPE_NOTIFICATIONS,
+                               std::string(),
+                               CONTENT_SETTING_BLOCK,
+                               pattern_pairs_settings);
+  }
+}
+
+void PrefProvider::SyncObsoletePrefs() {
   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.
+  // Clear obsolete preferences first. Then copy the settings from the new
+  // preference to the obsolete ones.
   prefs_->ClearPref(prefs::kGeolocationContentSettings);
+  prefs_->ClearPref(prefs::kDesktopNotificationAllowedOrigins);
+  prefs_->ClearPref(prefs::kDesktopNotificationDeniedOrigins);
+
+  ListPrefUpdate update_allowed_sites(
+        prefs_, prefs::kDesktopNotificationAllowedOrigins);

[battre, 2011/08/22 15:00:57]

nit: indentation -2

[markusheintz_, 2011/08/24 00:47:12]

Done.

+  ListPrefUpdate update_denied_sites(
+        prefs_, prefs::kDesktopNotificationDeniedOrigins);
+  ListValue* allowed_sites = update_allowed_sites.Get();
+  ListValue* denied_sites = update_denied_sites.Get();
+
   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_NOTIFICATIONS])) {
+      int setting_value;

[battre, 2011/08/22 15:00:57]

nit: initialize

[markusheintz_, 2011/08/24 00:47:12]

Done.

+      settings_dictionary->GetInteger(
+          kTypeNames[CONTENT_SETTINGS_TYPE_NOTIFICATIONS], &setting_value);

[battre, 2011/08/22 15:00:57]

Return value is not checked. I propose to simplify this, by replacing the HasKey
call above with the following.

int setting_value = 0;
if (settings_dictionary->GetInteger(..., &setting_value)) {
  UpdateObsolete...
}

(no previous check whether the key exists)

[markusheintz_, 2011/08/24 00:47:12]

Done.

+      UpdateObsoleteNotificationsSettings(pattern_pair.first,
+                                          pattern_pair.second,
+                                          ContentSetting(setting_value),
+                                          allowed_sites,
+                                          denied_sites);
+    }
+
+    if (settings_dictionary->HasKey(
             kTypeNames[CONTENT_SETTINGS_TYPE_GEOLOCATION])) {

[battre, 2011/08/22 15:00:57]

same here

[markusheintz_, 2011/08/24 00:47:12]

Done.

       int setting_value;
       settings_dictionary->GetInteger(
           kTypeNames[CONTENT_SETTINGS_TYPE_GEOLOCATION], &setting_value);
-
       UpdateObsoleteGeolocationPref(pattern_pair.first,
                                     pattern_pair.second,
                                     ContentSetting(setting_value));
     }
   }
 }
 
 }  // namespace content_settings