diff options
author | joi@chromium.org <joi@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-11-09 10:36:38 +0000 |
---|---|---|
committer | joi@chromium.org <joi@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-11-09 10:36:38 +0000 |
commit | 3fb6a6946e7d160f186255d8decd5eb67c011946 (patch) | |
tree | 46b2d5dcdca57a37be29bf80dcf68448c7af489a /chrome/browser/prefs | |
parent | 5ee200b423be84efa02659b972093fed44d987a2 (diff) | |
download | chromium_src-3fb6a6946e7d160f186255d8decd5eb67c011946.zip chromium_src-3fb6a6946e7d160f186255d8decd5eb67c011946.tar.gz chromium_src-3fb6a6946e7d160f186255d8decd5eb67c011946.tar.bz2 |
Revert 166670 - Add Closure-based API to PrefChangeObserver and PrefMember, deprecate PrefObserver-based API.
This switches the API with minimal implementation changes; PrefNotifierImpl still uses PrefObserver to accept registrations, and still filters on pref names (which PrefChangeObserver looks up again when it receives callbacks via its PrefObserver implementation).
This approach is chosen for now to establish the new API so that usages can be switched, because:
a) We need a way for PrefNotifierImpl to dispatch to both PrefChangeObserver and PrefMember, and it's unclear to me whether we want to switch that interface to base::Callback as well (if so, how to unregister?), or whether we want PrefMember to have a PrefChangeObserver instance (probably inefficient?) or something else.
b) There are plans to do performance measurements of a few different implementation approaches in how PrefNotifierImpl and PrefChangeObserver interact; that interaction can be changed "under the hood" while the new API stays unchanged, and this lets us start switching users to the new API and removing the now-deprecated PrefObserver-based API.
TBR=ben@chromium.org,finnur@chromium.org
BUG=155525,160177
Review URL: https://chromiumcodereview.appspot.com/11368098
Speculative revert, to check if memory usage increase from issue
160177 was because of this revision.
TBR=joi@chromium.org
Review URL: https://codereview.chromium.org/11364174
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@166888 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/prefs')
-rw-r--r-- | chrome/browser/prefs/pref_service_unittest.cc | 7 |
1 files changed, 2 insertions, 5 deletions
diff --git a/chrome/browser/prefs/pref_service_unittest.cc b/chrome/browser/prefs/pref_service_unittest.cc index 883f50a..9cacfb7 100644 --- a/chrome/browser/prefs/pref_service_unittest.cc +++ b/chrome/browser/prefs/pref_service_unittest.cc @@ -108,9 +108,6 @@ TEST(PrefServiceTest, Observers) { registrar.Init(&prefs); registrar.Add(pref_name, &obs); - PrefChangeRegistrar registrar_two; - registrar_two.Init(&prefs); - // This should fire the checks in PrefObserverMock::Observe. obs.Expect(&prefs, pref_name, &expected_new_pref_value); prefs.SetString(pref_name, new_pref_value); @@ -122,7 +119,7 @@ TEST(PrefServiceTest, Observers) { PrefObserverMock obs2; obs.Expect(&prefs, pref_name, &expected_new_pref_value2); obs2.Expect(&prefs, pref_name, &expected_new_pref_value2); - registrar_two.Add(pref_name, &obs2); + registrar.Add(pref_name, &obs2); // This should fire the checks in obs and obs2. prefs.SetString(pref_name, new_pref_value2); Mock::VerifyAndClearExpectations(&obs); @@ -139,7 +136,7 @@ TEST(PrefServiceTest, Observers) { Mock::VerifyAndClearExpectations(&obs2); // Make sure obs2 still works after removing obs. - registrar.Remove(pref_name); + registrar.Remove(pref_name, &obs); EXPECT_CALL(obs, OnPreferenceChanged(_, _)).Times(0); obs2.Expect(&prefs, pref_name, &expected_new_pref_value); // This should only fire the observer in obs2. |