Move post-signin confirmation bubble to OneClickSigninSyncStarter.

OneClickSigninSyncStarter now displays the post-signin confirmation bubble so
we can better integrate this with the rest of the signin UI (creating a new
profile, displaying the SAML dialog, etc).

BUG=174655,232339,236828,238173
R=rogerta@chromium.org

Review URL: https://codereview.chromium.org/14914003

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

chrome/browser/ui/sync/one_click_signin_helper.h

@@ -8,7 +8,7 @@
 #include <string>
 
 #include "base/gtest_prod_util.h"
-#include "chrome/browser/signin/signin_tracker.h"
+#include "chrome/browser/sync/profile_sync_service_observer.h"
 #include "chrome/browser/ui/webui/sync_promo/sync_promo_ui.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/web_contents_observer.h"
@@ -35,7 +35,6 @@ class URLRequest;
 class OneClickSigninHelper
     : public content::WebContentsObserver,
       public content::WebContentsUserData<OneClickSigninHelper>,
-      public SigninTracker::Observer,
       public ProfileSyncServiceObserver {
  public:
   // Represents user's decision about sign in process.
@@ -189,9 +188,9 @@ class OneClickSigninHelper
                                   int child_id,
                                   int route_id);
 
-  void RedirectToNtpOrAppsPage(bool show_bubble);
+  void RedirectToNtpOrAppsPage();
   void RedirectToSignin();
-  void ShowSyncConfirmationBubble(bool show_bubble);
+  void ShowSigninErrorBubble(const std::string& error);
 
   // Clear all data member of the helper, except for the error.
   void CleanTransientState();
@@ -207,11 +206,6 @@ class OneClickSigninHelper
   virtual void DidStopLoading(
       content::RenderViewHost* render_view_host) OVERRIDE;
 
-  // SigninTracker::Observer override.
-  virtual void GaiaCredentialsValid() OVERRIDE;
-  virtual void SigninFailed(const GoogleServiceAuthError& error) OVERRIDE;
-  virtual void SigninSuccess() OVERRIDE;
-
   // ProfileSyncServiceObserver.
   virtual void OnStateChanged() OVERRIDE;
 
@@ -234,7 +228,6 @@ class OneClickSigninHelper
   // Redirect URL after sync setup is complete.
   GURL redirect_url_;
   std::string error_message_;
-  scoped_ptr<SigninTracker> signin_tracker_;
 
   // Number of navigations since starting a sign in that is outside the
   // the set of trusted Gaia URLs.  Sign in attempts that include visits to
@@ -246,7 +239,7 @@ class OneClickSigninHelper
   // dedicated sign in process (e.g. SAML login, which redirects to a
   // non-google-controlled domain).
   // This is set to true if at least one such URL is detected.
-  bool confirmation_required_;
+  bool untrusted_confirmation_required_;
 
   DISALLOW_COPY_AND_ASSIGN(OneClickSigninHelper);
 };

chrome/browser/ui/sync/one_click_signin_sync_starter.cc

@@ -31,6 +31,10 @@
 #include "chrome/browser/ui/webui/signin/login_ui_service_factory.h"
 #include "chrome/browser/ui/webui/signin/profile_signin_confirmation_dialog.h"
 #include "chrome/common/url_constants.h"
+#include "grit/chromium_strings.h"
+#include "grit/generated_resources.h"
+#include "ui/base/l10n/l10n_util.h"
+#include "ui/base/resource/resource_bundle.h"
 
 OneClickSigninSyncStarter::OneClickSigninSyncStarter(
     Profile* profile,
@@ -40,7 +44,7 @@ OneClickSigninSyncStarter::OneClickSigninSyncStarter(
     const std::string& password,
     StartSyncMode start_mode,
     bool force_same_tab_navigation,
-    bool confirmation_required)
+    ConfirmationRequired confirmation_required)
     : start_mode_(start_mode),
       force_same_tab_navigation_(force_same_tab_navigation),
       confirmation_required_(confirmation_required),
@@ -107,7 +111,7 @@ void OneClickSigninSyncStarter::ConfirmSignin(const std::string& oauth_token) {
                    weak_pointer_factory_.GetWeakPtr()));
     return;
 #else
