Revert 44036 [unit_tests failed] - Since the host content settings preferences can be updated by the sync backend,

make sure HostContentSettingsMap correctly fires off notifications whenever
it modifies them, and reloads them when they are modified.

BUG=none
TEST=HostContentSettingsMapTest.*

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

TBR=albertb@chromium.org
Review URL: http://codereview.chromium.org/1589026

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

3 files changed, 35 insertions, 130 deletions

diff --git a/chrome/browser/host_content_settings_map.cc b/chrome/browser/host_content_settings_map.cc
index bd30cf7..fd7517a 100644
--- a/chrome/browser/host_content_settings_map.cc
+++ b/chrome/browser/host_content_settings_map.cc
@@ -9,7 +9,6 @@
 #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"
@@ -103,8 +102,7 @@ const ContentSetting
 
 HostContentSettingsMap::HostContentSettingsMap(Profile* profile)
     : profile_(profile),
-      block_third_party_cookies_(false),
-      updating_preferences_(false) {
+      block_third_party_cookies_(false) {
   PrefService* prefs = profile_->GetPrefs();
 
   // Migrate obsolete cookie pref.
@@ -162,7 +160,14 @@ HostContentSettingsMap::HostContentSettingsMap(Profile* profile)
   // Read global defaults.
   DCHECK_EQ(arraysize(kTypeNames),
             static_cast<size_t>(CONTENT_SETTINGS_NUM_TYPES));
-  ReadDefaultSettings(false);
+  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 misc. global settings.
   block_third_party_cookies_ =
@@ -180,13 +185,24 @@ HostContentSettingsMap::HostContentSettingsMap(Profile* profile)
   }
 
   // Read exceptions.
-  ReadExceptions(false);
-
-  prefs->AddPrefObserver(prefs::kDefaultContentSettings, this);
-  prefs->AddPrefObserver(prefs::kContentSettingsPatterns, this);
-  prefs->AddPrefObserver(prefs::kBlockThirdPartyCookies, this);
-  notification_registrar_.Add(this, NotificationType::PROFILE_DESTROYED,
-                              NotificationService::AllSources());
+  const DictionaryValue* all_settings_dictionary =
+      prefs->GetMutableDictionary(prefs::kContentSettingsPatterns);
+  // 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_pattern(*i);
+      if (!Pattern(WideToUTF8(wide_pattern)).IsValid())
+        LOG(WARNING) << "Invalid pattern stored in content settings";
+      DictionaryValue* pattern_settings_dictionary = NULL;
+      bool found = all_settings_dictionary->GetDictionaryWithoutPathExpansion(
+          wide_pattern, &pattern_settings_dictionary);
+      DCHECK(found);
+      ContentSettings settings;
+      GetSettingsFromDictionary(pattern_settings_dictionary, &settings);
+      host_content_settings_[WideToUTF8(wide_pattern)] = settings;
+    }
+  }
 }
 
 // static
@@ -279,15 +295,13 @@ 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 =
-      prefs->GetMutableDictionary(prefs::kDefaultContentSettings);
+      profile_->GetPrefs()->GetMutableDictionary(
+          prefs::kDefaultContentSettings);
   std::wstring dictionary_path(kTypeNames[content_type]);
-  updating_preferences_ = true;
   {
     AutoLock auto_lock(lock_);
-    ScopedPrefUpdate update(prefs, prefs::kDefaultContentSettings);
     if ((setting == CONTENT_SETTING_DEFAULT) ||
         (setting == kDefaultSettings[content_type])) {
       default_content_settings_.settings[content_type] =
@@ -300,7 +314,6 @@ void HostContentSettingsMap::SetDefaultContentSetting(
           dictionary_path, Value::CreateIntegerValue(setting));
     }
   }
-  updating_preferences_ = false;
 
   NotifyObservers(ContentSettingsDetails(true));
 }
