chromeos: Bind ParallelAuthenticator's scoped_refptr instead of raw pointer when calling async Cryptohome methods

Use scoped_refptr in some places. 
Remove TrigerResolveOnIoThread which does nothing.

Based on CL from hashimoto@ http://codereview.chromium.org/11196060/

BUG=156681
TEST=git try, manual (new/existing user sign in/unlock/changed password case/guest/sign in to one user with incorrect pw, then to another one).

Review URL: https://chromiumcodereview.appspot.com/11232003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@163018 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 49 insertions, 45 deletions

diff --git a/chrome/browser/chromeos/login/authenticator.cc b/chrome/browser/chromeos/login/authenticator.cc
index 20de695..7ac6ad5 100644
--- a/chrome/browser/chromeos/login/authenticator.cc
+++ b/chrome/browser/chromeos/login/authenticator.cc
@@ -15,4 +15,8 @@ Authenticator::Authenticator(LoginStatusConsumer* consumer)
 
 Authenticator::~Authenticator() {}
 
+void Authenticator::ResetConsumer() {
+  consumer_ = NULL;
+}
+
 }  // namespace chromeos
diff --git a/chrome/browser/chromeos/login/authenticator.h b/chrome/browser/chromeos/login/authenticator.h
index 7e08398..8d667c9 100644
--- a/chrome/browser/chromeos/login/authenticator.h
+++ b/chrome/browser/chromeos/login/authenticator.h
@@ -90,6 +90,9 @@ class Authenticator : public base::RefCountedThreadSafe<Authenticator> {
   // authentication process.
   Profile* authentication_profile() { return authentication_profile_; }
 
+  // Resets consumer.
+  void ResetConsumer();
+
  protected:
   virtual ~Authenticator();
 
diff --git a/chrome/browser/chromeos/login/login_utils.cc b/chrome/browser/chromeos/login/login_utils.cc
index 02119a0..4d1bd5f 100644
--- a/chrome/browser/chromeos/login/login_utils.cc
+++ b/chrome/browser/chromeos/login/login_utils.cc
@@ -886,8 +886,11 @@ void LoginUtilsImpl::SetFirstLoginPrefs(PrefService* prefs) {
 scoped_refptr<Authenticator> LoginUtilsImpl::CreateAuthenticator(
     LoginStatusConsumer* consumer) {
   // Screen locker needs new Authenticator instance each time.
-  if (ScreenLocker::default_screen_locker())
+  if (ScreenLocker::default_screen_locker()) {
+    if (authenticator_)
+      authenticator_->ResetConsumer();
     authenticator_ = NULL;
+  }
 
   if (authenticator_ == NULL)
     authenticator_ = new ParallelAuthenticator(consumer);
diff --git a/chrome/browser/chromeos/login/parallel_authenticator.cc b/chrome/browser/chromeos/login/parallel_authenticator.cc
index cf27c27..ef57bc9 100644
--- a/chrome/browser/chromeos/login/parallel_authenticator.cc
+++ b/chrome/browser/chromeos/login/parallel_authenticator.cc
@@ -44,7 +44,7 @@ const int kPasswordHashLength = 32;
 
 // Records status and calls resolver->Resolve().
 void TriggerResolve(AuthAttemptState* attempt,
-                    AuthAttemptStateResolver* resolver,
+                    scoped_refptr<ParallelAuthenticator> resolver,
                     bool success,
                     cryptohome::MountError return_code) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -52,31 +52,20 @@ void TriggerResolve(AuthAttemptState* attempt,
   resolver->Resolve();
 }
 
-// Calls TriggerResolve on the IO thread.
-void TriggerResolveOnIoThread(AuthAttemptState* attempt,
-                              AuthAttemptStateResolver* resolver,
-                              bool success,
-                              cryptohome::MountError return_code) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      base::Bind(&TriggerResolve, attempt, resolver, success, return_code));
-}
-
-// Calls TriggerResolve on the IO thread while adding login time marker.
-void TriggerResolveOnIoThreadWithLoginTimeMarker(
+// Calls TriggerResolve while adding login time marker.
+void TriggerResolveWithLoginTimeMarker(
     const std::string& marker_name,
     AuthAttemptState* attempt,
-    AuthAttemptStateResolver* resolver,
+    scoped_refptr<ParallelAuthenticator> resolver,
     bool success,
     cryptohome::MountError return_code) {
   chromeos::BootTimesLoader::Get()->AddLoginTimeMarker(marker_name, false);
-  TriggerResolveOnIoThread(attempt, resolver, success, return_code);
+  TriggerResolve(attempt, resolver, success, return_code);
 }
 
 // Calls cryptohome's mount method.
 void Mount(AuthAttemptState* attempt,
-           AuthAttemptStateResolver* resolver,
+           scoped_refptr<ParallelAuthenticator> resolver,
            bool create_if_missing) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   chromeos::BootTimesLoader::Get()->AddLoginTimeMarker(
@@ -85,7 +74,7 @@ void Mount(AuthAttemptState* attempt,
       attempt->username,
       attempt->ascii_hash,
       create_if_missing,
-      base::Bind(&TriggerResolveOnIoThreadWithLoginTimeMarker,
+      base::Bind(&TriggerResolveWithLoginTimeMarker,
                  "CryptohomeMount-End",
                  attempt,
                  resolver));
@@ -93,10 +82,10 @@ void Mount(AuthAttemptState* attempt,
 
 // Calls cryptohome's mount method for guest.
 void MountGuest(AuthAttemptState* attempt,
-                AuthAttemptStateResolver* resolver) {
+                scoped_refptr<ParallelAuthenticator> resolver) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   cryptohome::AsyncMethodCaller::GetInstance()->AsyncMountGuest(
-      base::Bind(&TriggerResolveOnIoThreadWithLoginTimeMarker,
+      base::Bind(&TriggerResolveWithLoginTimeMarker,
                  "CryptohomeMount-End",
                  attempt,
                  resolver));
@@ -104,7 +93,7 @@ void MountGuest(AuthAttemptState* attempt,
 
 // Calls cryptohome's key migration method.
 void Migrate(AuthAttemptState* attempt,
-             AuthAttemptStateResolver* resolver,
+             scoped_refptr<ParallelAuthenticator> resolver,
              bool passing_old_hash,
              const std::string& hash) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -117,7 +106,7 @@ void Migrate(AuthAttemptState* attempt,
         attempt->username,
         hash,
         attempt->ascii_hash,
-        base::Bind(&TriggerResolveOnIoThreadWithLoginTimeMarker,
+        base::Bind(&TriggerResolveWithLoginTimeMarker,
                    "CryptohomeMount-End",
                    attempt,
                    resolver));
@@ -126,7 +115,7 @@ void Migrate(AuthAttemptState* attempt,
         attempt->username,
         attempt->ascii_hash,
         hash,
-        base::Bind(&TriggerResolveOnIoThreadWithLoginTimeMarker,
+        base::Bind(&TriggerResolveWithLoginTimeMarker,
                    "CryptohomeMount-End",
                    attempt,
                    resolver));
@@ -135,13 +124,13 @@ void Migrate(AuthAttemptState* attempt,
 
 // Calls cryptohome's remove method.
 void Remove(AuthAttemptState* attempt,
-            AuthAttemptStateResolver* resolver) {
+            scoped_refptr<ParallelAuthenticator> resolver) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   chromeos::BootTimesLoader::Get()->AddLoginTimeMarker(
       "CryptohomeRemove-Start", false);
   cryptohome::AsyncMethodCaller::GetInstance()->AsyncRemove(
       attempt->username,
-      base::Bind(&TriggerResolveOnIoThreadWithLoginTimeMarker,
+      base::Bind(&TriggerResolveWithLoginTimeMarker,
                  "CryptohomeRemove-End",
                  attempt,
                  resolver));
@@ -149,12 +138,12 @@ void Remove(AuthAttemptState* attempt,
 
 // Calls cryptohome's key check method.
 void CheckKey(AuthAttemptState* attempt,
-              AuthAttemptStateResolver* resolver) {
+              scoped_refptr<ParallelAuthenticator> resolver) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   cryptohome::AsyncMethodCaller::GetInstance()->AsyncCheckKey(
       attempt->username,
       attempt->ascii_hash,
-      base::Bind(&TriggerResolveOnIoThread, attempt, resolver));
+      base::Bind(&TriggerResolve, attempt, resolver));
 }
 
 // Returns whether the login failure was connection issue.
@@ -226,7 +215,7 @@ void ParallelAuthenticator::AuthenticateToLogin(
       BrowserThread::UI, FROM_HERE,
       base::Bind(&Mount,
                  current_state_.get(),
-                 static_cast<AuthAttemptStateResolver*>(this),
+                 scoped_refptr<ParallelAuthenticator>(this),
                  create_if_missing));
   // ClientLogin authentication check should happen immediately here.
   // We should not try OAuthLogin check until the profile loads.
@@ -259,7 +248,7 @@ void ParallelAuthenticator::CompleteLogin(Profile* profile,
       BrowserThread::UI, FROM_HERE,
       base::Bind(&Mount,
                  current_state_.get(),
-                 static_cast<AuthAttemptStateResolver*>(this),
+                 scoped_refptr<ParallelAuthenticator>(this),
                  create_if_missing));
 
   if (!using_oauth_) {
@@ -292,7 +281,7 @@ void ParallelAuthenticator::AuthenticateToUnlock(const std::string& username,
       BrowserThread::UI, FROM_HERE,
       base::Bind(&CheckKey,
                  current_state_.get(),
-                 static_cast<AuthAttemptStateResolver*>(this)));
+                 scoped_refptr<ParallelAuthenticator>(this)));
 }
 
 void ParallelAuthenticator::LoginDemoUser() {
@@ -302,7 +291,7 @@ void ParallelAuthenticator::LoginDemoUser() {
       new AuthAttemptState(kDemoUser, "", "", "", "", false));
   mount_guest_attempted_ = true;
   MountGuest(current_state_.get(),
-             static_cast<AuthAttemptStateResolver*>(this));
+             scoped_refptr<ParallelAuthenticator>(this));
 }
 
 void ParallelAuthenticator::LoginOffTheRecord() {
@@ -310,7 +299,7 @@ void ParallelAuthenticator::LoginOffTheRecord() {
   current_state_.reset(new AuthAttemptState("", "", "", "", "", false));
   mount_guest_attempted_ = true;
   MountGuest(current_state_.get(),
-             static_cast<AuthAttemptStateResolver*>(this));
+             scoped_refptr<ParallelAuthenticator>(this));
 }
 
 void ParallelAuthenticator::OnDemoUserLoginSuccess() {
@@ -322,7 +311,8 @@ void ParallelAuthenticator::OnDemoUserLoginSuccess() {
       chrome::NOTIFICATION_LOGIN_AUTHENTICATION,
       content::NotificationService::AllSources(),
       content::Details<AuthenticationNotificationDetails>(&details));
-  consumer_->OnDemoUserLoginSuccess();
+  if (consumer_)
+    consumer_->OnDemoUserLoginSuccess();
 }
 
 void ParallelAuthenticator::OnLoginSuccess(bool request_pending) {
@@ -338,10 +328,11 @@ void ParallelAuthenticator::OnLoginSuccess(bool request_pending) {
     base::AutoLock for_this_block(success_lock_);
     already_reported_success_ = true;
   }
-  consumer_->OnLoginSuccess(current_state_->username,
-                            current_state_->password,
-                            request_pending,
-                            using_oauth_);
+  if (consumer_)
+    consumer_->OnLoginSuccess(current_state_->username,
+                              current_state_->password,
+                              request_pending,
+                              using_oauth_);
 }
 
 void ParallelAuthenticator::OnOffTheRecordLoginSuccess() {
@@ -352,12 +343,14 @@ void ParallelAuthenticator::OnOffTheRecordLoginSuccess() {
       chrome::NOTIFICATION_LOGIN_AUTHENTICATION,
       content::NotificationService::AllSources(),
       content::Details<AuthenticationNotificationDetails>(&details));
-  consumer_->OnOffTheRecordLoginSuccess();
+  if (consumer_)
+    consumer_->OnOffTheRecordLoginSuccess();
 }
 
 void ParallelAuthenticator::OnPasswordChangeDetected() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  consumer_->OnPasswordChangeDetected();
+  if (consumer_)
+    consumer_->OnPasswordChangeDetected();
 }
 
 void ParallelAuthenticator::OnLoginFailure(const LoginFailure& error) {
@@ -369,7 +362,8 @@ void ParallelAuthenticator::OnLoginFailure(const LoginFailure& error) {
       content::NotificationService::AllSources(),
       content::Details<AuthenticationNotificationDetails>(&details));
   LOG(WARNING) << "Login failed: " << error.GetErrorString();
-  consumer_->OnLoginFailure(error);
+  if (consumer_)
+    consumer_->OnLoginFailure(error);
 }
 
 void ParallelAuthenticator::RecordOAuthCheckFailure(
@@ -390,7 +384,7 @@ void ParallelAuthenticator::RecoverEncryptedData(
       BrowserThread::UI, FROM_HERE,
       base::Bind(&Migrate,
                  current_state_.get(),
-                 static_cast<AuthAttemptStateResolver*>(this),
+                 scoped_refptr<ParallelAuthenticator>(this),
                  true,
                  old_hash));
 }
@@ -402,7 +396,7 @@ void ParallelAuthenticator::ResyncEncryptedData() {
       BrowserThread::UI, FROM_HERE,
       base::Bind(&Remove,
                  current_state_.get(),
-                 static_cast<AuthAttemptStateResolver*>(this)));
+                 scoped_refptr<ParallelAuthenticator>(this)));
 }
 
 bool ParallelAuthenticator::VerifyOwner() {
@@ -504,7 +498,7 @@ void ParallelAuthenticator::Resolve() {
           BrowserThread::UI, FROM_HERE,
           base::Bind(&Mount,
                      current_state_.get(),
-                     static_cast<AuthAttemptStateResolver*>(this),
+                     scoped_refptr<ParallelAuthenticator>(this),
                      create));
       break;
     case NEED_OLD_PW:
@@ -558,7 +552,7 @@ void ParallelAuthenticator::Resolve() {
           BrowserThread::UI, FROM_HERE,
           base::Bind(&Migrate,
                      reauth_state_.get(),
-                     static_cast<AuthAttemptStateResolver*>(this),
+                     scoped_refptr<ParallelAuthenticator>(this),
                      true,
                      current_state_->ascii_hash));
       break;