Use ScopedPrefUpdate to correctly send out notifications when the host

content settnigs are update.

BUG=none
TEST=HostContentSettingsMapTest

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

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

1 files changed, 117 insertions, 66 deletions

diff --git a/chrome/browser/host_content_settings_map.cc b/chrome/browser/host_content_settings_map.cc
index a45dc8e..b812e09 100644
--- a/chrome/browser/host_content_settings_map.cc
+++ b/chrome/browser/host_content_settings_map.cc
@@ -8,6 +8,7 @@
 #include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/pref_service.h"
 #include "chrome/browser/profile.h"
+#include "chrome/browser/scoped_pref_update.h"
 #include "chrome/common/notification_service.h"
 #include "chrome/common/notification_type.h"
 #include "chrome/common/pref_names.h"
@@ -71,39 +72,16 @@ HostContentSettingsMap::HostContentSettingsMap(Profile* profile)
     prefs->ClearPref(prefs::kPopupWhitelistedHosts);
   }
 
-  // Read global defaults.
-  DCHECK_EQ(arraysize(kTypeNames),
-            static_cast<size_t>(CONTENT_SETTINGS_NUM_TYPES));
-  const DictionaryValue* default_settings_dictionary =
-      prefs->GetDictionary(prefs::kDefaultContentSettings);
-  // Careful: The returned value could be NULL if the pref has never been set.
-  if (default_settings_dictionary != NULL) {
-    GetSettingsFromDictionary(default_settings_dictionary,
-                              &default_content_settings_);
-  }
-  ForceDefaultsToBeExplicit();
-
-  // Read host-specific exceptions.
-  const DictionaryValue* all_settings_dictionary =
-      prefs->GetDictionary(prefs::kPerHostContentSettings);
-  // Careful: The returned value could be NULL if the pref has never been set.
-  if (all_settings_dictionary != NULL) {
-    for (DictionaryValue::key_iterator i(all_settings_dictionary->begin_keys());
-         i != all_settings_dictionary->end_keys(); ++i) {
-      std::wstring wide_host(*i);
-      DictionaryValue* host_settings_dictionary = NULL;
-      bool found = all_settings_dictionary->GetDictionaryWithoutPathExpansion(
-          wide_host, &host_settings_dictionary);
-      DCHECK(found);
-      ContentSettings settings;
-      GetSettingsFromDictionary(host_settings_dictionary, &settings);
-      host_content_settings_[WideToUTF8(wide_host)] = settings;
-    }
-  }
+  // Read global defaults and host-speficic exceptions from preferences.
+  ReadDefaultSettings(false);
+  ReadPerHostSettings(false);
 
   // Read misc. global settings.
   block_third_party_cookies_ =
       prefs->GetBoolean(prefs::kBlockThirdPartyCookies);
+
+  prefs->AddPrefObserver(prefs::kDefaultContentSettings, this);
+  prefs->AddPrefObserver(prefs::kPerHostContentSettings, this);
 }
 
 // static
@@ -191,13 +169,16 @@ void HostContentSettingsMap::SetDefaultContentSetting(
     ContentSetting setting) {
   DCHECK(kTypeNames[content_type] != NULL);  // Don't call this for Geolocation.
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
+  PrefService* prefs = profile_->GetPrefs();
 
   DictionaryValue* default_settings_dictionary =
-      profile_->GetPrefs()->GetMutableDictionary(
-          prefs::kDefaultContentSettings);
+      prefs->GetMutableDictionary(prefs::kDefaultContentSettings);
   std::wstring dictionary_path(kTypeNames[content_type]);
+
+  updating_settings_ = true;
   {
     AutoLock auto_lock(lock_);
+    ScopedPrefUpdate update_settings(prefs, prefs::kDefaultContentSettings);
     if ((setting == CONTENT_SETTING_DEFAULT) ||
         (setting == kDefaultSettings[content_type])) {
       default_content_settings_.settings[content_type] =
@@ -210,6 +191,7 @@ void HostContentSettingsMap::SetDefaultContentSetting(
           dictionary_path, Value::CreateIntegerValue(setting));
     }
   }
+  updating_settings_ = false;
 
   NotifyObservers(std::string());
 }
