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

Issue 654366 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: Unnecessary profile re-navigate to 'Person 2' even after clicking on 'Switch to Person 1' in ' New Signin modal error dialogue' .

Reported by dmascare...@etouch.net, Oct 10 2016

Issue description

Chrome Version:56.0.2886.0 (Official Build) aa3f2f3a48ffd2ac9ea7585f6a2ae1399f360532-refs/heads/master@{#424099}
OS: MAC (10.10.5, 10.11.4)

Pre condition:1. Sign in into Person 1 with Account A with valid credential.

What steps will reproduce the problem?
1. Launch chrome and Add person 2 from Switch person overlay.
2. Try to sign in with Account A in person 2 such that 'new signin modal error dialogue' opens,right click on 'Switch to Person 1' button
3. Then normally click on 'Switch to Person 1' and observe.

Actual: 1. Flickering of the 'Switch to Person 1' button is observe after step 2.
        2. Unnecessary profile re-navigate to 'Person 2' even after click on 'Switch to Person 1'.

Expected: 1. Flickering of button should not be seen.
          2. Person 2 should get opened after step 3.

This is regression issue,broken in 'M 56' and below is Manual bisect:

Good build:55.0.2883.6
Bad build:56.0.2886.0

Note: Above issue is not reproducible on Windows OS and will soon update the Linux OS info.

 
Note: Above issue is not reproducible on Linux OS.

Please refer the screencast
Actual_profile.mov
4.4 MB Download
Windows_behaviour.mp4
1.9 MB View Download

Comment 2 by hdodda@chromium.org, Oct 10 2016

Cc: pea...@gmail.com hdodda@chromium.org
Labels: hasbisect-per-revision ReleaseBlock-Stable
Owner: asargent@chromium.org
Status: Assigned (was: Unconfirmed)
Using the per-revision bisect providing the bisect results,
Good build: 55.0.2883.6 (Revision: 423768).
Bad build: 56.0.2886.0 (Revision: 424099).

You are probably looking for a change made after 423938 (known good), but no later than 423939 (first known bad).
CHANGELOG URL:
The script might not always return single CL as suspectas some perf builds might get missing due to failure.
  https://chromium.googlesource.com/chromium/src/+log/e0088f675519c6e1c90ff0717f09fb267d8a0e5f..adae398e9702324fb605c4bb0969c352363cadee

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

@asargent -- Unable to find the author of the CL from the owner's list, As a reviewer of the file assigning to you. Could you please look into the issue.

Note: Adding RB-stable label for now , please feel free to edit or remove , if this is not the case.

Thank You!
Cc: -hdodda@chromium.org
Owner: hdodda@chromium.org
https://codereview.chromium.org/2391243002 just removed a single LOG statement, which seems extremely unlikely to be the underlying cause of a problem like this. (Unless somehow this just happened to uncover a race condition in completely unrelated code to what the CL touched, that was always there?)

Maybe we accidentally got the wrong revision numbers for the bisect, or there's some bug in the bisect script?

Cc: hdodda@chromium.org
Owner: anthonyvd@chromium.org
Providing ChangeLog, since not able to sign in to chromium builds.
https://chromium.googlesource.com/chromium/src/+log/55.0.2883.0..56.0.2886.0?pretty=fuller&n=10000

Suspecting: r423943 ?

Could you please help to reassign if your change is not the cause for this change



Labels: -ReleaseBlock-Stable
Thanks for the report. This feature is new so it's normal not to find a regression CL.

Comment 6 by ew...@chromium.org, Oct 13 2016

Cc: ew...@chromium.org
Components: Services>SignIn
Labels: -M-56 M-55 signin-active-bug
Marking this as M55, since we need to merge this back to M55 (we're rolling out the user menu in M55).

Comment 7 by ew...@chromium.org, Oct 26 2016

Anthony, checking in here...We need to get this fixed for 55. Are you looking into it?

Comment 8 by ew...@chromium.org, Nov 1 2016

Checked in with Anthony offline. He's going to investigate a fix (note this is related to Issue 616242, the new user menu launch)
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 7 2016

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

commit 5cc51e0407c4d57ac23c418170481c16f8e27e4f
Author: anthonyvd <anthonyvd@chromium.org>
Date: Mon Nov 07 21:56:17 2016

Fix "Switch to {Profile}..." focusing the wrong window.

BUG= 654366 

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

[modify] https://crrev.com/5cc51e0407c4d57ac23c418170481c16f8e27e4f/chrome/browser/ui/webui/signin/signin_error_handler.cc

Cc: dmascare...@etouch.net
dmascarenhas@ - could you please help us verify this fix? Anthony landed the fix in comment #9 - can you still repro?

We'd like to request a merge once we've verified this.
Labels: TE-Verified-M56 TE-Verified-56.0.2914.0
Verified the issue on Mac OS 10.12 using chrome latest Dev M56-56.0.2914.0 by following steps mentioned in the original comment. Observed no flickering button and able to switch to profile 1 as expected. Hence adding TE-Verified label.

Note: For more info please refer screen-cast.
654366.mp4
1.2 MB View Download

Comment 12 by ew...@chromium.org, Nov 10 2016

Labels: Merge-Request-55
Thanks!

Comment 13 by dimu@chromium.org, Nov 10 2016

Labels: -Merge-Request-55 Merge-Approved-55 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M55 (branch: 2883)
Project Member

Comment 14 by bugdroid1@chromium.org, Nov 10 2016

Labels: -merge-approved-55 merge-merged-2883
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/11371901b59683f90cac854fa0bed19b7d4edcf1

commit 11371901b59683f90cac854fa0bed19b7d4edcf1
Author: Anthony Vallee-Dubois <anthonyvd@chromium.org>
Date: Thu Nov 10 17:52:59 2016

[Merge] Fix "Switch to {Profile}..." focusing the wrong window.

BUG= 654366 

Review-Url: https://codereview.chromium.org/2480393002
Cr-Commit-Position: refs/heads/master@{#430389}
(cherry picked from commit 5cc51e0407c4d57ac23c418170481c16f8e27e4f)

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

Cr-Commit-Position: refs/branch-heads/2883@{#520}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/11371901b59683f90cac854fa0bed19b7d4edcf1/chrome/browser/ui/webui/signin/signin_error_handler.cc

Comment 15 by ew...@chromium.org, Nov 10 2016

Status: Fixed (was: Assigned)
Thanks Anthony!
Labels: TE-Verified-55.0.2883.52 TE-Verified-M55
Rechecked the issue on chrome version 55.0.2883.52 on MAC 10.12.1 and merge is working as intended. 

Flickering of button are not observed and "Person 2" is displayed after switching person (Step 3).

Adding TE-Verified labels.


Sign in to add a comment