@@ -310,12 +323,12 @@ void HostContentSettingsMap::SetContentSetting(const Pattern& pattern,
                                                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_pattern(UTF8ToWide(pattern.AsString()));
   DictionaryValue* all_settings_dictionary =
-      prefs->GetMutableDictionary(prefs::kContentSettingsPatterns);
+      profile_->GetPrefs()->GetMutableDictionary(
+          prefs::kContentSettingsPatterns);
   {
     AutoLock auto_lock(lock_);
     if (!host_content_settings_.count(pattern.AsString()))
@@ -354,30 +367,22 @@ void HostContentSettingsMap::SetContentSetting(const Pattern& pattern,
     }
   }
 
-  updating_preferences_ = true;
-  {
-    ScopedPrefUpdate update(prefs, prefs::kContentSettingsPatterns);
-  }
-  updating_preferences_ = false;
-
   NotifyObservers(ContentSettingsDetails(pattern));
 }
 
 void HostContentSettingsMap::ClearSettingsForOneType(
     ContentSettingsType content_type) {
   DCHECK(kTypeNames[content_type] != NULL);  // Don't call this for Geolocation.
-  PrefService* prefs = profile_->GetPrefs();
-  updating_preferences_ = true;
   {
     AutoLock auto_lock(lock_);
-    ScopedPrefUpdate update(prefs, prefs::kContentSettingsPatterns);
     for (HostContentSettings::iterator i(host_content_settings_.begin());
          i != host_content_settings_.end(); ) {
       if (i->second.settings[content_type] != CONTENT_SETTING_DEFAULT) {
         i->second.settings[content_type] = CONTENT_SETTING_DEFAULT;
         std::wstring wide_host(UTF8ToWide(i->first));
         DictionaryValue* all_settings_dictionary =
-            prefs->GetMutableDictionary(prefs::kContentSettingsPatterns);
+            profile_->GetPrefs()->GetMutableDictionary(
+                prefs::kContentSettingsPatterns);
         if (AllDefault(i->second)) {
           all_settings_dictionary->RemoveWithoutPathExpansion(wide_host, NULL);
           host_content_settings_.erase(i++);
@@ -396,7 +401,6 @@ void HostContentSettingsMap::ClearSettingsForOneType(
       }
     }
   }
-  updating_preferences_ = false;
 
   NotifyObservers(ContentSettingsDetails(true));
 }
@@ -428,52 +432,13 @@ void HostContentSettingsMap::ResetToDefaults() {
   }
 
   PrefService* prefs = profile_->GetPrefs();
-  updating_preferences_ = true;
   prefs->ClearPref(prefs::kDefaultContentSettings);
   prefs->ClearPref(prefs::kContentSettingsPatterns);
   prefs->ClearPref(prefs::kBlockThirdPartyCookies);
-  updating_preferences_ = false;
 
   NotifyObservers(ContentSettingsDetails(true));
 }
 
-void HostContentSettingsMap::Observe(NotificationType type,
-                                     const NotificationSource& source,
-                                     const NotificationDetails& details) {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
-
-  if (NotificationType::PREF_CHANGED == type) {
-    if (updating_preferences_)
-      return;
-
-    std::wstring* name = Details<std::wstring>(details).ptr();
-    if (prefs::kDefaultContentSettings == *name) {
-      ReadDefaultSettings(true);
-    } else if (prefs::kContentSettingsPatterns == *name) {
-      ReadExceptions(true);
-    } else if (prefs::kBlockThirdPartyCookies == *name) {
-      updating_preferences_ = true;
-      SetBlockThirdPartyCookies(profile_->GetPrefs()->GetBoolean(
-          prefs::kBlockThirdPartyCookies));
-      updating_preferences_ = false;
-    } else {
-      NOTREACHED() << "Unexpected preference observed";
-      return;
-    }
-
-    NotifyObservers(ContentSettingsDetails(true));
-  } else if (NotificationType::PROFILE_DESTROYED == type) {
-    PrefService* prefs = profile_->GetPrefs();
-    prefs->RemovePrefObserver(prefs::kDefaultContentSettings, this);
-    prefs->RemovePrefObserver(prefs::kContentSettingsPatterns, this);
-    prefs->RemovePrefObserver(prefs::kBlockThirdPartyCookies, this);
-    notification_registrar_.Remove(this, NotificationType::PROFILE_DESTROYED,
-                                   NotificationService::AllSources());
-  } else {
-    NOTREACHED() << "Unexpected notification";
-  }
-}
-
 HostContentSettingsMap::~HostContentSettingsMap() {
 }
 
@@ -524,44 +489,6 @@ bool HostContentSettingsMap::AllDefault(const ContentSettings& settings) const {
   return true;
 }
 
-void HostContentSettingsMap::ReadDefaultSettings(bool overwrite) {
-  PrefService* prefs = profile_->GetPrefs();
-  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) {
-    if (overwrite)
-      default_content_settings_ = ContentSettings();
-    GetSettingsFromDictionary(default_settings_dictionary,
-                              &default_content_settings_);
-  }
-  ForceDefaultsToBeExplicit();
-}
-
-void HostContentSettingsMap::ReadExceptions(bool overwrite) {
-  PrefService* prefs = profile_->GetPrefs();
-  const DictionaryValue* all_settings_dictionary =
-      prefs->GetMutableDictionary(prefs::kContentSettingsPatterns);
-  // 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_pattern(*i);
-      if (!Pattern(WideToUTF8(wide_pattern)).IsValid())
-        LOG(WARNING) << "Invalid pattern stored in content settings";
-      DictionaryValue* pattern_settings_dictionary = NULL;
-      bool found = all_settings_dictionary->GetDictionaryWithoutPathExpansion(
-          wide_pattern, &pattern_settings_dictionary);
-      DCHECK(found);
-      ContentSettings settings;
-      GetSettingsFromDictionary(pattern_settings_dictionary, &settings);
-      host_content_settings_[WideToUTF8(wide_pattern)] = settings;
-    }
-  }
-}
-
 void HostContentSettingsMap::NotifyObservers(
     const ContentSettingsDetails& details) {
   NotificationService::current()->Notify(
diff --git a/chrome/browser/host_content_settings_map.h b/chrome/browser/host_content_settings_map.h
index 97a1c1b..babc769 100644
--- a/chrome/browser/host_content_settings_map.h
+++ b/chrome/browser/host_content_settings_map.h
@@ -17,8 +17,6 @@
 #include "base/lock.h"
 #include "base/ref_counted.h"
 #include "chrome/common/content_settings.h"
-#include "chrome/common/notification_observer.h"
-#include "chrome/common/notification_registrar.h"
 
 class DictionaryValue;
 class GURL;
@@ -26,8 +24,7 @@ class PrefService;
 class Profile;
 
 class HostContentSettingsMap
-    : public NotificationObserver,
-      public base::RefCountedThreadSafe<HostContentSettingsMap> {
+    : public base::RefCountedThreadSafe<HostContentSettingsMap> {
  public:
   // A hostname pattern. See |IsValid| for a description of possible patterns.
   class Pattern {
@@ -151,11 +148,6 @@ class HostContentSettingsMap
   // This should only be called on the UI thread.
   void ResetToDefaults();
 
-  // NotificationObserver implementation.
-  virtual void Observe(NotificationType type,
-                       const NotificationSource& source,
-                       const NotificationDetails& details);
-
  private:
   friend class base::RefCountedThreadSafe<HostContentSettingsMap>;
 
@@ -185,14 +177,6 @@ class HostContentSettingsMap
   // Returns true if |settings| consists entirely of CONTENT_SETTING_DEFAULT.
   bool AllDefault(const ContentSettings& settings) const;
 
-  // Reads the default settings from the prefereces service. If |overwrite| is
-  // true and the preference is missing, the local copy will be cleared as well.
-  void ReadDefaultSettings(bool overwrite);
-
-  // Reads the host exceptions from the prefereces service. If |overwrite| is
-  // true and the preference is missing, the local copy will be cleared as well.
-  void ReadExceptions(bool overwrite);
-
   // Informs observers that content settings have changed. Make sure that
   // |lock_| is not held when calling this, as listeners will usually call one
   // of the GetSettings functions in response, which would then lead to a
@@ -202,8 +186,6 @@ class HostContentSettingsMap
   // The profile we're associated with.
   Profile* profile_;
 
-  NotificationRegistrar notification_registrar_;
-
   // Copies of the pref data, so that we can read it on the IO thread.
   ContentSettings default_content_settings_;
   HostContentSettings host_content_settings_;
@@ -214,10 +196,6 @@ class HostContentSettingsMap
   // Used around accesses to the settings objects to guarantee thread safety.
   mutable Lock lock_;
 
-  // Whether we are currently updating preferences, this is used to ignore
-  // notifications from the preferences service that we triggered ourself.
-  bool updating_preferences_;
-
   DISALLOW_COPY_AND_ASSIGN(HostContentSettingsMap);
 };
 
diff --git a/chrome/browser/sync/glue/synchronized_preferences.h b/chrome/browser/sync/glue/synchronized_preferences.h
index 556720a..ecc16bf 100644
--- a/chrome/browser/sync/glue/synchronized_preferences.h
+++ b/chrome/browser/sync/glue/synchronized_preferences.h
@@ -52,7 +52,7 @@ static const wchar_t* kSynchronizedPreferences[] = {
   prefs::kNTPPromoImageRemaining,
   prefs::kNTPPromoLineRemaining,
   prefs::kPasswordManagerEnabled,
-  prefs::kContentSettingsPatterns,
+  prefs::kPerHostContentSettings,
   prefs::kPerHostZoomLevels,
   prefs::kPinnedTabs,
   prefs::kPrintingPageFooterCenter,