Make sure the new confirmation bubble is always shown upon signin

chrome/browser/ui/sync/one_click_signin_sync_starter.cc

Index: chrome/browser/ui/sync/one_click_signin_sync_starter.cc
diff --git a/chrome/browser/ui/sync/one_click_signin_sync_starter.cc b/chrome/browser/ui/sync/one_click_signin_sync_starter.cc
index 157d23b4dcf3204d6457285646d3bedeba4fd7f9..9ddb2fc1afa0289ef7392a06c58c108ba9712c0b 100644
--- a/chrome/browser/ui/sync/one_click_signin_sync_starter.cc
+++ b/chrome/browser/ui/sync/one_click_signin_sync_starter.cc
@@ -187,15 +187,10 @@ void OneClickSigninSyncStarter::SigninDialogDelegate::OnContinueSignin() {
 
   if (sync_starter_ != NULL) {
     // If the user signs in from the new avatar bubble, the enterprise
-    // confirmation dialog would dismiss the avatar bubble, thus it won't show
-    // any confirmation upon sign in completes. This cofirmation dialog already
-    // mentions that user data would be synced, thus we just start sync
-    // immediately.
-
-    // TODO(guohui): add a sync settings link to allow user to configure sync
-    // settings before sync starts.
+    // confirmation dialog would dismiss the avatar bubble, thus we need to
+    // manually re-show the avatar bubble to confirm sync settings.
     if (sync_starter_->GetStartSyncMode() == CONFIRM_SYNC_SETTINGS_FIRST)
-      sync_starter_->SetStartSyncMode(SYNC_WITH_DEFAULT_SETTINGS);
+      sync_starter_->SetConfirmationRequired(CONFIRM_AFTER_SIGNIN);
     sync_starter_->LoadPolicyWithCachedCredentials();
   }
 }
@@ -204,16 +199,15 @@ void OneClickSigninSyncStarter::SigninDialogDelegate::OnSigninWithNewProfile() {
   SetUserChoiceHistogram(SIGNIN_CHOICE_NEW_PROFILE);
 
   if (sync_starter_ != NULL) {
-    // TODO(guohui): add a sync settings link to allow user to configure sync
-    // settings before sync starts.
     if (sync_starter_->GetStartSyncMode() == CONFIRM_SYNC_SETTINGS_FIRST)
-      sync_starter_->SetStartSyncMode(SYNC_WITH_DEFAULT_SETTINGS);
+      sync_starter_->SetConfirmationRequired(CONFIRM_AFTER_SIGNIN);
     sync_starter_->CreateNewSignedInProfile();
   }
 }
 
-void OneClickSigninSyncStarter::SetStartSyncMode(StartSyncMode start_mode) {
-  start_mode_ = start_mode;
+void OneClickSigninSyncStarter::SetConfirmationRequired(
+    ConfirmationRequired confirmation_required) {
+  confirmation_required_ = confirmation_required;
 }
 
 OneClickSigninSyncStarter::StartSyncMode
@@ -482,6 +476,8 @@ void OneClickSigninSyncStarter::MergeSessionComplete(
     }
     case CONFIRM_SYNC_SETTINGS_FIRST:
       // Blocks sync until the sync settings confirmation UI is closed.
+      if (confirmation_required_ == CONFIRM_AFTER_SIGNIN)
+        DisplayFinalConfirmationBubble(base::string16());

[Roger Tawa OOO till Jul 10th, 2014/08/15 15:49:13]

What if you always call DisplayFinalConfirmationBubble() here, and remove the
code in ProfileChooserView::OnRefreshTokenAvailable() that switches to
TUTORIAL_MODE_CONFIRM_SIGNIN?

Maybe ProfileChooserView::OnRefreshTokenAvailable() should completely ignore
profiles::BUBBLE_VIEW_MODE_GAIA_SIGNIN?

[guohui, 2014/08/15 15:57:52]

as discussed yesterday, that would be a cleaner solution, however it requires
non-trivial changes. I am working on three other CLs for M38 blockers, thus I am
de-prioritizing any refactoring work for now. If time allows i ll upload another
patch with the refactoring, otherwise i will just land this CL as it is and
refactor in the next CL.

I am more concerned about the no-confirmation bug you mentioned in the email, do
you still see it?

       return;
     case CONFIGURE_SYNC_FIRST:
       ShowSettingsPage(true);  // Show sync config UI.
@@ -506,6 +502,16 @@ void OneClickSigninSyncStarter::MergeSessionComplete(
 void OneClickSigninSyncStarter::DisplayFinalConfirmationBubble(
     const base::string16& custom_message) {
   browser_ = EnsureBrowser(browser_, profile_, desktop_type_);
+  // Show the success confirmation message in the new avatar menu if it is
+  // enabled.
+  // TODO(guohui): needs to handle custom messages.
+  if (custom_message.empty() && switches::IsNewAvatarMenu()) {
+    browser_->window()->ShowAvatarBubbleFromAvatarButton(
+        BrowserWindow::AVATAR_BUBBLE_MODE_CONFIRM_SIGNIN,
+        signin::ManageAccountsParams());
+    return;
+  }
+
   browser_->window()->ShowOneClickSigninBubble(
       BrowserWindow::ONE_CLICK_SIGNIN_BUBBLE_TYPE_BUBBLE,
       base::string16(),  // No email required - this is not a SAML confirmation.