Start moving signin code out of browser/sync.

chrome/browser/ui/webui/sync_setup_handler.cc

Index: chrome/browser/ui/webui/sync_setup_handler.cc
diff --git a/chrome/browser/ui/webui/sync_setup_handler.cc b/chrome/browser/ui/webui/sync_setup_handler.cc
index 1dab8c848c537d8cc6908eb624590fb4c5473abb..7a8e7d10e6604f3aebe6b6af0127ad91a2cb3728 100644
--- a/chrome/browser/ui/webui/sync_setup_handler.cc
+++ b/chrome/browser/ui/webui/sync_setup_handler.cc
@@ -16,12 +16,15 @@
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/profiles/profile_metrics.h"
 #include "chrome/browser/signin/signin_manager.h"
+#include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/sync/protocol/service_constants.h"
 #include "chrome/browser/sync/sync_setup_flow.h"
 #include "chrome/browser/sync/util/oauth.h"
 #include "chrome/browser/ui/browser_list.h"
+#include "chrome/browser/ui/webui/signin/login_ui_service.h"
+#include "chrome/browser/ui/webui/signin/login_ui_service_factory.h"
 #include "chrome/browser/ui/webui/sync_promo/sync_promo_ui.h"
 #include "chrome/common/net/gaia/gaia_constants.h"
 #include "chrome/common/url_constants.h"
