PART1: Initiated the SignedSettings refactoring.

chrome/browser/chromeos/login/login_performer.cc

 // Copyright (c) 2011 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/login_performer.h"
 
 #include <string>
 
+#include "base/bind.h"
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/metrics/histogram.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/boot_times_loader.h"
 #include "chrome/browser/chromeos/cros/cros_library.h"
 #include "chrome/browser/chromeos/cros/screen_lock_library.h"
+#include "chrome/browser/chromeos/cros_settings.h"
 #include "chrome/browser/chromeos/cros_settings_names.h"
 #include "chrome/browser/chromeos/login/login_utils.h"
 #include "chrome/browser/chromeos/login/screen_locker.h"
-#include "chrome/browser/chromeos/user_cros_settings_provider.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "content/browser/browser_thread.h"
 #include "content/browser/user_metrics.h"
 #include "content/common/content_notification_types.h"
 #include "content/common/notification_service.h"
@@ skipping 16 matching lines @@
       delegate_(delegate),
       password_changed_(false),
       screen_lock_requested_(false),
       initial_online_auth_pending_(false),
       auth_mode_(AUTH_MODE_INTERNAL),
       using_oauth_(
           CommandLine::ForCurrentProcess()->HasSwitch(
               switches::kWebUILogin) &&
           !CommandLine::ForCurrentProcess()->HasSwitch(
               switches::kSkipOAuthLogin)),
-      method_factory_(this) {
+      pointer_factory_(this) {
   DCHECK(default_performer_ == NULL)
       << "LoginPerformer should have only one instance.";
   default_performer_ = this;
 }
 
 LoginPerformer::~LoginPerformer() {
   DVLOG(1) << "Deleting LoginPerformer";
   DCHECK(default_performer_ != NULL) << "Default instance should exist.";
   default_performer_ = NULL;
 }
@@ skipping 150 matching lines @@
     last_login_failure_ =
         LoginFailure::FromNetworkAuthFailure(GoogleServiceAuthError(
             GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS));
     password_changed_ = true;
     DVLOG(1) << "Password change detected - locking screen.";
     RequestScreenLock();
   }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-// LoginPerformer, SignedSettingsHelper::Callback implementation:
