New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 694476 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression : 'Sync confirmation' overlay does not dismiss after clicking on 'OK, GOT IT' or 'UNDO' button.

Reported by yfulgaon...@etouch.net, Feb 21 2017

Issue description

Chrome Version : 58.0.3018.2 (Official Build) 9684a56837d6ddb675c417f4adf046583c8db38a-refs/branch-heads/3018@{#2} 32/64 bit
OS : Windows (7,8,10)

What steps will reproduce the problem?
1. Launch chrome, press 'Ctrl' + 'N' (New Window appears), click on Avatar icon and click on 'Sign in to chrome' button.
2. Enter valid email id & password and proceed until 'Sync confirmation' overlay is seen.
3. When 'Sync confirmation' overlay appears, press 'Alt' + 'Tab' keys (user switch to the first window)
4. Now again press 'Alt' + 'Tab' (user switch back to the second window) and click on 'OK, GOT IT' button.
5. Observe.

Actual : 'Sync confirmation' overlay does not dismiss after clicking on 'OK, GOT IT' or 'UNDO' button.
Expected : 'Sync confirmation' overlay should dismiss after clicking on 'OK, GOT IT' or 'UNDO' button.

This is a regression issue broken in ‘M-57’, below is the Manual Regression range and will soon update other info.
Good build : 57.0.2959.0
Bad build : 57.0.2960.0

Note : 
1. This is Windows OS specific issue and the same is working fine on Mac(10.11.6, 10.12.1, 10.12) & Linux(14.04 LTS) OS.
2. Issue is also reproducible on latest windows canary build #58.0.3019.0
 
Actual_Result.mp4
1.3 MB View Download
Expected_Result.mp4
1.4 MB View Download
Cc: kkaluri@chromium.org
Labels: hasbisect-per-revision
Owner: msarda@chromium.org
Status: Assigned (was: Unconfirmed)
Bisect Info:
===========
Good build : 57.0.2959.0,   Revision Range - 440307
Bad build  : 57.0.2960.0,   Revision Range - 440547

After executing the per-revision bisect script , i got the following CL's between good and bad build versions
===========================================
https://chromium.googlesource.com/chromium/src/+log/b60675f1b7962b43597a6426e17db031f3d025d8..4d89528f55ebdfe107f61592694272af8b362a55


The suspecting Change Log is :
-----------
https://chromium.googlesource.com/chromium/src/+/4d89528f55ebdfe107f61592694272af8b362a55

Review-Url: https://codereview.chromium.org/2594353002

msarda@- Could you please look into this issue, if it's related to your change?  if not could you please help us to reassign this issue to the right owner.

Comment 2 by msarda@chromium.org, Mar 14 2017

Status: Started (was: Assigned)
I'll take a look next week once I have again access to my windows machine.

Comment 3 by msarda@chromium.org, Mar 24 2017

Labels: OS-Mac
I found the root cause and in some cases it may affect macOS users as well - if the user opens 2 browser windows, leaves the sync confirmation dialog on-screen on the first one, switches to the second one and then closes the first sync confirmation dialog without focusing the first browser window (using Command + mouse click).

The issue is the fact that when closing the dialog, we use the active browser and that may be a different one that the one showing the dialog.
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 29 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fc05776e6cc541dc9f4b6652876be72549f41682

commit fc05776e6cc541dc9f4b6652876be72549f41682
Author: msarda <msarda@chromium.org>
Date: Wed Mar 29 10:49:23 2017

Use the same browser instance in the sync confirmation dialog.

Before this CL, the sync confirmation dialog used the active browser
when dismissing the dialog. In some cases, the active browser is different
than the browser that actually presented the sync confirmation dialog
and this leads to unexpected behavior.

This CL fixes the browser instance used by the sync confirmation dialog
and ensures that the same instace is used throughout the lifetime of the
sync confirmation dialog.

TEST=See bug
BUG= 694476 

Review-Url: https://codereview.chromium.org/2771113003
Cr-Commit-Position: refs/heads/master@{#460345}

[modify] https://crrev.com/fc05776e6cc541dc9f4b6652876be72549f41682/chrome/browser/ui/cocoa/profiles/signin_view_controller_delegate_mac.h
[modify] https://crrev.com/fc05776e6cc541dc9f4b6652876be72549f41682/chrome/browser/ui/cocoa/profiles/signin_view_controller_delegate_mac.mm
[modify] https://crrev.com/fc05776e6cc541dc9f4b6652876be72549f41682/chrome/browser/ui/views/profiles/signin_view_controller_delegate_views.cc
[modify] https://crrev.com/fc05776e6cc541dc9f4b6652876be72549f41682/chrome/browser/ui/webui/signin/signin_error_handler.cc
[modify] https://crrev.com/fc05776e6cc541dc9f4b6652876be72549f41682/chrome/browser/ui/webui/signin/signin_utils.cc
[modify] https://crrev.com/fc05776e6cc541dc9f4b6652876be72549f41682/chrome/browser/ui/webui/signin/signin_utils.h
[modify] https://crrev.com/fc05776e6cc541dc9f4b6652876be72549f41682/chrome/browser/ui/webui/signin/sync_confirmation_handler.cc
[modify] https://crrev.com/fc05776e6cc541dc9f4b6652876be72549f41682/chrome/browser/ui/webui/signin/sync_confirmation_handler.h
[modify] https://crrev.com/fc05776e6cc541dc9f4b6652876be72549f41682/chrome/browser/ui/webui/signin/sync_confirmation_handler_unittest.cc
[modify] https://crrev.com/fc05776e6cc541dc9f4b6652876be72549f41682/chrome/browser/ui/webui/signin/sync_confirmation_ui.cc
[modify] https://crrev.com/fc05776e6cc541dc9f4b6652876be72549f41682/chrome/browser/ui/webui/signin/sync_confirmation_ui.h

Project Member

Comment 5 by bugdroid1@chromium.org, Mar 29 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/87301efc7eee4e37e3af750bc43f2977b386652b

commit 87301efc7eee4e37e3af750bc43f2977b386652b
Author: finnur <finnur@chromium.org>
Date: Wed Mar 29 11:32:34 2017

Revert of Use the same browser instance in the sync confirmation dialog. (patchset #4 id:120001 of https://codereview.chromium.org/2771113003/ )

Reason for revert:
Reverting due to linker error on Linux ChromiumOS Builder (dbg):

FAILED: chrome
../../chrome/browser/ui/views/profiles/signin_view_controller_delegate_views.cc:190: error: undefined reference to 'SyncConfirmationUI::InitializeMessageHandlerWithBrowser(Browser*)'
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Original issue's description:
> Use the same browser instance in the sync confirmation dialog.
>
> Before this CL, the sync confirmation dialog used the active browser
> when dismissing the dialog. In some cases, the active browser is different
> than the browser that actually presented the sync confirmation dialog
> and this leads to unexpected behavior.
>
> This CL fixes the browser instance used by the sync confirmation dialog
> and ensures that the same instace is used throughout the lifetime of the
> sync confirmation dialog.
>
> TEST=See bug
> BUG= 694476 
>
> Review-Url: https://codereview.chromium.org/2771113003
> Cr-Commit-Position: refs/heads/master@{#460345}
> Committed: https://chromium.googlesource.com/chromium/src/+/fc05776e6cc541dc9f4b6652876be72549f41682

TBR=anthonyvd@chromium.org,tommycli@chromium.org,pkasting@chromium.org,msarda@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 694476 

Review-Url: https://codereview.chromium.org/2787453002
Cr-Commit-Position: refs/heads/master@{#460349}

[modify] https://crrev.com/87301efc7eee4e37e3af750bc43f2977b386652b/chrome/browser/ui/cocoa/profiles/signin_view_controller_delegate_mac.h
[modify] https://crrev.com/87301efc7eee4e37e3af750bc43f2977b386652b/chrome/browser/ui/cocoa/profiles/signin_view_controller_delegate_mac.mm
[modify] https://crrev.com/87301efc7eee4e37e3af750bc43f2977b386652b/chrome/browser/ui/views/profiles/signin_view_controller_delegate_views.cc
[modify] https://crrev.com/87301efc7eee4e37e3af750bc43f2977b386652b/chrome/browser/ui/webui/signin/signin_error_handler.cc
[modify] https://crrev.com/87301efc7eee4e37e3af750bc43f2977b386652b/chrome/browser/ui/webui/signin/signin_utils.cc
[modify] https://crrev.com/87301efc7eee4e37e3af750bc43f2977b386652b/chrome/browser/ui/webui/signin/signin_utils.h
[modify] https://crrev.com/87301efc7eee4e37e3af750bc43f2977b386652b/chrome/browser/ui/webui/signin/sync_confirmation_handler.cc
[modify] https://crrev.com/87301efc7eee4e37e3af750bc43f2977b386652b/chrome/browser/ui/webui/signin/sync_confirmation_handler.h
[modify] https://crrev.com/87301efc7eee4e37e3af750bc43f2977b386652b/chrome/browser/ui/webui/signin/sync_confirmation_handler_unittest.cc
[modify] https://crrev.com/87301efc7eee4e37e3af750bc43f2977b386652b/chrome/browser/ui/webui/signin/sync_confirmation_ui.cc
[modify] https://crrev.com/87301efc7eee4e37e3af750bc43f2977b386652b/chrome/browser/ui/webui/signin/sync_confirmation_ui.h

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6883fe184564665aee120504f2d8b23310093eea

commit 6883fe184564665aee120504f2d8b23310093eea
Author: msarda <msarda@chromium.org>
Date: Fri Mar 31 10:02:46 2017

Reland "Use the same browser instance in the sync confirmation dialog.

This CL relands the CL https://codereview.chromium.org/2771113003
(the compile error was fixed in CL https://codereview.chromium.org/2785753003/).

TBR=pkasting,anthonyvd

--------------------------------------------------------------------
CL description for https://codereview.chromium.org/2771113003
Use the same browser instance in the sync confirmation dialog.

Before this CL, the sync confirmation dialog used the active browser
when dismissing the dialog. In some cases, the active browser is different
than the browser that actually presented the sync confirmation dialog
and this leads to unexpected behavior.

This CL fixes the browser instance used by the sync confirmation dialog
and ensures that the same instance is used throughout the lifetime of the
sync confirmation dialog.

TEST=See bug
BUG= 694476 
--------------------------------------------------------------------

Review-Url: https://codereview.chromium.org/2784823002
Cr-Commit-Position: refs/heads/master@{#461091}

[modify] https://crrev.com/6883fe184564665aee120504f2d8b23310093eea/chrome/browser/ui/cocoa/profiles/signin_view_controller_delegate_mac.h
[modify] https://crrev.com/6883fe184564665aee120504f2d8b23310093eea/chrome/browser/ui/cocoa/profiles/signin_view_controller_delegate_mac.mm
[modify] https://crrev.com/6883fe184564665aee120504f2d8b23310093eea/chrome/browser/ui/views/profiles/signin_view_controller_delegate_views.cc
[modify] https://crrev.com/6883fe184564665aee120504f2d8b23310093eea/chrome/browser/ui/webui/signin/signin_error_handler.cc
[modify] https://crrev.com/6883fe184564665aee120504f2d8b23310093eea/chrome/browser/ui/webui/signin/signin_utils.cc
[modify] https://crrev.com/6883fe184564665aee120504f2d8b23310093eea/chrome/browser/ui/webui/signin/signin_utils.h
[modify] https://crrev.com/6883fe184564665aee120504f2d8b23310093eea/chrome/browser/ui/webui/signin/sync_confirmation_handler.cc
[modify] https://crrev.com/6883fe184564665aee120504f2d8b23310093eea/chrome/browser/ui/webui/signin/sync_confirmation_handler.h
[modify] https://crrev.com/6883fe184564665aee120504f2d8b23310093eea/chrome/browser/ui/webui/signin/sync_confirmation_handler_unittest.cc
[modify] https://crrev.com/6883fe184564665aee120504f2d8b23310093eea/chrome/browser/ui/webui/signin/sync_confirmation_ui.cc
[modify] https://crrev.com/6883fe184564665aee120504f2d8b23310093eea/chrome/browser/ui/webui/signin/sync_confirmation_ui.h

Comment 7 by msarda@chromium.org, Mar 31 2017

Status: Fixed (was: Started)

Sign in to add a comment