Explicitly ShutdownOnUIThread the HostContentSettingsMap when destroying the Profile.

chrome/browser/content_settings/content_settings_pref_provider.cc

Index: chrome/browser/content_settings/content_settings_pref_provider.cc
diff --git a/chrome/browser/content_settings/content_settings_pref_provider.cc b/chrome/browser/content_settings/content_settings_pref_provider.cc
index 3d2805edd1f684e2005966da7d46d38031b213d6..93879539cd89c0680feac380e283e34e4aab8c0c 100644
--- a/chrome/browser/content_settings/content_settings_pref_provider.cc
+++ b/chrome/browser/content_settings/content_settings_pref_provider.cc
@@ -9,6 +9,7 @@
 #include <utility>
 #include <vector>
 
+#include "base/auto_reset.h"
 #include "base/command_line.h"
 #include "base/metrics/histogram.h"
 #include "chrome/browser/content_settings/content_settings_details.h"
@@ -148,14 +149,14 @@ ContentSetting FixObsoleteCookiePromptMode(ContentSettingsType content_type,
 
 namespace content_settings {
 
-PrefDefaultProvider::PrefDefaultProvider(Profile* profile)
-    : profile_(profile),
-      is_incognito_(profile_->IsOffTheRecord()),
+PrefDefaultProvider::PrefDefaultProvider(HostContentSettingsMap* map,
+                                         PrefService* prefs,
+                                         bool incognito)
+    : map_(map),
+      prefs_(prefs),
+      is_incognito_(incognito),
       updating_preferences_(false) {
-  initializing_ = true;
-  PrefService* prefs = profile->GetPrefs();
-
-  MigrateObsoleteNotificationPref(prefs);
+  MigrateObsoleteNotificationPref();
 
   // Read global defaults.
   DCHECK_EQ(arraysize(kTypeNames),
@@ -170,15 +171,12 @@ PrefDefaultProvider::PrefDefaultProvider(Profile* profile)
         UserMetricsAction("CookieBlockingDisabledPerDefault"));
   }
 
-  pref_change_registrar_.Init(prefs);
+  pref_change_registrar_.Init(prefs_);
   pref_change_registrar_.Add(prefs::kDefaultContentSettings, this);
-  notification_registrar_.Add(this, NotificationType::PROFILE_DESTROYED,
-                              Source<Profile>(profile_));
-  initializing_ = false;
 }
 
 PrefDefaultProvider::~PrefDefaultProvider() {
-  UnregisterObservers();
+  DCHECK(!prefs_);
 }
 
 ContentSetting PrefDefaultProvider::ProvideDefaultSetting(
@@ -198,13 +196,11 @@ void PrefDefaultProvider::UpdateDefaultSetting(
   if (is_incognito_)
     return;
 
-  PrefService* prefs = profile_->GetPrefs();
-
   std::string dictionary_path(kTypeNames[content_type]);
   updating_preferences_ = true;
   {
     base::AutoLock lock(lock_);
-    DictionaryPrefUpdate update(prefs, prefs::kDefaultContentSettings);
+    DictionaryPrefUpdate update(prefs_, prefs::kDefaultContentSettings);
     DictionaryValue* default_settings_dictionary = update.Get();
     if ((setting == CONTENT_SETTING_DEFAULT) ||
         (setting == kDefaultSettings[content_type])) {
@@ -240,9 +236,8 @@ void PrefDefaultProvider::ResetToDefaults() {
   ForceDefaultsToBeExplicit();
 
   if (!is_incognito_) {
-    PrefService* prefs = profile_->GetPrefs();
     updating_preferences_ = true;
-    prefs->ClearPref(prefs::kDefaultContentSettings);
+    prefs_->ClearPref(prefs::kDefaultContentSettings);
     updating_preferences_ = false;
   }
 }
@@ -253,7 +248,7 @@ void PrefDefaultProvider::Observe(NotificationType type,
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   if (type == NotificationType::PREF_CHANGED) {
-    DCHECK_EQ(profile_->GetPrefs(), Source<PrefService>(source).ptr());
+    DCHECK_EQ(prefs_, Source<PrefService>(source).ptr());
     if (updating_preferences_)
       return;
 
@@ -265,35 +260,28 @@ void PrefDefaultProvider::Observe(NotificationType type,
       return;
     }
 
-    if (!is_incognito_) {
-      ContentSettingsDetails details(ContentSettingsPattern(),
-                                     ContentSettingsPattern(),
-                                     CONTENT_SETTINGS_TYPE_DEFAULT,
-                                     std::string());
-      NotifyObservers(details);
-    }
-  } else if (type == NotificationType::PROFILE_DESTROYED) {
-    DCHECK_EQ(profile_, Source<Profile>(source).ptr());
-    UnregisterObservers();
+    ContentSettingsDetails details(ContentSettingsPattern(),
+                                   ContentSettingsPattern(),
+                                   CONTENT_SETTINGS_TYPE_DEFAULT,
+                                   std::string());
+    NotifyObservers(details);
   } else {
     NOTREACHED() << "Unexpected notification";
   }
 }
 
-void PrefDefaultProvider::UnregisterObservers() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (!profile_)
+void PrefDefaultProvider::ShutdownOnUIThread() {
+  if (!prefs_)
     return;
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   pref_change_registrar_.RemoveAll();
-  notification_registrar_.Remove(this, NotificationType::PROFILE_DESTROYED,
-                                 Source<Profile>(profile_));
-  profile_ = NULL;
+  prefs_ = NULL;
+  map_ = NULL;
 }
 
 void PrefDefaultProvider::ReadDefaultSettings(bool overwrite) {
-  PrefService* prefs = profile_->GetPrefs();
   const DictionaryValue* default_settings_dictionary =
-      prefs->GetDictionary(prefs::kDefaultContentSettings);
+      prefs_->GetDictionary(prefs::kDefaultContentSettings);
 
   base::AutoLock lock(lock_);
 
@@ -348,20 +336,20 @@ void PrefDefaultProvider::GetSettingsFromDictionary(
 void PrefDefaultProvider::NotifyObservers(
     const ContentSettingsDetails& details) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (initializing_ || profile_ == NULL)
+  if (map_ == NULL)
     return;
   NotificationService::current()->Notify(
       NotificationType::CONTENT_SETTINGS_CHANGED,
-      Source<HostContentSettingsMap>(profile_->GetHostContentSettingsMap()),
+      Source<HostContentSettingsMap>(map_),
       Details<const ContentSettingsDetails>(&details));
 }
 
-void PrefDefaultProvider::MigrateObsoleteNotificationPref(PrefService* prefs) {
-  if (prefs->HasPrefPath(prefs::kDesktopNotificationDefaultContentSetting)) {
+void PrefDefaultProvider::MigrateObsoleteNotificationPref() {
+  if (prefs_->HasPrefPath(prefs::kDesktopNotificationDefaultContentSetting)) {
     ContentSetting setting = IntToContentSetting(
-        prefs->GetInteger(prefs::kDesktopNotificationDefaultContentSetting));
+        prefs_->GetInteger(prefs::kDesktopNotificationDefaultContentSetting));
     UpdateDefaultSetting(CONTENT_SETTINGS_TYPE_NOTIFICATIONS, setting);
-    prefs->ClearPref(prefs::kDesktopNotificationDefaultContentSetting);
+    prefs_->ClearPref(prefs::kDesktopNotificationDefaultContentSetting);
   }
 }
 
@@ -408,28 +396,26 @@ void PrefProvider::RegisterUserPrefs(PrefService* prefs) {
                                 PrefService::UNSYNCABLE_PREF);
 }
 
-PrefProvider::PrefProvider(Profile* profile)
-  : profile_(profile),
-    is_incognito_(profile_->IsOffTheRecord()),
+PrefProvider::PrefProvider(HostContentSettingsMap* map,
+                           PrefService* prefs,
+                           bool incognito)
+  : prefs_(prefs),
+    map_(map),
+    is_incognito_(incognito),
     updating_preferences_(false) {
-  Init();
-}
-
-void PrefProvider::Init() {
-  initializing_ = true;
-  PrefService* prefs = profile_->GetPrefs();
-
-  // Migrate obsolete preferences.
-  MigrateObsoletePerhostPref(prefs);
-  MigrateObsoletePopupsPref(prefs);
-  MigrateObsoleteContentSettingsPatternPref(prefs);
+  if (!is_incognito_) {
+    // Migrate obsolete preferences.
+    MigrateObsoletePerhostPref();
+    MigrateObsoletePopupsPref();
+    MigrateObsoleteContentSettingsPatternPref();
+  }
 
   // Verify preferences version.
-  if (!prefs->HasPrefPath(prefs::kContentSettingsVersion)) {
-    prefs->SetInteger(prefs::kContentSettingsVersion,
+  if (!prefs_->HasPrefPath(prefs::kContentSettingsVersion)) {
+    prefs_->SetInteger(prefs::kContentSettingsVersion,
                       ContentSettingsPattern::kContentSettingsPatternVersion);
   }
-  if (prefs->GetInteger(prefs::kContentSettingsVersion) >
+  if (prefs_->GetInteger(prefs::kContentSettingsVersion) >
       ContentSettingsPattern::kContentSettingsPatternVersion) {
     LOG(ERROR) << "Unknown content settings version in preferences.";
     return;
@@ -443,13 +429,9 @@ void PrefProvider::Init() {
                          value_map_.size());
   }
 
-  pref_change_registrar_.Init(prefs);
+  pref_change_registrar_.Init(prefs_);
   pref_change_registrar_.Add(prefs::kContentSettingsPatterns, this);
   pref_change_registrar_.Add(prefs::kContentSettingsPatternPairs, this);
-
-  notification_registrar_.Add(this, NotificationType::PROFILE_DESTROYED,
-                              Source<Profile>(profile_));
-  initializing_ = false;
 }
 
 ContentSetting PrefProvider::GetContentSetting(
@@ -540,7 +522,7 @@ void PrefProvider::SetContentSetting(
   }
 
   // Update the content settings preference.
-  if (!is_incognito_ && profile_) {
+  if (!is_incognito_) {
     UpdatePref(primary_pattern,
                secondary_pattern,
                content_type,
@@ -555,6 +537,7 @@ void PrefProvider::SetContentSetting(
 
 void PrefProvider::ResetToDefaults() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(prefs_);
 
   {
     base::AutoLock auto_lock(lock_);
@@ -563,10 +546,9 @@ void PrefProvider::ResetToDefaults() {
   }
 
   if (!is_incognito_) {
-    PrefService* prefs = profile_->GetPrefs();
     updating_preferences_ = true;
-    prefs->ClearPref(prefs::kContentSettingsPatternPairs);
-    prefs->ClearPref(prefs::kContentSettingsPatterns);
+    prefs_->ClearPref(prefs::kContentSettingsPatternPairs);
+    prefs_->ClearPref(prefs::kContentSettingsPatterns);
     updating_preferences_ = false;
   }
 }
@@ -574,6 +556,7 @@ void PrefProvider::ResetToDefaults() {
 void PrefProvider::ClearAllContentSettingsRules(
     ContentSettingsType content_type) {
   DCHECK(kTypeNames[content_type] != NULL);  // Don't call this for Geolocation.
+  DCHECK(prefs_);
 
   OriginIdentifierValueMap* map_to_modify = &incognito_value_map_;
   if (!is_incognito_)
@@ -613,41 +596,35 @@ void PrefProvider::Observe(
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   if (type == NotificationType::PREF_CHANGED) {
-    DCHECK_EQ(profile_->GetPrefs(), Source<PrefService>(source).ptr());
+    DCHECK_EQ(prefs_, Source<PrefService>(source).ptr());
     if (updating_preferences_)
       return;
 
     std::string* name = Details<std::string>(details).ptr();
     if (*name == prefs::kContentSettingsPatternPairs) {
-      SyncObsoletePref(profile_->GetPrefs());
+      SyncObsoletePref();
       ReadContentSettingsFromPref(true);
     } else if (*name == prefs::kContentSettingsPatterns) {
-      updating_preferences_ = true;
-      MigrateObsoleteContentSettingsPatternPref(profile_->GetPrefs());
-      updating_preferences_ = false;
+      AutoReset<bool> tmp(&updating_preferences_, true);
+      MigrateObsoleteContentSettingsPatternPref();
       ReadContentSettingsFromPref(true);
     } else {
       NOTREACHED() << "Unexpected preference observed";
       return;
     }
 
-    if (!is_incognito_) {
-      ContentSettingsDetails details(ContentSettingsPattern(),
-                                     ContentSettingsPattern(),
-                                     CONTENT_SETTINGS_TYPE_DEFAULT,
-                                     std::string());
-      NotifyObservers(details);
-    }
-  } else if (type == NotificationType::PROFILE_DESTROYED) {
-    DCHECK_EQ(profile_, Source<Profile>(source).ptr());
-    UnregisterObservers();
+    ContentSettingsDetails details(ContentSettingsPattern(),
+                                   ContentSettingsPattern(),
+                                   CONTENT_SETTINGS_TYPE_DEFAULT,
+                                   std::string());
+    NotifyObservers(details);
   } else {
     NOTREACHED() << "Unexpected notification";
   }
 }
 
 PrefProvider::~PrefProvider() {
-  UnregisterObservers();
+  DCHECK(!prefs_);
 }
 
 // ////////////////////////////////////////////////////////////////////////////
@@ -659,7 +636,8 @@ void PrefProvider::UpdatePref(
     ContentSettingsType content_type,
     const ResourceIdentifier& resource_identifier,
     ContentSetting setting) {
-  updating_preferences_ = true;
+  DCHECK(prefs_);
+  AutoReset<bool> tmp(&updating_preferences_, true);
   UpdatePatternPairsPref(primary_pattern,
                          secondary_pattern,
                          content_type,
@@ -670,23 +648,22 @@ void PrefProvider::UpdatePref(
                      content_type,
                      resource_identifier,
                      setting);
-  updating_preferences_ = false;
 }
 
 void PrefProvider::ReadContentSettingsFromPref(bool overwrite) {
+  DCHECK(prefs_);
   base::AutoLock auto_lock(lock_);
 
-  PrefService* prefs = profile_->GetPrefs();
   const DictionaryValue* all_settings_dictionary =
-      prefs->GetDictionary(prefs::kContentSettingsPatternPairs);
+      prefs_->GetDictionary(prefs::kContentSettingsPatternPairs);
 
   if (overwrite)
     value_map_.clear();
 
-  updating_preferences_ = true;
+  AutoReset<bool> tmp(&updating_preferences_, true);
   // Careful: The returned value could be NULL if the pref has never been set.
   if (all_settings_dictionary != NULL) {
-    DictionaryPrefUpdate update(prefs, prefs::kContentSettingsPatternPairs);
+    DictionaryPrefUpdate update(prefs_, prefs::kContentSettingsPatternPairs);
     DictionaryValue* mutable_settings;
     scoped_ptr<DictionaryValue> mutable_settings_scope;
 
@@ -773,7 +750,8 @@ void PrefProvider::UpdatePatternsPref(
       ContentSettingsType content_type,
       const ResourceIdentifier& resource_identifier,
       ContentSetting setting) {
-  DictionaryPrefUpdate update(profile_->GetPrefs(),
+  DCHECK(prefs_);
+  DictionaryPrefUpdate update(prefs_,
                               prefs::kContentSettingsPatterns);
   DictionaryValue* all_settings_dictionary = update.Get();
 
@@ -839,7 +817,8 @@ void PrefProvider::UpdatePatternPairsPref(
       ContentSettingsType content_type,
       const ResourceIdentifier& resource_identifier,
       ContentSetting setting) {
-  DictionaryPrefUpdate update(profile_->GetPrefs(),
+  DCHECK(prefs_);
+  DictionaryPrefUpdate update(prefs_,
                               prefs::kContentSettingsPatternPairs);
   DictionaryValue* all_settings_dictionary = update.Get();
 
@@ -952,29 +931,27 @@ void PrefProvider::CanonicalizeContentSettingsExceptions(
 void PrefProvider::NotifyObservers(
     const ContentSettingsDetails& details) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (initializing_ || profile_ == NULL)
+  if (map_ == NULL)
     return;
   NotificationService::current()->Notify(
       NotificationType::CONTENT_SETTINGS_CHANGED,
-      Source<HostContentSettingsMap>(
-          profile_->GetHostContentSettingsMap()),
+      Source<HostContentSettingsMap>(map_),
       Details<const ContentSettingsDetails>(&details));
 }
 
-void PrefProvider::UnregisterObservers() {
+void PrefProvider::ShutdownOnUIThread() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (!profile_)
+  if (!prefs_)
     return;
   pref_change_registrar_.RemoveAll();
-  notification_registrar_.Remove(this, NotificationType::PROFILE_DESTROYED,
-                                 Source<Profile>(profile_));
-  profile_ = NULL;
+  prefs_ = NULL;
+  map_ = NULL;
 }
 
-void PrefProvider::MigrateObsoletePerhostPref(PrefService* prefs) {
-  if (prefs->HasPrefPath(prefs::kPerHostContentSettings)) {
+void PrefProvider::MigrateObsoletePerhostPref() {
+  if (prefs_->HasPrefPath(prefs::kPerHostContentSettings)) {
     const DictionaryValue* all_settings_dictionary =
-        prefs->GetDictionary(prefs::kPerHostContentSettings);
+        prefs_->GetDictionary(prefs::kPerHostContentSettings);
     DCHECK(all_settings_dictionary);
     for (DictionaryValue::key_iterator
          i(all_settings_dictionary->begin_keys());
@@ -1026,14 +1003,14 @@ void PrefProvider::MigrateObsoletePerhostPref(PrefService* prefs) {
         }
       }
     }
-    prefs->ClearPref(prefs::kPerHostContentSettings);
+    prefs_->ClearPref(prefs::kPerHostContentSettings);
   }
 }
 
-void PrefProvider::MigrateObsoletePopupsPref(PrefService* prefs) {
-  if (prefs->HasPrefPath(prefs::kPopupWhitelistedHosts)) {
+void PrefProvider::MigrateObsoletePopupsPref() {
+  if (prefs_->HasPrefPath(prefs::kPopupWhitelistedHosts)) {
     const ListValue* whitelist_pref =
-        prefs->GetList(prefs::kPopupWhitelistedHosts);
+        prefs_->GetList(prefs::kPopupWhitelistedHosts);
     for (ListValue::const_iterator i(whitelist_pref->begin());
          i != whitelist_pref->end(); ++i) {
       std::string host;
@@ -1044,18 +1021,17 @@ void PrefProvider::MigrateObsoletePopupsPref(PrefService* prefs) {
                         "",
                         CONTENT_SETTING_ALLOW);
     }
-    prefs->ClearPref(prefs::kPopupWhitelistedHosts);
+    prefs_->ClearPref(prefs::kPopupWhitelistedHosts);
   }
 }
 
-void PrefProvider::MigrateObsoleteContentSettingsPatternPref(
-    PrefService* prefs) {
-  if (prefs->HasPrefPath(prefs::kContentSettingsPatterns) &&
-      !is_incognito_) {
+void PrefProvider::MigrateObsoleteContentSettingsPatternPref() {
+  DCHECK(prefs_);
+  if (prefs_->HasPrefPath(prefs::kContentSettingsPatterns)) {
     const DictionaryValue* all_settings_dictionary =
-      prefs->GetDictionary(prefs::kContentSettingsPatterns);
+      prefs_->GetDictionary(prefs::kContentSettingsPatterns);
 
-    DictionaryPrefUpdate update(prefs, prefs::kContentSettingsPatternPairs);
+    DictionaryPrefUpdate update(prefs_, prefs::kContentSettingsPatternPairs);
     DictionaryValue* exceptions_dictionary;
     exceptions_dictionary = update.Get();
     for (DictionaryValue::key_iterator i(all_settings_dictionary->begin_keys());
@@ -1088,14 +1064,15 @@ void PrefProvider::MigrateObsoleteContentSettingsPatternPref(
   }
 }
 
-void PrefProvider::SyncObsoletePref(PrefService* prefs) {
-  updating_preferences_ = true;
-  if (prefs->HasPrefPath(prefs::kContentSettingsPatternPairs) &&
+void PrefProvider::SyncObsoletePref() {
+  DCHECK(prefs_);
+  AutoReset<bool> tmp(&updating_preferences_, true);

[markusheintz_, 2011/07/06 15:56:24]

This is up to you:
Could we use a more descriptive name than tmp?
Maybe auto_reset or something like this?

+  if (prefs_->HasPrefPath(prefs::kContentSettingsPatternPairs) &&
       !is_incognito_) {
     const DictionaryValue* pattern_pairs_dictionary =
-      prefs->GetDictionary(prefs::kContentSettingsPatternPairs);
+      prefs_->GetDictionary(prefs::kContentSettingsPatternPairs);
 
-    DictionaryPrefUpdate update(prefs, prefs::kContentSettingsPatterns);
+    DictionaryPrefUpdate update(prefs_, prefs::kContentSettingsPatterns);
     DictionaryValue* obsolete_settings_dictionary = update.Get();
 
     for (DictionaryValue::key_iterator i =
@@ -1120,7 +1097,6 @@ void PrefProvider::SyncObsoletePref(PrefService* prefs) {
           new_key, dictionary->DeepCopy());
     }
   }
-  updating_preferences_ = false;
 }
 
 }  // namespace content_settings