@@ -219,48 +201,54 @@ void HostContentSettingsMap::SetContentSetting(const std::string& host,
                                                ContentSetting setting) {
   DCHECK(kTypeNames[content_type] != NULL);  // Don't call this for Geolocation.
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
+  PrefService* prefs = profile_->GetPrefs();
 
   bool early_exit = false;
   std::wstring wide_host(UTF8ToWide(host));
   DictionaryValue* all_settings_dictionary =
-      profile_->GetPrefs()->GetMutableDictionary(
-          prefs::kPerHostContentSettings);
+      prefs->GetMutableDictionary(prefs::kPerHostContentSettings);
+
+  updating_settings_ = true;
   {
-    AutoLock auto_lock(lock_);
-    if (!host_content_settings_.count(host))
-      host_content_settings_[host] = ContentSettings();
-    HostContentSettings::iterator i(host_content_settings_.find(host));
-    ContentSettings& settings = i->second;
-    settings.settings[content_type] = setting;
-    if (AllDefault(settings)) {
-      host_content_settings_.erase(i);
-      all_settings_dictionary->RemoveWithoutPathExpansion(wide_host, NULL);
-
-      // We can't just return because |NotifyObservers()| needs to be called,
-      // without |lock_| being held.
-      early_exit = true;
+    ScopedPrefUpdate update_settings(prefs, prefs::kPerHostContentSettings);
+    {
+      AutoLock auto_lock(lock_);
+      if (!host_content_settings_.count(host))
+        host_content_settings_[host] = ContentSettings();
+      HostContentSettings::iterator i(host_content_settings_.find(host));
+      ContentSettings& settings = i->second;
+      settings.settings[content_type] = setting;
+      if (AllDefault(settings)) {
+        host_content_settings_.erase(i);
+        all_settings_dictionary->RemoveWithoutPathExpansion(wide_host, NULL);
+
+        // We can't just return because |NotifyObservers()| needs to be called,
+        // without |lock_| being held.
+        early_exit = true;
+      }
     }
-  }
 
-  if (!early_exit) {
-    DictionaryValue* host_settings_dictionary;
-    bool found = all_settings_dictionary->GetDictionaryWithoutPathExpansion(
-        wide_host, &host_settings_dictionary);
-    if (!found) {
-      host_settings_dictionary = new DictionaryValue;
-      all_settings_dictionary->SetWithoutPathExpansion(
-          wide_host, host_settings_dictionary);
-      DCHECK_NE(setting, CONTENT_SETTING_DEFAULT);
-    }
-    std::wstring dictionary_path(kTypeNames[content_type]);
-    if (setting == CONTENT_SETTING_DEFAULT) {
-      host_settings_dictionary->RemoveWithoutPathExpansion(dictionary_path,
-                                                           NULL);
-    } else {
-      host_settings_dictionary->SetWithoutPathExpansion(
-          dictionary_path, Value::CreateIntegerValue(setting));
+    if (!early_exit) {
+      DictionaryValue* host_settings_dictionary;
+      bool found = all_settings_dictionary->GetDictionaryWithoutPathExpansion(
+          wide_host, &host_settings_dictionary);
+      if (!found) {
+        host_settings_dictionary = new DictionaryValue;
+        all_settings_dictionary->SetWithoutPathExpansion(
+            wide_host, host_settings_dictionary);
+        DCHECK_NE(setting, CONTENT_SETTING_DEFAULT);
+      }
+      std::wstring dictionary_path(kTypeNames[content_type]);
+      if (setting == CONTENT_SETTING_DEFAULT) {
+        host_settings_dictionary->RemoveWithoutPathExpansion(dictionary_path,
+                                                             NULL);
+      } else {
+        host_settings_dictionary->SetWithoutPathExpansion(
+            dictionary_path, Value::CreateIntegerValue(setting));
+      }
     }
   }
+  updating_settings_ = false;
 
   NotifyObservers(host);
 }
@@ -268,6 +256,9 @@ void HostContentSettingsMap::SetContentSetting(const std::string& host,
 void HostContentSettingsMap::ClearSettingsForOneType(
     ContentSettingsType content_type) {
   DCHECK(kTypeNames[content_type] != NULL);  // Don't call this for Geolocation.
+  PrefService* prefs = profile_->GetPrefs();
+
+  updating_settings_ = true;
   {
     AutoLock auto_lock(lock_);
     for (HostContentSettings::iterator i(host_content_settings_.begin());
@@ -276,8 +267,8 @@ void HostContentSettingsMap::ClearSettingsForOneType(
         i->second.settings[content_type] = CONTENT_SETTING_DEFAULT;
         std::wstring wide_host(UTF8ToWide(i->first));
         DictionaryValue* all_settings_dictionary =
-            profile_->GetPrefs()->GetMutableDictionary(
-                prefs::kPerHostContentSettings);
+            prefs->GetMutableDictionary(prefs::kPerHostContentSettings);
+        ScopedPrefUpdate update_settings(prefs, prefs::kPerHostContentSettings);
         if (AllDefault(i->second)) {
           all_settings_dictionary->RemoveWithoutPathExpansion(wide_host, NULL);
           host_content_settings_.erase(i++);
@@ -296,6 +287,7 @@ void HostContentSettingsMap::ClearSettingsForOneType(
       }
     }
   }
+  updating_settings_ = true;
 
   NotifyObservers(std::string());
 }
@@ -327,16 +319,75 @@ void HostContentSettingsMap::ResetToDefaults() {
   }
 
   PrefService* prefs = profile_->GetPrefs();
+  updating_settings_ = true;
   prefs->ClearPref(prefs::kDefaultContentSettings);
   prefs->ClearPref(prefs::kPerHostContentSettings);
   prefs->ClearPref(prefs::kBlockThirdPartyCookies);
+  updating_settings_ = false;
 
   NotifyObservers(std::string());
 }
 
+void HostContentSettingsMap::Observe(NotificationType type,
+                                     const NotificationSource& source,
+                                     const NotificationDetails& details) {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
+  DCHECK(NotificationType::PREF_CHANGED == type);
+  DCHECK_EQ(profile_->GetPrefs(), Source<PrefService>(source).ptr());
+  if (updating_settings_)
+    return;
+
+  std::wstring* name = Details<std::wstring>(details).ptr();
+  if (prefs::kDefaultContentSettings == *name) {
+    ReadDefaultSettings(true);
+    NotifyObservers(std::string());
+  } else if (prefs::kPerHostContentSettings == *name) {
+    ReadPerHostSettings(true);
+    NotifyObservers(std::string());
+  } else {
+    NOTREACHED() << "Unexpected preference observed.";
+  }
+}
+
 HostContentSettingsMap::~HostContentSettingsMap() {
 }
 
+void HostContentSettingsMap::ReadDefaultSettings(bool overwrite) {
+  DCHECK_EQ(arraysize(kTypeNames),
+            static_cast<size_t>(CONTENT_SETTINGS_NUM_TYPES));
+  const DictionaryValue* default_settings_dictionary =
+      profile_->GetPrefs()->GetDictionary(prefs::kDefaultContentSettings);
+  AutoLock auto_lock(lock_);
+  // Careful: The returned value could be NULL if the pref has never been set.
+  if (default_settings_dictionary != NULL) {
+    if (overwrite) default_content_settings_ = ContentSettings();
+    GetSettingsFromDictionary(default_settings_dictionary,
+                              &default_content_settings_);
+  }
+  ForceDefaultsToBeExplicit();
+}
+
+void HostContentSettingsMap::ReadPerHostSettings(bool overwrite) {
+  const DictionaryValue* all_settings_dictionary =
+      profile_->GetPrefs()->GetDictionary(prefs::kPerHostContentSettings);
+  AutoLock auto_lock(lock_);
+  // Careful: The returned value could be NULL if the pref has never been set.
+  if (all_settings_dictionary != NULL) {
+    if (overwrite) host_content_settings_.clear();
+    for (DictionaryValue::key_iterator i(all_settings_dictionary->begin_keys());
+         i != all_settings_dictionary->end_keys(); ++i) {
+      std::wstring wide_host(*i);
+      DictionaryValue* host_settings_dictionary = NULL;
+      bool found = all_settings_dictionary->GetDictionaryWithoutPathExpansion(
+          wide_host, &host_settings_dictionary);
+      DCHECK(found);
+      ContentSettings settings;
+      GetSettingsFromDictionary(host_settings_dictionary, &settings);
+      host_content_settings_[WideToUTF8(wide_host)] = settings;
+    }
+  }
+}
+
 // static
 bool HostContentSettingsMap::ShouldAllowAllContent(const GURL& url) {
   return url.SchemeIs(chrome::kChromeInternalScheme) ||