summaryrefslogtreecommitdiffstats
path: root/chrome
diff options
context:
space:
mode:
authordanno@chromium.org <danno@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-09-29 12:24:28 +0000
committerdanno@chromium.org <danno@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-09-29 12:24:28 +0000
commit2fb7dc983456e980d631501f4a120eb091d197e7 (patch)
treefa96aef59bf1900f56ae1b0457c4f9d36e0fb38e /chrome
parent0ce04e5a148136f3849e8a8b0dd351a0fc4798b5 (diff)
downloadchromium_src-2fb7dc983456e980d631501f4a120eb091d197e7.zip
chromium_src-2fb7dc983456e980d631501f4a120eb091d197e7.tar.gz
chromium_src-2fb7dc983456e980d631501f4a120eb091d197e7.tar.bz2
Use PrefChangeRegistrar everywhere
BUG=54955 TEST=PrefChangeRegistrarTest.* Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=60169 Review URL: http://codereview.chromium.org/3304015 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@60935 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome')
-rw-r--r--chrome/browser/background_mode_manager.cc7
-rw-r--r--chrome/browser/background_mode_manager.h4
-rw-r--r--chrome/browser/cocoa/content_settings_dialog_controller.h2
-rw-r--r--chrome/browser/cocoa/content_settings_dialog_controller.mm24
-rw-r--r--chrome/browser/cocoa/extensions/extension_action_context_menu.mm12
-rw-r--r--chrome/browser/cocoa/extensions/extension_popup_controller_unittest.mm3
-rw-r--r--chrome/browser/cocoa/preferences_window_controller.h2
-rw-r--r--chrome/browser/cocoa/preferences_window_controller.mm16
-rw-r--r--chrome/browser/dom_ui/core_options_handler.cc15
-rw-r--r--chrome/browser/dom_ui/core_options_handler.h5
-rw-r--r--chrome/browser/dom_ui/ntp_resource_cache.cc12
-rw-r--r--chrome/browser/dom_ui/ntp_resource_cache.h4
-rw-r--r--chrome/browser/dom_ui/shown_sections_handler.cc7
-rw-r--r--chrome/browser/dom_ui/shown_sections_handler.h4
-rw-r--r--chrome/browser/extensions/extension_toolbar_model.cc4
-rw-r--r--chrome/browser/extensions/extension_toolbar_model.h4
-rw-r--r--chrome/browser/extensions/extensions_service.cc21
-rw-r--r--chrome/browser/extensions/extensions_service.h3
-rw-r--r--chrome/browser/extensions/extensions_service_unittest.cc70
-rw-r--r--chrome/browser/extensions/extensions_service_unittest.h1
-rw-r--r--chrome/browser/gtk/gtk_theme_provider.cc4
-rw-r--r--chrome/browser/gtk/gtk_theme_provider.h3
-rw-r--r--chrome/browser/gtk/options/general_page_gtk.cc9
-rw-r--r--chrome/browser/gtk/options/general_page_gtk.h3
-rw-r--r--chrome/browser/host_content_settings_map.cc15
-rw-r--r--chrome/browser/host_content_settings_map.h2
-rw-r--r--chrome/browser/host_zoom_map.cc8
-rw-r--r--chrome/browser/host_zoom_map.h2
-rw-r--r--chrome/browser/host_zoom_map_unittest.cc10
-rw-r--r--chrome/browser/net/chrome_url_request_context.cc21
-rw-r--r--chrome/browser/net/chrome_url_request_context.h4
-rw-r--r--chrome/browser/notifications/desktop_notification_service.cc15
-rw-r--r--chrome/browser/notifications/desktop_notification_service.h3
-rw-r--r--chrome/browser/prefs/pref_change_registrar.cc23
-rw-r--r--chrome/browser/prefs/pref_change_registrar.h9
-rw-r--r--chrome/browser/prefs/pref_change_registrar_unittest.cc31
-rw-r--r--chrome/browser/prefs/pref_service.h26
-rw-r--r--chrome/browser/prefs/pref_service_unittest.cc40
-rw-r--r--chrome/browser/prefs/pref_set_observer.cc10
-rw-r--r--chrome/browser/prefs/pref_set_observer.h4
-rw-r--r--chrome/browser/profile_impl.cc15
-rw-r--r--chrome/browser/profile_impl.h2
-rw-r--r--chrome/browser/sync/glue/preference_change_processor.cc5
-rw-r--r--chrome/browser/sync/glue/preference_change_processor.h3
-rw-r--r--chrome/browser/tab_contents/tab_contents.cc11
-rw-r--r--chrome/browser/tab_contents/tab_contents.h4
-rw-r--r--chrome/browser/translate/translate_manager.cc6
-rw-r--r--chrome/browser/translate/translate_manager.h2
-rw-r--r--chrome/browser/translate/translate_manager_unittest.cc25
-rw-r--r--chrome/browser/views/options/general_page_view.cc8
-rw-r--r--chrome/browser/views/options/general_page_view.h3
-rw-r--r--chrome/test/testing_profile.cc42
-rw-r--r--chrome/test/testing_profile.h26
53 files changed, 368 insertions, 246 deletions
diff --git a/chrome/browser/background_mode_manager.cc b/chrome/browser/background_mode_manager.cc
index 01a9a36..b0110a1 100644
--- a/chrome/browser/background_mode_manager.cc
+++ b/chrome/browser/background_mode_manager.cc
@@ -163,7 +163,8 @@ BackgroundModeManager::BackgroundModeManager(Profile* profile,
NotificationService::AllSources());
// Listen for changes to the background mode preference.
- profile_->GetPrefs()->AddPrefObserver(prefs::kBackgroundModeEnabled, this);
+ pref_registrar_.Init(profile_->GetPrefs());
+ pref_registrar_.Add(prefs::kBackgroundModeEnabled, this);
}
BackgroundModeManager::~BackgroundModeManager() {
@@ -172,10 +173,6 @@ BackgroundModeManager::~BackgroundModeManager() {
// because in an actual running system we'd get an APP_TERMINATING
// notification before being destroyed.
EndBackgroundMode();
- // Manually remove our pref observer so we don't get notified for prefs
- // changes (have to do it manually because we can't use the registrar for
- // prefs notifications).
- profile_->GetPrefs()->RemovePrefObserver(prefs::kBackgroundModeEnabled, this);
}
bool BackgroundModeManager::IsBackgroundModeEnabled() {
diff --git a/chrome/browser/background_mode_manager.h b/chrome/browser/background_mode_manager.h
index 46e8847..52f37da 100644
--- a/chrome/browser/background_mode_manager.h
+++ b/chrome/browser/background_mode_manager.h
@@ -8,6 +8,7 @@
#include "app/menus/simple_menu_model.h"
#include "base/gtest_prod_util.h"
+#include "chrome/browser/prefs/pref_change_registrar.h"
#include "chrome/browser/status_icons/status_icon.h"
#include "chrome/common/notification_observer.h"
#include "chrome/common/notification_registrar.h"
@@ -142,6 +143,9 @@ class BackgroundModeManager
// Reference to our status icon (if any) - owned by the StatusTray.
StatusIcon* status_icon_;
+ // Ensure observed preferences are properly cleaned up.
+ PrefChangeRegistrar pref_registrar_;
+
DISALLOW_COPY_AND_ASSIGN(BackgroundModeManager);
};
diff --git a/chrome/browser/cocoa/content_settings_dialog_controller.h b/chrome/browser/cocoa/content_settings_dialog_controller.h
index d48b962..8fe92da 100644
--- a/chrome/browser/cocoa/content_settings_dialog_controller.h
+++ b/chrome/browser/cocoa/content_settings_dialog_controller.h
@@ -7,6 +7,7 @@
#import "base/cocoa_protocols_mac.h"
#include "base/scoped_ptr.h"
#include "chrome/common/content_settings_types.h"
+#include "chrome/browser/prefs/pref_change_registrar.h"
#include "chrome/browser/prefs/pref_member.h"
// Index of the "enabled" and "disabled" radio group settings in all tabs except
@@ -50,6 +51,7 @@ class Profile;
Profile* profile_; // weak
IntegerPrefMember lastSelectedTab_;
BooleanPrefMember clearSiteDataOnExit_;
+ PrefChangeRegistrar registrar_;
scoped_ptr<ContentSettingsDialogControllerInternal::PrefObserverBridge>
observer_; // Watches for pref changes.
}
diff --git a/chrome/browser/cocoa/content_settings_dialog_controller.mm b/chrome/browser/cocoa/content_settings_dialog_controller.mm
index 9ed2078..001a3ec 100644
--- a/chrome/browser/cocoa/content_settings_dialog_controller.mm
+++ b/chrome/browser/cocoa/content_settings_dialog_controller.mm
@@ -140,11 +140,11 @@ class PrefObserverDisabler {
// Manually observe notifications for preferences that are grouped in
// the HostContentSettingsMap or GeolocationContentSettingsMap.
PrefService* prefs = profile_->GetPrefs();
- prefs->AddPrefObserver(prefs::kBlockThirdPartyCookies, observer_.get());
- prefs->AddPrefObserver(prefs::kBlockNonsandboxedPlugins, observer_.get());
- prefs->AddPrefObserver(prefs::kDefaultContentSettings, observer_.get());
- prefs->AddPrefObserver(prefs::kGeolocationDefaultContentSetting,
- observer_.get());
+ registrar_.Init(prefs);
+ registrar_.Add(prefs::kBlockThirdPartyCookies, observer_.get());
+ registrar_.Add(prefs::kBlockNonsandboxedPlugins, observer_.get());
+ registrar_.Add(prefs::kDefaultContentSettings, observer_.get());
+ registrar_.Add(prefs::kGeolocationDefaultContentSetting, observer_.get());
// We don't need to observe changes in this value.
lastSelectedTab_.Init(prefs::kContentSettingsWindowLastTabIndex,
@@ -153,20 +153,6 @@ class PrefObserverDisabler {
return self;
}
-- (void)dealloc {
- if (profile_) {
- PrefService* prefs = profile_->GetPrefs();
- prefs->RemovePrefObserver(prefs::kBlockThirdPartyCookies, observer_.get());
- prefs->RemovePrefObserver(prefs::kBlockNonsandboxedPlugins,
- observer_.get());
- prefs->RemovePrefObserver(prefs::kDefaultContentSettings, observer_.get());
- prefs->RemovePrefObserver(prefs::kGeolocationDefaultContentSetting,
- observer_.get());
- }
-
- [super dealloc];
-}
-
- (void)closeExceptionsSheet {
NSWindow* attachedSheet = [[self window] attachedSheet];
if (attachedSheet) {
diff --git a/chrome/browser/cocoa/extensions/extension_action_context_menu.mm b/chrome/browser/cocoa/extensions/extension_action_context_menu.mm
index 10ea079..81aa61e 100644
--- a/chrome/browser/cocoa/extensions/extension_action_context_menu.mm
+++ b/chrome/browser/cocoa/extensions/extension_action_context_menu.mm
@@ -18,6 +18,7 @@
#include "chrome/browser/extensions/extension_install_ui.h"
#include "chrome/browser/extensions/extensions_service.h"
#include "chrome/browser/extensions/extension_tabs_module.h"
+#include "chrome/browser/prefs/pref_change_registrar.h"
#include "chrome/browser/prefs/pref_service.h"
#include "chrome/browser/profile.h"
#include "chrome/common/extensions/extension.h"
@@ -71,14 +72,10 @@ class DevmodeObserver : public NotificationObserver {
DevmodeObserver(ExtensionActionContextMenu* menu,
PrefService* service)
: menu_(menu), pref_service_(service) {
- pref_service_->AddPrefObserver(prefs::kExtensionsUIDeveloperMode,
- this);
- }
-
- ~DevmodeObserver() {
- pref_service_->RemovePrefObserver(prefs::kExtensionsUIDeveloperMode,
- this);
+ registrar_.Init(pref_service_);
+ registrar_.Add(prefs::kExtensionsUIDeveloperMode, this);
}
+ virtual ~DevmodeObserver() {}
void Observe(NotificationType type,
const NotificationSource& source,
@@ -92,6 +89,7 @@ class DevmodeObserver : public NotificationObserver {
private:
ExtensionActionContextMenu* menu_;
PrefService* pref_service_;
+ PrefChangeRegistrar registrar_;
};
} // namespace extension_action_context_menu
diff --git a/chrome/browser/cocoa/extensions/extension_popup_controller_unittest.mm b/chrome/browser/cocoa/extensions/extension_popup_controller_unittest.mm
index 110f719..ba7e2b7 100644
--- a/chrome/browser/cocoa/extensions/extension_popup_controller_unittest.mm
+++ b/chrome/browser/cocoa/extensions/extension_popup_controller_unittest.mm
@@ -27,8 +27,7 @@ class ExtensionTestingProfile : public TestingProfile {
manager_.reset(ExtensionProcessManager::Create(this));
service_ = new ExtensionsService(this,
CommandLine::ForCurrentProcess(),
- GetPrefs(),
- GetExtensionsInstallDir(),
+ GetExtensionsInstallDir(),
false);
service_->set_extensions_enabled(true);
service_->set_show_extensions_prompts(false);
diff --git a/chrome/browser/cocoa/preferences_window_controller.h b/chrome/browser/cocoa/preferences_window_controller.h
index e7b8383..54d4778 100644
--- a/chrome/browser/cocoa/preferences_window_controller.h
+++ b/chrome/browser/cocoa/preferences_window_controller.h
@@ -9,6 +9,7 @@
#include "chrome/browser/options_window.h"
#include "chrome/browser/prefs/pref_member.h"
#include "chrome/browser/prefs/pref_set_observer.h"
+#include "chrome/browser/prefs/pref_change_registrar.h"
namespace PreferencesWindowControllerInternal {
class PrefObserverBridge;
@@ -45,6 +46,7 @@ class ProfileSyncService;
ProfileSyncService* syncService_;
scoped_ptr<PreferencesWindowControllerInternal::PrefObserverBridge>
observer_; // Watches for pref changes.
+ PrefChangeRegistrar registrar_; // Manages pref change observer registration.
scoped_nsobject<WindowSizeAutosaver> sizeSaver_;
NSView* currentPrefsView_; // weak ref - current prefs page view.
scoped_ptr<PreferencesWindowControllerInternal::ManagedPrefsBannerState>
diff --git a/chrome/browser/cocoa/preferences_window_controller.mm b/chrome/browser/cocoa/preferences_window_controller.mm
index c7c2b7b..0f11f63 100644
--- a/chrome/browser/cocoa/preferences_window_controller.mm
+++ b/chrome/browser/cocoa/preferences_window_controller.mm
@@ -330,7 +330,6 @@ CGFloat AutoSizeUnderTheHoodContent(NSView* view,
// Record the user performed a certain action and save the preferences.
- (void)recordUserAction:(const UserMetricsAction&) action;
- (void)registerPrefObservers;
-- (void)unregisterPrefObservers;
// KVC setter methods.
- (void)setNewTabPageIsHomePageIndex:(NSInteger)val;
@@ -761,7 +760,6 @@ class ManagedPrefsBannerState : public policy::ManagedPrefsBannerBase {
syncService_->RemoveObserver(observer_.get());
}
[[NSNotificationCenter defaultCenter] removeObserver:self];
- [self unregisterPrefObservers];
[animation_ setDelegate:nil];
[animation_ stopAnimation];
[super dealloc];
@@ -781,7 +779,8 @@ class ManagedPrefsBannerState : public policy::ManagedPrefsBannerBase {
if (!prefs_) return;
// Basics panel
- prefs_->AddPrefObserver(prefs::kURLsToRestoreOnStartup, observer_.get());
+ registrar_.Init(prefs_);
+ registrar_.Add(prefs::kURLsToRestoreOnStartup, observer_.get());
restoreOnStartup_.Init(prefs::kRestoreOnStartup, prefs_, observer_.get());
newTabPageIsHomePage_.Init(prefs::kHomePageIsNewTabPage,
prefs_, observer_.get());
@@ -823,17 +822,6 @@ class ManagedPrefsBannerState : public policy::ManagedPrefsBannerBase {
lastSelectedPage_.Init(prefs::kOptionsWindowLastTabIndex, local, NULL);
}
-// Clean up what was registered in -registerPrefObservers. We only have to
-// clean up the non-PrefMember registrations.
-- (void)unregisterPrefObservers {
- if (!prefs_) return;
-
- // Basics
- prefs_->RemovePrefObserver(prefs::kURLsToRestoreOnStartup, observer_.get());
-
- // Nothing to do for other panels...
-}
-
// Called when the window wants to be closed.
- (BOOL)windowShouldClose:(id)sender {
// Stop any animation and clear the delegate to avoid stale pointers.
diff --git a/chrome/browser/dom_ui/core_options_handler.cc b/chrome/browser/dom_ui/core_options_handler.cc
index 2739ebc..849e768b 100644
--- a/chrome/browser/dom_ui/core_options_handler.cc
+++ b/chrome/browser/dom_ui/core_options_handler.cc
@@ -88,6 +88,13 @@ void CoreOptionsHandler::Uninitialize() {
}
}
+DOMMessageHandler* CoreOptionsHandler::Attach(DOMUI* dom_ui) {
+ DOMMessageHandler* result = DOMMessageHandler::Attach(dom_ui);
+ DCHECK(dom_ui_);
+ registrar_.Init(dom_ui_->GetProfile()->GetPrefs());
+ return result;
+}
+
void CoreOptionsHandler::Observe(NotificationType type,
const NotificationSource& source,
const NotificationDetails& details) {
@@ -138,9 +145,7 @@ Value* CoreOptionsHandler::FetchPref(const std::string& pref_name) {
}
void CoreOptionsHandler::ObservePref(const std::string& pref_name) {
- DCHECK(dom_ui_);
- PrefService* pref_service = dom_ui_->GetProfile()->GetPrefs();
- pref_service->AddPrefObserver(pref_name.c_str(), this);
+ registrar_.Add(pref_name.c_str(), this);
}
void CoreOptionsHandler::SetPref(const std::string& pref_name,
@@ -185,9 +190,7 @@ void CoreOptionsHandler::ProcessUserMetric(Value::ValueType pref_type,
}
void CoreOptionsHandler::StopObservingPref(const std::string& path) {
- DCHECK(dom_ui_);
- PrefService* pref_service = dom_ui_->GetProfile()->GetPrefs();
- pref_service->RemovePrefObserver(path.c_str(), this);
+ registrar_.Remove(path.c_str(), this);
}
void CoreOptionsHandler::HandleFetchPrefs(const ListValue* args) {
diff --git a/chrome/browser/dom_ui/core_options_handler.h b/chrome/browser/dom_ui/core_options_handler.h
index 748ab83..fd94d10 100644
--- a/chrome/browser/dom_ui/core_options_handler.h
+++ b/chrome/browser/dom_ui/core_options_handler.h
@@ -11,6 +11,7 @@
#include "base/values.h"
#include "chrome/browser/dom_ui/options_ui.h"
+#include "chrome/browser/prefs/pref_change_registrar.h"
// Core options UI handler.
// Handles resource and JS calls common to all options sub-pages.
@@ -22,7 +23,6 @@ class CoreOptionsHandler : public OptionsPageUIHandler {
virtual void GetLocalizedValues(DictionaryValue* localized_strings);
virtual void Uninitialize();
-
// NotificationObserver implementation.
virtual void Observe(NotificationType type,
const NotificationSource& source,
@@ -30,6 +30,7 @@ class CoreOptionsHandler : public OptionsPageUIHandler {
// DOMMessageHandler implementation.
virtual void RegisterMessages();
+ virtual DOMMessageHandler* Attach(DOMUI* dom_ui);
protected:
// Fetches a pref value of given |pref_name|.
@@ -87,6 +88,8 @@ class CoreOptionsHandler : public OptionsPageUIHandler {
void NotifyPrefChanged(const std::string* pref_name);
+ PrefChangeRegistrar registrar_;
+
DISALLOW_COPY_AND_ASSIGN(CoreOptionsHandler);
};
diff --git a/chrome/browser/dom_ui/ntp_resource_cache.cc b/chrome/browser/dom_ui/ntp_resource_cache.cc
index 600710f..bd7ff37 100644
--- a/chrome/browser/dom_ui/ntp_resource_cache.cc
+++ b/chrome/browser/dom_ui/ntp_resource_cache.cc
@@ -133,15 +133,9 @@ NTPResourceCache::NTPResourceCache(Profile* profile) : profile_(profile) {
NotificationService::AllSources());
// Watch for pref changes that cause us to need to invalidate the HTML cache.
- PrefService* pref_service = profile_->GetPrefs();
- pref_service->AddPrefObserver(prefs::kShowBookmarkBar, this);
- pref_service->AddPrefObserver(prefs::kNTPShownSections, this);
-}
-
-NTPResourceCache::~NTPResourceCache() {
- PrefService* pref_service = profile_->GetPrefs();
- pref_service->RemovePrefObserver(prefs::kShowBookmarkBar, this);
- pref_service->RemovePrefObserver(prefs::kNTPShownSections, this);
+ pref_change_registrar_.Init(profile_->GetPrefs());
+ pref_change_registrar_.Add(prefs::kShowBookmarkBar, this);
+ pref_change_registrar_.Add(prefs::kNTPShownSections, this);
}
RefCountedBytes* NTPResourceCache::GetNewTabHTML(bool is_off_the_record) {
diff --git a/chrome/browser/dom_ui/ntp_resource_cache.h b/chrome/browser/dom_ui/ntp_resource_cache.h
index f669325..3cdc3e5 100644
--- a/chrome/browser/dom_ui/ntp_resource_cache.h
+++ b/chrome/browser/dom_ui/ntp_resource_cache.h
@@ -10,6 +10,7 @@
#include "base/ref_counted.h"
#include "chrome/common/notification_observer.h"
#include "chrome/common/notification_registrar.h"
+#include "chrome/browser/prefs/pref_change_registrar.h"
class Profile;
class RefCountedBytes;
@@ -19,7 +20,7 @@ class RefCountedBytes;
class NTPResourceCache : public NotificationObserver {
public:
explicit NTPResourceCache(Profile* profile);
- virtual ~NTPResourceCache();
+ virtual ~NTPResourceCache() {}
RefCountedBytes* GetNewTabHTML(bool is_off_the_record);
RefCountedBytes* GetNewTabCSS(bool is_off_the_record);
@@ -43,6 +44,7 @@ class NTPResourceCache : public NotificationObserver {
scoped_refptr<RefCountedBytes> new_tab_css_;
NotificationRegistrar registrar_;
+ PrefChangeRegistrar pref_change_registrar_;
DISALLOW_COPY_AND_ASSIGN(NTPResourceCache);
};
diff --git a/chrome/browser/dom_ui/shown_sections_handler.cc b/chrome/browser/dom_ui/shown_sections_handler.cc
index 78ff716..e32511d 100644
--- a/chrome/browser/dom_ui/shown_sections_handler.cc
+++ b/chrome/browser/dom_ui/shown_sections_handler.cc
@@ -48,11 +48,8 @@ int ShownSectionsHandler::GetShownSections(PrefService* prefs) {
ShownSectionsHandler::ShownSectionsHandler(PrefService* pref_service)
: pref_service_(pref_service) {
- pref_service_->AddPrefObserver(prefs::kNTPShownSections, this);
-}
-
-ShownSectionsHandler::~ShownSectionsHandler() {
- pref_service_->RemovePrefObserver(prefs::kNTPShownSections, this);
+ registrar_.Init(pref_service);
+ registrar_.Add(prefs::kNTPShownSections, this);
}
void ShownSectionsHandler::RegisterMessages() {
diff --git a/chrome/browser/dom_ui/shown_sections_handler.h b/chrome/browser/dom_ui/shown_sections_handler.h
index 14a3215..a281346 100644
--- a/chrome/browser/dom_ui/shown_sections_handler.h
+++ b/chrome/browser/dom_ui/shown_sections_handler.h
@@ -8,6 +8,7 @@
#include "chrome/browser/dom_ui/dom_ui.h"
#include "chrome/common/notification_observer.h"
+#include "chrome/browser/prefs/pref_change_registrar.h"
class DOMUI;
class Value;
@@ -25,7 +26,7 @@ class ShownSectionsHandler : public DOMMessageHandler,
public NotificationObserver {
public:
explicit ShownSectionsHandler(PrefService* pref_service);
- virtual ~ShownSectionsHandler();
+ virtual ~ShownSectionsHandler() {}
// Helper to get the current shown sections.
static int GetShownSections(PrefService* pref_service);
@@ -52,6 +53,7 @@ class ShownSectionsHandler : public DOMMessageHandler,
private:
PrefService* pref_service_;
+ PrefChangeRegistrar registrar_;
DISALLOW_COPY_AND_ASSIGN(ShownSectionsHandler);
};
diff --git a/chrome/browser/extensions/extension_toolbar_model.cc b/chrome/browser/extensions/extension_toolbar_model.cc
index 325f0c4..100c7aa 100644
--- a/chrome/browser/extensions/extension_toolbar_model.cc
+++ b/chrome/browser/extensions/extension_toolbar_model.cc
@@ -33,6 +33,10 @@ ExtensionToolbarModel::ExtensionToolbarModel(ExtensionsService* service)
ExtensionToolbarModel::~ExtensionToolbarModel() {
}
+void ExtensionToolbarModel::DestroyingProfile() {
+ registrar_.RemoveAll();
+}
+
void ExtensionToolbarModel::AddObserver(Observer* observer) {
observers_.AddObserver(observer);
}
diff --git a/chrome/browser/extensions/extension_toolbar_model.h b/chrome/browser/extensions/extension_toolbar_model.h
index 028c8b5..a38fc8c 100644
--- a/chrome/browser/extensions/extension_toolbar_model.h
+++ b/chrome/browser/extensions/extension_toolbar_model.h
@@ -20,6 +20,10 @@ class ExtensionToolbarModel : public NotificationObserver {
explicit ExtensionToolbarModel(ExtensionsService* service);
~ExtensionToolbarModel();
+ // Notifies the toolbar model that the Profile that suplied its
+ // prefs is being destroyed.
+ void DestroyingProfile();
+
// A class which is informed of changes to the model; represents the view of
// MVC.
class Observer {
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index e2e6342..017890f 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -528,11 +528,11 @@ bool ExtensionsService::IsDownloadFromMiniGallery(const GURL& download_url) {
ExtensionsService::ExtensionsService(Profile* profile,
const CommandLine* command_line,
- PrefService* prefs,
const FilePath& install_directory,
bool autoupdate_enabled)
: profile_(profile),
- extension_prefs_(new ExtensionPrefs(prefs, install_directory)),
+ extension_prefs_(new ExtensionPrefs(profile->GetPrefs(),
+ install_directory)),
install_directory_(install_directory),
extensions_enabled_(true),
show_extensions_prompts_(true),
@@ -547,8 +547,9 @@ ExtensionsService::ExtensionsService(Profile* profile,
registrar_.Add(this, NotificationType::EXTENSION_PROCESS_TERMINATED,
NotificationService::AllSources());
- prefs->AddPrefObserver(prefs::kExtensionInstallAllowList, this);
- prefs->AddPrefObserver(prefs::kExtensionInstallDenyList, this);
+ pref_change_registrar_.Init(profile->GetPrefs());
+ pref_change_registrar_.Add(prefs::kExtensionInstallAllowList, this);
+ pref_change_registrar_.Add(prefs::kExtensionInstallDenyList, this);
// Set up the ExtensionUpdater
if (autoupdate_enabled) {
@@ -558,7 +559,9 @@ ExtensionsService::ExtensionsService(Profile* profile,
switches::kExtensionsUpdateFrequency),
&update_frequency);
}
- updater_ = new ExtensionUpdater(this, prefs, update_frequency);
+ updater_ = new ExtensionUpdater(this,
+ profile->GetPrefs(),
+ update_frequency);
}
backend_ = new ExtensionsServiceBackend(install_directory_,
@@ -572,6 +575,7 @@ ExtensionsService::ExtensionsService(Profile* profile,
}
ExtensionsService::~ExtensionsService() {
+ DCHECK(!profile_); // Profile should have told us it's going away.
UnloadAllExtensions();
if (updater_.get()) {
updater_->Stop();
@@ -1186,12 +1190,9 @@ void ExtensionsService::UpdateExtensionBlacklist(
}
void ExtensionsService::DestroyingProfile() {
- profile_->GetPrefs()->RemovePrefObserver(
- prefs::kExtensionInstallAllowList, this);
- profile_->GetPrefs()->RemovePrefObserver(
- prefs::kExtensionInstallDenyList, this);
-
+ pref_change_registrar_.RemoveAll();
profile_ = NULL;
+ toolbar_model_.DestroyingProfile();
}
void ExtensionsService::CheckAdminBlacklist() {
diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h
index cc9bb769..7d5ad97 100644
--- a/chrome/browser/extensions/extensions_service.h
+++ b/chrome/browser/extensions/extensions_service.h
@@ -28,6 +28,7 @@
#include "chrome/browser/extensions/extensions_quota_service.h"
#include "chrome/browser/extensions/external_extension_provider.h"
#include "chrome/browser/extensions/sandboxed_extension_unpacker.h"
+#include "chrome/browser/prefs/pref_change_registrar.h"
#include "chrome/common/notification_observer.h"
#include "chrome/common/notification_registrar.h"
#include "chrome/common/extensions/extension.h"
@@ -140,7 +141,6 @@ class ExtensionsService
ExtensionsService(Profile* profile,
const CommandLine* command_line,
- PrefService* prefs,
const FilePath& install_directory,
bool autoupdate_enabled);
@@ -486,6 +486,7 @@ class ExtensionsService
OrphanedDevTools orphaned_dev_tools_;
NotificationRegistrar registrar_;
+ PrefChangeRegistrar pref_change_registrar_;
// Keeps track of menu items added by extensions.
ExtensionMenuManager menu_manager_;
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index a434b8f..7781945 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -287,23 +287,22 @@ void ExtensionsServiceTestBase::InitializeExtensionsService(
ExtensionTestingProfile* profile = new ExtensionTestingProfile();
// Create a preference service that only contains user defined
// preference values.
- prefs_.reset(PrefService::CreateUserPrefService(pref_file));
+ PrefService* prefs = PrefService::CreateUserPrefService(pref_file);
+ Profile::RegisterUserPrefs(prefs);
+ browser::RegisterUserPrefs(prefs);
+ profile->SetPrefService(prefs);
- Profile::RegisterUserPrefs(prefs_.get());
- browser::RegisterUserPrefs(prefs_.get());
profile_.reset(profile);
// TODO(scherkus): Remove this when we no longer need to have Talk
// component extension state as a preference http://crbug.com/56429
DictionaryValue* dict =
- prefs_->GetMutableDictionary("extensions.settings");
+ profile->GetPrefs()->GetMutableDictionary("extensions.settings");
dict->Remove("ggnioahjipcehijkhpdjekioddnjoben", NULL);
- service_ = new ExtensionsService(profile_.get(),
- CommandLine::ForCurrentProcess(),
- prefs_.get(),
- extensions_install_dir,
- false);
+ service_ = profile->CreateExtensionsService(
+ CommandLine::ForCurrentProcess(),
+ extensions_install_dir);
service_->set_extensions_enabled(true);
service_->set_show_extensions_prompts(false);
profile->set_extensions_service(service_.get());
@@ -524,7 +523,7 @@ class ExtensionsServiceTest
void ValidatePrefKeyCount(size_t count) {
DictionaryValue* dict =
- prefs_->GetMutableDictionary("extensions.settings");
+ profile_->GetPrefs()->GetMutableDictionary("extensions.settings");
ASSERT_TRUE(dict != NULL);
EXPECT_EQ(count, dict->size());
}
@@ -539,7 +538,9 @@ class ExtensionsServiceTest
msg += " == ";
msg += expected_val ? "true" : "false";
- const DictionaryValue* dict = prefs_->GetDictionary("extensions.settings");
+ PrefService* prefs = profile_->GetPrefs();
+ const DictionaryValue* dict =
+ prefs->GetDictionary("extensions.settings");
ASSERT_TRUE(dict != NULL) << msg;
DictionaryValue* pref = NULL;
ASSERT_TRUE(dict->GetDictionary(extension_id, &pref)) << msg;
@@ -551,7 +552,8 @@ class ExtensionsServiceTest
bool IsPrefExist(const std::string& extension_id,
const std::string& pref_path) {
- const DictionaryValue* dict = prefs_->GetDictionary("extensions.settings");
+ const DictionaryValue* dict =
+ profile_->GetPrefs()->GetDictionary("extensions.settings");
if (dict == NULL) return false;
DictionaryValue* pref = NULL;
if (!dict->GetDictionary(extension_id, &pref)) {
@@ -577,7 +579,9 @@ class ExtensionsServiceTest
msg += " == ";
msg += base::IntToString(expected_val);
- const DictionaryValue* dict = prefs_->GetDictionary("extensions.settings");
+ PrefService* prefs = profile_->GetPrefs();
+ const DictionaryValue* dict =
+ prefs->GetDictionary("extensions.settings");
ASSERT_TRUE(dict != NULL) << msg;
DictionaryValue* pref = NULL;
ASSERT_TRUE(dict->GetDictionary(extension_id, &pref)) << msg;
@@ -597,7 +601,8 @@ class ExtensionsServiceTest
msg += " == ";
msg += expected_val;
- const DictionaryValue* dict = prefs_->GetDictionary("extensions.settings");
+ const DictionaryValue* dict =
+ profile_->GetPrefs()->GetDictionary("extensions.settings");
ASSERT_TRUE(dict != NULL) << msg;
DictionaryValue* pref = NULL;
std::string manifest_path = extension_id + ".manifest";
@@ -619,7 +624,7 @@ class ExtensionsServiceTest
msg += base::IntToString(value);
const DictionaryValue* dict =
- prefs_->GetMutableDictionary("extensions.settings");
+ profile_->GetPrefs()->GetMutableDictionary("extensions.settings");
ASSERT_TRUE(dict != NULL) << msg;
DictionaryValue* pref = NULL;
ASSERT_TRUE(dict->GetDictionary(extension_id, &pref)) << msg;
@@ -845,7 +850,8 @@ TEST_F(ExtensionsServiceTest, CleanupOnStartup) {
InitializeInstalledExtensionsService(pref_path, source_install_dir);
// Simulate that one of them got partially deleted by clearing its pref.
- DictionaryValue* dict = prefs_->GetMutableDictionary("extensions.settings");
+ DictionaryValue* dict =
+ profile_->GetPrefs()->GetMutableDictionary("extensions.settings");
ASSERT_TRUE(dict != NULL);
dict->Remove("behllobkkfkfnphdnhnkndlbkcpglgmj", NULL);
@@ -1868,8 +1874,10 @@ TEST_F(ExtensionsServiceTest, WillNotLoadPluginExtensionsFromDirectory) {
TEST_F(ExtensionsServiceTest, BlacklistedByPolicyWillNotInstall) {
InitializeEmptyExtensionsService();
- ListValue* whitelist = prefs_->GetMutableList("extensions.install.allowlist");
- ListValue* blacklist = prefs_->GetMutableList("extensions.install.denylist");
+ ListValue* whitelist =
+ profile_->GetPrefs()->GetMutableList("extensions.install.allowlist");
+ ListValue* blacklist =
+ profile_->GetPrefs()->GetMutableList("extensions.install.denylist");
ASSERT_TRUE(whitelist != NULL && blacklist != NULL);
// Blacklist everything.
@@ -1906,16 +1914,18 @@ TEST_F(ExtensionsServiceTest, BlacklistedByPolicyRemovedIfRunning) {
loop_.RunAllPending();
EXPECT_EQ(1u, service_->extensions()->size());
- ListValue* blacklist = prefs_->GetMutableList("extensions.install.denylist");
+ PrefService* prefs = profile_->GetPrefs();
+ ListValue* blacklist =
+ prefs->GetMutableList("extensions.install.denylist");
ASSERT_TRUE(blacklist != NULL);
// Blacklist this extension.
blacklist->Append(Value::CreateStringValue(good_crx));
- prefs_->ScheduleSavePersistentPrefs();
+ prefs->ScheduleSavePersistentPrefs();
// Programmatically appending to the prefs doesn't seem to notify the
// observers... :/
- prefs_->pref_notifier()->FireObservers("extensions.install.denylist");
+ prefs->pref_notifier()->FireObservers("extensions.install.denylist");
// Extension should not be running now.
loop_.RunAllPending();
@@ -2233,7 +2243,7 @@ void ExtensionsServiceTest::TestExternalProvider(
// Now clear the preference and reinstall.
SetPrefInteg(good_crx, "state", Extension::ENABLED);
- prefs_->ScheduleSavePersistentPrefs();
+ profile_->GetPrefs()->ScheduleSavePersistentPrefs();
loaded_.clear();
service_->CheckForExternalUpdates();
@@ -2461,8 +2471,8 @@ TEST(ExtensionsServiceTestSimple, Enabledness) {
// By default, we are enabled.
command_line.reset(new CommandLine(CommandLine::ARGUMENTS_ONLY));
- service = new ExtensionsService(profile.get(), command_line.get(),
- profile->GetPrefs(), install_dir, false);
+ service = profile->CreateExtensionsService(command_line.get(),
+ install_dir);
EXPECT_TRUE(service->extensions_enabled());
service->Init();
loop.RunAllPending();
@@ -2472,8 +2482,8 @@ TEST(ExtensionsServiceTestSimple, Enabledness) {
recorder.set_ready(false);
profile.reset(new TestingProfile());
command_line->AppendSwitch(switches::kDisableExtensions);
- service = new ExtensionsService(profile.get(), command_line.get(),
- profile->GetPrefs(), install_dir, false);
+ service = profile->CreateExtensionsService(command_line.get(),
+ install_dir);
EXPECT_FALSE(service->extensions_enabled());
service->Init();
loop.RunAllPending();
@@ -2482,8 +2492,8 @@ TEST(ExtensionsServiceTestSimple, Enabledness) {
recorder.set_ready(false);
profile.reset(new TestingProfile());
profile->GetPrefs()->SetBoolean(prefs::kDisableExtensions, true);
- service = new ExtensionsService(profile.get(), command_line.get(),
- profile->GetPrefs(), install_dir, false);
+ service = profile->CreateExtensionsService(command_line.get(),
+ install_dir);
EXPECT_FALSE(service->extensions_enabled());
service->Init();
loop.RunAllPending();
@@ -2493,8 +2503,8 @@ TEST(ExtensionsServiceTestSimple, Enabledness) {
profile.reset(new TestingProfile());
profile->GetPrefs()->SetBoolean(prefs::kDisableExtensions, true);
command_line.reset(new CommandLine(CommandLine::ARGUMENTS_ONLY));
- service = new ExtensionsService(profile.get(), command_line.get(),
- profile->GetPrefs(), install_dir, false);
+ service = profile->CreateExtensionsService(command_line.get(),
+ install_dir);
EXPECT_FALSE(service->extensions_enabled());
service->Init();
loop.RunAllPending();
diff --git a/chrome/browser/extensions/extensions_service_unittest.h b/chrome/browser/extensions/extensions_service_unittest.h
index 62f6a95..fe298b4b 100644
--- a/chrome/browser/extensions/extensions_service_unittest.h
+++ b/chrome/browser/extensions/extensions_service_unittest.h
@@ -38,7 +38,6 @@ class ExtensionsServiceTestBase : public testing::Test {
protected:
ScopedTempDir temp_dir_;
- scoped_ptr<PrefService> prefs_;
scoped_ptr<Profile> profile_;
FilePath extensions_install_dir_;
scoped_refptr<ExtensionsService> service_;
diff --git a/chrome/browser/gtk/gtk_theme_provider.cc b/chrome/browser/gtk/gtk_theme_provider.cc
index 7e5a2c2..5bcd8a3 100644
--- a/chrome/browser/gtk/gtk_theme_provider.cc
+++ b/chrome/browser/gtk/gtk_theme_provider.cc
@@ -258,7 +258,6 @@ GtkThemeProvider::GtkThemeProvider()
}
GtkThemeProvider::~GtkThemeProvider() {
- profile()->GetPrefs()->RemovePrefObserver(prefs::kUsesSystemTheme, this);
gtk_widget_destroy(fake_window_);
gtk_widget_destroy(fake_frame_);
fake_label_.Destroy();
@@ -273,7 +272,8 @@ GtkThemeProvider::~GtkThemeProvider() {
}
void GtkThemeProvider::Init(Profile* profile) {
- profile->GetPrefs()->AddPrefObserver(prefs::kUsesSystemTheme, this);
+ registrar_.Init(profile->GetPrefs());
+ registrar_.Add(prefs::kUsesSystemTheme, this);
use_gtk_ = profile->GetPrefs()->GetBoolean(prefs::kUsesSystemTheme);
BrowserThemeProvider::Init(profile);
diff --git a/chrome/browser/gtk/gtk_theme_provider.h b/chrome/browser/gtk/gtk_theme_provider.h
index 33b1ec2..2ecef7a 100644
--- a/chrome/browser/gtk/gtk_theme_provider.h
+++ b/chrome/browser/gtk/gtk_theme_provider.h
@@ -12,6 +12,7 @@
#include "app/gtk_integers.h"
#include "app/gtk_signal.h"
#include "base/scoped_ptr.h"
+#include "chrome/browser/prefs/pref_change_registrar.h"
#include "chrome/browser/gtk/owned_widget_gtk.h"
#include "chrome/browser/themes/browser_theme_provider.h"
#include "chrome/common/notification_observer.h"
@@ -275,6 +276,8 @@ class GtkThemeProvider : public BrowserThemeProvider,
PerDisplaySurfaceMap per_display_surfaces_;
PerDisplaySurfaceMap per_display_unthemed_surfaces_;
+ PrefChangeRegistrar registrar_;
+
// This is a dummy widget that only exists so we have something to pass to
// gtk_widget_render_icon().
static GtkWidget* icon_widget_;
diff --git a/chrome/browser/gtk/options/general_page_gtk.cc b/chrome/browser/gtk/options/general_page_gtk.cc
index fb4db2b..734ee8c 100644
--- a/chrome/browser/gtk/options/general_page_gtk.cc
+++ b/chrome/browser/gtk/options/general_page_gtk.cc
@@ -95,8 +95,9 @@ GeneralPageGtk::GeneralPageGtk(Profile* profile)
InitDefaultBrowserGroup(), false);
#endif
- profile->GetPrefs()->AddPrefObserver(prefs::kRestoreOnStartup, this);
- profile->GetPrefs()->AddPrefObserver(prefs::kURLsToRestoreOnStartup, this);
+ registrar_.Init(profile->GetPrefs());
+ registrar_.Add(prefs::kRestoreOnStartup, this);
+ registrar_.Add(prefs::kURLsToRestoreOnStartup, this);
new_tab_page_is_home_page_.Init(prefs::kHomePageIsNewTabPage,
profile->GetPrefs(), this);
@@ -108,10 +109,6 @@ GeneralPageGtk::GeneralPageGtk(Profile* profile)
}
GeneralPageGtk::~GeneralPageGtk() {
- profile()->GetPrefs()->RemovePrefObserver(prefs::kRestoreOnStartup, this);
- profile()->GetPrefs()->RemovePrefObserver(
- prefs::kURLsToRestoreOnStartup, this);
-
if (template_url_model_)
template_url_model_->RemoveObserver(this);
diff --git a/chrome/browser/gtk/options/general_page_gtk.h b/chrome/browser/gtk/options/general_page_gtk.h
index 3a072ac..143f447 100644
--- a/chrome/browser/gtk/options/general_page_gtk.h
+++ b/chrome/browser/gtk/options/general_page_gtk.h
@@ -14,6 +14,7 @@
#include "chrome/browser/gtk/gtk_tree.h"
#include "chrome/browser/gtk/options/managed_prefs_banner_gtk.h"
#include "chrome/browser/options_page_base.h"
+#include "chrome/browser/prefs/pref_change_registrar.h"
#include "chrome/browser/prefs/pref_member.h"
#include "chrome/browser/search_engines/template_url_model_observer.h"
#include "chrome/browser/shell_integration.h"
@@ -160,6 +161,8 @@ class GeneralPageGtk : public OptionsPageBase,
// Tracks managed preference warning banner state.
ManagedPrefsBannerGtk managed_prefs_banner_;
+ PrefChangeRegistrar registrar_;
+
DISALLOW_COPY_AND_ASSIGN(GeneralPageGtk);
};
diff --git a/chrome/browser/host_content_settings_map.cc b/chrome/browser/host_content_settings_map.cc
index cffe57e..4ac9f405 100644
--- a/chrome/browser/host_content_settings_map.cc
+++ b/chrome/browser/host_content_settings_map.cc
@@ -230,10 +230,11 @@ HostContentSettingsMap::HostContentSettingsMap(Profile* profile)
// Read exceptions.
ReadExceptions(false);
- prefs->AddPrefObserver(prefs::kDefaultContentSettings, this);
- prefs->AddPrefObserver(prefs::kContentSettingsPatterns, this);
- prefs->AddPrefObserver(prefs::kBlockThirdPartyCookies, this);
- prefs->AddPrefObserver(prefs::kBlockNonsandboxedPlugins, this);
+ pref_change_registrar_.Init(prefs);
+ pref_change_registrar_.Add(prefs::kDefaultContentSettings, this);
+ pref_change_registrar_.Add(prefs::kContentSettingsPatterns, this);
+ pref_change_registrar_.Add(prefs::kBlockThirdPartyCookies, this);
+ pref_change_registrar_.Add(prefs::kBlockNonsandboxedPlugins, this);
notification_registrar_.Add(this, NotificationType::PROFILE_DESTROYED,
Source<Profile>(profile_));
}
@@ -909,11 +910,7 @@ void HostContentSettingsMap::UnregisterObservers() {
DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
if (!profile_)
return;
- PrefService* prefs = profile_->GetPrefs();
- prefs->RemovePrefObserver(prefs::kDefaultContentSettings, this);
- prefs->RemovePrefObserver(prefs::kContentSettingsPatterns, this);
- prefs->RemovePrefObserver(prefs::kBlockThirdPartyCookies, this);
- prefs->RemovePrefObserver(prefs::kBlockNonsandboxedPlugins, this);
+ pref_change_registrar_.RemoveAll();
notification_registrar_.Remove(this, NotificationType::PROFILE_DESTROYED,
Source<Profile>(profile_));
profile_ = NULL;
diff --git a/chrome/browser/host_content_settings_map.h b/chrome/browser/host_content_settings_map.h
index ff9c8f0..7afb808 100644
--- a/chrome/browser/host_content_settings_map.h
+++ b/chrome/browser/host_content_settings_map.h
@@ -18,6 +18,7 @@
#include "base/lock.h"
#include "base/ref_counted.h"
#include "chrome/browser/chrome_thread.h"
+#include "chrome/browser/prefs/pref_change_registrar.h"
#include "chrome/common/content_settings.h"
#include "chrome/common/notification_observer.h"
#include "chrome/common/notification_registrar.h"
@@ -285,6 +286,7 @@ class HostContentSettingsMap
Profile* profile_;
NotificationRegistrar notification_registrar_;
+ PrefChangeRegistrar pref_change_registrar_;
// Copies of the pref data, so that we can read it on the IO thread.
ContentSettings default_content_settings_;
diff --git a/chrome/browser/host_zoom_map.cc b/chrome/browser/host_zoom_map.cc
index 0c1d96a..cb67a04 100644
--- a/chrome/browser/host_zoom_map.cc
+++ b/chrome/browser/host_zoom_map.cc
@@ -27,8 +27,10 @@ HostZoomMap::HostZoomMap(Profile* profile)
// Don't observe pref changes (e.g. from sync) in Incognito; once we create
// the incognito window it should have no further connection to the main
// profile/prefs.
- if (!profile_->IsOffTheRecord())
- profile_->GetPrefs()->AddPrefObserver(prefs::kPerHostZoomLevels, this);
+ if (!profile_->IsOffTheRecord()) {
+ pref_change_registrar_.Init(profile_->GetPrefs());
+ pref_change_registrar_.Add(prefs::kPerHostZoomLevels, this);
+ }
}
void HostZoomMap::Load() {
@@ -128,7 +130,7 @@ void HostZoomMap::Shutdown() {
NotificationType::PROFILE_DESTROYED,
Source<Profile>(profile_));
if (!profile_->IsOffTheRecord())
- profile_->GetPrefs()->RemovePrefObserver(prefs::kPerHostZoomLevels, this);
+ pref_change_registrar_.RemoveAll();
profile_ = NULL;
}
diff --git a/chrome/browser/host_zoom_map.h b/chrome/browser/host_zoom_map.h
index caa60b7..3142024 100644
--- a/chrome/browser/host_zoom_map.h
+++ b/chrome/browser/host_zoom_map.h
@@ -15,6 +15,7 @@
#include "base/basictypes.h"
#include "base/lock.h"
#include "base/ref_counted.h"
+#include "chrome/browser/prefs/pref_change_registrar.h"
#include "chrome/common/notification_observer.h"
#include "chrome/common/notification_registrar.h"
@@ -83,6 +84,7 @@ class HostZoomMap : public NotificationObserver,
bool updating_preferences_;
NotificationRegistrar registrar_;
+ PrefChangeRegistrar pref_change_registrar_;
DISALLOW_COPY_AND_ASSIGN(HostZoomMap);
};
diff --git a/chrome/browser/host_zoom_map_unittest.cc b/chrome/browser/host_zoom_map_unittest.cc
index e6b1f72..5d206e1 100644
--- a/chrome/browser/host_zoom_map_unittest.cc
+++ b/chrome/browser/host_zoom_map_unittest.cc
@@ -70,7 +70,9 @@ TEST_F(HostZoomMapTest, Load) {
TEST_F(HostZoomMapTest, SetZoomLevel) {
scoped_refptr<HostZoomMap> map(new HostZoomMap(&profile_));
- prefs_->AddPrefObserver(prefs::kPerHostZoomLevels, &pref_observer_);
+ PrefChangeRegistrar registrar;
+ registrar.Init(prefs_);
+ registrar.Add(prefs::kPerHostZoomLevels, &pref_observer_);
SetPrefObserverExpectation();
map->SetZoomLevel(url_, kZoomLevel);
EXPECT_EQ(kZoomLevel, map->GetZoomLevel(url_));
@@ -84,19 +86,19 @@ TEST_F(HostZoomMapTest, SetZoomLevel) {
map->SetZoomLevel(url_, 0);
EXPECT_EQ(0, map->GetZoomLevel(url_));
EXPECT_FALSE(dict->HasKey(host_));
- prefs_->RemovePrefObserver(prefs::kPerHostZoomLevels, &pref_observer_);
}
TEST_F(HostZoomMapTest, ResetToDefaults) {
scoped_refptr<HostZoomMap> map(new HostZoomMap(&profile_));
map->SetZoomLevel(url_, kZoomLevel);
- prefs_->AddPrefObserver(prefs::kPerHostZoomLevels, &pref_observer_);
+ PrefChangeRegistrar registrar;
+ registrar.Init(prefs_);
+ registrar.Add(prefs::kPerHostZoomLevels, &pref_observer_);
SetPrefObserverExpectation();
map->ResetToDefaults();
EXPECT_EQ(0, map->GetZoomLevel(url_));
EXPECT_EQ(NULL, prefs_->GetDictionary(prefs::kPerHostZoomLevels));
- prefs_->RemovePrefObserver(prefs::kPerHostZoomLevels, &pref_observer_);
}
TEST_F(HostZoomMapTest, ReloadOnPrefChange) {
diff --git a/chrome/browser/net/chrome_url_request_context.cc b/chrome/browser/net/chrome_url_request_context.cc
index c9271d9..88f8ee0 100644
--- a/chrome/browser/net/chrome_url_request_context.cc
+++ b/chrome/browser/net/chrome_url_request_context.cc
@@ -526,8 +526,7 @@ ChromeURLRequestContext* FactoryForMedia::Create() {
ChromeURLRequestContextGetter::ChromeURLRequestContextGetter(
Profile* profile,
ChromeURLRequestContextFactory* factory)
- : prefs_(NULL),
- factory_(factory),
+ : factory_(factory),
url_request_context_(NULL) {
DCHECK(factory);
@@ -539,7 +538,7 @@ ChromeURLRequestContextGetter::ChromeURLRequestContextGetter(
ChromeURLRequestContextGetter::~ChromeURLRequestContextGetter() {
CheckCurrentlyOnIOThread();
- DCHECK(!prefs_) << "Probably didn't call CleanupOnUIThread";
+ DCHECK(registrar_.IsEmpty()) << "Probably didn't call CleanupOnUIThread";
// Either we already transformed the factory into a URLRequestContext, or
// we still have a pending factory.
@@ -652,13 +651,8 @@ ChromeURLRequestContextGetter::CreateOffTheRecordForExtensions(
void ChromeURLRequestContextGetter::CleanupOnUIThread() {
CheckCurrentlyOnMainThread();
-
- if (prefs_) {
- // Unregister for pref notifications.
- prefs_->RemovePrefObserver(prefs::kAcceptLanguages, this);
- prefs_->RemovePrefObserver(prefs::kDefaultCharset, this);
- prefs_ = NULL;
- }
+ // Unregister for pref notifications.
+ registrar_.RemoveAll();
}
void ChromeURLRequestContextGetter::OnNewExtensions(
@@ -710,10 +704,9 @@ void ChromeURLRequestContextGetter::Observe(
void ChromeURLRequestContextGetter::RegisterPrefsObserver(Profile* profile) {
CheckCurrentlyOnMainThread();
- prefs_ = profile->GetPrefs();
-
- prefs_->AddPrefObserver(prefs::kAcceptLanguages, this);
- prefs_->AddPrefObserver(prefs::kDefaultCharset, this);
+ registrar_.Init(profile->GetPrefs());
+ registrar_.Add(prefs::kAcceptLanguages, this);
+ registrar_.Add(prefs::kDefaultCharset, this);
}
// static
diff --git a/chrome/browser/net/chrome_url_request_context.h b/chrome/browser/net/chrome_url_request_context.h
index b7ff017..3141364 100644
--- a/chrome/browser/net/chrome_url_request_context.h
+++ b/chrome/browser/net/chrome_url_request_context.h
@@ -19,6 +19,7 @@
#include "chrome/browser/host_zoom_map.h"
#include "chrome/browser/io_thread.h"
#include "chrome/browser/net/chrome_cookie_policy.h"
+#include "chrome/browser/prefs/pref_change_registrar.h"
#include "chrome/browser/prefs/pref_service.h"
#include "chrome/common/extensions/extension.h"
#include "chrome/common/extensions/extension_icon_set.h"
@@ -382,8 +383,7 @@ class ChromeURLRequestContextGetter : public URLRequestContextGetter,
void GetCookieStoreAsyncHelper(base::WaitableEvent* completion,
net::CookieStore** result);
- // Access only from the UI thread.
- PrefService* prefs_;
+ PrefChangeRegistrar registrar_;
// Deferred logic for creating a ChromeURLRequestContext.
// Access only from the IO thread.
diff --git a/chrome/browser/notifications/desktop_notification_service.cc b/chrome/browser/notifications/desktop_notification_service.cc
index b55c573..1b3df09 100644
--- a/chrome/browser/notifications/desktop_notification_service.cc
+++ b/chrome/browser/notifications/desktop_notification_service.cc
@@ -210,6 +210,7 @@ DesktopNotificationService::DesktopNotificationService(Profile* profile,
NotificationUIManager* ui_manager)
: profile_(profile),
ui_manager_(ui_manager) {
+ registrar_.Init(profile_->GetPrefs());
InitPrefs();
StartObserving();
}
@@ -254,21 +255,15 @@ void DesktopNotificationService::InitPrefs() {
void DesktopNotificationService::StartObserving() {
if (!profile_->IsOffTheRecord()) {
- PrefService* prefs = profile_->GetPrefs();
- prefs->AddPrefObserver(prefs::kDesktopNotificationDefaultContentSetting,
- this);
- prefs->AddPrefObserver(prefs::kDesktopNotificationAllowedOrigins, this);
- prefs->AddPrefObserver(prefs::kDesktopNotificationDeniedOrigins, this);
+ registrar_.Add(prefs::kDesktopNotificationDefaultContentSetting, this);
+ registrar_.Add(prefs::kDesktopNotificationAllowedOrigins, this);
+ registrar_.Add(prefs::kDesktopNotificationDeniedOrigins, this);
}
}
void DesktopNotificationService::StopObserving() {
if (!profile_->IsOffTheRecord()) {
- PrefService* prefs = profile_->GetPrefs();
- prefs->RemovePrefObserver(prefs::kDesktopNotificationDefaultContentSetting,
- this);
- prefs->RemovePrefObserver(prefs::kDesktopNotificationAllowedOrigins, this);
- prefs->RemovePrefObserver(prefs::kDesktopNotificationDeniedOrigins, this);
+ registrar_.RemoveAll();
}
}
diff --git a/chrome/browser/notifications/desktop_notification_service.h b/chrome/browser/notifications/desktop_notification_service.h
index d1a4ccc..2282cd2 100644
--- a/chrome/browser/notifications/desktop_notification_service.h
+++ b/chrome/browser/notifications/desktop_notification_service.h
@@ -11,6 +11,7 @@
#include "base/basictypes.h"
#include "base/string16.h"
#include "chrome/browser/notifications/notification.h"
+#include "chrome/browser/prefs/pref_change_registrar.h"
#include "chrome/common/content_settings.h"
#include "chrome/common/notification_observer.h"
#include "chrome/common/notification_registrar.h"
@@ -136,6 +137,8 @@ class DesktopNotificationService : public NotificationObserver {
// UI for desktop toasts.
NotificationUIManager* ui_manager_;
+ PrefChangeRegistrar registrar_;
+
DISALLOW_COPY_AND_ASSIGN(DesktopNotificationService);
};
diff --git a/chrome/browser/prefs/pref_change_registrar.cc b/chrome/browser/prefs/pref_change_registrar.cc
index dc8bb97..05372b1 100644
--- a/chrome/browser/prefs/pref_change_registrar.cc
+++ b/chrome/browser/prefs/pref_change_registrar.cc
@@ -9,16 +9,11 @@
PrefChangeRegistrar::PrefChangeRegistrar() : service_(NULL) {}
PrefChangeRegistrar::~PrefChangeRegistrar() {
- if (service_) {
- for (std::set<ObserverRegistration>::const_iterator it = observers_.begin();
- it != observers_.end(); ++it) {
- service_->RemovePrefObserver(it->first.c_str(), it->second);
- }
- }
+ RemoveAll();
}
void PrefChangeRegistrar::Init(PrefService* service) {
- DCHECK(!service_);
+ DCHECK(IsEmpty() || service_ == service);
service_ = service;
}
@@ -51,3 +46,17 @@ void PrefChangeRegistrar::Remove(const char* path, NotificationObserver* obs) {
service_->RemovePrefObserver(it->first.c_str(), it->second);
observers_.erase(it);
}
+
+void PrefChangeRegistrar::RemoveAll() {
+ if (service_) {
+ for (std::set<ObserverRegistration>::const_iterator it = observers_.begin();
+ it != observers_.end(); ++it) {
+ service_->RemovePrefObserver(it->first.c_str(), it->second);
+ }
+ observers_.clear();
+ }
+}
+
+bool PrefChangeRegistrar::IsEmpty() const {
+ return observers_.empty();
+}
diff --git a/chrome/browser/prefs/pref_change_registrar.h b/chrome/browser/prefs/pref_change_registrar.h
index cd8f5eb..773c556 100644
--- a/chrome/browser/prefs/pref_change_registrar.h
+++ b/chrome/browser/prefs/pref_change_registrar.h
@@ -23,7 +23,8 @@ class PrefChangeRegistrar {
PrefChangeRegistrar();
virtual ~PrefChangeRegistrar();
- // Must be called before adding or removing observers.
+ // Must be called before adding or removing observers. Can be called more
+ // than once as long as the value of |service| doesn't change.
void Init(PrefService* service);
// Adds an pref observer for the specified pref |path| and |obs| observer
@@ -38,6 +39,12 @@ class PrefChangeRegistrar {
void Remove(const char* path,
NotificationObserver* obs);
+ // Removes all observers that have been previously added with a call to Add.
+ void RemoveAll();
+
+ // Returns true if no pref observers are registered.
+ bool IsEmpty() const;
+
private:
typedef std::pair<std::string, NotificationObserver*> ObserverRegistration;
diff --git a/chrome/browser/prefs/pref_change_registrar_unittest.cc b/chrome/browser/prefs/pref_change_registrar_unittest.cc
index 4d7f747..8096ee6 100644
--- a/chrome/browser/prefs/pref_change_registrar_unittest.cc
+++ b/chrome/browser/prefs/pref_change_registrar_unittest.cc
@@ -63,6 +63,7 @@ TEST_F(PrefChangeRegistrarTest, AddAndRemove) {
AddPrefObserver(Eq(std::string("test.pref.2")), observer()));
registrar.Add("test.pref.1", observer());
registrar.Add("test.pref.2", observer());
+ EXPECT_FALSE(registrar.IsEmpty());
// Test removing.
Mock::VerifyAndClearExpectations(service());
@@ -72,6 +73,11 @@ TEST_F(PrefChangeRegistrarTest, AddAndRemove) {
RemovePrefObserver(Eq(std::string("test.pref.2")), observer()));
registrar.Remove("test.pref.1", observer());
registrar.Remove("test.pref.2", observer());
+ EXPECT_TRUE(registrar.IsEmpty());
+
+ // Explicitly check the expectations now to make sure that the Removes
+ // worked (rather than the registrar destructor doing the work).
+ Mock::VerifyAndClearExpectations(service());
}
TEST_F(PrefChangeRegistrarTest, AutoRemove) {
@@ -83,8 +89,33 @@ TEST_F(PrefChangeRegistrarTest, AutoRemove) {
AddPrefObserver(Eq(std::string("test.pref.1")), observer()));
registrar.Add("test.pref.1", observer());
Mock::VerifyAndClearExpectations(service());
+ EXPECT_FALSE(registrar.IsEmpty());
// Test auto-removing.
EXPECT_CALL(*service(),
RemovePrefObserver(Eq(std::string("test.pref.1")), observer()));
}
+
+TEST_F(PrefChangeRegistrarTest, RemoveAll) {
+ PrefChangeRegistrar registrar;
+ registrar.Init(service());
+
+ EXPECT_CALL(*service(),
+ AddPrefObserver(Eq(std::string("test.pref.1")), observer()));
+ EXPECT_CALL(*service(),
+ AddPrefObserver(Eq(std::string("test.pref.2")), observer()));
+ registrar.Add("test.pref.1", observer());
+ registrar.Add("test.pref.2", observer());
+ Mock::VerifyAndClearExpectations(service());
+
+ EXPECT_CALL(*service(),
+ RemovePrefObserver(Eq(std::string("test.pref.1")), observer()));
+ EXPECT_CALL(*service(),
+ RemovePrefObserver(Eq(std::string("test.pref.2")), observer()));
+ registrar.RemoveAll();
+ EXPECT_TRUE(registrar.IsEmpty());
+
+ // Explicitly check the expectations now to make sure that the RemoveAll
+ // worked (rather than the registrar destructor doing the work).
+ Mock::VerifyAndClearExpectations(service());
+}
diff --git a/chrome/browser/prefs/pref_service.h b/chrome/browser/prefs/pref_service.h
index 450c6c1..5cedd52 100644
--- a/chrome/browser/prefs/pref_service.h
+++ b/chrome/browser/prefs/pref_service.h
@@ -19,9 +19,14 @@
class FilePath;
class NotificationObserver;
+class PrefChangeObserver;
class PrefNotifier;
class Profile;
+namespace subtle {
+ class PrefMemberBase;
+};
+
class PrefService : public NonThreadSafe {
public:
// A helper class to store all the information associated with a preference.
@@ -161,11 +166,6 @@ class PrefService : public NonThreadSafe {
const DictionaryValue* GetDictionary(const char* path) const;
const ListValue* GetList(const char* path) const;
- // If the pref at the given path changes, we call the observer's Observe
- // method with NOTIFY_PREF_CHANGED.
- virtual void AddPrefObserver(const char* path, NotificationObserver* obs);
- virtual void RemovePrefObserver(const char* path, NotificationObserver* obs);
-
// Removes a user pref and restores the pref to its default value.
void ClearPref(const char* path);
@@ -229,6 +229,22 @@ class PrefService : public NonThreadSafe {
scoped_ptr<PrefNotifier> pref_notifier_;
private:
+ // Registration of pref change observers must be done using the
+ // PrefChangeRegistrar, which is declared as a friend here to grant it
+ // access to the otherwise protected members Add/RemovePrefObserver.
+ // PrefMember registers for preferences changes notification directly to
+ // avoid the storage overhead of the registrar, so its base class must be
+ // declared as a friend, too.
+ friend class PrefChangeRegistrar;
+ friend class subtle::PrefMemberBase;
+
+ // If the pref at the given path changes, we call the observer's Observe
+ // method with NOTIFY_PREF_CHANGED. Note that observers should not call
+ // these methods directly but rather use a PrefChangeRegistrar to make sure
+ // the observer gets cleaned up properly.
+ virtual void AddPrefObserver(const char* path, NotificationObserver* obs);
+ virtual void RemovePrefObserver(const char* path, NotificationObserver* obs);
+
// Add a preference to the PreferenceMap. If the pref already exists, return
// false. This method takes ownership of |default_value|.
void RegisterPreference(const char* path, Value* default_value);
diff --git a/chrome/browser/prefs/pref_service_unittest.cc b/chrome/browser/prefs/pref_service_unittest.cc
index 461f108..d62679d 100644
--- a/chrome/browser/prefs/pref_service_unittest.cc
+++ b/chrome/browser/prefs/pref_service_unittest.cc
@@ -8,6 +8,7 @@
#include "base/scoped_ptr.h"
#include "base/values.h"
#include "chrome/browser/prefs/dummy_pref_store.h"
+#include "chrome/browser/prefs/pref_change_registrar.h"
#include "chrome/browser/prefs/pref_value_store.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/notification_observer_mock.h"
@@ -94,7 +95,10 @@ TEST(PrefServiceTest, NoObserverFire) {
const std::string new_pref_value("http://www.google.com/");
TestPrefObserver obs(&prefs, pref_name, new_pref_value);
- prefs.AddPrefObserver(pref_name, &obs);
+
+ PrefChangeRegistrar registrar;
+ registrar.Init(&prefs);
+ registrar.Add(pref_name, &obs);
// This should fire the checks in TestPrefObserver::Observe.
prefs.SetString(pref_name, new_pref_value);
@@ -116,9 +120,6 @@ TEST(PrefServiceTest, NoObserverFire) {
obs.Reset("");
prefs.ClearPref(pref_name);
EXPECT_FALSE(obs.observer_fired());
-
- // Ok, clean up.
- prefs.RemovePrefObserver(pref_name, &obs);
}
TEST(PrefServiceTest, HasPrefPath) {
@@ -148,7 +149,9 @@ TEST(PrefServiceTest, Observers) {
const std::string new_pref_value("http://www.google.com/");
TestPrefObserver obs(&prefs, pref_name, new_pref_value);
- prefs.AddPrefObserver(pref_name, &obs);
+ PrefChangeRegistrar registrar;
+ registrar.Init(&prefs);
+ registrar.Add(pref_name, &obs);
// This should fire the checks in TestPrefObserver::Observe.
prefs.SetString(pref_name, new_pref_value);
@@ -159,23 +162,20 @@ TEST(PrefServiceTest, Observers) {
const std::string new_pref_value2("http://www.youtube.com/");
obs.Reset(new_pref_value2);
TestPrefObserver obs2(&prefs, pref_name, new_pref_value2);
- prefs.AddPrefObserver(pref_name, &obs2);
+ registrar.Add(pref_name, &obs2);
// This should fire the checks in obs and obs2.
prefs.SetString(pref_name, new_pref_value2);
EXPECT_TRUE(obs.observer_fired());
EXPECT_TRUE(obs2.observer_fired());
// Make sure obs2 still works after removing obs.
- prefs.RemovePrefObserver(pref_name, &obs);
+ registrar.Remove(pref_name, &obs);
obs.Reset("");
obs2.Reset(new_pref_value);
// This should only fire the observer in obs2.
prefs.SetString(pref_name, new_pref_value);
EXPECT_FALSE(obs.observer_fired());
EXPECT_TRUE(obs2.observer_fired());
-
- // Ok, clean up.
- prefs.RemovePrefObserver(pref_name, &obs2);
}
class PrefServiceSetValueTest : public testing::Test {
@@ -211,7 +211,11 @@ TEST_F(PrefServiceSetValueTest, SetStringValue) {
const char default_string[] = "default";
scoped_ptr<Value> default_value(Value::CreateStringValue(default_string));
prefs_.RegisterStringPref(name_, default_string);
- prefs_.AddPrefObserver(name_, &observer_);
+
+ PrefChangeRegistrar registrar;
+ registrar.Init(&prefs_);
+ registrar.Add(name_, &observer_);
+
// Changing the controlling store from default to user triggers notification.
SetExpectPrefChanged();
prefs_.Set(name_, *default_value);
@@ -225,13 +229,13 @@ TEST_F(PrefServiceSetValueTest, SetStringValue) {
SetExpectPrefChanged();
prefs_.Set(name_, *new_value);
EXPECT_EQ(value_, prefs_.GetString(name_));
-
- prefs_.RemovePrefObserver(name_, &observer_);
}
TEST_F(PrefServiceSetValueTest, SetDictionaryValue) {
prefs_.RegisterDictionaryPref(name_);
- prefs_.AddPrefObserver(name_, &observer_);
+ PrefChangeRegistrar registrar;
+ registrar.Init(&prefs_);
+ registrar.Add(name_, &observer_);
// Dictionary values are special: setting one to NULL is the same as clearing
// the user value, allowing the NULL default to take (or keep) control.
@@ -259,13 +263,13 @@ TEST_F(PrefServiceSetValueTest, SetDictionaryValue) {
Mock::VerifyAndClearExpectations(&observer_);
dict = prefs_.GetMutableDictionary(name_);
EXPECT_EQ(0U, dict->size());
-
- prefs_.RemovePrefObserver(name_, &observer_);
}
TEST_F(PrefServiceSetValueTest, SetListValue) {
prefs_.RegisterListPref(name_);
- prefs_.AddPrefObserver(name_, &observer_);
+ PrefChangeRegistrar registrar;
+ registrar.Init(&prefs_);
+ registrar.Add(name_, &observer_);
// List values are special: setting one to NULL is the same as clearing the
// user value, allowing the NULL default to take (or keep) control.
@@ -293,6 +297,4 @@ TEST_F(PrefServiceSetValueTest, SetListValue) {
Mock::VerifyAndClearExpectations(&observer_);
list = prefs_.GetMutableList(name_);
EXPECT_EQ(0U, list->GetSize());
-
- prefs_.RemovePrefObserver(name_, &observer_);
}
diff --git a/chrome/browser/prefs/pref_set_observer.cc b/chrome/browser/prefs/pref_set_observer.cc
index a4ecf6f..dd067d3 100644
--- a/chrome/browser/prefs/pref_set_observer.cc
+++ b/chrome/browser/prefs/pref_set_observer.cc
@@ -11,23 +11,19 @@ PrefSetObserver::PrefSetObserver(PrefService* pref_service,
NotificationObserver* observer)
: pref_service_(pref_service),
observer_(observer) {
-}
-
-PrefSetObserver::~PrefSetObserver() {
- for (PrefSet::const_iterator i(prefs_.begin()); i != prefs_.end(); ++i)
- pref_service_->RemovePrefObserver(i->c_str(), this);
+ registrar_.Init(pref_service);
}
void PrefSetObserver::AddPref(const std::string& pref) {
if (!prefs_.count(pref) && pref_service_->FindPreference(pref.c_str())) {
prefs_.insert(pref);
- pref_service_->AddPrefObserver(pref.c_str(), this);
+ registrar_.Add(pref.c_str(), this);
}
}
void PrefSetObserver::RemovePref(const std::string& pref) {
if (prefs_.erase(pref))
- pref_service_->RemovePrefObserver(pref.c_str(), this);
+ registrar_.Remove(pref.c_str(), this);
}
bool PrefSetObserver::IsObserved(const std::string& pref) {
diff --git a/chrome/browser/prefs/pref_set_observer.h b/chrome/browser/prefs/pref_set_observer.h
index beaec01..ff350df 100644
--- a/chrome/browser/prefs/pref_set_observer.h
+++ b/chrome/browser/prefs/pref_set_observer.h
@@ -10,6 +10,7 @@
#include "base/basictypes.h"
#include "chrome/browser/prefs/pref_service.h"
+#include "chrome/browser/prefs/pref_change_registrar.h"
#include "chrome/common/notification_observer.h"
// Observes the state of a set of preferences and allows to query their combined
@@ -19,7 +20,7 @@ class PrefSetObserver : public NotificationObserver {
// Initialize with an empty set of preferences.
PrefSetObserver(PrefService* pref_service,
NotificationObserver* observer);
- virtual ~PrefSetObserver();
+ virtual ~PrefSetObserver() {}
// Add a |pref| to the set of preferences to observe.
void AddPref(const std::string& pref);
@@ -51,6 +52,7 @@ class PrefSetObserver : public NotificationObserver {
PrefSet prefs_;
PrefService* pref_service_;
+ PrefChangeRegistrar registrar_;
NotificationObserver* observer_;
DISALLOW_COPY_AND_ASSIGN(PrefSetObserver);
diff --git a/chrome/browser/profile_impl.cc b/chrome/browser/profile_impl.cc
index 5a61e1c..f72a6a5 100644
--- a/chrome/browser/profile_impl.cc
+++ b/chrome/browser/profile_impl.cc
@@ -292,9 +292,10 @@ ProfileImpl::ProfileImpl(const FilePath& path)
&ProfileImpl::EnsureSessionServiceCreated);
PrefService* prefs = GetPrefs();
- prefs->AddPrefObserver(prefs::kSpellCheckDictionary, this);
- prefs->AddPrefObserver(prefs::kEnableSpellCheck, this);
- prefs->AddPrefObserver(prefs::kEnableAutoSpellCorrect, this);
+ pref_change_registrar_.Init(prefs);
+ pref_change_registrar_.Add(prefs::kSpellCheckDictionary, this);
+ pref_change_registrar_.Add(prefs::kEnableSpellCheck, this);
+ pref_change_registrar_.Add(prefs::kEnableAutoSpellCorrect, this);
#if defined(OS_MACOSX)
// If the profile directory doesn't already have a cache directory and it
@@ -390,7 +391,6 @@ void ProfileImpl::InitExtensions() {
extensions_service_ = new ExtensionsService(
this,
CommandLine::ForCurrentProcess(),
- GetPrefs(),
GetPath().AppendASCII(ExtensionsService::kInstallDirectoryName),
true);
@@ -501,11 +501,8 @@ ProfileImpl::~ProfileImpl() {
// The theme provider provides bitmaps to whoever wants them.
theme_provider_.reset();
- // Remove pref observers.
- PrefService* prefs = GetPrefs();
- prefs->RemovePrefObserver(prefs::kSpellCheckDictionary, this);
- prefs->RemovePrefObserver(prefs::kEnableSpellCheck, this);
- prefs->RemovePrefObserver(prefs::kEnableAutoSpellCorrect, this);
+ // Remove pref observers
+ pref_change_registrar_.RemoveAll();
// Delete the NTP resource cache so we can unregister pref observers.
ntp_resource_cache_.reset();
diff --git a/chrome/browser/profile_impl.h b/chrome/browser/profile_impl.h
index 9331d34..f324fbd 100644
--- a/chrome/browser/profile_impl.h
+++ b/chrome/browser/profile_impl.h
@@ -13,6 +13,7 @@
#include "base/scoped_ptr.h"
#include "base/timer.h"
#include "chrome/browser/profile.h"
+#include "chrome/browser/prefs/pref_change_registrar.h"
#include "chrome/browser/spellcheck_host_observer.h"
#include "chrome/common/notification_observer.h"
#include "chrome/common/notification_registrar.h"
@@ -144,6 +145,7 @@ class ProfileImpl : public Profile,
}
NotificationRegistrar registrar_;
+ PrefChangeRegistrar pref_change_registrar_;
FilePath path_;
FilePath base_cache_path_;
diff --git a/chrome/browser/sync/glue/preference_change_processor.cc b/chrome/browser/sync/glue/preference_change_processor.cc
index ec0eeb3..874f786 100644
--- a/chrome/browser/sync/glue/preference_change_processor.cc
+++ b/chrome/browser/sync/glue/preference_change_processor.cc
@@ -195,6 +195,7 @@ Value* PreferenceChangeProcessor::ReadPreference(
void PreferenceChangeProcessor::StartImpl(Profile* profile) {
DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
pref_service_ = profile->GetPrefs();
+ registrar_.Init(pref_service_);
StartObserving();
}
@@ -210,7 +211,7 @@ void PreferenceChangeProcessor::StartObserving() {
for (std::set<std::string>::const_iterator it =
model_associator_->synced_preferences().begin();
it != model_associator_->synced_preferences().end(); ++it) {
- pref_service_->AddPrefObserver((*it).c_str(), this);
+ registrar_.Add((*it).c_str(), this);
}
}
@@ -219,7 +220,7 @@ void PreferenceChangeProcessor::StopObserving() {
for (std::set<std::string>::const_iterator it =
model_associator_->synced_preferences().begin();
it != model_associator_->synced_preferences().end(); ++it) {
- pref_service_->RemovePrefObserver((*it).c_str(), this);
+ registrar_.Remove((*it).c_str(), this);
}
}
diff --git a/chrome/browser/sync/glue/preference_change_processor.h b/chrome/browser/sync/glue/preference_change_processor.h
index cb25511..1ef88da 100644
--- a/chrome/browser/sync/glue/preference_change_processor.h
+++ b/chrome/browser/sync/glue/preference_change_processor.h
@@ -8,6 +8,7 @@
#include <string>
+#include "chrome/browser/prefs/pref_change_registrar.h"
#include "chrome/browser/prefs/pref_service.h"
#include "chrome/browser/sync/engine/syncapi.h"
#include "chrome/browser/sync/glue/change_processor.h"
@@ -60,6 +61,8 @@ class PreferenceChangeProcessor : public ChangeProcessor,
// Whether we are currently processing a preference change notification.
bool processing_pref_change_;
+ PrefChangeRegistrar registrar_;
+
DISALLOW_COPY_AND_ASSIGN(PreferenceChangeProcessor);
};
diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc
index 086d1f1..b33f3d8 100644
--- a/chrome/browser/tab_contents/tab_contents.cc
+++ b/chrome/browser/tab_contents/tab_contents.cc
@@ -400,9 +400,10 @@ TabContents::TabContents(Profile* profile,
// Register for notifications about all interested prefs change.
PrefService* prefs = profile->GetPrefs();
+ pref_change_registrar_.Init(prefs);
if (prefs) {
for (int i = 0; i < kPrefsToObserveLength; ++i)
- prefs->AddPrefObserver(kPrefsToObserve[i], this);
+ pref_change_registrar_.Add(kPrefsToObserve[i], this);
}
// Register for notifications about URL starredness changing on any profile.
@@ -442,13 +443,7 @@ TabContents::~TabContents() {
// We don't want any notifications while we're running our destructor.
registrar_.RemoveAll();
-
- // Unregister the notifications of all observed prefs change.
- PrefService* prefs = profile()->GetPrefs();
- if (prefs) {
- for (int i = 0; i < kPrefsToObserveLength; ++i)
- prefs->RemovePrefObserver(kPrefsToObserve[i], this);
- }
+ pref_change_registrar_.RemoveAll();
NotifyDisconnected();
hung_renderer_dialog::HideForTabContents(this);
diff --git a/chrome/browser/tab_contents/tab_contents.h b/chrome/browser/tab_contents/tab_contents.h
index 90ded2e..d37aab8 100644
--- a/chrome/browser/tab_contents/tab_contents.h
+++ b/chrome/browser/tab_contents/tab_contents.h
@@ -24,6 +24,7 @@
#include "chrome/browser/find_bar_controller.h"
#include "chrome/browser/find_notification_details.h"
#include "chrome/browser/jsmessage_box_client.h"
+#include "chrome/browser/prefs/pref_change_registrar.h"
#include "chrome/browser/password_manager/password_manager_delegate.h"
#include "chrome/browser/renderer_host/render_view_host_delegate.h"
#include "chrome/browser/tab_contents/constrained_window.h"
@@ -1057,6 +1058,9 @@ class TabContents : public PageNavigator,
// Registers and unregisters us for notifications.
NotificationRegistrar registrar_;
+ // Registers and unregisters for pref notifications.
+ PrefChangeRegistrar pref_change_registrar_;
+
// Handles print preview and print job for this contents.
scoped_ptr<printing::PrintViewManager> printing_;
diff --git a/chrome/browser/translate/translate_manager.cc b/chrome/browser/translate/translate_manager.cc
index 6f649fd..1c70dd4 100644
--- a/chrome/browser/translate/translate_manager.cc
+++ b/chrome/browser/translate/translate_manager.cc
@@ -249,7 +249,7 @@ void TranslateManager::Observe(NotificationType type,
// We should know about this profile since we are listening for
// notifications on it.
DCHECK(count > 0);
- profile->GetPrefs()->RemovePrefObserver(prefs::kAcceptLanguages, this);
+ pref_change_registrar_.Remove(prefs::kAcceptLanguages, this);
break;
}
case NotificationType::PREF_CHANGED: {
@@ -342,6 +342,8 @@ void TranslateManager::InitiateTranslation(TabContents* tab,
if (!prefs->GetBoolean(prefs::kEnableTranslate))
return;
+ pref_change_registrar_.Init(prefs);
+
// Allow disabling of translate from the command line to assist with
// automated browser testing.
if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kDisableTranslate))
@@ -534,7 +536,7 @@ bool TranslateManager::IsAcceptLanguage(TabContents* tab,
notification_registrar_.Add(this, NotificationType::PROFILE_DESTROYED,
Source<Profile>(tab->profile()));
// Also start listening for changes in the accept languages.
- tab->profile()->GetPrefs()->AddPrefObserver(prefs::kAcceptLanguages, this);
+ pref_change_registrar_.Add(prefs::kAcceptLanguages, this);
iter = accept_languages_.find(pref_service);
}
diff --git a/chrome/browser/translate/translate_manager.h b/chrome/browser/translate/translate_manager.h
index adec281..3ddcabf 100644
--- a/chrome/browser/translate/translate_manager.h
+++ b/chrome/browser/translate/translate_manager.h
@@ -14,6 +14,7 @@
#include "base/lazy_instance.h"
#include "base/singleton.h"
#include "base/task.h"
+#include "chrome/browser/prefs/pref_change_registrar.h"
#include "chrome/common/net/url_fetcher.h"
#include "chrome/common/notification_observer.h"
#include "chrome/common/notification_registrar.h"
@@ -153,6 +154,7 @@ class TranslateManager : public NotificationObserver,
TabContents* tab);
NotificationRegistrar notification_registrar_;
+ PrefChangeRegistrar pref_change_registrar_;
// A map that associates a profile with its parsed "accept languages".
typedef std::set<std::string> LanguageSet;
diff --git a/chrome/browser/translate/translate_manager_unittest.cc b/chrome/browser/translate/translate_manager_unittest.cc
index 8f2b801..f578b3f 100644
--- a/chrome/browser/translate/translate_manager_unittest.cc
+++ b/chrome/browser/translate/translate_manager_unittest.cc
@@ -7,6 +7,7 @@
#include "base/utf_string_conversions.h"
#include "chrome/app/chrome_dll_resource.h"
#include "chrome/browser/prefs/pref_service.h"
+#include "chrome/browser/prefs/pref_change_registrar.h"
#include "chrome/browser/renderer_host/mock_render_process_host.h"
#include "chrome/browser/tab_contents/navigation_controller.h"
#include "chrome/browser/tab_contents/render_view_context_menu.h"
@@ -851,8 +852,10 @@ TEST_F(TranslateManagerTest, NeverTranslateLanguagePref) {
// Select never translate this language.
PrefService* prefs = contents()->profile()->GetPrefs();
- prefs->AddPrefObserver(TranslatePrefs::kPrefTranslateLanguageBlacklist,
- &pref_observer_);
+ PrefChangeRegistrar registrar;
+ registrar.Init(prefs);
+ registrar.Add(TranslatePrefs::kPrefTranslateLanguageBlacklist,
+ &pref_observer_);
TranslatePrefs translate_prefs(prefs);
EXPECT_FALSE(translate_prefs.IsLanguageBlacklisted("fr"));
EXPECT_TRUE(translate_prefs.CanTranslate(prefs, "fr", url));
@@ -881,8 +884,6 @@ TEST_F(TranslateManagerTest, NeverTranslateLanguagePref) {
// There should be a translate infobar.
EXPECT_TRUE(GetTranslateInfoBar() != NULL);
- prefs->RemovePrefObserver(TranslatePrefs::kPrefTranslateLanguageBlacklist,
- &pref_observer_);
}
// Tests the "Never translate this site" pref.
@@ -897,8 +898,10 @@ TEST_F(TranslateManagerTest, NeverTranslateSitePref) {
// Select never translate this site.
PrefService* prefs = contents()->profile()->GetPrefs();
- prefs->AddPrefObserver(TranslatePrefs::kPrefTranslateSiteBlacklist,
- &pref_observer_);
+ PrefChangeRegistrar registrar;
+ registrar.Init(prefs);
+ registrar.Add(TranslatePrefs::kPrefTranslateSiteBlacklist,
+ &pref_observer_);
TranslatePrefs translate_prefs(prefs);
EXPECT_FALSE(translate_prefs.IsSiteBlacklisted(host));
EXPECT_TRUE(translate_prefs.CanTranslate(prefs, "fr", url));
@@ -927,16 +930,16 @@ TEST_F(TranslateManagerTest, NeverTranslateSitePref) {
// There should be a translate infobar.
EXPECT_TRUE(GetTranslateInfoBar() != NULL);
- prefs->RemovePrefObserver(TranslatePrefs::kPrefTranslateSiteBlacklist,
- &pref_observer_);
}
// Tests the "Always translate this language" pref.
TEST_F(TranslateManagerTest, AlwaysTranslateLanguagePref) {
// Select always translate French to English.
PrefService* prefs = contents()->profile()->GetPrefs();
- prefs->AddPrefObserver(TranslatePrefs::kPrefTranslateWhitelists,
- &pref_observer_);
+ PrefChangeRegistrar registrar;
+ registrar.Init(prefs);
+ registrar.Add(TranslatePrefs::kPrefTranslateWhitelists,
+ &pref_observer_);
TranslatePrefs translate_prefs(prefs);
SetPrefObserverExpectation(TranslatePrefs::kPrefTranslateWhitelists);
translate_prefs.WhitelistLanguagePair("fr", "en");
@@ -985,8 +988,6 @@ TEST_F(TranslateManagerTest, AlwaysTranslateLanguagePref) {
infobar = GetTranslateInfoBar();
ASSERT_TRUE(infobar != NULL);
EXPECT_EQ(TranslateInfoBarDelegate::BEFORE_TRANSLATE, infobar->type());
- prefs->RemovePrefObserver(TranslatePrefs::kPrefTranslateWhitelists,
- &pref_observer_);
}
// Context menu.
diff --git a/chrome/browser/views/options/general_page_view.cc b/chrome/browser/views/options/general_page_view.cc
index 830dac1..d618770 100644
--- a/chrome/browser/views/options/general_page_view.cc
+++ b/chrome/browser/views/options/general_page_view.cc
@@ -217,9 +217,6 @@ GeneralPageView::GeneralPageView(Profile* profile)
}
GeneralPageView::~GeneralPageView() {
- profile()->GetPrefs()->RemovePrefObserver(prefs::kRestoreOnStartup, this);
- profile()->GetPrefs()->RemovePrefObserver(
- prefs::kURLsToRestoreOnStartup, this);
if (startup_custom_pages_table_)
startup_custom_pages_table_->SetModel(NULL);
default_browser_worker_->ObserverDestroyed();
@@ -356,8 +353,9 @@ void GeneralPageView::InitControlLayout() {
#endif
// Register pref observers that update the controls when a pref changes.
- profile()->GetPrefs()->AddPrefObserver(prefs::kRestoreOnStartup, this);
- profile()->GetPrefs()->AddPrefObserver(prefs::kURLsToRestoreOnStartup, this);
+ registrar_.Init(profile()->GetPrefs());
+ registrar_.Add(prefs::kRestoreOnStartup, this);
+ registrar_.Add(prefs::kURLsToRestoreOnStartup, this);
new_tab_page_is_home_page_.Init(prefs::kHomePageIsNewTabPage,
profile()->GetPrefs(), this);
diff --git a/chrome/browser/views/options/general_page_view.h b/chrome/browser/views/options/general_page_view.h
index 93a7af5..f2b126f 100644
--- a/chrome/browser/views/options/general_page_view.h
+++ b/chrome/browser/views/options/general_page_view.h
@@ -6,6 +6,7 @@
#define CHROME_BROWSER_VIEWS_OPTIONS_GENERAL_PAGE_VIEW_H_
#pragma once
+#include "chrome/browser/prefs/pref_change_registrar.h"
#include "chrome/browser/prefs/pref_member.h"
#include "chrome/browser/shell_integration.h"
#include "chrome/browser/views/options/options_page_view.h"
@@ -156,6 +157,8 @@ class GeneralPageView : public OptionsPageView,
// The helper object that performs default browser set/check tasks.
scoped_refptr<ShellIntegration::DefaultBrowserWorker> default_browser_worker_;
+ PrefChangeRegistrar registrar_;
+
DISALLOW_COPY_AND_ASSIGN(GeneralPageView);
};
diff --git a/chrome/test/testing_profile.cc b/chrome/test/testing_profile.cc
index 6fd04fc..359f64d 100644
--- a/chrome/test/testing_profile.cc
+++ b/chrome/test/testing_profile.cc
@@ -16,6 +16,7 @@
#include "chrome/browser/bookmarks/bookmark_model.h"
#include "chrome/browser/chrome_thread.h"
#include "chrome/browser/dom_ui/ntp_resource_cache.h"
+#include "chrome/browser/extensions/extensions_service.h"
#include "chrome/browser/favicon_service.h"
#include "chrome/browser/find_bar_state.h"
#include "chrome/browser/geolocation/geolocation_content_settings_map.h"
@@ -166,6 +167,7 @@ class TestExtensionURLRequestContextGetter : public URLRequestContextGetter {
TestingProfile::TestingProfile()
: start_time_(Time::Now()),
+ testing_prefs_(NULL),
created_theme_provider_(false),
has_history_service_(false),
off_the_record_(false),
@@ -207,6 +209,10 @@ TestingProfile::~TestingProfile() {
if (top_sites_.get())
top_sites_->ClearProfile();
history::TopSites::DeleteTopSites(top_sites_);
+ if (extensions_service_.get()) {
+ extensions_service_->DestroyingProfile();
+ extensions_service_ = NULL;
+ }
}
void TestingProfile::CreateFaviconService() {
@@ -315,13 +321,26 @@ void TestingProfile::UseThemeProvider(BrowserThemeProvider* theme_provider) {
theme_provider_.reset(theme_provider);
}
+scoped_refptr<ExtensionsService> TestingProfile::CreateExtensionsService(
+ const CommandLine* command_line,
+ const FilePath& install_directory) {
+ extensions_service_ = new ExtensionsService(this,
+ command_line,
+ install_directory,
+ false);
+ return extensions_service_;
+}
+
FilePath TestingProfile::GetPath() {
DCHECK(temp_dir_.IsValid()); // TODO(phajdan.jr): do it better.
return temp_dir_.path();
}
TestingPrefService* TestingProfile::GetTestingPrefService() {
- return static_cast<TestingPrefService*>(GetPrefs());
+ if (!prefs_.get())
+ CreateTestingPrefService();
+ DCHECK(testing_prefs_);
+ return testing_prefs_;
}
webkit_database::DatabaseTracker* TestingProfile::GetDatabaseTracker() {
@@ -330,6 +349,10 @@ webkit_database::DatabaseTracker* TestingProfile::GetDatabaseTracker() {
return db_tracker_;
}
+ExtensionsService* TestingProfile::GetExtensionsService() {
+ return extensions_service_.get();
+}
+
net::CookieMonster* TestingProfile::GetCookieMonster() {
if (!GetRequestContext())
return NULL;
@@ -348,11 +371,22 @@ void TestingProfile::InitThemes() {
}
}
+void TestingProfile::SetPrefService(PrefService* prefs) {
+ DCHECK(!prefs_.get());
+ prefs_.reset(prefs);
+}
+
+void TestingProfile::CreateTestingPrefService() {
+ DCHECK(!prefs_.get());
+ testing_prefs_ = new TestingPrefService();
+ prefs_.reset(testing_prefs_);
+ Profile::RegisterUserPrefs(prefs_.get());
+ browser::RegisterAllPrefs(prefs_.get(), prefs_.get());
+}
+
PrefService* TestingProfile::GetPrefs() {
if (!prefs_.get()) {
- prefs_.reset(new TestingPrefService());
- Profile::RegisterUserPrefs(prefs_.get());
- browser::RegisterAllPrefs(prefs_.get(), prefs_.get());
+ CreateTestingPrefService();
}
return prefs_.get();
}
diff --git a/chrome/test/testing_profile.h b/chrome/test/testing_profile.h
index d75b7c1..b1e466d 100644
--- a/chrome/test/testing_profile.h
+++ b/chrome/test/testing_profile.h
@@ -23,6 +23,7 @@ class CookieMonster;
class AutocompleteClassifier;
class BookmarkModel;
class BrowserThemeProvider;
+class CommandLine;
class DesktopNotificationService;
class FaviconService;
class FindBarState;
@@ -86,6 +87,14 @@ class TestingProfile : public Profile {
// ownership of |theme_provider|.
void UseThemeProvider(BrowserThemeProvider* theme_provider);
+ // Creates an ExtensionsService initialized with the testing profile and
+ // returns it. The profile keeps its own copy of a scoped_refptr to the
+ // ExtensionsService to make sure that is still alive to be notified when the
+ // profile is destroyed.
+ scoped_refptr<ExtensionsService> CreateExtensionsService(
+ const CommandLine* command_line,
+ const FilePath& install_directory);
+
TestingPrefService* GetTestingPrefService();
virtual ProfileId GetRuntimeId() {
@@ -109,7 +118,7 @@ class TestingProfile : public Profile {
virtual ChromeAppCacheService* GetAppCacheService() { return NULL; }
virtual webkit_database::DatabaseTracker* GetDatabaseTracker();
virtual VisitedLinkMaster* GetVisitedLinkMaster() { return NULL; }
- virtual ExtensionsService* GetExtensionsService() { return NULL; }
+ virtual ExtensionsService* GetExtensionsService();
virtual UserScriptMaster* GetUserScriptMaster() { return NULL; }
virtual ExtensionDevToolsManager* GetExtensionDevToolsManager() {
return NULL;
@@ -148,6 +157,10 @@ class TestingProfile : public Profile {
virtual PasswordStore* GetPasswordStore(ServiceAccessType access) {
return NULL;
}
+ // Initialized the profile's PrefService with an explicity specified
+ // PrefService. Must be called before the TestingProfile.
+ // The profile takes ownership of |pref|.
+ void SetPrefService(PrefService* prefs);
virtual PrefService* GetPrefs();
virtual TemplateURLModel* GetTemplateURLModel() {
return template_url_model_.get();
@@ -263,7 +276,9 @@ class TestingProfile : public Profile {
protected:
base::Time start_time_;
- scoped_ptr<TestingPrefService> prefs_;
+ scoped_ptr<PrefService> prefs_;
+ // ref only for right type, lifecycle is managed by prefs_
+ TestingPrefService* testing_prefs_;
private:
// Destroys favicon service if it has been created.
@@ -277,6 +292,9 @@ class TestingProfile : public Profile {
// from the destructor.
void DestroyWebDataService();
+ // Creates a TestingPrefService and associates it with the TestingProfile
+ void CreateTestingPrefService();
+
// The favicon service. Only created if CreateFaviconService is invoked.
scoped_refptr<FaviconService> favicon_service_;
@@ -346,6 +364,10 @@ class TestingProfile : public Profile {
FilePath last_selected_directory_;
scoped_refptr<history::TopSites> top_sites_; // For history and thumbnails.
+ // For properly notifying the ExtensionsService when the profile
+ // is disposed.
+ scoped_refptr<ExtensionsService> extensions_service_;
+
// We use a temporary directory to store testing profile data.
ScopedTempDir temp_dir_;
};