Make sure sync setup completes when signing in from the avatar bubble with modal warnings

Make sure sync setup completes when signing in from the avatar bubble with modal warnings

When signing in from the new avatar bubble, since the inline template has no "choose what to sync" link, thus Chrome shows an inline confirmation in the avatar bubble upon signin completes and blocks sync until the bubble is closed.

However, when some modal warnings show up before signin completes, they steal the focus thus the avatar bubble wont be able to show confirmation for sync settings and sync will be blocked infinitely.

A temporary workaround is to just start sync immediately if a modal warning is shown, since in all cases the dialog mentions that user data will be synced.

I will add a sync settings link to all modal dialogs later so that user could configure sync settings before sync starts.

BUG=435423005
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=288122

[guohui, 2014-08-06 20:32:16]

Hey,

could you please review the CL?

Thanks,

Hui

[Roger Tawa OOO till Jul 10th, 2014-08-06 21:04:42]

lgtm

Maybe add more comments in all the places where the code will still need to be
modified once all dialogs have the settings link, so people reading the code
know that the change to default is a temporary hack.

https://codereview.chromium.org/447883002/diff/1/chrome/browser/ui/sync/one_c...
File chrome/browser/ui/sync/one_click_signin_helper.cc (right):

https://codereview.chromium.org/447883002/diff/1/chrome/browser/ui/sync/one_c...
chrome/browser/ui/sync/one_click_signin_helper.cc:1190: // sign in completes.
This cofirmation dialog already mentions that user
cofirmation --> confirmation

[guohui, 2014-08-06 21:19:40]

The CQ bit was checked by guohui@chromium.org

[guohui, 2014-08-06 21:19:45]

https://codereview.chromium.org/447883002/diff/1/chrome/browser/ui/sync/one_c...
File chrome/browser/ui/sync/one_click_signin_helper.cc (right):

https://codereview.chromium.org/447883002/diff/1/chrome/browser/ui/sync/one_c...
chrome/browser/ui/sync/one_click_signin_helper.cc:1190: // sign in completes.
This cofirmation dialog already mentions that user
On 2014/08/06 21:04:42, Roger Tawa (build sheriff) wrote:
> cofirmation --> confirmation

Done.

[commit-bot: I haz the power, 2014-08-06 21:21:52]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/guohui@chromium.org/447883002/20001

[commit-bot: I haz the power, 2014-08-06 23:21:47]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/38205)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/43413)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel/...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...)
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-06 23:32:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-06 23:32:18]

Try jobs failed on following builders:
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[guohui, 2014-08-07 00:15:34]

The CQ bit was checked by guohui@chromium.org

[commit-bot: I haz the power, 2014-08-07 00:18:51]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/guohui@chromium.org/447883002/40001

[commit-bot: I haz the power, 2014-08-07 05:21:24]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)

[commit-bot: I haz the power, 2014-08-07 07:16:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-07 07:16:33]

Failed to apply patch for chrome/browser/ui/sync/one_click_signin_helper.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/ui/sync/one_click_signin_helper.cc
  Hunk #1 FAILED at 1184.
  1 out of 1 hunk FAILED -- saving rejects to file
chrome/browser/ui/sync/one_click_signin_helper.cc.rej

Patch:       chrome/browser/ui/sync/one_click_signin_helper.cc
Index: chrome/browser/ui/sync/one_click_signin_helper.cc
diff --git a/chrome/browser/ui/sync/one_click_signin_helper.cc
b/chrome/browser/ui/sync/one_click_signin_helper.cc
index
c7662588446dc679b019007d0207a18279b608e3..e15ca3d5cb7b68752215c4261b9c032b3871b021
100644
--- a/chrome/browser/ui/sync/one_click_signin_helper.cc
+++ b/chrome/browser/ui/sync/one_click_signin_helper.cc
@@ -1184,6 +1184,16 @@ bool OneClickSigninHelper::HandleCrossAccountError(
     // TODO(atwilson): Move this into OneClickSigninSyncStarter.
     // The tab modal dialog always executes its callback before |contents|
     // is deleted.
+
+    // If the user signs in from the new avatar bubble, the confirm email
dialog
+    // would dismiss the avatar bubble, thus it won't show any confirmation
upon
+    // sign in completes. This confirmation 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.
+    if (start_mode == OneClickSigninSyncStarter::CONFIRM_SYNC_SETTINGS_FIRST)
+      start_mode = OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS;
+
     Browser* browser = chrome::FindBrowserWithWebContents(contents);
     ConfirmEmailDialogDelegate::AskForConfirmation(
         contents,

[guohui, 2014-08-07 13:58:17]

The CQ bit was checked by guohui@chromium.org

[commit-bot: I haz the power, 2014-08-07 13:59:34]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/guohui@chromium.org/447883002/60001

[commit-bot: I haz the power, 2014-08-07 19:52:35]

Change committed as 288122