-    SigninAfterSAMLConfirmation();
+    ConfirmAndSignin();
 #endif
   } else {
     // The user is already signed in - just tell SigninManager to continue
@@ -124,7 +128,7 @@ void OneClickSigninSyncStarter::OnRegisteredForPolicy(
   // finish signing in.
   if (!client.get()) {
     DVLOG(1) << "Policy registration failed";
-    SigninAfterSAMLConfirmation();
+    ConfirmAndSignin();
     return;
   }
 
@@ -235,24 +239,24 @@ void OneClickSigninSyncStarter::CompleteSigninForNewProfile(
 }
 #endif
 
-void OneClickSigninSyncStarter::SigninAfterSAMLConfirmation() {
+void OneClickSigninSyncStarter::ConfirmAndSignin() {
   SigninManager* signin = SigninManagerFactory::GetForProfile(profile_);
   // browser_ can be null for unit tests.
-  if (!browser_ || !confirmation_required_) {
-    // No confirmation required - just sign in the user.
-    signin->CompletePendingSignin();
-  } else {
+  if (browser_ && confirmation_required_ == CONFIRM_UNTRUSTED_SIGNIN) {
     // Display a confirmation dialog to the user.
     browser_->window()->ShowOneClickSigninBubble(
         BrowserWindow::ONE_CLICK_SIGNIN_BUBBLE_TYPE_SAML_MODAL_DIALOG,
         UTF8ToUTF16(signin->GetUsernameForAuthInProgress()),
         string16(), // No error message to display.
-        base::Bind(&OneClickSigninSyncStarter::SigninConfirmationComplete,
+        base::Bind(&OneClickSigninSyncStarter::UntrustedSigninConfirmed,
                    weak_pointer_factory_.GetWeakPtr()));
+  } else {
+    // No confirmation required - just sign in the user.
+    signin->CompletePendingSignin();
   }
 }
 
-void OneClickSigninSyncStarter::SigninConfirmationComplete(
+void OneClickSigninSyncStarter::UntrustedSigninConfirmed(
     StartSyncMode response) {
   if (response == UNDO_SYNC) {
     CancelSigninAndDelete();
@@ -272,6 +276,21 @@ void OneClickSigninSyncStarter::SigninFailed(
   ProfileSyncService* profile_sync_service = GetProfileSyncService();
   if (profile_sync_service)
     profile_sync_service->SetSetupInProgress(false);
+  if (confirmation_required_ == CONFIRM_AFTER_SIGNIN) {
+    switch (error.state()) {
+      case GoogleServiceAuthError::SERVICE_UNAVAILABLE:
+        DisplayFinalConfirmationBubble(l10n_util::GetStringUTF16(
+            IDS_SYNC_UNRECOVERABLE_ERROR));
+        break;
+      case GoogleServiceAuthError::REQUEST_CANCELED:
+        // No error notification needed if the user manually cancelled signin.
+        break;
+      default:
+        DisplayFinalConfirmationBubble(l10n_util::GetStringUTF16(
+            IDS_SYNC_ERROR_SIGNING_IN));
+        break;
+    }
+  }
   delete this;
 }
 
@@ -286,6 +305,15 @@ void OneClickSigninSyncStarter::SigninSuccess() {
         profile_sync_service->SetSyncSetupCompleted();
         profile_sync_service->SetSetupInProgress(false);
       }
+      if (confirmation_required_ == CONFIRM_AFTER_SIGNIN) {
+        string16 message;
+        if (!profile_sync_service) {
+          // Sync is disabled by policy.
+          message = l10n_util::GetStringUTF16(
+              IDS_ONE_CLICK_SIGNIN_BUBBLE_SYNC_DISABLED_MESSAGE);
+        }
+        DisplayFinalConfirmationBubble(message);
+      }
       break;
     case CONFIGURE_SYNC_FIRST:
       ConfigureSync();
@@ -293,10 +321,34 @@ void OneClickSigninSyncStarter::SigninSuccess() {
     default:
       NOTREACHED() << "Invalid start_mode=" << start_mode_;
   }
-
   delete this;
 }
 
+void OneClickSigninSyncStarter::DisplayFinalConfirmationBubble(
+    const string16& custom_message) {
+  EnsureBrowser();
+  browser_->window()->ShowOneClickSigninBubble(
+      BrowserWindow::ONE_CLICK_SIGNIN_BUBBLE_TYPE_BUBBLE,
+      string16(),  // No email required - this is not a SAML confirmation.
+      custom_message,
+      // Callback is ignored.
+      BrowserWindow::StartSyncCallback());
+}
+
+void OneClickSigninSyncStarter::EnsureBrowser() {
+  if (!browser_) {
+    // The user just created a new profile so we need to figure out what
+    // browser to use to display settings. Grab the most recently active
+    // browser or else create a new one.
+    browser_ = chrome::FindLastActiveWithProfile(profile_, desktop_type_);
+    if (!browser_) {
+      browser_ = new Browser(Browser::CreateParams(profile_,
+                                                   desktop_type_));
+    }
+    browser_->window()->Show();
+  }
+}
+
 void OneClickSigninSyncStarter::ConfigureSync() {
   // Give the user a chance to configure things. We don't clear the
   // ProfileSyncService::setup_in_progress flag because we don't want sync
@@ -307,17 +359,7 @@ void OneClickSigninSyncStarter::ConfigureSync() {
   if (login_ui->current_login_ui()) {
     login_ui->current_login_ui()->FocusUI();
   } else {
-    if (!browser_) {
-      // The user just created a new profile so we need to figure out what
-      // browser to use to display settings. Grab the most recently active
-      // browser or else create a new one.
-      browser_ = chrome::FindLastActiveWithProfile(profile_, desktop_type_);
-      if (!browser_) {
-        browser_ = new Browser(Browser::CreateParams(profile_,
-                                                     desktop_type_));
-      }
-      browser_->window()->Show();
-    }
+    EnsureBrowser();
     if (profile_sync_service) {
       // Need to navigate to the settings page and display the sync UI.
       if (force_same_tab_navigation_) {

chrome/browser/ui/sync/one_click_signin_sync_starter.h

@@ -38,6 +38,19 @@ class OneClickSigninSyncStarter : public SigninTracker::Observer {
     UNDO_SYNC
   };
 
+  enum ConfirmationRequired {
+    // No need to display a "post-signin" confirmation bubble (for example, if
+    // the user was doing a re-auth flow).
+    NO_CONFIRMATION,
+
+    // Signin flow redirected outside of trusted domains, so ask the user to
+    // confirm before signing in.
+    CONFIRM_UNTRUSTED_SIGNIN,
+
+    // Display a confirmation after signing in.
+    CONFIRM_AFTER_SIGNIN
+  };
+
   // |profile| must not be NULL, however |browser| can be. When using the
   // OneClickSigninSyncStarter from a browser, provide both.
   // If |display_confirmation| is true, the user will be prompted to confirm the
@@ -49,7 +62,7 @@ class OneClickSigninSyncStarter : public SigninTracker::Observer {
                             const std::string& password,
                             StartSyncMode start_mode,
                             bool force_same_tab_navigation,
-                            bool display_confirmation);
+                            ConfirmationRequired display_confirmation);
 
  private:
   virtual ~OneClickSigninSyncStarter();
@@ -94,28 +107,37 @@ class OneClickSigninSyncStarter : public SigninTracker::Observer {
   // confirmation is required (in which case we have to prompt the user first).
   void ConfirmSignin(const std::string& oauth_token);
 
-  // Displays confirmation UI to the user if confirmation_required_ is true,
-  // otherwise completes the pending signin process.
-  void SigninAfterSAMLConfirmation();
+  // Displays confirmation UI to the user if confirmation_required_ ==
+  // CONFIRM_UNTRUSTED_SIGNIN, otherwise completes the pending signin process.
+  void ConfirmAndSignin();
 
   // Callback invoked once the user has responded to the signin confirmation UI.
   // If response == UNDO_SYNC, the signin is cancelled, otherwise the pending
   // signin is completed.
-  void SigninConfirmationComplete(StartSyncMode response);
+  void UntrustedSigninConfirmed(StartSyncMode response);
 
   ProfileSyncService* GetProfileSyncService();
 
-  // Displays the sync configuration UI, then frees this object.
+  // Displays the sync configuration UI.
   void ConfigureSync();
   void ShowSyncSettingsPageOnSameTab();
 
+  // Shows the post-signin confirmation bubble. If |custom_message| is empty,
+  // the default "You are signed in" message is displayed.
+  void DisplayFinalConfirmationBubble(const string16& custom_message);
+
+  // Makes sure browser_ points to a valid browser (opens a new browser if
+  // necessary). Useful in the case where the user has created a new Profile as
+  // part of the signin process.
+  void EnsureBrowser();
+
   Profile* profile_;
   Browser* browser_;
   scoped_ptr<SigninTracker> signin_tracker_;
   StartSyncMode start_mode_;
   chrome::HostDesktopType desktop_type_;
   bool force_same_tab_navigation_;
-  bool confirmation_required_;
+  ConfirmationRequired confirmation_required_;
   base::WeakPtrFactory<OneClickSigninSyncStarter> weak_pointer_factory_;
 
 #if defined(ENABLE_CONFIGURATION_POLICY)