Remove NOTIFICATION_SYSTEM_SETTING_CHANGED, switch CrosSettings to base::CallbackRegistry.

chrome/browser/chromeos/login/existing_user_controller.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/login/existing_user_controller.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 133 matching lines @@
                  content::NotificationService::AllSources());
   registrar_.Add(this,
                  chrome::NOTIFICATION_USER_LIST_CHANGED,
                  content::NotificationService::AllSources());
   registrar_.Add(this,
                  chrome::NOTIFICATION_AUTH_SUPPLIED,
                  content::NotificationService::AllSources());
   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) {
   time_init_ = base::Time::Now();
   UpdateLoginDisplay(users);
   ConfigurePublicSessionAutoLogin();
 
   DBusThreadManager::Get()->GetSessionManagerClient()->EmitLoginPromptReady();
 }
 
@@ skipping 51 matching lines @@
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
   if (type == chrome::NOTIFICATION_SESSION_STARTED) {
     // Stop listening to any notification once session has started.
     // Sign in screen objects are marked for deletion with DeleteSoon so
     // make sure no object would be used after session has started.
     // http://crbug.com/125276
     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
     // need the credentials for enrollment and other system requests from the
     // main |g_browser_process| request context (see bug
     // http://crosbug.com/24861). So we transfer any credentials to the global
     // request context here.
     // The issue we have here is that the NOTIFICATION_AUTH_SUPPLIED is sent
     // just after the UI is closed but before the new credentials were stored
     // in the profile. Therefore we have to give it some time to make sure it
@@ skipping 17 matching lines @@
     return;
   login_display_->OnUserImageChanged(*content::Details<User>(details).ptr());
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // ExistingUserController, private:
 
 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 {
     NOTREACHED() << "More than one controller are alive.";
   }
   DCHECK(login_display_.get());
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // ExistingUserController, LoginDisplay::Delegate implementation:
@@ skipping 623 matching lines @@
     online_succeeded_for_ = username;
     // Wait for login attempt to end, if it hasn't yet.
     if (offline_failed_ && !is_login_in_progress_)
       ShowGaiaPasswordChanged(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());
 }
 
 void ExistingUserController::ConfigurePublicSessionAutoLogin() {
   std::string auto_login_account_id;
   cros_settings_->GetString(kAccountsPrefDeviceLocalAccountAutoLoginId,
                             &auto_login_account_id);
   const std::vector<policy::DeviceLocalAccount> device_local_accounts =
@@ skipping 158 matching lines @@
   // changed.
   UserManager::Get()->SaveUserOAuthStatus(
       username,
       User::OAUTH2_TOKEN_STATUS_INVALID);
 
   login_display_->SetUIEnabled(true);
   login_display_->ShowGaiaPasswordChanged(username);
 }
 
 }  // namespace chromeos