Properly handle user cancellation of signin.

chrome/browser/signin/signin_manager.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/signin/signin_manager.h"
 
 #include <string>
 #include <vector>
 
 #include "base/command_line.h"
@@ skipping 497 matching lines @@
   last_result_ = ClientLoginResult();
   possibly_invalid_username_.clear();
   password_.clear();
   had_two_factor_error_ = false;
   type_ = SIGNIN_TYPE_NONE;
   temp_oauth_login_tokens_ = ClientOAuthResult();
 }
 
 void SigninManager::HandleAuthError(const GoogleServiceAuthError& error,
                                     bool clear_transient_data) {
+  // In some cases, the user should not be signed out.  For example, the failure
+  // may be due to a captcha or OTP challenge.  In these cases, the transient
+  // data must be kept to properly handle the follow up. This routine clears
+  // the data before sending out the notification so the SigninManager is no
+  // longer in the AuthInProgress state when the notification goes out.
+  if (clear_transient_data)
+    ClearTransientSigninData();
+
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_GOOGLE_SIGNIN_FAILED,
       content::Source<Profile>(profile_),
       content::Details<const GoogleServiceAuthError>(&error));
-
-  // In some cases, the user should not be signed out.  For example, the failure
-  // may be due to a captcha or OTP challenge.  In these cases, the transient
-  // data must be kept to properly handle the follow up.
-  if (clear_transient_data)
-    ClearTransientSigninData();
 }
 
 void SigninManager::SignOut() {
   DCHECK(IsInitialized());
+
+  if (authenticated_username_.empty()) {
+    if (AuthInProgress()) {
+      // If the user is in the process of signing in, then treat a call to
+      // SignOut as a cancellation request.
+      GoogleServiceAuthError error(GoogleServiceAuthError::REQUEST_CANCELED);
+      HandleAuthError(error, true);
+    } else {
+      // Clean up our transient data and exit if we aren't signed in.
+      // This avoids a perf regression from clearing out the TokenDB if
+      // SignOut() is invoked on startup to clean up any incomplete previous
+      // signin attempts.
+      ClearTransientSigninData();
+    }
+    return;
+  }
+
   if (prohibit_signout_) {
     DVLOG(1) << "Ignoring attempt to sign out while signout is prohibited";
     return;
   }
-  if (authenticated_username_.empty() && !client_login_.get()) {
-    // Clean up our transient data and exit if we aren't signed in (or in the
-    // process of signing in). This avoids a perf regression from clearing out
-    // the TokenDB if SignOut() is invoked on startup to clean up any
-    // incomplete previous signin attempts.
-    ClearTransientSigninData();
-    return;
-  }
-
+  DCHECK(!authenticated_username_.empty());
   GoogleServiceSignoutDetails details(authenticated_username_);
 
   ClearTransientSigninData();
   authenticated_username_.clear();
   profile_->GetPrefs()->ClearPref(prefs::kGoogleServicesUsername);
 
   // Erase (now) stale information from AboutSigninInternals.
   NotifyDiagnosticsObservers(USERNAME, std::string());
   NotifyDiagnosticsObservers(LSID, std::string());
   NotifyDiagnosticsObservers(signin_internals_util::SID, std::string());
@@ skipping 192 matching lines @@
   policy::UserPolicySigninService* policy_service =
       policy::UserPolicySigninServiceFactory::GetForProfile(profile_);
   policy_service->FetchPolicyForSignedInUser(
       policy_client_.Pass(),
       base::Bind(&SigninManager::OnPolicyFetchComplete,
                  weak_pointer_factory_.GetWeakPtr()));
 }
 
 void SigninManager::OnPolicyFetchComplete(bool success) {
   // For now, we allow signin to complete even if the policy fetch fails. If
-  // we ever want to change this behavior, we could call SignOut() here
+  // we ever want to change this behavior, we could call HandleAuthError() here
   // instead.
   DLOG_IF(ERROR, !success) << "Error fetching policy for user";
   DVLOG_IF(1, success) << "Policy fetch successful - completing signin";
   CompleteSigninAfterPolicyLoad();
 }
 
 void SigninManager::TransferCredentialsToNewProfile() {
   DCHECK(!possibly_invalid_username_.empty());
   DCHECK(policy_client_);
   // Create a new profile and have it call back when done so we can inject our
   // signin credentials.
   ProfileManager::CreateMultiProfileAsync(
       UTF8ToUTF16(possibly_invalid_username_),
       UTF8ToUTF16(ProfileInfoCache::GetDefaultAvatarIconUrl(1)),
       base::Bind(&SigninManager::CompleteSigninForNewProfile,
                  weak_pointer_factory_.GetWeakPtr()),
       chrome::GetActiveDesktop(),
       false);
 }
 
 void SigninManager::CompleteSigninForNewProfile(
     Profile* profile,
     Profile::CreateStatus status) {
   DCHECK_NE(profile_, profile);
-  // TODO(atwilson): On error, unregister the client to release the DMToken.
   if (status == Profile::CREATE_STATUS_FAIL) {
+    // TODO(atwilson): On error, unregister the client to release the DMToken
+    // and surface a better error for the user.
     NOTREACHED() << "Error creating new profile";
-    SignOut();
+    GoogleServiceAuthError error(GoogleServiceAuthError::SERVICE_UNAVAILABLE);
+    HandleAuthError(error, true);
     return;
   }
 
   // Wait until the profile is initialized before we transfer credentials.
   if (status == Profile::CREATE_STATUS_INITIALIZED) {
     DCHECK(!possibly_invalid_username_.empty());
     DCHECK(policy_client_);
     // Sign in to the just-created profile and fetch policy for it.
     SigninManager* signin_manager =
         SigninManagerFactory::GetForProfile(profile);
     DCHECK(signin_manager);
     signin_manager->possibly_invalid_username_ = possibly_invalid_username_;
     signin_manager->last_result_ = last_result_;
     signin_manager->temp_oauth_login_tokens_ = temp_oauth_login_tokens_;
     signin_manager->policy_client_.reset(policy_client_.release());
     signin_manager->LoadPolicyWithCachedClient();
     // Allow sync to start up if it is not overridden by policy.
     browser_sync::SyncPrefs prefs(profile->GetPrefs());
     prefs.SetSyncSetupCompleted();
 
-    // We've transferred our credentials to the new profile - sign out.
+    // We've transferred our credentials to the new profile - notify that
+    // the signin for this profile was cancelled.
     SignOut();
   }
 }
 #endif
 
 void SigninManager::CompleteSigninAfterPolicyLoad() {
   DCHECK(!possibly_invalid_username_.empty());
   SetAuthenticatedUsername(possibly_invalid_username_);
   possibly_invalid_username_.clear();
   profile_->GetPrefs()->SetString(prefs::kGoogleServicesUsername,
@@ skipping 136 matching lines @@
                     NotifySigninValueChanged(field, value));
 }
 
 void SigninManager::NotifyDiagnosticsObservers(
     const TimedSigninStatusField& field,
     const std::string& value) {
   FOR_EACH_OBSERVER(SigninDiagnosticsObserver,
                     signin_diagnostics_observers_,
                     NotifySigninValueChanged(field, value));
 }