Replace SetContentSetting method of the content_settings::Provider interface with GetWebsiteSetting

chrome/browser/content_settings/content_settings_default_provider.cc

Index: chrome/browser/content_settings/content_settings_default_provider.cc
diff --git a/chrome/browser/content_settings/content_settings_default_provider.cc b/chrome/browser/content_settings/content_settings_default_provider.cc
index d1ff97751f78043ac69cc0b3469a54c374e1b846..39dc6ee8a18157afde6298868f7608c17b4e2b16 100644
--- a/chrome/browser/content_settings/content_settings_default_provider.cc
+++ b/chrome/browser/content_settings/content_settings_default_provider.cc
@@ -70,6 +70,7 @@ class DefaultRuleIterator : public RuleIterator {
   }
 
  private:
+  // TODO(markusheintz): |ContentSetting| should be replaced with a |Value|.
   ContentSetting setting_;
 };
 
@@ -109,8 +110,10 @@ DefaultProvider::DefaultProvider(PrefService* prefs, bool incognito)
 
   // Read global defaults.
   ReadDefaultSettings(true);
-  if (default_content_settings_.settings[CONTENT_SETTINGS_TYPE_COOKIES] ==
-      CONTENT_SETTING_BLOCK) {
+
+  ContentSetting cookie_setting = ValueToContentSetting(
+      default_settings_[CONTENT_SETTINGS_TYPE_COOKIES].get());
+  if (cookie_setting == CONTENT_SETTING_BLOCK) {
     UserMetrics::RecordAction(
         UserMetricsAction("CookieBlockingEnabledPerDefault"));
   } else {
@@ -126,25 +129,25 @@ DefaultProvider::DefaultProvider(PrefService* prefs, bool incognito)
 DefaultProvider::~DefaultProvider() {
 }
 
-void DefaultProvider::SetContentSetting(
+bool DefaultProvider::SetWebsiteSetting(
     const ContentSettingsPattern& primary_pattern,
     const ContentSettingsPattern& secondary_pattern,
     ContentSettingsType content_type,
     const ResourceIdentifier& resource_identifier,
-    ContentSetting setting) {
+    Value* value) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(prefs_);
 
   // Ignore non default settings
   if (primary_pattern != ContentSettingsPattern::Wildcard() ||
       secondary_pattern != ContentSettingsPattern::Wildcard()) {
-    return;
+    return false;
   }
 
   // The default settings may not be directly modified for OTR sessions.
   // Instead, they are synced to the main profile's setting.
   if (is_incognito_)
-    return;
+    return false;
 
   std::string dictionary_path = GetTypeName(content_type);
   {
@@ -157,27 +160,38 @@ void DefaultProvider::SetContentSetting(
     // |PrefService::SetInteger()| send out notifications. As a response, the
     // upper layers may call |GetAllContentSettingRules| which acquires |lock_|
     // again.
+    // TODO(markusheintz): Hardcoded default settings should be handled by the
+    // HostContentSettingsMap itself. The DefaultProvider should only deal with
+    // user set default values and should return NULL if no user defined
+    // settings are avaliable.
     {
       base::AutoLock lock(lock_);
-      if (setting == CONTENT_SETTING_DEFAULT ||
-          setting == kDefaultSettings[content_type]) {
-        default_content_settings_.settings[content_type] =
-            kDefaultSettings[content_type];
+      if (value == NULL ||
+          ValueToContentSetting(value) == kDefaultSettings[content_type]) {
+        // If |value| is NULL we need to reset the default setting the the
+        // hardcoded default.
+        default_settings_[content_type].reset(
+            Value::CreateIntegerValue(kDefaultSettings[content_type]));
+
+        // If |value| is null or equal to the hardcoded default setting then
+        // remove the corresponding pref entry as it is not needed.
         default_settings_dictionary->RemoveWithoutPathExpansion(dictionary_path,
                                                                 NULL);
       } else {
-        default_content_settings_.settings[content_type] = setting;
+        default_settings_[content_type].reset(value->DeepCopy());
         default_settings_dictionary->SetWithoutPathExpansion(
-            dictionary_path, Value::CreateIntegerValue(setting));
+            dictionary_path, value);
       }
     }
 
     // Keep the obsolete pref in sync as long as backwards compatibility is
     // required. This is required to keep sync working correctly.
     if (content_type == CONTENT_SETTINGS_TYPE_GEOLOCATION) {
-       prefs_->SetInteger(prefs::kGeolocationDefaultContentSetting,
-                          setting == CONTENT_SETTING_DEFAULT ?
-                              kDefaultSettings[content_type] : setting);
+      if (value) {
+        prefs_->Set(prefs::kGeolocationDefaultContentSetting, *value);

[Bernhard Bauer, 2011/11/15 15:40:28]

I'm not happy that we pass ownership of |value| to |default_settings_dictionary|
but still keep using it. That can lead to subtle bugs if we change something and
|value| suddenly gets invalidated from under us.

[markusheintz_, 2011/11/15 17:36:58]

Done.

+      } else {
+        prefs_->ClearPref(prefs::kGeolocationDefaultContentSetting);
+      }
     }
   }
 
@@ -185,6 +199,8 @@ void DefaultProvider::SetContentSetting(
                   ContentSettingsPattern(),
                   content_type,
                   std::string());
+
+  return true;
 }
 
 RuleIterator* DefaultProvider::GetRuleIterator(
@@ -193,8 +209,14 @@ RuleIterator* DefaultProvider::GetRuleIterator(
     bool incognito) const {
   base::AutoLock lock(lock_);
   if (resource_identifier.empty()) {
-    return new DefaultRuleIterator(
-        default_content_settings_.settings[content_type]);
+    int int_value = 0;
+    ValueMap::const_iterator it(default_settings_.find(content_type));
+    if (it != default_settings_.end()) {
+      it->second->GetAsInteger(&int_value);
+    } else {
+      NOTREACHED();
+    }
+    return new DefaultRuleIterator(ContentSetting(int_value));
   } else {
     return new EmptyRuleIterator();
   }
@@ -255,76 +277,87 @@ void DefaultProvider::ReadDefaultSettings(bool overwrite) {
       prefs_->GetDictionary(prefs::kDefaultContentSettings);
 
   if (overwrite)
-    default_content_settings_ = ContentSettings();
+    default_settings_.clear();
 
   // Careful: The returned value could be NULL if the pref has never been set.
-  if (default_settings_dictionary) {
-    GetSettingsFromDictionary(default_settings_dictionary,
-                              &default_content_settings_);
-  }
+  if (default_settings_dictionary)
+    GetSettingsFromDictionary(default_settings_dictionary);
+
   ForceDefaultsToBeExplicit();
 }
 
 void DefaultProvider::ForceDefaultsToBeExplicit() {
   for (int i = 0; i < CONTENT_SETTINGS_NUM_TYPES; ++i) {
-    if (default_content_settings_.settings[i] == CONTENT_SETTING_DEFAULT)
-      default_content_settings_.settings[i] = kDefaultSettings[i];
+    ContentSettingsType type = ContentSettingsType(i);
+    if (!default_settings_[type].get())
+      default_settings_[type].reset(
+          Value::CreateIntegerValue(kDefaultSettings[i]));
   }
 }
 
 void DefaultProvider::GetSettingsFromDictionary(
-    const DictionaryValue* dictionary,
-    ContentSettings* settings) {
+    const DictionaryValue* dictionary) {
   for (DictionaryValue::key_iterator i(dictionary->begin_keys());
        i != dictionary->end_keys(); ++i) {
     const std::string& content_type(*i);
     for (size_t type = 0; type < CONTENT_SETTINGS_NUM_TYPES; ++type) {
       if (content_type == GetTypeName(ContentSettingsType(type))) {
-        int setting = CONTENT_SETTING_DEFAULT;
+        int int_value = CONTENT_SETTING_DEFAULT;
         bool found = dictionary->GetIntegerWithoutPathExpansion(content_type,
-                                                                &setting);
+                                                                &int_value);
         DCHECK(found);
-        settings->settings[type] = IntToContentSetting(setting);
+        default_settings_[ContentSettingsType(type)].reset(
+            Value::CreateIntegerValue(int_value));
         break;
       }
     }
   }
-  // Migrate obsolete cookie prompt mode/
-  if (settings->settings[CONTENT_SETTINGS_TYPE_COOKIES] ==
-      CONTENT_SETTING_ASK)
-    settings->settings[CONTENT_SETTINGS_TYPE_COOKIES] = CONTENT_SETTING_BLOCK;
-
-  settings->settings[CONTENT_SETTINGS_TYPE_PLUGINS] =
-      ClickToPlayFixup(CONTENT_SETTINGS_TYPE_PLUGINS,
-                       settings->settings[CONTENT_SETTINGS_TYPE_PLUGINS]);
+  // Migrate obsolete cookie prompt mode.
+  if (ValueToContentSetting(
+          default_settings_[CONTENT_SETTINGS_TYPE_COOKIES].get()) ==
+              CONTENT_SETTING_ASK) {
+    default_settings_[CONTENT_SETTINGS_TYPE_COOKIES].reset(
+        Value::CreateIntegerValue(CONTENT_SETTING_BLOCK));
+  }
+
+  if (default_settings_[CONTENT_SETTINGS_TYPE_PLUGINS].get()) {
+    ContentSetting plugin_setting = ValueToContentSetting(
+        default_settings_[CONTENT_SETTINGS_TYPE_PLUGINS].get());
+    plugin_setting =
+        ClickToPlayFixup(CONTENT_SETTINGS_TYPE_PLUGINS, plugin_setting);
+    default_settings_[CONTENT_SETTINGS_TYPE_PLUGINS].reset(
+        Value::CreateIntegerValue(plugin_setting));
+  }
 }
 
 void DefaultProvider::MigrateObsoleteNotificationPref() {
   if (prefs_->HasPrefPath(prefs::kDesktopNotificationDefaultContentSetting)) {
-    ContentSetting setting = IntToContentSetting(
-        prefs_->GetInteger(prefs::kDesktopNotificationDefaultContentSetting));
-    SetContentSetting(
+    const base::Value* value = prefs_->FindPreference(
+        prefs::kDesktopNotificationDefaultContentSetting)->GetValue();
+    // Do not clear the old preference yet as long as we need to maintain
+    // backward compatibility.
+    SetWebsiteSetting(
         ContentSettingsPattern::Wildcard(),
         ContentSettingsPattern::Wildcard(),
         CONTENT_SETTINGS_TYPE_NOTIFICATIONS,
         std::string(),
-        setting);
+        value->DeepCopy());
     prefs_->ClearPref(prefs::kDesktopNotificationDefaultContentSetting);
   }
 }
 
 void DefaultProvider::MigrateObsoleteGeolocationPref() {
   if (prefs_->HasPrefPath(prefs::kGeolocationDefaultContentSetting)) {
-    ContentSetting setting = IntToContentSetting(
-        prefs_->GetInteger(prefs::kGeolocationDefaultContentSetting));
+    const base::Value* value = prefs_->FindPreference(
+        prefs::kGeolocationDefaultContentSetting)->GetValue();
     // Do not clear the old preference yet as long as we need to maintain
     // backward compatibility.
-    SetContentSetting(
+    SetWebsiteSetting(
         ContentSettingsPattern::Wildcard(),
         ContentSettingsPattern::Wildcard(),
         CONTENT_SETTINGS_TYPE_GEOLOCATION,
         std::string(),
-        setting);
+        value->DeepCopy());
   }
 }