Remove NOTIFICATION_SYSTEM_SETTING_CHANGED, switch CrosSettings to base::CallbackRegistry.

chrome/browser/chromeos/settings/cros_settings.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/values.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/chromeos/settings/device_settings_provider.h"
 #include "chrome/browser/chromeos/settings/device_settings_service.h"
 #include "chrome/browser/chromeos/settings/stub_cros_settings_provider.h"
 #include "chrome/browser/chromeos/settings/system_settings_provider.h"
 #include "chromeos/chromeos_switches.h"
 #include "content/public/browser/notification_details.h"

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

Remove

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

Done.

 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/notification_types.h"
 #include "google_apis/gaia/gaia_auth_util.h"
 
 namespace chromeos {
 
 static CrosSettings*  g_cros_settings = NULL;
 
 // static
 void CrosSettings::Initialize() {
@@ skipping 225 matching lines @@
   DCHECK(CalledOnValidThread());
   std::vector<CrosSettingsProvider*>::iterator it =
       std::find(providers_.begin(), providers_.end(), provider);
   if (it != providers_.end()) {
     providers_.erase(it);
     return true;
   }
   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;
+    registry = 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;
-  }
-
-  // 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(
     const std::string& path) const {
   for (size_t i = 0; i < providers_.size(); ++i) {
     if (providers_[i]->HandlesSetting(path))
       return providers_[i];
   }
   return NULL;
 }
 
 void CrosSettings::FireObservers(const std::string& path) {
   DCHECK(CalledOnValidThread());
   SettingsObserverMap::iterator observer_iterator =
       settings_observers_.find(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() {
   CrosSettings::Initialize();
 }
 
 ScopedTestCrosSettings::~ScopedTestCrosSettings() {
   CrosSettings::Shutdown();
 }
 
 }  // namespace chromeos