Remove NOTIFICATION_SYSTEM_SETTING_CHANGED, switch CrosSettings to base::CallbackRegistry.

chrome/browser/chromeos/login/existing_user_controller.cc

Index: chrome/browser/chromeos/login/existing_user_controller.cc
diff --git a/chrome/browser/chromeos/login/existing_user_controller.cc b/chrome/browser/chromeos/login/existing_user_controller.cc
index 5530fb773176d96e0ef61c62010ab4c7d202f2eb..05e75473c3cdadf0030d8e4bc2aef1009c35da95 100644
--- a/chrome/browser/chromeos/login/existing_user_controller.cc
+++ b/chrome/browser/chromeos/login/existing_user_controller.cc
@@ -151,16 +151,32 @@ ExistingUserController::ExistingUserController(LoginDisplayHost* host)
   registrar_.Add(this,
                  chrome::NOTIFICATION_SESSION_STARTED,
                  content::NotificationService::AllSources());
-  cros_settings_->AddSettingsObserver(kAccountsPrefShowUserNamesOnSignIn, this);
-  cros_settings_->AddSettingsObserver(kAccountsPrefAllowNewUser, this);
-  cros_settings_->AddSettingsObserver(kAccountsPrefAllowGuest, this);
-  cros_settings_->AddSettingsObserver(kAccountsPrefUsers, this);
-  cros_settings_->AddSettingsObserver(
-      kAccountsPrefDeviceLocalAccountAutoLoginId,
-      this);
-  cros_settings_->AddSettingsObserver(
-      kAccountsPrefDeviceLocalAccountAutoLoginDelay,
-      this);
+  show_user_names_subscription_ = cros_settings_->AddSettingsObserver(
+      kAccountsPrefShowUserNamesOnSignIn,
+      base::Bind(&ExistingUserController::SettingsChanged,
+                 base::Unretained(this)));
+  allow_new_user_subscription_ = cros_settings_->AddSettingsObserver(
+      kAccountsPrefAllowNewUser,
+      base::Bind(&ExistingUserController::SettingsChanged,
+                 base::Unretained(this)));
+  allow_guest_subscription_ = cros_settings_->AddSettingsObserver(
+      kAccountsPrefAllowGuest,
+      base::Bind(&ExistingUserController::SettingsChanged,
+                 base::Unretained(this)));
+  users_subscription_ = cros_settings_->AddSettingsObserver(
+      kAccountsPrefUsers,
+      base::Bind(&ExistingUserController::SettingsChanged,
+                 base::Unretained(this)));
+  local_account_auto_login_id_subscription_ =
+      cros_settings_->AddSettingsObserver(
+          kAccountsPrefDeviceLocalAccountAutoLoginId,
+          base::Bind(&ExistingUserController::SettingsChanged,
+                     base::Unretained(this)));
+  local_account_auto_login_delay_subscription_ =
+      cros_settings_->AddSettingsObserver(
+          kAccountsPrefDeviceLocalAccountAutoLoginDelay,
+          base::Bind(&ExistingUserController::SettingsChanged,
+                     base::Unretained(this)));
 }
 
 void ExistingUserController::Init(const UserList& users) {
@@ -232,22 +248,9 @@ void ExistingUserController::Observe(
     registrar_.RemoveAll();
     return;
   }
-  if (type == chrome::NOTIFICATION_SYSTEM_SETTING_CHANGED) {
-    const std::string setting = *content::Details<const std::string>(
-        details).ptr();
-    if (setting == kAccountsPrefDeviceLocalAccountAutoLoginId ||
-        setting == kAccountsPrefDeviceLocalAccountAutoLoginDelay) {
-      ConfigurePublicSessionAutoLogin();
-    }
-  }
-  if (type == chrome::NOTIFICATION_SYSTEM_SETTING_CHANGED ||
-      type == chrome::NOTIFICATION_USER_LIST_CHANGED) {
-    if (host_ != NULL) {
-      // Signed settings or user list changed. Notify views and update them.
-      UpdateLoginDisplay(chromeos::UserManager::Get()->GetUsers());
-      ConfigurePublicSessionAutoLogin();
-      return;
-    }
+  if (type == chrome::NOTIFICATION_USER_LIST_CHANGED) {
+    SettingsChanged();
+    return;
   }
   if (type == chrome::NOTIFICATION_AUTH_SUPPLIED) {
     // Possibly the user has authenticated against a proxy server and we might
@@ -285,18 +288,6 @@ void ExistingUserController::Observe(
 ExistingUserController::~ExistingUserController() {
   LoginUtils::Get()->DelegateDeleted(this);
 
-  cros_settings_->RemoveSettingsObserver(kAccountsPrefShowUserNamesOnSignIn,
-                                         this);
-  cros_settings_->RemoveSettingsObserver(kAccountsPrefAllowNewUser, this);
-  cros_settings_->RemoveSettingsObserver(kAccountsPrefAllowGuest, this);
-  cros_settings_->RemoveSettingsObserver(kAccountsPrefUsers, this);
-  cros_settings_->RemoveSettingsObserver(
-      kAccountsPrefDeviceLocalAccountAutoLoginId,
-      this);
-  cros_settings_->RemoveSettingsObserver(
-      kAccountsPrefDeviceLocalAccountAutoLoginDelay,
-      this);
-
   if (current_controller_ == this) {
     current_controller_ = NULL;
   } else {
@@ -940,6 +931,15 @@ void ExistingUserController::OnOnlineChecked(const std::string& username,
 ////////////////////////////////////////////////////////////////////////////////
 // ExistingUserController, private:
 
+void ExistingUserController::SettingsChanged() {

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

Not sure if this is this equivalent to the old code. I'm leaving it to the other
reviewers to have a closer look.

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

The old code doesn't make much sense either ;)

I think this should actually be split into two callbacks. Calling
ConfigurePublicSessionAutoLogin() is sufficient for
kAccountsPrefDeviceLocalAccountAutoLoginId and
kAccountsPrefDeviceLocalAccountAutoLoginDelay. The remaining observed settings
should be wired to SettingsChanged() (which should retain the call to
ConfigurePublicSessionAutoLogin()).

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

Tell me about it.
Done.

+  if (host_ != NULL) {
+    // Signed settings or user list changed. Notify views and update them.
+    UpdateLoginDisplay(chromeos::UserManager::Get()->GetUsers());
+    ConfigurePublicSessionAutoLogin();
+    return;
+  }
+}
+
 void ExistingUserController::ActivateWizard(const std::string& screen_name) {
   scoped_ptr<DictionaryValue> params;
   host_->StartWizard(screen_name, params.Pass());