PART1: Initiated the SignedSettings refactoring.

chrome/browser/chromeos/login/login_performer.cc

Index: chrome/browser/chromeos/login/login_performer.cc
diff --git a/chrome/browser/chromeos/login/login_performer.cc b/chrome/browser/chromeos/login/login_performer.cc
index 161684d1d3d2aabd05418575125f039a9c2dceab..a6d4b10f2fae1f097e8240527d81a6f8ac151b28 100644
--- a/chrome/browser/chromeos/login/login_performer.cc
+++ b/chrome/browser/chromeos/login/login_performer.cc
@@ -6,6 +6,7 @@
 
 #include <string>
 
+#include "base/bind.h"
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
@@ -15,10 +16,10 @@
 #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"
@@ -55,7 +56,7 @@ LoginPerformer::LoginPerformer(Delegate* delegate)
               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;
@@ -226,26 +227,6 @@ void LoginPerformer::OnPasswordChangeDetected(
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-// 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:
 //
 
@@ -273,44 +254,46 @@ void LoginPerformer::CompleteLogin(const std::string& username,
   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();
     }
   }
 }
@@ -321,44 +304,45 @@ void LoginPerformer::Login(const std::string& username,
   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();
     }
   }
 }
@@ -367,26 +351,23 @@ 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();
 }
 
@@ -551,11 +532,10 @@ void LoginPerformer::StartLoginCompletion() {
   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();
 }
@@ -568,13 +548,12 @@ void LoginPerformer::StartAuthentication() {
     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.";
@@ -582,13 +561,12 @@ void LoginPerformer::StartAuthentication() {
     // 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();
 }