-
-void LoginPerformer::OnCheckWhitelistCompleted(SignedSettings::ReturnCode code,
-                                               const std::string& email) {
-  if (code == SignedSettings::SUCCESS) {
-    // Whitelist check passed, continue with authentication.
-    if (auth_mode_ == AUTH_MODE_EXTENSION) {
-      StartLoginCompletion();
-    } else {
-      StartAuthentication();
-    }
-  } else {
-    if (delegate_)
-      delegate_->WhiteListCheckFailed(email);
-    else
-      NOTREACHED();
-  }
-}
-
-////////////////////////////////////////////////////////////////////////////////
 // LoginPerformer, NotificationObserver implementation:
 //
 
 void LoginPerformer::Observe(int type,
                              const NotificationSource& source,
                              const NotificationDetails& details) {
   if (type != chrome::NOTIFICATION_SCREEN_LOCK_STATE_CHANGED)
     return;
 
   bool is_screen_locked = *Details<bool>(details).ptr();
   if (is_screen_locked) {
     if (screen_lock_requested_) {
       screen_lock_requested_ = false;
       ResolveScreenLocked();
     }
   } else {
     ResolveScreenUnlocked();
   }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // LoginPerformer, public:
 void LoginPerformer::CompleteLogin(const std::string& username,
                                    const std::string& password) {
   auth_mode_ = AUTH_MODE_EXTENSION;
   username_ = username;
   password_ = password;
+
+  CrosSettings* cros_settings = CrosSettings::Get();
+
   // Whitelist check is always performed during initial login and
   // should not be performed when ScreenLock is active (pending online auth).
   if (!ScreenLocker::default_screen_locker()) {
-    // Must not proceed without signature verification.
-    UserCrosSettingsProvider user_settings;
-    bool trusted_setting_available = user_settings.RequestTrustedAllowNewUser(
-        method_factory_.NewRunnableMethod(&LoginPerformer::CompleteLogin,
-                                          username,
-                                          password));
-    if (!trusted_setting_available) {
+    // Must not proceed without signature verification or valid user list.
+    bool trusted_settings_available =
+        cros_settings->GetTrusted(
+            kAccountsPrefAllowNewUser,
+            base::Bind(&LoginPerformer::CompleteLogin,
+                       pointer_factory_.GetWeakPtr(),
+                       username, password)) ||
+        cros_settings->GetTrusted(
+            kAccountsPrefAllowNewUser,
+            base::Bind(&LoginPerformer::CompleteLogin,
+                       pointer_factory_.GetWeakPtr(),
+                       username, password));

[Mattias Nissler (ping if slow), 2011/09/21 11:12:59]

Doing the same thing twice to be extra-sure?

[pastarmovj, 2011/09/23 15:19:32]

Done.

+    if (!trusted_settings_available) {
       // Value of AllowNewUser setting is still not verified.
       // Another attempt will be invoked after verification completion.
       return;
     }
   }
 
-  if (ScreenLocker::default_screen_locker() ||
-      UserCrosSettingsProvider::cached_allow_new_user()) {
+  bool allow_new_user = false;
+  cros_settings->GetBoolean(kAccountsPrefAllowNewUser, &allow_new_user);

[Mattias Nissler (ping if slow), 2011/09/21 11:12:59]

no guarantee that you have trusted values here if
ScreenLocker::default_screen_locker() is NULL. Is that intentional?

[pastarmovj, 2011/09/23 15:19:32]

I must admit I didn't really try to verify the logic that was already existing
here only updated it to use the right calls.
But looking in the code it seems right to me. If the screen locker is null the
code above will trigger trust check and if it is not null the if will be true
regardless of the allow_new_user value.

+  if (ScreenLocker::default_screen_locker() || allow_new_user) {
     // Starts authentication if guest login is allowed or online auth pending.
     StartLoginCompletion();
   } else {
-    // Otherwise, do whitelist check first.
-    PrefService* local_state = g_browser_process->local_state();
-    CHECK(local_state);
-    if (local_state->IsManagedPreference(kAccountsPrefUsers)) {
-      if (UserCrosSettingsProvider::IsEmailInCachedWhitelist(username)) {
-        StartLoginCompletion();
-      } else {
-        if (delegate_)
-          delegate_->WhiteListCheckFailed(username);
-        else
-          NOTREACHED();
-      }
+    const ListValue *user_list;
+    if (cros_settings->GetList(kAccountsPrefUsers, &user_list) &&
+        user_list->Find(StringValue(username)) != user_list->end()) {
+      StartLoginCompletion();
     } else {
-      // In case of signed settings: with current implementation we do not
-      // trust whitelist returned by PrefService.  So make separate check.
-      SignedSettingsHelper::Get()->StartCheckWhitelistOp(
-          username, this);
+      if (delegate_)
+        delegate_->WhiteListCheckFailed(username);
+      else
+        NOTREACHED();
     }
   }
 }
 
 void LoginPerformer::Login(const std::string& username,
                            const std::string& password) {
   auth_mode_ = AUTH_MODE_INTERNAL;
   username_ = username;
   password_ = password;
 
+  CrosSettings* cros_settings = CrosSettings::Get();
+
   // Whitelist check is always performed during initial login and
   // should not be performed when ScreenLock is active (pending online auth).
   if (!ScreenLocker::default_screen_locker()) {
     // Must not proceed without signature verification.
-    UserCrosSettingsProvider user_settings;
-    bool trusted_setting_available = user_settings.RequestTrustedAllowNewUser(
-        method_factory_.NewRunnableMethod(&LoginPerformer::Login,
-                                          username,
-                                          password));
-    if (!trusted_setting_available) {
+    bool trusted_settings_available =
+        cros_settings->GetTrusted(
+            kAccountsPrefAllowNewUser,
+            base::Bind(&LoginPerformer::CompleteLogin,
+                       pointer_factory_.GetWeakPtr(),
+                       username, password)) ||
+        cros_settings->GetTrusted(
+            kAccountsPrefAllowNewUser,
+            base::Bind(&LoginPerformer::CompleteLogin,
+                       pointer_factory_.GetWeakPtr(),
+                       username, password));

[Mattias Nissler (ping if slow), 2011/09/21 11:12:59]

Same two identical things again. Also, this is pretty inconvenient to use if you
want to check several different prefs. Have you considered decoupling the
GetTrusted() check from the actual pref path? I.e. introduce a way to be
notified whenever we learn that all settings are trusted?

[pastarmovj, 2011/09/23 15:19:32]

Done.

+    if (!trusted_settings_available) {
       // Value of AllowNewUser setting is still not verified.
       // Another attempt will be invoked after verification completion.
       return;
     }
   }
 
-  if (ScreenLocker::default_screen_locker() ||
-      UserCrosSettingsProvider::cached_allow_new_user()) {
+  bool allow_new_user = false;
+  cros_settings->GetBoolean(kAccountsPrefAllowNewUser, &allow_new_user);

[Mattias Nissler (ping if slow), 2011/09/21 11:12:59]

same comment regarding trustedness as above.

[pastarmovj, 2011/09/23 15:19:32]

Same answer as above too :)

+  if (ScreenLocker::default_screen_locker() || allow_new_user) {
     // Starts authentication if guest login is allowed or online auth pending.
     StartAuthentication();
   } else {
-    // Otherwise, do whitelist check first.
-    PrefService* local_state = g_browser_process->local_state();
-    CHECK(local_state);
-    if (local_state->IsManagedPreference(kAccountsPrefUsers)) {
-      if (UserCrosSettingsProvider::IsEmailInCachedWhitelist(username)) {
-        StartAuthentication();
-      } else {
-        if (delegate_)
-          delegate_->WhiteListCheckFailed(username);
-        else
-          NOTREACHED();
-      }
+    const ListValue *user_list;
+    if (cros_settings->GetList(kAccountsPrefUsers, &user_list) &&
+        user_list->Find(StringValue(username)) != user_list->end()) {
+      StartAuthentication();
     } else {
-      // In case of signed settings: with current implementation we do not
-      // trust whitelist returned by PrefService.  So make separate check.
-      SignedSettingsHelper::Get()->StartCheckWhitelistOp(
-          username, this);
+      if (delegate_)
+        delegate_->WhiteListCheckFailed(username);
+      else
+        NOTREACHED();
     }
   }
 }
 
 void LoginPerformer::LoginOffTheRecord() {
   authenticator_ = LoginUtils::Get()->CreateAuthenticator(this);
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
-      NewRunnableMethod(authenticator_.get(),
-                        &Authenticator::LoginOffTheRecord));
+      base::Bind(&Authenticator::LoginOffTheRecord, authenticator_.get()));
 }
 
 void LoginPerformer::RecoverEncryptedData(const std::string& old_password) {
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
-      NewRunnableMethod(authenticator_.get(),
-                        &Authenticator::RecoverEncryptedData,
-                        old_password,
-                        cached_credentials_));
+      base::Bind(&Authenticator::RecoverEncryptedData, authenticator_.get(),
+                 old_password,
+                 cached_credentials_));
   cached_credentials_ = GaiaAuthConsumer::ClientLoginResult();
 }
 
 void LoginPerformer::ResyncEncryptedData() {
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
-      NewRunnableMethod(authenticator_.get(),
-                        &Authenticator::ResyncEncryptedData,
-                        cached_credentials_));
+      base::Bind(&Authenticator::ResyncEncryptedData, authenticator_.get(),
+                 cached_credentials_));
   cached_credentials_ = GaiaAuthConsumer::ClientLoginResult();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // LoginPerformer, private:
 
 void LoginPerformer::RequestScreenLock() {
   DVLOG(1) << "Screen lock requested";
   // Will receive notifications on screen unlock and delete itself.
   registrar_.Add(this,
@@ skipping 144 matching lines @@
 }
 
 void LoginPerformer::StartLoginCompletion() {
   DVLOG(1) << "Login completion started";
   BootTimesLoader::Get()->AddLoginTimeMarker("AuthStarted", false);
   Profile* profile = g_browser_process->profile_manager()->GetDefaultProfile();
 
   authenticator_ = LoginUtils::Get()->CreateAuthenticator(this);
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
-      NewRunnableMethod(authenticator_.get(),
-                        &Authenticator::CompleteLogin,
-                        profile,
-                        username_,
-                        password_));
+      base::Bind(&Authenticator::CompleteLogin, authenticator_.get(),
+                 profile,
+                 username_,
+                 password_));
 
   password_.clear();
 }
 
 void LoginPerformer::StartAuthentication() {
   DVLOG(1) << "Auth started";
   BootTimesLoader::Get()->AddLoginTimeMarker("AuthStarted", false);
   Profile* profile = g_browser_process->profile_manager()->GetDefaultProfile();
   if (delegate_) {
     authenticator_ = LoginUtils::Get()->CreateAuthenticator(this);
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,
-        NewRunnableMethod(authenticator_.get(),
-                          &Authenticator::AuthenticateToLogin,
-                          profile,
-                          username_,
-                          password_,
-                          captcha_token_,
-                          captcha_));
+        base::Bind(&Authenticator::AuthenticateToLogin, authenticator_.get(),
+                   profile,
+                   username_,
+                   password_,
+                   captcha_token_,
+                   captcha_));
   } else {
     DCHECK(authenticator_.get())
         << "Authenticator instance doesn't exist for login attempt retry.";
     // At this point offline auth has been successful,
     // retry online auth, using existing Authenticator instance.
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,
-        NewRunnableMethod(authenticator_.get(),
-                          &Authenticator::RetryAuth,
-                          profile,
-                          username_,
-                          password_,
-                          captcha_token_,
-                          captcha_));
+        base::Bind(&Authenticator::RetryAuth, authenticator_.get(),
+                   profile,
+                   username_,
+                   password_,
+                   captcha_token_,
+                   captcha_));
   }
   password_.clear();
 }
 
 }  // namespace chromeos