Remove NOTIFICATION_SYSTEM_SETTING_CHANGED, switch CrosSettings to base::CallbackRegistry.

chrome/browser/chromeos/settings/cros_settings.cc

Index: chrome/browser/chromeos/settings/cros_settings.cc
diff --git a/chrome/browser/chromeos/settings/cros_settings.cc b/chrome/browser/chromeos/settings/cros_settings.cc
index 901ebcf91ee1ec0fcad3fa8d5329a1e884386572..7ac3603b93b934b1f0203979dcc4fa58f3ffcbd2 100644
--- a/chrome/browser/chromeos/settings/cros_settings.cc
+++ b/chrome/browser/chromeos/settings/cros_settings.cc
@@ -262,52 +262,31 @@ bool CrosSettings::RemoveSettingsProvider(CrosSettingsProvider* provider) {
   return false;
 }
 
-void CrosSettings::AddSettingsObserver(const char* path,
-                                       content::NotificationObserver* obs) {
+scoped_ptr<CrosSettings::ObserverSubscription>
+CrosSettings::AddSettingsObserver(
+    const char* path, const base::Closure& callback) {

[Lei Zhang, 2013/09/18 05:00:53]

Why can't |path| be a const std::string& ?

[Mattias Nissler (ping if slow), 2013/09/18 09:09:48]

nit: parameters on separate lines.

[Avi (use Gerrit), 2013/09/18 16:41:44]

It's been that way since day 1, three years ago:
https://codereview.chromium.org/3143009

However...

If you look at browser/chromeos/login/version_info_updater, it uses a loop to
register interest, and you can't define a vector of string objects.

[Avi (use Gerrit), 2013/09/18 16:41:44]

Done.

[Avi (use Gerrit), 2013/09/18 18:50:38]

Actually, you're right in that you will construct a temporary. Lemme try that.

   DCHECK(path);
-  DCHECK(obs);
+  DCHECK(!callback.is_null());
   DCHECK(CalledOnValidThread());
 
   if (!GetProvider(std::string(path))) {
     NOTREACHED() << "Trying to add an observer for an unregistered setting: "
                  << path;
-    return;
+    return scoped_ptr<CrosSettings::ObserverSubscription>();
   }
 
-  // Get the settings observer list associated with the path.
-  NotificationObserverList* observer_list = NULL;
+  // Get the callback registry associated with the path.
+  base::CallbackRegistry<void>* registry = NULL;
   SettingsObserverMap::iterator observer_iterator =
       settings_observers_.find(path);
   if (observer_iterator == settings_observers_.end()) {
-    observer_list = new NotificationObserverList;
-    settings_observers_[path] = observer_list;
+    registry = new base::CallbackRegistry<void>;
+    settings_observers_[path] = registry;
   } else {
-    observer_list = observer_iterator->second;
-  }
-
-  // Verify that this observer doesn't already exist.
-  NotificationObserverList::Iterator it(*observer_list);
-  content::NotificationObserver* existing_obs;
-  while ((existing_obs = it.GetNext()) != NULL) {
-    if (existing_obs == obs)
-      return;
+    registry = observer_iterator->second;
   }
 
-  // Ok, safe to add the pref observer.
-  observer_list->AddObserver(obs);
-}
-
-void CrosSettings::RemoveSettingsObserver(const char* path,
-                                          content::NotificationObserver* obs) {
-  DCHECK(CalledOnValidThread());
-
-  SettingsObserverMap::iterator observer_iterator =
-      settings_observers_.find(path);
-  if (observer_iterator == settings_observers_.end())
-    return;
-
-  NotificationObserverList* observer_list = observer_iterator->second;
-  observer_list->RemoveObserver(obs);
+  return registry->Add(callback);
 }
 
 CrosSettingsProvider* CrosSettings::GetProvider(
@@ -326,13 +305,7 @@ void CrosSettings::FireObservers(const std::string& path) {
   if (observer_iterator == settings_observers_.end())
     return;
 
-  NotificationObserverList::Iterator it(*(observer_iterator->second));
-  content::NotificationObserver* observer;
-  while ((observer = it.GetNext()) != NULL) {
-    observer->Observe(chrome::NOTIFICATION_SYSTEM_SETTING_CHANGED,
-                      content::Source<CrosSettings>(this),
-                      content::Details<const std::string>(&path));
-  }
+  observer_iterator->second->Notify();
 }
 
 ScopedTestCrosSettings::ScopedTestCrosSettings() {