@@ -182,9 +185,12 @@ SyncSetupHandler::SyncSetupHandler(ProfileManager* profile_manager)
 }
 
 SyncSetupHandler::~SyncSetupHandler() {
+  // Just exit if running unit tests (no actual WebUI is attached).
+  if (!web_ui())
+    return;
+
   // This case is hit when the user performs a back navigation.
-  if (flow_)
-    flow_->OnDialogClosed("");
+  CloseSyncSetup();
 }
 
 void SyncSetupHandler::GetLocalizedValues(DictionaryValue* localized_strings) {
@@ -349,16 +355,41 @@ void SyncSetupHandler::GetStaticLocalizedValues(
   RegisterTitle(localized_strings, "syncSetupOverlay", IDS_SYNC_SETUP_TITLE);
 }
 
-void SyncSetupHandler::Initialize() {
+void SyncSetupHandler::StartConfigureSync() {
+  DCHECK(!flow_);
+  // We only get here if we're signed in, so no longer need our SigninTracker.
+  signin_tracker_.reset();
+  ProfileSyncService* service = GetSyncService();
+  service->get_wizard().Step(
+      service->HasSyncSetupCompleted() ?
+      SyncSetupWizard::CONFIGURE : SyncSetupWizard::SYNC_EVERYTHING);
+
+  // Attach this as the sync setup handler.
+  if (!service->get_wizard().AttachSyncSetupHandler(this)) {
+    LOG(ERROR) << "SyncSetupHandler attach failed!";
+    CloseOverlay();
+  }
+}
+
+bool SyncSetupHandler::IsActiveLogin() const {
+  LoginUIService* service = GetLoginUIService();
+  return service->current_login_ui() == web_ui();
 }
 
 void SyncSetupHandler::OnGetOAuthTokenSuccess(const std::string& oauth_token) {
-  flow_->OnUserSubmittedOAuth(oauth_token);
+  Profile* profile = Profile::FromWebUI(web_ui());
+  SigninManager* signin = GetSignin();
+  GaiaOAuthFetcher* fetcher = new GaiaOAuthFetcher(
+      signin,
+      profile->GetRequestContext(),
+      profile,
+      GaiaConstants::kSyncServiceOAuth);
+  signin->StartOAuthSignIn(oauth_token, fetcher);
 }
 
 void SyncSetupHandler::OnGetOAuthTokenFailure(
     const GoogleServiceAuthError& error) {
-  CloseSyncSetup();
+  CloseOverlay();
 }
 
 void SyncSetupHandler::RegisterMessages() {
@@ -388,37 +419,80 @@ void SyncSetupHandler::RegisterMessages() {
                  base::Unretained(this)));
 }
 
-// Ideal(?) solution here would be to mimic the ClientLogin overlay.  Since
-// this UI must render an external URL, that overlay cannot be used directly.
-// The current implementation is functional, but fails asthetically.
-// TODO(rickcam): Bug 90711: Update UI for OAuth sign-in flow
-void SyncSetupHandler::ShowOAuthLogin() {
-  DCHECK(browser_sync::IsUsingOAuth());
-
+SigninManager* SyncSetupHandler::GetSignin() const {
   Profile* profile = Profile::FromWebUI(web_ui());
-  oauth_login_.reset(new GaiaOAuthFetcher(this,
-                                          profile->GetRequestContext(),
-                                          profile,
-                                          GaiaConstants::kSyncServiceOAuth));
-  oauth_login_->SetAutoFetchLimit(GaiaOAuthFetcher::OAUTH1_REQUEST_TOKEN);
-  oauth_login_->StartGetOAuthToken();
+  return SigninManagerFactory::GetForProfile(profile);
+}
+
+void SyncSetupHandler::DisplayGaiaLogin(bool fatal_error) {
+  DisplayGaiaLoginWithErrorMessage(string16(), fatal_error);
 }
 
-void SyncSetupHandler::ShowGaiaLogin(const DictionaryValue& args) {
+void SyncSetupHandler::DisplayGaiaLoginWithErrorMessage(
+    const string16& error_message, bool fatal_error) {
   DCHECK(!browser_sync::IsUsingOAuth());
+  // If we're exiting from sync config (due to some kind of error), notify
+  // SyncSetupFlow.
+  if (flow_) {
+    flow_->OnDialogClosed(std::string());
+    flow_ = NULL;
+  }
+
+  // Setup args for the GAIA login screen:
+  // error_message: custom error message to display
+  // error: GoogleServiceAuthError from previous login attempt (0 if none)
+  // user: The email the user most recently entered.
+  // editable_user: Whether the username field should be editable.
+  SigninManager* signin = GetSignin();
+  std::string user;
+  int error;
+  bool editable_user;
+  if (!last_attempted_user_email_.empty()) {
+    // This is a repeat of a login attempt.
+    user = last_attempted_user_email_;
+    error = signin->GetLoginAuthError().state();
+    editable_user = true;
+  } else {
+    // Fresh login attempt - lock in the authenticated username if there is
+    // one (don't let the user change it).
+    user = signin->GetAuthenticatedUsername();
+    error = 0;
+    editable_user = user.empty();
+  }
+  DictionaryValue args;
+  args.SetString("user", user);
+  args.SetInteger("error", error);
+  args.SetBoolean("editable_user", editable_user);
+  if (!error_message.empty())
+    args.SetString("error_message", error_message);
+  if (fatal_error)
+    args.SetBoolean("fatalError", true);
   StringValue page("login");
   web_ui()->CallJavascriptFunction(
       "SyncSetupOverlay.showSyncSetupPage", page, args);
 }
 
-void SyncSetupHandler::ShowGaiaSuccessAndClose() {
+void SyncSetupHandler::RecordSignin() {
+  // By default, do nothing - subclasses can override.

[tim (not reviewing), 2012/02/14 17:22:29]

Wait, what is the subclass again? I'm tempted to ask for a paragraph-or-two
comment / ascii art at the top of one of the new files, perhaps LoginUIService,
explaining how the Signin Process works at a high level and the relationships
between the different classes.  It might rot, but I think it'll be useful long
enough to make it worthwhile.

[Andrew T Wilson (Slow), 2012/02/14 20:52:36]

The subclasses are SyncPromoHandler and OptionsSyncSetupHandler. The promo
handler wants to do special logging of signins which is why this API is here.

I've added documentation to SigninTracker as that's more of a core class than
LoginUIService, I think. Let me know if you disagree.

+}
+
+void SyncSetupHandler::DisplayGaiaSuccessAndClose() {
+  // TODO(atwilson): Can we remove this now that we've changed the signin flow?
+  RecordSignin();
   web_ui()->CallJavascriptFunction("SyncSetupOverlay.showSuccessAndClose");
 }
 
-void SyncSetupHandler::ShowGaiaSuccessAndSettingUp() {
+void SyncSetupHandler::DisplayGaiaSuccessAndSettingUp() {
+  RecordSignin();
   web_ui()->CallJavascriptFunction("SyncSetupOverlay.showSuccessAndSettingUp");
 }
 
+void SyncSetupHandler::ShowFatalError() {
+  // For now, just send the user back to the login page. Ultimately we may want
+  // to give different feedback (especially for chromeos).
+  DisplayGaiaLogin(true);
+}
+
 void SyncSetupHandler::ShowConfigure(const DictionaryValue& args) {
   StringValue page("configure");
   web_ui()->CallJavascriptFunction(
@@ -447,9 +521,7 @@ void SyncSetupHandler::ShowSetupDone(const string16& user) {
   SyncPromoUI::SetUserSkippedSyncPromo(Profile::FromWebUI(web_ui()));
 
   Profile* profile = Profile::FromWebUI(web_ui());
-  ProfileSyncService* service =
-      ProfileSyncServiceFactory::GetInstance()->GetForProfile(
-          profile);
+  ProfileSyncService* service = GetSyncService();
   if (!service->HasSyncSetupCompleted()) {
     FilePath profile_file_path = profile->GetPath();
     ProfileMetrics::LogProfileSyncSignIn(profile_file_path);
@@ -488,12 +560,58 @@ void SyncSetupHandler::HandleSubmitAuth(const ListValue* args) {
 
   string16 error_message;
   if (!IsLoginAuthDataValid(username, &error_message)) {
-    ShowLoginErrorMessage(error_message);
+    DisplayGaiaLoginWithErrorMessage(error_message, false);
+    return;
+  }
+
+  TryLogin(username, password, captcha, access_code);
+}
+
+void SyncSetupHandler::TryLogin(const std::string& username,
+                                const std::string& password,
+                                const std::string& captcha,
+                                const std::string& access_code) {
+  DCHECK(IsActiveLogin());
+  // Make sure we are listening for signin traffic.
+  if (!signin_tracker_.get())
+    signin_tracker_.reset(new SigninTracker(Profile::FromWebUI(web_ui()),
+                                            this));
+
+  last_attempted_user_email_ = username;
+  // If we're just being called to provide an ASP, then pass it to the
+  // SigninManager and wait for the next step.
+  SigninManager* signin = GetSignin();
+  if (!access_code.empty()) {
+    signin->ProvideSecondFactorAccessCode(access_code);
     return;
   }
 
-  if (flow_)
-    flow_->OnUserSubmittedAuth(username, password, captcha, access_code);
+  // Kick off a sign-in through the signin manager.
+  signin->StartSignIn(username,
+                      password,
+                      signin->GetLoginAuthError().captcha().token,
+                      captcha);
+}
+
+void SyncSetupHandler::GaiaCredentialsValid() {
+  DCHECK(IsActiveLogin());
+  // Gaia credentials are valid - update the UI.
+  DisplayGaiaSuccessAndSettingUp();
+}
+
+void SyncSetupHandler::SigninFailed() {
+  // Got a failed signin - this is either just a typical auth error, or a
+  // sync error (treat sync errors as "fatal errors" - i.e. non-auth errors).
+  DisplayGaiaLogin(GetSyncService()->unrecoverable_error_detected());
+}
+
+ProfileSyncService* SyncSetupHandler::GetSyncService() const {
+  return ProfileSyncServiceFactory::GetForProfile(Profile::FromWebUI(web_ui()));
+}
+
+void SyncSetupHandler::SigninSuccess() {
+  DCHECK(GetSyncService()->sync_initialized());
+  StartConfigureSync();
 }
 
 void SyncSetupHandler::HandleConfigure(const ListValue* args) {
@@ -563,9 +681,7 @@ void SyncSetupHandler::HandleAttachHandler(const ListValue* args) {
 void SyncSetupHandler::HandleShowErrorUI(const ListValue* args) {
   DCHECK(!flow_);
 
-  Profile* profile = Profile::FromWebUI(web_ui());
-  ProfileSyncService* service = ProfileSyncServiceFactory::GetInstance()->
-      GetForProfile(profile);
+  ProfileSyncService* service = GetSyncService();
   DCHECK(service);
 
   service->ShowErrorUI();
@@ -573,52 +689,54 @@ void SyncSetupHandler::HandleShowErrorUI(const ListValue* args) {
 
 void SyncSetupHandler::HandleShowSetupUI(const ListValue* args) {
   DCHECK(!flow_);
-  if (FocusExistingWizard()) {
-    CloseOverlay();
-    return;
-  }
-
-  StepWizardForShowSetupUI();
-  ShowSetupUI();
+  OpenSyncSetup();
 }
 
 void SyncSetupHandler::CloseSyncSetup() {
+  // TODO(atwilson): Move UMA tracking of signin events out of sync module.
+  if (IsActiveLogin()) {
+    if (signin_tracker_.get()) {
+      ProfileSyncService::SyncEvent(
+          ProfileSyncService::CANCEL_DURING_SIGNON);
+    } else if (!flow_) {
+      ProfileSyncService::SyncEvent(
+          ProfileSyncService::CANCEL_FROM_SIGNON_WITHOUT_AUTH);
+    }
+  }
+
   if (flow_) {
     flow_->OnDialogClosed(std::string());
     flow_ = NULL;
   }
+  signin_tracker_.reset();
+  GetLoginUIService()->ResetLoginUI(web_ui());
 }
 
 void SyncSetupHandler::OpenSyncSetup() {
-  DCHECK(!flow_);
-
-  Profile* profile = Profile::FromWebUI(web_ui());
-  ProfileSyncService* service = ProfileSyncServiceFactory::GetInstance()->
-      GetForProfile(profile);
+  ProfileSyncService* service = GetSyncService();
   if (!service) {
     // If there's no sync service, the user tried to manually invoke a syncSetup
     // URL, but sync features are disabled.  We need to close the overlay for
     // this (rare) case.
+    DLOG(WARNING) << "Closing sync UI because sync is disabled";
     CloseOverlay();
     return;
   }
 
-  // If the wizard is already visible, it must be attached to another flow
-  // handler.
+  // If the wizard is already visible, just focus that one.
   if (FocusExistingWizard()) {
-    CloseOverlay();
+    if (!IsActiveLogin())
+        CloseOverlay();
     return;
   }
 
-  // The wizard must be stepped before attaching. Allow subclasses to step the
-  // wizard to appropriate state.
-  StepWizardForShowSetupUI();
+  GetLoginUIService()->SetLoginUI(web_ui());
 
-  // Attach this as the sync setup handler, before calling ShowSetupUI().
-  if (!service->get_wizard().AttachSyncSetupHandler(this)) {
-    LOG(ERROR) << "SyncSetupHandler attach failed!";
-    CloseOverlay();
-    return;
+  if (!SigninTracker::AreServicesSignedIn(Profile::FromWebUI(web_ui()))) {
+    // User is not logged in - need to display login UI.

[tim (not reviewing), 2012/02/14 17:22:29]

My initial reaction to the "User is not logged in" comment made me wonder why
you're not calling IsUserLoggedIn instead.  What's the difference? Where is that
explained?

I think here you mean User is not logged in _to at least one service_, and
presumably we can't mint tokens for that service from whatever we have in the
TokenService, even though the user may have logged in in the past.  Or something
like that?

[Andrew T Wilson (Slow), 2012/02/14 20:52:36]

IsUserLoggedIn() was obsolete/never actually called. I decided to move that
logic into SigninTracker, but forgot to remove the obsolete routine. Thanks for
catching that!

+    DisplayGaiaLogin(false);
+  } else {
+    StartConfigureSync();

[tim (not reviewing), 2012/02/14 17:22:29]

So this case is reached if the user clicks 'Advanced' for example, on the
settings page? (May be nice to have a comment).

[Andrew T Wilson (Slow), 2012/02/14 20:52:36]

Done. BTW, there are possibly lots of ways for this to get called (through
wrench menus, etc) and I'm trying not to embed comments here that will easily
get obsolete as the specifics of the UI change. In theory this class shouldn't
care about exactly what UI elements lead to OpenSyncSetup() being called.

   }
 
   ShowSetupUI();
@@ -626,22 +744,40 @@ void SyncSetupHandler::OpenSyncSetup() {
 
 // Private member functions.
 
-bool SyncSetupHandler::FocusExistingWizard() {
+bool SyncSetupHandler::IsUserLoggedIn() const {
   Profile* profile = Profile::FromWebUI(web_ui());
-  ProfileSyncService* service = ProfileSyncServiceFactory::GetInstance()->
-      GetForProfile(profile);
-  if (!service)
+  std::string user = GetSignin()->GetAuthenticatedUsername();
+  if (user.empty())
     return false;
 
-  // If the wizard is already visible, focus it.
-  if (service->get_wizard().IsVisible()) {
-    service->get_wizard().Focus();
-    return true;
-  }
-  return false;
+  // Now, make sure the services all have proper auth tokens.
+  // TODO(atwilson): Remove this dependency on the ProfileSyncService here.
+  // Ultimately, we'd like individual services like sync to report their auth
+  // errors via the TokenService, so we can query the TokenService to check for
+  // invalid credentials instead of asking the services themselves.
+  ProfileSyncService* service =
+      ProfileSyncServiceFactory::GetForProfile(profile);
+  // We're signed in if we have a token and our last attempt to access the
+  // server succeeded.
+  return !service->waiting_for_auth() &&
+      service->AreCredentialsAvailable() &&
+      service->GetAuthError().state() == GoogleServiceAuthError::NONE;
+}
+
+bool SyncSetupHandler::FocusExistingWizard() {

[tim (not reviewing), 2012/02/14 17:22:29]

Maybe 'FocusExistingWizardIfPresent'? Something to convey the fact that this
function doesn't require an existing wizard as a precondition.

[Andrew T Wilson (Slow), 2012/02/14 20:52:36]

Sure. It's a pre-existing function (I just updated the implementation to work in
the new refactored world), but I'll rename it.

+  LoginUIService* service = GetLoginUIService();
+  if (!service->current_login_ui())
+    return false;
+  service->FocusLoginUI();
+  return true;
+}
+
+LoginUIService* SyncSetupHandler::GetLoginUIService() const {
+  return LoginUIServiceFactory::GetForProfile(Profile::FromWebUI(web_ui()));
 }
 
 void SyncSetupHandler::CloseOverlay() {
+  CloseSyncSetup();
   web_ui()->CallJavascriptFunction("OptionsPage.closeOverlay");
 }
 
@@ -671,11 +807,3 @@ bool SyncSetupHandler::IsLoginAuthDataValid(const std::string& username,
 
   return true;
 }
-
-void SyncSetupHandler::ShowLoginErrorMessage(const string16& error_message) {
-  DCHECK(flow_);
-  DictionaryValue args;
-  flow_->GetArgsForGaiaLogin(&args);
-  args.SetString("error_message", error_message);
-  ShowGaiaLogin(args);
-}