diff options
21 files changed, 347 insertions, 394 deletions
diff --git a/base/values.cc b/base/values.cc index 0f30e379..adfc9a8 100644 --- a/base/values.cc +++ b/base/values.cc @@ -123,6 +123,10 @@ bool Value::GetAsList(ListValue** out_value) { return false; } +bool Value::GetAsList(const ListValue** out_value) const { + return false; +} + Value* Value::DeepCopy() const { // This method should only be getting called for null Values--all subclasses // need to provide their own implementation;. @@ -866,6 +870,12 @@ bool ListValue::GetAsList(ListValue** out_value) { return true; } +bool ListValue::GetAsList(const ListValue** out_value) const { + if (out_value) + *out_value = this; + return true; +} + ListValue* ListValue::DeepCopy() const { ListValue* result = new ListValue; diff --git a/base/values.h b/base/values.h index 43894d1..a00a17c 100644 --- a/base/values.h +++ b/base/values.h @@ -94,6 +94,7 @@ class BASE_API Value { virtual bool GetAsString(std::string* out_value) const; virtual bool GetAsString(string16* out_value) const; virtual bool GetAsList(ListValue** out_value); + virtual bool GetAsList(const ListValue** out_value) const; // This creates a deep copy of the entire Value tree, and returns a pointer // to the copy. The caller gets ownership of the copy, of course. @@ -427,6 +428,7 @@ class BASE_API ListValue : public Value { // Overridden from Value: virtual bool GetAsList(ListValue** out_value); + virtual bool GetAsList(const ListValue** out_value) const; virtual ListValue* DeepCopy() const; virtual bool Equals(const Value* other) const; diff --git a/chrome/browser/content_settings/content_settings_mock_provider.cc b/chrome/browser/content_settings/content_settings_mock_provider.cc index d2eaaa3..78751d8 100644 --- a/chrome/browser/content_settings/content_settings_mock_provider.cc +++ b/chrome/browser/content_settings/content_settings_mock_provider.cc @@ -37,6 +37,9 @@ bool MockDefaultProvider::DefaultSettingIsManaged( return content_type == content_type_ && is_managed_; } +void MockDefaultProvider::ShutdownOnUIThread() { +} + MockProvider::MockProvider() : requesting_url_pattern_(ContentSettingsPattern()), embedding_url_pattern_(ContentSettingsPattern()), diff --git a/chrome/browser/content_settings/content_settings_mock_provider.h b/chrome/browser/content_settings/content_settings_mock_provider.h index dbf006ce..3962dea 100644 --- a/chrome/browser/content_settings/content_settings_mock_provider.h +++ b/chrome/browser/content_settings/content_settings_mock_provider.h @@ -29,6 +29,7 @@ class MockDefaultProvider : public DefaultProviderInterface { virtual void UpdateDefaultSetting(ContentSettingsType content_type, ContentSetting setting); virtual bool DefaultSettingIsManaged(ContentSettingsType content_type) const; + virtual void ShutdownOnUIThread(); private: ContentSettingsType content_type_; @@ -76,6 +77,8 @@ class MockProvider : public ProviderInterface { virtual void ClearAllContentSettingsRules( ContentSettingsType content_type) {} + virtual void ShutdownOnUIThread() {} + // Accessors void set_requesting_url_pattern(ContentSettingsPattern pattern) { requesting_url_pattern_ = pattern; diff --git a/chrome/browser/content_settings/content_settings_notification_provider.cc b/chrome/browser/content_settings/content_settings_notification_provider.cc index 1c71b4a..04fb00a 100644 --- a/chrome/browser/content_settings/content_settings_notification_provider.cc +++ b/chrome/browser/content_settings/content_settings_notification_provider.cc @@ -156,6 +156,9 @@ void NotificationProvider::ClearAllContentSettingsRules( ResetAllOrigins(); } +void NotificationProvider::ShutdownOnUIThread() { +} + void NotificationProvider::Observe(int type, const NotificationSource& source, const NotificationDetails& details) { diff --git a/chrome/browser/content_settings/content_settings_notification_provider.h b/chrome/browser/content_settings/content_settings_notification_provider.h index 69055f8..1f9caf7 100644 --- a/chrome/browser/content_settings/content_settings_notification_provider.h +++ b/chrome/browser/content_settings/content_settings_notification_provider.h @@ -55,6 +55,8 @@ class NotificationProvider : public ProviderInterface, virtual void ClearAllContentSettingsRules( ContentSettingsType content_type); + virtual void ShutdownOnUIThread(); + // NotificationObserver implementation. virtual void Observe(int type, const NotificationSource& source, diff --git a/chrome/browser/content_settings/content_settings_policy_provider.cc b/chrome/browser/content_settings/content_settings_policy_provider.cc index ace204a..a13ca77 100644 --- a/chrome/browser/content_settings/content_settings_policy_provider.cc +++ b/chrome/browser/content_settings/content_settings_policy_provider.cc @@ -100,17 +100,16 @@ const PrefsForManagedContentSettingsMapEntry namespace content_settings { -PolicyDefaultProvider::PolicyDefaultProvider(Profile* profile) - : profile_(profile), - is_off_the_record_(profile_->IsOffTheRecord()) { - PrefService* prefs = profile->GetPrefs(); - +PolicyDefaultProvider::PolicyDefaultProvider(HostContentSettingsMap* map, + PrefService* prefs) + : host_content_settings_map_(map), + prefs_(prefs) { // Read global defaults. DCHECK_EQ(arraysize(kPrefToManageType), static_cast<size_t>(CONTENT_SETTINGS_NUM_TYPES)); ReadManagedDefaultSettings(); - pref_change_registrar_.Init(prefs); + pref_change_registrar_.Init(prefs_); // The following preferences are only used to indicate if a // default-content-setting is managed and to hold the managed default-setting // value. If the value for any of the following perferences is set then the @@ -123,12 +122,10 @@ PolicyDefaultProvider::PolicyDefaultProvider(Profile* profile) pref_change_registrar_.Add(prefs::kManagedDefaultJavaScriptSetting, this); pref_change_registrar_.Add(prefs::kManagedDefaultPluginsSetting, this); pref_change_registrar_.Add(prefs::kManagedDefaultPopupsSetting, this); - notification_registrar_.Add(this, chrome::NOTIFICATION_PROFILE_DESTROYED, - Source<Profile>(profile_)); } PolicyDefaultProvider::~PolicyDefaultProvider() { - UnregisterObservers(); + DCHECK(!prefs_); } ContentSetting PolicyDefaultProvider::ProvideDefaultSetting( @@ -159,7 +156,7 @@ void PolicyDefaultProvider::Observe(int type, DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); if (type == chrome::NOTIFICATION_PREF_CHANGED) { - DCHECK_EQ(profile_->GetPrefs(), Source<PrefService>(source).ptr()); + DCHECK_EQ(prefs_, Source<PrefService>(source).ptr()); std::string* name = Details<std::string>(details).ptr(); if (*name == prefs::kManagedDefaultCookiesSetting) { UpdateManagedDefaultSetting(CONTENT_SETTINGS_TYPE_COOKIES); @@ -176,40 +173,33 @@ void PolicyDefaultProvider::Observe(int type, return; } - if (!is_off_the_record_) { - ContentSettingsDetails details(ContentSettingsPattern(), - ContentSettingsPattern(), - CONTENT_SETTINGS_TYPE_DEFAULT, - std::string()); - NotifyObservers(details); - } - } else if (type == chrome::NOTIFICATION_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 PolicyDefaultProvider::UnregisterObservers() { +void PolicyDefaultProvider::ShutdownOnUIThread() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - if (!profile_) - return; + DCHECK(prefs_); pref_change_registrar_.RemoveAll(); - notification_registrar_.Remove(this, chrome::NOTIFICATION_PROFILE_DESTROYED, - Source<Profile>(profile_)); - profile_ = NULL; + prefs_ = NULL; + host_content_settings_map_ = NULL; } void PolicyDefaultProvider::NotifyObservers( const ContentSettingsDetails& details) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - if (profile_ == NULL) + if (host_content_settings_map_ == NULL) return; NotificationService::current()->Notify( chrome::NOTIFICATION_CONTENT_SETTINGS_CHANGED, - Source<HostContentSettingsMap>(profile_->GetHostContentSettingsMap()), + Source<HostContentSettingsMap>(host_content_settings_map_), Details<const ContentSettingsDetails>(&details)); } @@ -230,12 +220,11 @@ void PolicyDefaultProvider::UpdateManagedDefaultSetting( // preference to manage a default-content-settings is CONTENT_SETTING_DEFAULT. // This indicates that no managed value is set. If a pref was set, than it // MUST be managed. - PrefService* prefs = profile_->GetPrefs(); - DCHECK(!prefs->HasPrefPath(kPrefToManageType[type]) || - prefs->IsManagedPreference(kPrefToManageType[type])); + DCHECK(!prefs_->HasPrefPath(kPrefToManageType[type]) || + prefs_->IsManagedPreference(kPrefToManageType[type])); base::AutoLock auto_lock(lock_); managed_default_content_settings_.settings[type] = IntToContentSetting( - prefs->GetInteger(kPrefToManageType[type])); + prefs_->GetInteger(kPrefToManageType[type])); } // static @@ -288,23 +277,15 @@ void PolicyProvider::RegisterUserPrefs(PrefService* prefs) { PrefService::UNSYNCABLE_PREF); } -PolicyProvider::PolicyProvider(Profile* profile, +PolicyProvider::PolicyProvider(HostContentSettingsMap* map, + PrefService* prefs, DefaultProviderInterface* default_provider) - : profile_(profile), + : host_content_settings_map_(map), + prefs_(prefs), default_provider_(default_provider) { - Init(); -} - -PolicyProvider::~PolicyProvider() { - UnregisterObservers(); -} - -void PolicyProvider::Init() { - PrefService* prefs = profile_->GetPrefs(); - ReadManagedContentSettings(false); - pref_change_registrar_.Init(prefs); + pref_change_registrar_.Init(prefs_); pref_change_registrar_.Add(prefs::kManagedCookiesBlockedForUrls, this); pref_change_registrar_.Add(prefs::kManagedCookiesAllowedForUrls, this); pref_change_registrar_.Add(prefs::kManagedCookiesSessionOnlyForUrls, this); @@ -316,29 +297,32 @@ void PolicyProvider::Init() { pref_change_registrar_.Add(prefs::kManagedPluginsAllowedForUrls, this); pref_change_registrar_.Add(prefs::kManagedPopupsBlockedForUrls, this); pref_change_registrar_.Add(prefs::kManagedPopupsAllowedForUrls, this); +} - notification_registrar_.Add(this, chrome::NOTIFICATION_PROFILE_DESTROYED, - Source<Profile>(profile_)); +PolicyProvider::~PolicyProvider() { + DCHECK(!prefs_); } void PolicyProvider::GetContentSettingsFromPreferences( - PrefService* prefs, ContentSettingsRules* rules) { for (size_t i = 0; i < arraysize(kPrefsForManagedContentSettingsMap); ++i) { const char* pref_name = kPrefsForManagedContentSettingsMap[i].pref_name; // Skip unset policies. - if (!prefs->HasPrefPath(pref_name)) { + if (!prefs_->HasPrefPath(pref_name)) { VLOG(2) << "Skipping unset preference: " << pref_name; continue; } - const PrefService::Preference* pref = prefs->FindPreference(pref_name); + const PrefService::Preference* pref = prefs_->FindPreference(pref_name); DCHECK(pref); DCHECK(pref->IsManaged()); - DCHECK_EQ(Value::TYPE_LIST, pref->GetType()); - const ListValue* pattern_str_list = - static_cast<const ListValue*>(pref->GetValue()); + const ListValue* pattern_str_list = NULL; + if (!pref->GetValue()->GetAsList(&pattern_str_list)) { + NOTREACHED(); + return; + } + for (size_t j = 0; j < pattern_str_list->GetSize(); ++j) { std::string original_pattern_str; pattern_str_list->GetString(j, &original_pattern_str); @@ -366,8 +350,7 @@ void PolicyProvider::GetContentSettingsFromPreferences( void PolicyProvider::ReadManagedContentSettings(bool overwrite) { ContentSettingsRules rules; - PrefService* prefs = profile_->GetPrefs(); - GetContentSettingsFromPreferences(prefs, &rules); + GetContentSettingsFromPreferences(&rules); { base::AutoLock auto_lock(lock_); if (overwrite) @@ -443,24 +426,23 @@ void PolicyProvider::ClearAllContentSettingsRules( ContentSettingsType content_type) { } -void PolicyProvider::UnregisterObservers() { +void PolicyProvider::ShutdownOnUIThread() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - if (!profile_) + if (!prefs_) return; pref_change_registrar_.RemoveAll(); - notification_registrar_.Remove(this, chrome::NOTIFICATION_PROFILE_DESTROYED, - Source<Profile>(profile_)); - profile_ = NULL; + prefs_ = NULL; + host_content_settings_map_ = NULL; } void PolicyProvider::NotifyObservers( const ContentSettingsDetails& details) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - if (profile_ == NULL) + if (host_content_settings_map_ == NULL) return; NotificationService::current()->Notify( chrome::NOTIFICATION_CONTENT_SETTINGS_CHANGED, - Source<HostContentSettingsMap>(profile_->GetHostContentSettingsMap()), + Source<HostContentSettingsMap>(host_content_settings_map_), Details<const ContentSettingsDetails>(&details)); } @@ -470,7 +452,7 @@ void PolicyProvider::Observe(int type, DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); if (type == chrome::NOTIFICATION_PREF_CHANGED) { - DCHECK_EQ(profile_->GetPrefs(), Source<PrefService>(source).ptr()); + DCHECK_EQ(prefs_, Source<PrefService>(source).ptr()); std::string* name = Details<std::string>(details).ptr(); if (*name == prefs::kManagedCookiesAllowedForUrls || *name == prefs::kManagedCookiesBlockedForUrls || @@ -490,9 +472,6 @@ void PolicyProvider::Observe(int type, std::string()); NotifyObservers(details); } - } else if (type == chrome::NOTIFICATION_PROFILE_DESTROYED) { - DCHECK_EQ(profile_, Source<Profile>(source).ptr()); - UnregisterObservers(); } else { NOTREACHED() << "Unexpected notification"; } diff --git a/chrome/browser/content_settings/content_settings_policy_provider.h b/chrome/browser/content_settings/content_settings_policy_provider.h index a992c51..394e5a7 100644 --- a/chrome/browser/content_settings/content_settings_policy_provider.h +++ b/chrome/browser/content_settings/content_settings_policy_provider.h @@ -21,15 +21,15 @@ class ContentSettingsDetails; class DictionaryValue; +class HostContentSettingsMap; class PrefService; -class Profile; namespace content_settings { class PolicyDefaultProvider : public DefaultProviderInterface, public NotificationObserver { public: - explicit PolicyDefaultProvider(Profile* profile); + PolicyDefaultProvider(HostContentSettingsMap* map, PrefService* prefs); virtual ~PolicyDefaultProvider(); // DefaultContentSettingsProvider implementation. @@ -39,6 +39,8 @@ class PolicyDefaultProvider : public DefaultProviderInterface, ContentSetting setting); virtual bool DefaultSettingIsManaged(ContentSettingsType content_type) const; + void ShutdownOnUIThread(); + static void RegisterUserPrefs(PrefService* prefs); // NotificationObserver implementation. @@ -53,8 +55,6 @@ class PolicyDefaultProvider : public DefaultProviderInterface, // mutex deadlock. void NotifyObservers(const ContentSettingsDetails& details); - void UnregisterObservers(); - // Reads the policy managed default settings. void ReadManagedDefaultSettings(); @@ -64,10 +64,8 @@ class PolicyDefaultProvider : public DefaultProviderInterface, // Copies of the pref data, so that we can read it on the IO thread. ContentSettings managed_default_content_settings_; - Profile* profile_; - - // Whether this settings map is for an OTR session. - bool is_off_the_record_; + HostContentSettingsMap* host_content_settings_map_; + PrefService* prefs_; // Used around accesses to the managed_default_content_settings_ object to // guarantee thread safety. @@ -83,14 +81,13 @@ class PolicyDefaultProvider : public DefaultProviderInterface, class PolicyProvider : public ProviderInterface, public NotificationObserver { public: - explicit PolicyProvider(Profile* profile, - DefaultProviderInterface* default_provider); + PolicyProvider(HostContentSettingsMap* map, + PrefService* prefs, + DefaultProviderInterface* default_provider); virtual ~PolicyProvider(); static void RegisterUserPrefs(PrefService* prefs); // ProviderInterface Implementation - virtual void Init(); - virtual void SetContentSetting( const ContentSettingsPattern& primary_pattern, const ContentSettingsPattern& secondary_pattern, @@ -112,6 +109,8 @@ class PolicyProvider : public ProviderInterface, virtual void ClearAllContentSettingsRules( ContentSettingsType content_type); + virtual void ShutdownOnUIThread(); + // NotificationObserver implementation. virtual void Observe(int type, const NotificationSource& source, @@ -128,25 +127,21 @@ class PolicyProvider : public ProviderInterface, void ReadManagedContentSettings(bool overwrite); - void GetContentSettingsFromPreferences(PrefService* prefs, - ContentSettingsRules* rules); + void GetContentSettingsFromPreferences(ContentSettingsRules* rules); - void ReadManagedContentSettingsTypes( - ContentSettingsType content_type); + void ReadManagedContentSettingsTypes(ContentSettingsType content_type); void NotifyObservers(const ContentSettingsDetails& details); - void UnregisterObservers(); - OriginIdentifierValueMap value_map_; - Profile* profile_; + HostContentSettingsMap* host_content_settings_map_; + PrefService* prefs_; // Weak, owned by HostContentSettingsMap. DefaultProviderInterface* default_provider_; PrefChangeRegistrar pref_change_registrar_; - NotificationRegistrar notification_registrar_; // Used around accesses to the content_settings_ object to guarantee // thread safety. diff --git a/chrome/browser/content_settings/content_settings_policy_provider_unittest.cc b/chrome/browser/content_settings/content_settings_policy_provider_unittest.cc index 8cedeca..36f321f 100644 --- a/chrome/browser/content_settings/content_settings_policy_provider_unittest.cc +++ b/chrome/browser/content_settings/content_settings_policy_provider_unittest.cc @@ -35,8 +35,8 @@ class PolicyDefaultProviderTest : public TestingBrowserProcessTest { TEST_F(PolicyDefaultProviderTest, DefaultValues) { TestingProfile profile; - PolicyDefaultProvider provider(&profile); TestingPrefService* prefs = profile.GetTestingPrefService(); + PolicyDefaultProvider provider(profile.GetHostContentSettingsMap(), prefs); // By default, policies should be off. ASSERT_FALSE( @@ -54,6 +54,8 @@ TEST_F(PolicyDefaultProviderTest, DefaultValues) { prefs->RemoveManagedPref(prefs::kManagedDefaultCookiesSetting); ASSERT_FALSE( provider.DefaultSettingIsManaged(CONTENT_SETTINGS_TYPE_COOKIES)); + + provider.ShutdownOnUIThread(); } // When a default-content-setting is set to a managed setting a @@ -107,7 +109,7 @@ TEST_F(PolicyProviderTest, Default) { prefs->SetManagedPref(prefs::kManagedImagesBlockedForUrls, value); - PolicyProvider provider(&profile, NULL); + PolicyProvider provider(profile.GetHostContentSettingsMap(), prefs, NULL); ContentSettingsPattern yt_url_pattern = ContentSettingsPattern::FromString("www.youtube.com"); @@ -130,6 +132,8 @@ TEST_F(PolicyProviderTest, Default) { EXPECT_EQ(CONTENT_SETTING_DEFAULT, provider.GetContentSetting( youtube_url, youtube_url, CONTENT_SETTINGS_TYPE_COOKIES, "")); + + provider.ShutdownOnUIThread(); } TEST_F(PolicyProviderTest, ResourceIdentifier) { @@ -145,7 +149,7 @@ TEST_F(PolicyProviderTest, ResourceIdentifier) { prefs->SetManagedPref(prefs::kManagedPluginsAllowedForUrls, value); - PolicyProvider provider(&profile, NULL); + PolicyProvider provider(profile.GetHostContentSettingsMap(), prefs, NULL); GURL youtube_url("http://www.youtube.com"); GURL google_url("http://mail.google.com"); @@ -173,6 +177,8 @@ TEST_F(PolicyProviderTest, ResourceIdentifier) { google_url, CONTENT_SETTINGS_TYPE_PLUGINS, "someplugin")); + + provider.ShutdownOnUIThread(); } } // namespace content_settings diff --git a/chrome/browser/content_settings/content_settings_pref_provider.cc b/chrome/browser/content_settings/content_settings_pref_provider.cc index 56ac49b..e6c7c90 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" @@ -149,14 +150,15 @@ 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) + : host_content_settings_map_(map), + prefs_(prefs), + is_incognito_(incognito), updating_preferences_(false) { - initializing_ = true; - PrefService* prefs = profile->GetPrefs(); - - MigrateObsoleteNotificationPref(prefs); + DCHECK(prefs_); + MigrateObsoleteNotificationPref(); // Read global defaults. DCHECK_EQ(arraysize(kTypeNames), @@ -171,15 +173,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, chrome::NOTIFICATION_PROFILE_DESTROYED, - Source<Profile>(profile_)); - initializing_ = false; } PrefDefaultProvider::~PrefDefaultProvider() { - UnregisterObservers(); + DCHECK(!prefs_); } ContentSetting PrefDefaultProvider::ProvideDefaultSetting( @@ -192,6 +191,7 @@ void PrefDefaultProvider::UpdateDefaultSetting( ContentSettingsType content_type, ContentSetting setting) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + DCHECK(prefs_); DCHECK(kTypeNames[content_type] != NULL); // Don't call this for Geolocation. // The default settings may not be directly modified for OTR sessions. @@ -199,13 +199,11 @@ void PrefDefaultProvider::UpdateDefaultSetting( if (is_incognito_) return; - PrefService* prefs = profile_->GetPrefs(); - std::string dictionary_path(kTypeNames[content_type]); - updating_preferences_ = true; { + AutoReset<bool> auto_reset(&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])) { @@ -219,7 +217,6 @@ void PrefDefaultProvider::UpdateDefaultSetting( dictionary_path, Value::CreateIntegerValue(setting)); } } - updating_preferences_ = false; ContentSettingsDetails details( ContentSettingsPattern(), @@ -240,7 +237,7 @@ void PrefDefaultProvider::Observe(int type, DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); if (type == chrome::NOTIFICATION_PREF_CHANGED) { - DCHECK_EQ(profile_->GetPrefs(), Source<PrefService>(source).ptr()); + DCHECK_EQ(prefs_, Source<PrefService>(source).ptr()); if (updating_preferences_) return; @@ -252,35 +249,27 @@ void PrefDefaultProvider::Observe(int type, return; } - if (!is_incognito_) { - ContentSettingsDetails details(ContentSettingsPattern(), - ContentSettingsPattern(), - CONTENT_SETTINGS_TYPE_DEFAULT, - std::string()); - NotifyObservers(details); - } - } else if (type == chrome::NOTIFICATION_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() { +void PrefDefaultProvider::ShutdownOnUIThread() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - if (!profile_) - return; + DCHECK(prefs_); pref_change_registrar_.RemoveAll(); - notification_registrar_.Remove(this, chrome::NOTIFICATION_PROFILE_DESTROYED, - Source<Profile>(profile_)); - profile_ = NULL; + prefs_ = NULL; + host_content_settings_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_); @@ -335,20 +324,20 @@ void PrefDefaultProvider::GetSettingsFromDictionary( void PrefDefaultProvider::NotifyObservers( const ContentSettingsDetails& details) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - if (initializing_ || profile_ == NULL) + if (host_content_settings_map_ == NULL) return; NotificationService::current()->Notify( chrome::NOTIFICATION_CONTENT_SETTINGS_CHANGED, - Source<HostContentSettingsMap>(profile_->GetHostContentSettingsMap()), + Source<HostContentSettingsMap>(host_content_settings_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); } } @@ -395,28 +384,27 @@ 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), + host_content_settings_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); + DCHECK(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; @@ -430,20 +418,16 @@ 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, chrome::NOTIFICATION_PROFILE_DESTROYED, - Source<Profile>(profile_)); - initializing_ = false; } ContentSetting PrefProvider::GetContentSetting( - const GURL& primary_url, - const GURL& secondary_url, - ContentSettingsType content_type, - const ResourceIdentifier& resource_identifier) const { + 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 @@ -469,9 +453,9 @@ ContentSetting PrefProvider::GetContentSetting( } void PrefProvider::GetAllContentSettingsRules( - ContentSettingsType content_type, - const ResourceIdentifier& resource_identifier, - Rules* content_setting_rules) const { + ContentSettingsType content_type, + const ResourceIdentifier& resource_identifier, + Rules* content_setting_rules) const { DCHECK(content_setting_rules); content_setting_rules->clear(); @@ -500,8 +484,9 @@ void PrefProvider::SetContentSetting( ContentSettingsType content_type, const ResourceIdentifier& resource_identifier, ContentSetting setting) { - DCHECK(kTypeNames[content_type] != NULL); // Don't call this for Geolocation. DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + DCHECK(prefs_); + DCHECK(kTypeNames[content_type] != NULL); // Don't call this for Geolocation. // Update in memory value map. OriginIdentifierValueMap* map_to_modify = &incognito_value_map_; @@ -527,7 +512,7 @@ void PrefProvider::SetContentSetting( } // Update the content settings preference. - if (!is_incognito_ && profile_) { + if (!is_incognito_) { UpdatePref(primary_pattern, secondary_pattern, content_type, @@ -536,13 +521,15 @@ void PrefProvider::SetContentSetting( } ContentSettingsDetails details( - primary_pattern, secondary_pattern, content_type, std::string()); + primary_pattern, secondary_pattern, content_type, resource_identifier); NotifyObservers(details); } void PrefProvider::ClearAllContentSettingsRules( ContentSettingsType content_type) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); DCHECK(kTypeNames[content_type] != NULL); // Don't call this for Geolocation. + DCHECK(prefs_); OriginIdentifierValueMap* map_to_modify = &incognito_value_map_; if (!is_incognito_) @@ -582,41 +569,35 @@ void PrefProvider::Observe( DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); if (type == chrome::NOTIFICATION_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> auto_reset(&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 == chrome::NOTIFICATION_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_); } // //////////////////////////////////////////////////////////////////////////// @@ -628,7 +609,7 @@ void PrefProvider::UpdatePref( ContentSettingsType content_type, const ResourceIdentifier& resource_identifier, ContentSetting setting) { - updating_preferences_ = true; + AutoReset<bool> auto_reset(&updating_preferences_, true); UpdatePatternPairsPref(primary_pattern, secondary_pattern, content_type, @@ -639,23 +620,21 @@ void PrefProvider::UpdatePref( content_type, resource_identifier, setting); - updating_preferences_ = false; } void PrefProvider::ReadContentSettingsFromPref(bool overwrite) { 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> auto_reset(&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; @@ -733,7 +712,6 @@ void PrefProvider::ReadContentSettingsFromPref(bool overwrite) { } } } - updating_preferences_ = false; } void PrefProvider::UpdatePatternsPref( @@ -742,7 +720,7 @@ void PrefProvider::UpdatePatternsPref( ContentSettingsType content_type, const ResourceIdentifier& resource_identifier, ContentSetting setting) { - DictionaryPrefUpdate update(profile_->GetPrefs(), + DictionaryPrefUpdate update(prefs_, prefs::kContentSettingsPatterns); DictionaryValue* all_settings_dictionary = update.Get(); @@ -808,13 +786,13 @@ void PrefProvider::UpdatePatternPairsPref( ContentSettingsType content_type, const ResourceIdentifier& resource_identifier, ContentSetting setting) { - DictionaryPrefUpdate update(profile_->GetPrefs(), + DictionaryPrefUpdate update(prefs_, prefs::kContentSettingsPatternPairs); DictionaryValue* all_settings_dictionary = update.Get(); // Get settings dictionary for the given patterns. std::string pattern_str(CreatePatternString(primary_pattern, - secondary_pattern)); + secondary_pattern)); DictionaryValue* settings_dictionary = NULL; bool found = all_settings_dictionary->GetDictionaryWithoutPathExpansion( pattern_str, &settings_dictionary); @@ -869,6 +847,7 @@ void PrefProvider::UpdatePatternPairsPref( } } +// static void PrefProvider::CanonicalizeContentSettingsExceptions( DictionaryValue* all_settings_dictionary) { DCHECK(all_settings_dictionary); @@ -921,29 +900,25 @@ void PrefProvider::CanonicalizeContentSettingsExceptions( void PrefProvider::NotifyObservers( const ContentSettingsDetails& details) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - if (initializing_ || profile_ == NULL) - return; + DCHECK(host_content_settings_map_); NotificationService::current()->Notify( chrome::NOTIFICATION_CONTENT_SETTINGS_CHANGED, - Source<HostContentSettingsMap>( - profile_->GetHostContentSettingsMap()), + Source<HostContentSettingsMap>(host_content_settings_map_), Details<const ContentSettingsDetails>(&details)); } -void PrefProvider::UnregisterObservers() { +void PrefProvider::ShutdownOnUIThread() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - if (!profile_) - return; + DCHECK(prefs_); pref_change_registrar_.RemoveAll(); - notification_registrar_.Remove(this, chrome::NOTIFICATION_PROFILE_DESTROYED, - Source<Profile>(profile_)); - profile_ = NULL; + prefs_ = NULL; + host_content_settings_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()); @@ -995,14 +970,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; @@ -1013,18 +988,16 @@ 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() { + if (prefs_->HasPrefPath(prefs::kContentSettingsPatterns) && !is_incognito_) { 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()); @@ -1057,14 +1030,14 @@ void PrefProvider::MigrateObsoleteContentSettingsPatternPref( } } -void PrefProvider::SyncObsoletePref(PrefService* prefs) { - updating_preferences_ = true; - if (prefs->HasPrefPath(prefs::kContentSettingsPatternPairs) && +void PrefProvider::SyncObsoletePref() { + AutoReset<bool> auto_reset(&updating_preferences_, true); + 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 = @@ -1089,7 +1062,6 @@ void PrefProvider::SyncObsoletePref(PrefService* prefs) { new_key, dictionary->DeepCopy()); } } - updating_preferences_ = false; } } // namespace content_settings diff --git a/chrome/browser/content_settings/content_settings_pref_provider.h b/chrome/browser/content_settings/content_settings_pref_provider.h index 89d1e93..6320568 100644 --- a/chrome/browser/content_settings/content_settings_pref_provider.h +++ b/chrome/browser/content_settings/content_settings_pref_provider.h @@ -23,8 +23,8 @@ class ContentSettingsDetails; class DictionaryValue; +class HostContentSettingsMap; class PrefService; -class Profile; namespace content_settings { @@ -33,7 +33,9 @@ namespace content_settings { class PrefDefaultProvider : public DefaultProviderInterface, public NotificationObserver { public: - explicit PrefDefaultProvider(Profile* profile); + PrefDefaultProvider(HostContentSettingsMap* map, + PrefService* prefs, + bool incognito); virtual ~PrefDefaultProvider(); // DefaultContentSettingsProvider implementation. @@ -43,6 +45,8 @@ class PrefDefaultProvider : public DefaultProviderInterface, ContentSetting setting); virtual bool DefaultSettingIsManaged(ContentSettingsType content_type) const; + void ShutdownOnUIThread(); + static void RegisterUserPrefs(PrefService* prefs); // NotificationObserver implementation. @@ -57,8 +61,6 @@ class PrefDefaultProvider : public DefaultProviderInterface, // mutex deadlock. void NotifyObservers(const ContentSettingsDetails& details); - void UnregisterObservers(); - // Sets the fields of |settings| based on the values in |dictionary|. void GetSettingsFromDictionary(const DictionaryValue* dictionary, ContentSettings* settings); @@ -71,12 +73,13 @@ class PrefDefaultProvider : public DefaultProviderInterface, // true and the preference is missing, the local copy will be cleared as well. void ReadDefaultSettings(bool overwrite); - void MigrateObsoleteNotificationPref(PrefService* prefs); + void MigrateObsoleteNotificationPref(); // Copies of the pref data, so that we can read it on the IO thread. ContentSettings default_content_settings_; - Profile* profile_; + HostContentSettingsMap* host_content_settings_map_; + PrefService* prefs_; // Whether this settings map is for an Incognito session. bool is_incognito_; @@ -86,14 +89,11 @@ class PrefDefaultProvider : public DefaultProviderInterface, mutable base::Lock lock_; PrefChangeRegistrar pref_change_registrar_; - NotificationRegistrar notification_registrar_; // Whether we are currently updating preferences, this is used to ignore // notifications from the preferences service that we triggered ourself. bool updating_preferences_; - bool initializing_; - DISALLOW_COPY_AND_ASSIGN(PrefDefaultProvider); }; @@ -104,7 +104,9 @@ class PrefProvider : public ProviderInterface, public: static void RegisterUserPrefs(PrefService* prefs); - explicit PrefProvider(Profile* profile); + PrefProvider(HostContentSettingsMap* map, + PrefService* prefs, + bool incognito); virtual ~PrefProvider(); // ProviderInterface implementations. @@ -129,6 +131,8 @@ class PrefProvider : public ProviderInterface, virtual void ClearAllContentSettingsRules( ContentSettingsType content_type); + virtual void ShutdownOnUIThread(); + // NotificationObserver implementation. virtual void Observe(int type, const NotificationSource& source, @@ -172,37 +176,35 @@ class PrefProvider : public ProviderInterface, // Various migration methods (old cookie, popup and per-host data gets // migrated to the new format). - void MigrateObsoletePerhostPref(PrefService* prefs); - void MigrateObsoletePopupsPref(PrefService* prefs); - void MigrateObsoleteContentSettingsPatternPref(PrefService* prefs); + void MigrateObsoletePerhostPref(); + void MigrateObsoletePopupsPref(); + void MigrateObsoleteContentSettingsPatternPref(); // Copies the value of the preference that stores the content settings // exceptions to the obsolete preference for content settings exceptions. This // is necessary to allow content settings exceptions beeing synced to older // versions of chrome that only use the obsolete. - void SyncObsoletePref(PrefService* pref); + void SyncObsoletePref(); - void CanonicalizeContentSettingsExceptions( + static void CanonicalizeContentSettingsExceptions( DictionaryValue* all_settings_dictionary); void NotifyObservers(const ContentSettingsDetails& details); - void UnregisterObservers(); + // Weak; owned by the Profile and reset in ShutdownOnUIThread. + PrefService* prefs_; - Profile* profile_; + // Weak; owns us + HostContentSettingsMap* host_content_settings_map_; bool is_incognito_; PrefChangeRegistrar pref_change_registrar_; - NotificationRegistrar notification_registrar_; // Whether we are currently updating preferences, this is used to ignore // notifications from the preferences service that we triggered ourself. bool updating_preferences_; - // Do not fire any Notifications as long as we are in the constructor. - bool initializing_; - OriginIdentifierValueMap value_map_; OriginIdentifierValueMap incognito_value_map_; diff --git a/chrome/browser/content_settings/content_settings_pref_provider_unittest.cc b/chrome/browser/content_settings/content_settings_pref_provider_unittest.cc index df3b8708..9ff4e38 100644 --- a/chrome/browser/content_settings/content_settings_pref_provider_unittest.cc +++ b/chrome/browser/content_settings/content_settings_pref_provider_unittest.cc @@ -30,63 +30,63 @@ namespace content_settings { class PrefDefaultProviderTest : public TestingBrowserProcessTest { public: PrefDefaultProviderTest() - : ui_thread_(BrowserThread::UI, &message_loop_) { + : ui_thread_(BrowserThread::UI, &message_loop_), + provider_(profile_.GetHostContentSettingsMap(), + profile_.GetPrefs(), + false) { + } + ~PrefDefaultProviderTest() { + provider_.ShutdownOnUIThread(); } protected: MessageLoop message_loop_; BrowserThread ui_thread_; + TestingProfile profile_; + PrefDefaultProvider provider_; }; TEST_F(PrefDefaultProviderTest, DefaultValues) { - TestingProfile profile; - content_settings::PrefDefaultProvider provider(&profile); - ASSERT_FALSE( - provider.DefaultSettingIsManaged(CONTENT_SETTINGS_TYPE_COOKIES)); + provider_.DefaultSettingIsManaged(CONTENT_SETTINGS_TYPE_COOKIES)); // Check setting defaults. EXPECT_EQ(CONTENT_SETTING_ALLOW, - provider.ProvideDefaultSetting(CONTENT_SETTINGS_TYPE_COOKIES)); - provider.UpdateDefaultSetting(CONTENT_SETTINGS_TYPE_COOKIES, + provider_.ProvideDefaultSetting(CONTENT_SETTINGS_TYPE_COOKIES)); + provider_.UpdateDefaultSetting(CONTENT_SETTINGS_TYPE_COOKIES, CONTENT_SETTING_BLOCK); EXPECT_EQ(CONTENT_SETTING_BLOCK, - provider.ProvideDefaultSetting(CONTENT_SETTINGS_TYPE_COOKIES)); + provider_.ProvideDefaultSetting(CONTENT_SETTINGS_TYPE_COOKIES)); } TEST_F(PrefDefaultProviderTest, Observer) { - TestingProfile profile; - PrefDefaultProvider provider(&profile); MockSettingsObserver observer; EXPECT_CALL(observer, - OnContentSettingsChanged(profile.GetHostContentSettingsMap(), + OnContentSettingsChanged(profile_.GetHostContentSettingsMap(), CONTENT_SETTINGS_TYPE_IMAGES, false, _, _, true)); // Expect a second call because the PrefDefaultProvider in the TestingProfile // also observes the default content settings preference. EXPECT_CALL(observer, - OnContentSettingsChanged(profile.GetHostContentSettingsMap(), + OnContentSettingsChanged(profile_.GetHostContentSettingsMap(), CONTENT_SETTINGS_TYPE_DEFAULT, true, _, _, true)); - provider.UpdateDefaultSetting( + provider_.UpdateDefaultSetting( CONTENT_SETTINGS_TYPE_IMAGES, CONTENT_SETTING_BLOCK); } TEST_F(PrefDefaultProviderTest, ObserveDefaultPref) { - TestingProfile profile; - PrefDefaultProvider provider(&profile); - - PrefService* prefs = profile.GetPrefs(); + PrefService* prefs = profile_.GetPrefs(); // Make a copy of the default pref value so we can reset it later. scoped_ptr<Value> default_value(prefs->FindPreference( prefs::kDefaultContentSettings)->GetValue()->DeepCopy()); - provider.UpdateDefaultSetting( + provider_.UpdateDefaultSetting( CONTENT_SETTINGS_TYPE_COOKIES, CONTENT_SETTING_BLOCK); EXPECT_EQ(CONTENT_SETTING_BLOCK, - provider.ProvideDefaultSetting(CONTENT_SETTINGS_TYPE_COOKIES)); + provider_.ProvideDefaultSetting(CONTENT_SETTINGS_TYPE_COOKIES)); // Make a copy of the pref's new value so we can reset it later. scoped_ptr<Value> new_value(prefs->FindPreference( @@ -95,33 +95,30 @@ TEST_F(PrefDefaultProviderTest, ObserveDefaultPref) { // Clearing the backing pref should also clear the internal cache. prefs->Set(prefs::kDefaultContentSettings, *default_value); EXPECT_EQ(CONTENT_SETTING_ALLOW, - provider.ProvideDefaultSetting(CONTENT_SETTINGS_TYPE_COOKIES)); + provider_.ProvideDefaultSetting(CONTENT_SETTINGS_TYPE_COOKIES)); // Reseting the pref to its previous value should update the cache. prefs->Set(prefs::kDefaultContentSettings, *new_value); EXPECT_EQ(CONTENT_SETTING_BLOCK, - provider.ProvideDefaultSetting(CONTENT_SETTINGS_TYPE_COOKIES)); + provider_.ProvideDefaultSetting(CONTENT_SETTINGS_TYPE_COOKIES)); } TEST_F(PrefDefaultProviderTest, OffTheRecord) { - TestingProfile profile; - PrefDefaultProvider provider(&profile); - - profile.set_incognito(true); - PrefDefaultProvider otr_provider(&profile); - profile.set_incognito(false); + PrefDefaultProvider otr_provider(profile_.GetHostContentSettingsMap(), + profile_.GetPrefs(), + true); EXPECT_EQ(CONTENT_SETTING_ALLOW, - provider.ProvideDefaultSetting(CONTENT_SETTINGS_TYPE_COOKIES)); + provider_.ProvideDefaultSetting(CONTENT_SETTINGS_TYPE_COOKIES)); EXPECT_EQ(CONTENT_SETTING_ALLOW, otr_provider.ProvideDefaultSetting(CONTENT_SETTINGS_TYPE_COOKIES)); // Changing content settings on the main provider should also affect the // incognito map. - provider.UpdateDefaultSetting(CONTENT_SETTINGS_TYPE_COOKIES, + provider_.UpdateDefaultSetting(CONTENT_SETTINGS_TYPE_COOKIES, CONTENT_SETTING_BLOCK); EXPECT_EQ(CONTENT_SETTING_BLOCK, - provider.ProvideDefaultSetting(CONTENT_SETTINGS_TYPE_COOKIES)); + provider_.ProvideDefaultSetting(CONTENT_SETTINGS_TYPE_COOKIES)); EXPECT_EQ(CONTENT_SETTING_BLOCK, otr_provider.ProvideDefaultSetting(CONTENT_SETTINGS_TYPE_COOKIES)); @@ -129,9 +126,11 @@ TEST_F(PrefDefaultProviderTest, OffTheRecord) { otr_provider.UpdateDefaultSetting(CONTENT_SETTINGS_TYPE_COOKIES, CONTENT_SETTING_ALLOW); EXPECT_EQ(CONTENT_SETTING_BLOCK, - provider.ProvideDefaultSetting(CONTENT_SETTINGS_TYPE_COOKIES)); + provider_.ProvideDefaultSetting(CONTENT_SETTINGS_TYPE_COOKIES)); EXPECT_EQ(CONTENT_SETTING_BLOCK, otr_provider.ProvideDefaultSetting(CONTENT_SETTINGS_TYPE_COOKIES)); + + otr_provider.ShutdownOnUIThread(); } // //////////////////////////////////////////////////////////////////////////// @@ -160,17 +159,8 @@ class PrefProviderTest : public TestingBrowserProcessTest { TEST_F(PrefProviderTest, Observer) { TestingProfile profile; - // Get the |HostContentSettingsMap| one time in order to initialize it. - // Otherwise we end up in a dead lock when the |PrefProvider| tries to notify - // content settings change observers. The |PrefProvider| set's the - // |HostContentSettingsMap| as notification source and would trigger in - // infinite recursive instantiation loop. - // TODO(markusheintz): Let the HostContentSettingsMap sent out notifications. - // Providers and HostContentSettingsMap should communicate via a observer - // pattern. - profile.GetHostContentSettingsMap(); - Profile* p = &profile; - PrefProvider pref_content_settings_provider(p); + PrefProvider pref_content_settings_provider( + profile.GetHostContentSettingsMap(), profile.GetPrefs(), false); MockSettingsObserver observer; ContentSettingsPattern pattern = ContentSettingsPattern::FromString("[*.]example.com"); @@ -198,6 +188,8 @@ TEST_F(PrefProviderTest, Observer) { CONTENT_SETTINGS_TYPE_IMAGES, "", CONTENT_SETTING_ALLOW); + + pref_content_settings_provider.ShutdownOnUIThread(); } // Test for regression in which the PrefProvider modified the user pref store @@ -224,10 +216,11 @@ TEST_F(PrefProviderTest, Incognito) { profile.SetPrefService(regular_prefs); otr_profile->set_incognito(true); otr_profile->SetPrefService(otr_prefs); - profile.GetHostContentSettingsMap(); - PrefProvider pref_content_settings_provider(&profile); - PrefProvider pref_content_settings_provider_incognito(otr_profile); + PrefProvider pref_content_settings_provider( + profile.GetHostContentSettingsMap(), regular_prefs, false); + PrefProvider pref_content_settings_provider_incognito( + otr_profile->GetHostContentSettingsMap(), otr_prefs, true); ContentSettingsPattern pattern = ContentSettingsPattern::FromString("[*.]example.com"); pref_content_settings_provider.SetContentSetting( @@ -248,13 +241,16 @@ TEST_F(PrefProviderTest, Incognito) { host, host, CONTENT_SETTINGS_TYPE_IMAGES, "")); // But the value should not be overridden in the OTR user prefs accidentally. EXPECT_FALSE(otr_user_prefs->IsSetInOverlay(prefs::kContentSettingsPatterns)); + + pref_content_settings_provider.ShutdownOnUIThread(); + pref_content_settings_provider_incognito.ShutdownOnUIThread(); } TEST_F(PrefProviderTest, Patterns) { TestingProfile testing_profile; - testing_profile.GetHostContentSettingsMap(); PrefProvider pref_content_settings_provider( - testing_profile.GetOriginalProfile()); + testing_profile.GetHostContentSettingsMap(), + testing_profile.GetPrefs(), false); GURL host1("http://example.com/"); GURL host2("http://www.example.com/"); @@ -308,6 +304,8 @@ TEST_F(PrefProviderTest, Patterns) { EXPECT_EQ(CONTENT_SETTING_BLOCK, pref_content_settings_provider.GetContentSetting( host4, host4, CONTENT_SETTINGS_TYPE_IMAGES, "")); + + pref_content_settings_provider.ShutdownOnUIThread(); } TEST_F(PrefProviderTest, ResourceIdentifier) { @@ -317,9 +315,10 @@ TEST_F(PrefProviderTest, ResourceIdentifier) { cmd->AppendSwitch(switches::kEnableResourceContentSettings); TestingProfile testing_profile; - testing_profile.GetHostContentSettingsMap(); PrefProvider pref_content_settings_provider( - testing_profile.GetOriginalProfile()); + testing_profile.GetHostContentSettingsMap(), + testing_profile.GetPrefs(), + false); GURL host("http://example.com/"); ContentSettingsPattern pattern = @@ -342,6 +341,8 @@ TEST_F(PrefProviderTest, ResourceIdentifier) { EXPECT_EQ(CONTENT_SETTING_DEFAULT, pref_content_settings_provider.GetContentSetting( host, host, CONTENT_SETTINGS_TYPE_PLUGINS, resource2)); + + pref_content_settings_provider.ShutdownOnUIThread(); } TEST_F(PrefProviderTest, MigrateSinglePatternSettings) { @@ -364,7 +365,8 @@ TEST_F(PrefProviderTest, MigrateSinglePatternSettings) { prefs->Set(prefs::kContentSettingsPatterns, *all_settings_dictionary); // Test if single pattern settings are properly migrated. - content_settings::PrefProvider provider(profile.GetOriginalProfile()); + content_settings::PrefProvider provider(profile.GetHostContentSettingsMap(), + prefs, false); // Validate migrated preferences const DictionaryValue* const_all_settings_dictionary = @@ -386,6 +388,8 @@ TEST_F(PrefProviderTest, MigrateSinglePatternSettings) { GURL("http://www.example.com"), CONTENT_SETTINGS_TYPE_POPUPS, "")); + + provider.ShutdownOnUIThread(); } } // namespace content_settings diff --git a/chrome/browser/content_settings/content_settings_provider.h b/chrome/browser/content_settings/content_settings_provider.h index b94697b..8065dbe 100644 --- a/chrome/browser/content_settings/content_settings_provider.h +++ b/chrome/browser/content_settings/content_settings_provider.h @@ -42,6 +42,8 @@ class DefaultProviderInterface { // there shouldn't be any UI shown to modify this setting. virtual bool DefaultSettingIsManaged( ContentSettingsType content_type) const = 0; + + virtual void ShutdownOnUIThread() = 0; }; class ProviderInterface { @@ -103,7 +105,7 @@ class ProviderInterface { // Resets all content settings for the given |content_type| to // CONTENT_SETTING_DEFAULT. For content types that require a resource - // identifier all content settings for any resource identifieres of the given + // identifier all content settings for any resource identifiers of the given // |content_type| will be reset to CONTENT_SETTING_DEFAULT. // // This should only be called on the UI thread, and not after @@ -115,7 +117,7 @@ class ProviderInterface { // This methods needs to be called before destroying the Profile. // Afterwards, none of the methods above that should only be called on the UI // thread should be called anymore. - virtual void ShutdownOnUIThread() {} + virtual void ShutdownOnUIThread() = 0; }; } // namespace content_settings diff --git a/chrome/browser/content_settings/content_settings_provider_unittest.cc b/chrome/browser/content_settings/content_settings_provider_unittest.cc index 0b41475..3155430 100644 --- a/chrome/browser/content_settings/content_settings_provider_unittest.cc +++ b/chrome/browser/content_settings/content_settings_provider_unittest.cc @@ -11,9 +11,9 @@ namespace content_settings { TEST(ContentSettingsProviderTest, Mock) { MockDefaultProvider provider(CONTENT_SETTINGS_TYPE_COOKIES, - CONTENT_SETTING_ALLOW, - false, - true); + CONTENT_SETTING_ALLOW, + false, + true); EXPECT_EQ(CONTENT_SETTING_ALLOW, provider.ProvideDefaultSetting(CONTENT_SETTINGS_TYPE_COOKIES)); EXPECT_EQ(CONTENT_SETTING_DEFAULT, diff --git a/chrome/browser/content_settings/host_content_settings_map.cc b/chrome/browser/content_settings/host_content_settings_map.cc index f8c3484..e0e1e56 100644 --- a/chrome/browser/content_settings/host_content_settings_map.cc +++ b/chrome/browser/content_settings/host_content_settings_map.cc @@ -68,9 +68,12 @@ const char* kProviderNames[] = { } // namespace -HostContentSettingsMap::HostContentSettingsMap(Profile* profile) - : profile_(profile), - is_off_the_record_(profile_->IsOffTheRecord()), +HostContentSettingsMap::HostContentSettingsMap( + PrefService* prefs, + ExtensionService* extension_service, + bool incognito) + : prefs_(prefs), + is_off_the_record_(incognito), updating_preferences_(false), block_third_party_cookies_(false), is_block_third_party_cookies_managed_(false) { @@ -78,21 +81,20 @@ HostContentSettingsMap::HostContentSettingsMap(Profile* profile) // critical, as providers that are further down in the list (i.e. added later) // override providers further up. default_content_settings_providers_.push_back( - make_linked_ptr(new content_settings::PrefDefaultProvider(profile))); + make_linked_ptr(new content_settings::PrefDefaultProvider( + this, prefs_, is_off_the_record_))); content_settings::DefaultProviderInterface* policy_default_provider = - new content_settings::PolicyDefaultProvider(profile); + new content_settings::PolicyDefaultProvider(this, prefs_); default_content_settings_providers_.push_back( make_linked_ptr(policy_default_provider)); - PrefService* prefs = profile_->GetPrefs(); - // TODO(markusheintz): Discuss whether it is sensible to move migration code // to PrefContentSettingsProvider. - MigrateObsoleteCookiePref(prefs); + MigrateObsoleteCookiePref(); // Read misc. global settings. block_third_party_cookies_ = - prefs->GetBoolean(prefs::kBlockThirdPartyCookies); + prefs_->GetBoolean(prefs::kBlockThirdPartyCookies); if (block_third_party_cookies_) { UserMetrics::RecordAction( UserMetricsAction("ThirdPartyCookieBlockingEnabled")); @@ -101,15 +103,16 @@ HostContentSettingsMap::HostContentSettingsMap(Profile* profile) UserMetricsAction("ThirdPartyCookieBlockingDisabled")); } is_block_third_party_cookies_managed_ = - prefs->IsManagedPreference(prefs::kBlockThirdPartyCookies); + prefs_->IsManagedPreference(prefs::kBlockThirdPartyCookies); // User defined non default content settings are provided by the PrefProvider. // The order in which the content settings providers are created is critical, // as providers that are further up in the list (i.e. added earlier) override // providers further down. content_settings_providers_.push_back(make_linked_ptr( - new content_settings::PolicyProvider(profile, policy_default_provider))); - ExtensionService* extension_service = profile->GetExtensionService(); + new content_settings::PolicyProvider(this, + prefs_, + policy_default_provider))); if (extension_service) { // |extension_service| can be NULL in unit tests. content_settings_providers_.push_back(make_linked_ptr( @@ -118,13 +121,11 @@ HostContentSettingsMap::HostContentSettingsMap(Profile* profile) extension_service->GetExtensionContentSettingsStore(), is_off_the_record_))); } - content_settings_providers_.push_back( - make_linked_ptr(new content_settings::PrefProvider(profile))); + content_settings_providers_.push_back(make_linked_ptr( + new content_settings::PrefProvider(this, prefs_, is_off_the_record_))); - pref_change_registrar_.Init(prefs); + pref_change_registrar_.Init(prefs_); pref_change_registrar_.Add(prefs::kBlockThirdPartyCookies, this); - notification_registrar_.Add(this, chrome::NOTIFICATION_PROFILE_DESTROYED, - Source<Profile>(profile_)); } // static @@ -422,6 +423,7 @@ bool HostContentSettingsMap::IsSettingAllowedForType( void HostContentSettingsMap::SetBlockThirdPartyCookies(bool block) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + DCHECK(prefs_); // This setting may not be directly modified for OTR sessions. Instead, it // is synced to the main profile's setting. @@ -430,10 +432,9 @@ void HostContentSettingsMap::SetBlockThirdPartyCookies(bool block) { return; } - PrefService* prefs = profile_->GetPrefs(); // If the preference block-third-party-cookies is managed then do not allow to // change it. - if (prefs->IsManagedPreference(prefs::kBlockThirdPartyCookies)) { + if (prefs_->IsManagedPreference(prefs::kBlockThirdPartyCookies)) { NOTREACHED(); return; } @@ -443,7 +444,7 @@ void HostContentSettingsMap::SetBlockThirdPartyCookies(bool block) { block_third_party_cookies_ = block; } - profile_->GetPrefs()->SetBoolean(prefs::kBlockThirdPartyCookies, block); + prefs_->SetBoolean(prefs::kBlockThirdPartyCookies, block); } void HostContentSettingsMap::Observe(int type, @@ -452,32 +453,29 @@ void HostContentSettingsMap::Observe(int type, DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); if (type == chrome::NOTIFICATION_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::kBlockThirdPartyCookies) { base::AutoLock auto_lock(lock_); - block_third_party_cookies_ = profile_->GetPrefs()->GetBoolean( + block_third_party_cookies_ = prefs_->GetBoolean( prefs::kBlockThirdPartyCookies); is_block_third_party_cookies_managed_ = - profile_->GetPrefs()->IsManagedPreference( + prefs_->IsManagedPreference( prefs::kBlockThirdPartyCookies); } else { NOTREACHED() << "Unexpected preference observed"; return; } - } else if (type == chrome::NOTIFICATION_PROFILE_DESTROYED) { - DCHECK_EQ(profile_, Source<Profile>(source).ptr()); - UnregisterObservers(); } else { NOTREACHED() << "Unexpected notification"; } } HostContentSettingsMap::~HostContentSettingsMap() { - UnregisterObservers(); + DCHECK(!prefs_); } bool HostContentSettingsMap::IsDefaultContentSettingManaged( @@ -492,33 +490,32 @@ bool HostContentSettingsMap::IsDefaultContentSettingManaged( } void HostContentSettingsMap::ShutdownOnUIThread() { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + DCHECK(prefs_); + pref_change_registrar_.RemoveAll(); + prefs_ = NULL; for (ProviderIterator it = content_settings_providers_.begin(); it != content_settings_providers_.end(); ++it) { (*it)->ShutdownOnUIThread(); } + for (DefaultProviderIterator it = default_content_settings_providers_.begin(); + it != default_content_settings_providers_.end(); + ++it) { + (*it)->ShutdownOnUIThread(); + } } -void HostContentSettingsMap::UnregisterObservers() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - if (!profile_) - return; - pref_change_registrar_.RemoveAll(); - notification_registrar_.Remove(this, chrome::NOTIFICATION_PROFILE_DESTROYED, - Source<Profile>(profile_)); - profile_ = NULL; -} - -void HostContentSettingsMap::MigrateObsoleteCookiePref(PrefService* prefs) { - if (prefs->HasPrefPath(prefs::kCookieBehavior)) { - int cookie_behavior = prefs->GetInteger(prefs::kCookieBehavior); - prefs->ClearPref(prefs::kCookieBehavior); - if (!prefs->HasPrefPath(prefs::kDefaultContentSettings)) { +void HostContentSettingsMap::MigrateObsoleteCookiePref() { + if (prefs_->HasPrefPath(prefs::kCookieBehavior)) { + int cookie_behavior = prefs_->GetInteger(prefs::kCookieBehavior); + prefs_->ClearPref(prefs::kCookieBehavior); + if (!prefs_->HasPrefPath(prefs::kDefaultContentSettings)) { SetDefaultContentSetting(CONTENT_SETTINGS_TYPE_COOKIES, (cookie_behavior == net::StaticCookiePolicy::BLOCK_ALL_COOKIES) ? CONTENT_SETTING_BLOCK : CONTENT_SETTING_ALLOW); } - if (!prefs->HasPrefPath(prefs::kBlockThirdPartyCookies)) { + if (!prefs_->HasPrefPath(prefs::kBlockThirdPartyCookies)) { SetBlockThirdPartyCookies(cookie_behavior == net::StaticCookiePolicy::BLOCK_SETTING_THIRD_PARTY_COOKIES); } diff --git a/chrome/browser/content_settings/host_content_settings_map.h b/chrome/browser/content_settings/host_content_settings_map.h index f3046ad..10fda10 100644 --- a/chrome/browser/content_settings/host_content_settings_map.h +++ b/chrome/browser/content_settings/host_content_settings_map.h @@ -32,20 +32,22 @@ class ProviderInterface; class ContentSettingsDetails; class DictionaryValue; +class ExtensionService; class GURL; class PrefService; class Profile; class HostContentSettingsMap : public NotificationObserver, - public base::RefCountedThreadSafe<HostContentSettingsMap, - BrowserThread::DeleteOnUIThread> { + public base::RefCountedThreadSafe<HostContentSettingsMap> { public: typedef Tuple3<ContentSettingsPattern, ContentSetting, std::string> PatternSettingSourceTriple; typedef std::vector<PatternSettingSourceTriple> SettingsForOneType; - explicit HostContentSettingsMap(Profile* profile); + HostContentSettingsMap(PrefService* prefs, + ExtensionService* extension_service, + bool incognito); static void RegisterUserPrefs(PrefService* prefs); @@ -194,8 +196,7 @@ class HostContentSettingsMap const NotificationDetails& details); private: - friend struct BrowserThread::DeleteOnThread<BrowserThread::UI>; - friend class DeleteTask<HostContentSettingsMap>; + friend class base::RefCountedThreadSafe<HostContentSettingsMap>; virtual ~HostContentSettingsMap(); @@ -205,16 +206,13 @@ class HostContentSettingsMap ContentSettingsType content_type, const std::string& resource_identifier) const; - void UnregisterObservers(); - // Various migration methods (old cookie, popup and per-host data gets // migrated to the new format). - void MigrateObsoleteCookiePref(PrefService* prefs); + void MigrateObsoleteCookiePref(); - // The profile we're associated with. - Profile* profile_; + // Weak; owned by the Profile. + PrefService* prefs_; - NotificationRegistrar notification_registrar_; PrefChangeRegistrar pref_change_registrar_; // Whether this settings map is for an OTR session. diff --git a/chrome/browser/content_settings/host_content_settings_map_unittest.cc b/chrome/browser/content_settings/host_content_settings_map_unittest.cc index 4563cbb..5490638 100644 --- a/chrome/browser/content_settings/host_content_settings_map_unittest.cc +++ b/chrome/browser/content_settings/host_content_settings_map_unittest.cc @@ -617,10 +617,10 @@ TEST_F(HostContentSettingsMapTest, OffTheRecord) { TestingProfile profile; HostContentSettingsMap* host_content_settings_map = profile.GetHostContentSettingsMap(); - profile.set_incognito(true); scoped_refptr<HostContentSettingsMap> otr_map( - new HostContentSettingsMap(&profile)); - profile.set_incognito(false); + new HostContentSettingsMap(profile.GetPrefs(), + profile.GetExtensionService(), + true)); GURL host("http://example.com/"); ContentSettingsPattern pattern = @@ -660,6 +660,8 @@ TEST_F(HostContentSettingsMapTest, OffTheRecord) { EXPECT_EQ(CONTENT_SETTING_ALLOW, otr_map->GetContentSetting( host, host, CONTENT_SETTINGS_TYPE_IMAGES, "")); + + otr_map->ShutdownOnUIThread(); } TEST_F(HostContentSettingsMapTest, MigrateObsoletePrefs) { diff --git a/chrome/browser/profiles/profile.cc b/chrome/browser/profiles/profile.cc index 2e23117..ab8534d 100644 --- a/chrome/browser/profiles/profile.cc +++ b/chrome/browser/profiles/profile.cc @@ -517,8 +517,10 @@ class OffTheRecordProfileImpl : public Profile, // Retrieve the host content settings map of the parent profile in order to // ensure the preferences have been migrated. profile_->GetHostContentSettingsMap(); - if (!host_content_settings_map_.get()) - host_content_settings_map_ = new HostContentSettingsMap(this); + if (!host_content_settings_map_.get()) { + host_content_settings_map_ = new HostContentSettingsMap( + GetPrefs(), GetExtensionService(), true); + } return host_content_settings_map_.get(); } diff --git a/chrome/browser/profiles/profile_impl.cc b/chrome/browser/profiles/profile_impl.cc index b6bef63..619ff13 100644 --- a/chrome/browser/profiles/profile_impl.cc +++ b/chrome/browser/profiles/profile_impl.cc @@ -947,8 +947,10 @@ net::SSLConfigService* ProfileImpl::GetSSLConfigService() { } HostContentSettingsMap* ProfileImpl::GetHostContentSettingsMap() { - if (!host_content_settings_map_.get()) - host_content_settings_map_ = new HostContentSettingsMap(this); + if (!host_content_settings_map_.get()) { + host_content_settings_map_ = new HostContentSettingsMap( + GetPrefs(), GetExtensionService(), false); + } return host_content_settings_map_.get(); } diff --git a/chrome/test/testing_profile.cc b/chrome/test/testing_profile.cc index 2738406..ec65b2b 100644 --- a/chrome/test/testing_profile.cc +++ b/chrome/test/testing_profile.cc @@ -175,14 +175,14 @@ TestingProfile::~TestingProfile() { profile_dependency_manager_->DestroyProfileServices(this); + if (host_content_settings_map_) + host_content_settings_map_->ShutdownOnUIThread(); + DestroyTopSites(); DestroyHistoryService(); // FaviconService depends on HistoryServce so destroying it later. DestroyFaviconService(); DestroyWebDataService(); - if (extension_service_.get()) { - extension_service_.reset(); - } if (pref_proxy_config_tracker_.get()) pref_proxy_config_tracker_->DetachFromPrefService(); @@ -611,8 +611,10 @@ FindBarState* TestingProfile::GetFindBarState() { } HostContentSettingsMap* TestingProfile::GetHostContentSettingsMap() { - if (!host_content_settings_map_.get()) - host_content_settings_map_ = new HostContentSettingsMap(this); + if (!host_content_settings_map_.get()) { + host_content_settings_map_ = new HostContentSettingsMap( + GetPrefs(), GetExtensionService(), false); + } return host_content_settings_map_.get(); } diff --git a/tools/valgrind/memcheck/suppressions.txt b/tools/valgrind/memcheck/suppressions.txt index 7b39526..3920dda 100644 --- a/tools/valgrind/memcheck/suppressions.txt +++ b/tools/valgrind/memcheck/suppressions.txt @@ -3328,23 +3328,6 @@ fun:_ZN14ProfileManager10AddProfileEP7Profileb } { - bug_66853_b - Memcheck:Leak - fun:_Znw* - fun:_ZN11ProfileImpl25GetHostContentSettingsMapEv - ... - fun:_ZNK17ProfileImplIOData6Handle27GetMainRequestContextGetterEv - fun:_ZN11ProfileImpl17GetRequestContextEv - fun:_ZN19SafeBrowsingService5StartEv - fun:_ZN19SafeBrowsingService10InitializeEv - fun:_ZN22ResourceDispatcherHost10InitializeEv - fun:_ZN18BrowserProcessImpl28CreateResourceDispatcherHostEv - fun:_ZN18BrowserProcessImpl24resource_dispatcher_hostEv - fun:_ZN16ExtensionService4InitEv - fun:_ZN11ProfileImpl14InitExtensionsE* - fun:_ZN14ProfileManager10AddProfileEP7Profileb -} -{ bug_67142 Memcheck:Leak fun:_Znw* @@ -4804,22 +4787,6 @@ } { - bug_88557 - Memcheck:Leak - fun:_Znw* - fun:_ZN14ExtensionPrefsC1EP11PrefServiceRK8FilePathP21ExtensionPrefValueMap - fun:_ZN11ProfileImpl13OnPrefsLoadedEb - fun:_ZN11ProfileImplC1ERK8FilePathPN7Profile8DelegateE - fun:_ZN7Profile13CreateProfileERK8FilePath - fun:_ZN14ProfileManager10GetProfileERK8FilePath - fun:_ZN14ProfileManager17GetDefaultProfileERK8FilePath - fun:_ZN12_GLOBAL__N_113CreateProfileERK18MainFunctionParamsRK8FilePath - fun:_Z11BrowserMainRK18MainFunctionParams - fun:_ZN12_GLOBAL__N_123RunNamedProcessTypeMainERKSsRK18MainFunctionParams - fun:ChromeMain - fun:main -} -{ bug_88735 Memcheck:Leak fun:_Znw* |