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

Regression:Unable to sign in with saved credentials in gmail.com

Reported by shruti.j...@etouch.net, Jul 3

Issue description

Chrome Version : 69.0.3480.0 (Official Build) 3c4342c43a5e8e33042613038d4777cc1c9349af-refs/branch-heads/3480@{#1} 64 bit

OS: Mac(10.12.6, 10.13.1, 10.13.6, 10.14).

Pre-Condiiton: Enable 'Use Views browser windows instead of Cocoa' flag from chrome://flags and relaunch the browser.
               
Steps to reproduce:
1. Launch chrome and Login into gmail.com with valid credentials.
2. Click on save password bubble and save password.
3. Click on 'Add account' by clicking on profile pic and Try sign in with saved credentials.

Actual Result:  Unable to sign in with saved credentials in gmail.com
Expected Result: User should be able to sign in with valid credentials.

This is a regression issue, broken in 'M-69', and below is the bisect provided using per-revision script.
Good Build:69.0.3445.0 (Revision: 563011)
Bad Build: 69.0.3446.0 (Revision: 562691)

You are probably looking for a change made after 563007 (known good), but no later than 563008 (first known bad).
CHANGE-LOG URL:
The script might not always return single CL as suspect as some perf builds might get missing due to failure.
https://chromium.googlesource.com/chromium/src/+log/1e665d5f61e0558ef0b3dfe5029152f237ad9618..6be8acd7b09a2378d8c47ca9fa91b9fd9b4839ba

Suspect: https://chromium.googlesource.com/chromium/src/+/6be8acd7b09a2378d8c47ca9fa91b9fd9b4839ba

@Christopher Cameron: Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.

Note: Note: Issue is Mac specific and is not reproducible on Windows(7,8,8.1,10) and Linux(14.04 LTS).

Kindly refer the screen-cast using the below given link.

https://drive.google.com/open?id=13KhlaKeVqEwUgjTG6PUKEbxIJcTOA-pP

Thank You.
 
 
Cc: battre@chromium.org msarda@chromium.org
Components: -UI>Browser>Passwords Internals>Services>Identity
Does not look like related to the password manager. Adding other identity folks.
Cc: manoranj...@chromium.org
Labels: ReleaseBlock-Stable
Adding release blocker label for this issue.Please reduce priority or remove if not the case.

Thank You!
Cc: droger@chromium.org yananj@google.com
CC+ Yanan as this may be a server-side issue.

Yanan: Would you mind taking a look at this and check whether this is a server-side issue?
User is signed in from server's point of view: credentials are issued and user is redirected to https://mail.google.com/mail/#inbox. But user remains on password page for a long time even after gmail redirect, and eventually after a minute or so, gmail is shown.

This is only the case when 'Use Views browser windows instead of Cocoa' flag is turned on, the feature it's enabled seems to interfere the view.
Please attach a video so that I can see how to reproduce this.
I was using the screencast in comment #1 and could reproduce, https://drive.google.com/open?id=13KhlaKeVqEwUgjTG6PUKEbxIJcTOA-pP
Ah, thanks, sorry, didn't look carefully enough.
Found the root cause here.

The issue is that we are not being consistent about
- the ordering of child RWHVCocoas under WebContentsViewCocoas
- the ordering of child ui::Layers under NativeWidgetHost

We have a bad habit of just adding a pile of child RWHVCocoa children to WebContentsViewCocoa and not really tracking them. I'll have to add some code to maintain this structure.
Likely same root cause as  issue 864021 .
Thank you, this is an excellent bug report of a very subtle issue. I have a fix ready.
Labels: TE-Goodbug
Project Member

Comment 12 by bugdroid1@chromium.org, Jul 23

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

commit c27876f59a3df5b19b54dccc4e59894928665f1f
Author: Christopher Cameron <ccameron@chromium.org>
Date: Mon Jul 23 23:44:13 2018

MacViews: Fix RenderWidgetHostViewMac ui::Layer visibility

Two bugs here.

First bug: Interactions with calling RWHVMac::Show/Hide.

With the non-unified compositor, calling RWHVMac::Show/Hide will cause
the Cocoa NSView to have its show or hide method called.

With the unified compositor, calls to RWHVMac::Show/Hide are largely
ignored. The ui::Layer will always be visible if it has a parent
ui::Layer. This is a bug -- the ui::Layer should be made visible or not
based on the visibility specified by the calls to RWHVMac::Show/Hide.

Second bug: Child NSViews not matching child ui::Layers

It is possible for a WebContentsViewCocoa NSView to have multiple
RenderWidgetHostViewCocoa NSViews. Which of these views is visible
is controlled by called to RWHVMac::Show/Hide.

Track the parent RenderWidgetHostViewBases that added NSViews, and
iterate through all of them when updating the parent ui::Layer.

Unrelated cleanup: Remove parameter from BrowserCompositorViewMac that
is always false (and add a DCHECK that it is false, just for good
measure).

Bug:  859834 ,  865227 
Change-Id: I8767692443d5eb6381a8ea4b087c5c312fef305d
Reviewed-on: https://chromium-review.googlesource.com/1141148
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577334}
[modify] https://crrev.com/c27876f59a3df5b19b54dccc4e59894928665f1f/content/browser/renderer_host/browser_compositor_view_mac.h
[modify] https://crrev.com/c27876f59a3df5b19b54dccc4e59894928665f1f/content/browser/renderer_host/browser_compositor_view_mac.mm
[modify] https://crrev.com/c27876f59a3df5b19b54dccc4e59894928665f1f/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/c27876f59a3df5b19b54dccc4e59894928665f1f/content/browser/web_contents/web_contents_view_mac.h
[modify] https://crrev.com/c27876f59a3df5b19b54dccc4e59894928665f1f/content/browser/web_contents/web_contents_view_mac.mm

Update : 
Retested above issue on Mac(10.12.6,10.13.1,10.13.6,10.14.0)OS using latest Canary #70.0.3501.0 and issue is still reproducible.gmail window gets stuck and seen blank.Kindly review the attached screen-cast(https://drive.google.com/open?id=19aErbq9-PHgTSXT7BLpDwR8dyGP2Vcvw)
Thank you!
Project Member

Comment 14 by bugdroid1@chromium.org, Jul 25

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

commit 33211d468f7334a4cba855bef6194e502202cdf4
Author: Christopher Cameron <ccameron@chromium.org>
Date: Wed Jul 25 17:26:16 2018

WebContentsViewMac: Save parent ui::Layer in SetParentUiLayer

This was not being saved, resulting in new child RWHViewMacs having
their parent ui::Layer set to nullptr.

Bug:  859834 
Change-Id: I8e66370381406d8cc1dfb2aac9d81397c1775831
Reviewed-on: https://chromium-review.googlesource.com/1148940
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577958}
[modify] https://crrev.com/33211d468f7334a4cba855bef6194e502202cdf4/content/browser/web_contents/web_contents_view_mac.mm

Labels: TE-Verified-M70 TE-Verified-70.0.3503.0
Update : 
Retested above issue on Mac(10.12.6,10.13.1,10.13.6,10.14.0)OS using latest Canary #70.0.3503.0 and issue is fixed.gmail window does not get stuck and opens properly.Kindly review the attached screen-cast.
Thank you!
Canary_Behaviour#70.0.3503.0.mov
6.3 MB View Download
Cc: groby@chromium.org ccameron@chromium.org sdy@chromium.org
 Issue 865227  has been merged into this issue.
Cc: ellyjo...@chromium.org rpop@chromium.org
 Issue 864021  has been merged into this issue.
Labels: Merge-Request-69
Project Member

Comment 19 by sheriffbot@chromium.org, Jul 28

Labels: -Merge-Request-69 Hotlist-Merge-Approved Merge-Approved-69
Your change meets the bar and is auto-approved for M69. Please go ahead and merge the CL to branch 3497 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Please merge your change to M69 branch 3497 by 2:00 PM PT Monday, 07/30, so we can pick it up for next week last M69 Dev release. Thank you.

Project Member

Comment 21 by bugdroid1@chromium.org, Jul 29

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c9dfbbcb39c587ec98214f7c847569ec163eaf56

commit c9dfbbcb39c587ec98214f7c847569ec163eaf56
Author: Christopher Cameron <ccameron@chromium.org>
Date: Sun Jul 29 22:43:44 2018

MacViews: Fix RenderWidgetHostViewMac ui::Layer visibility

Two bugs here.

First bug: Interactions with calling RWHVMac::Show/Hide.

With the non-unified compositor, calling RWHVMac::Show/Hide will cause
the Cocoa NSView to have its show or hide method called.

With the unified compositor, calls to RWHVMac::Show/Hide are largely
ignored. The ui::Layer will always be visible if it has a parent
ui::Layer. This is a bug -- the ui::Layer should be made visible or not
based on the visibility specified by the calls to RWHVMac::Show/Hide.

Second bug: Child NSViews not matching child ui::Layers

It is possible for a WebContentsViewCocoa NSView to have multiple
RenderWidgetHostViewCocoa NSViews. Which of these views is visible
is controlled by called to RWHVMac::Show/Hide.

Track the parent RenderWidgetHostViewBases that added NSViews, and
iterate through all of them when updating the parent ui::Layer.

Unrelated cleanup: Remove parameter from BrowserCompositorViewMac that
is always false (and add a DCHECK that it is false, just for good
measure).

TBR=ccameron@chromium.org

(cherry picked from commit c27876f59a3df5b19b54dccc4e59894928665f1f)

Bug:  859834 ,  865227 
Change-Id: I8767692443d5eb6381a8ea4b087c5c312fef305d
Reviewed-on: https://chromium-review.googlesource.com/1141148
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#577334}
Reviewed-on: https://chromium-review.googlesource.com/1154359
Reviewed-by: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#193}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/c9dfbbcb39c587ec98214f7c847569ec163eaf56/content/browser/renderer_host/browser_compositor_view_mac.h
[modify] https://crrev.com/c9dfbbcb39c587ec98214f7c847569ec163eaf56/content/browser/renderer_host/browser_compositor_view_mac.mm
[modify] https://crrev.com/c9dfbbcb39c587ec98214f7c847569ec163eaf56/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/c9dfbbcb39c587ec98214f7c847569ec163eaf56/content/browser/web_contents/web_contents_view_mac.h
[modify] https://crrev.com/c9dfbbcb39c587ec98214f7c847569ec163eaf56/content/browser/web_contents/web_contents_view_mac.mm

Project Member

Comment 22 by bugdroid1@chromium.org, Jul 29

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

commit ea3c3cbced733bd1d00aa04618f57416f0d90ba0
Author: Christopher Cameron <ccameron@chromium.org>
Date: Sun Jul 29 22:46:09 2018

WebContentsViewMac: Save parent ui::Layer in SetParentUiLayer

This was not being saved, resulting in new child RWHViewMacs having
their parent ui::Layer set to nullptr.

TBR=ccameron@chromium.org

(cherry picked from commit 33211d468f7334a4cba855bef6194e502202cdf4)

Bug:  859834 
Change-Id: I8e66370381406d8cc1dfb2aac9d81397c1775831
Reviewed-on: https://chromium-review.googlesource.com/1148940
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#577958}
Reviewed-on: https://chromium-review.googlesource.com/1154360
Reviewed-by: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#194}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/ea3c3cbced733bd1d00aa04618f57416f0d90ba0/content/browser/web_contents/web_contents_view_mac.mm

Status: Fixed (was: Assigned)
Labels: TE-Verified-M69 TE-Verified-69.0.3497.23
Update : 
Retested above issue on Mac(10.12.6,10.13.1,10.13.6,10.14.0)OS using Dev #69.0.3497.23 and issue is fixed.Gmail window does not get stuck and opens properly.Kindly review the attached screen-cast.
Thank you!


Dev###69.0.3497.23.mov
9.8 MB View Download
 Issue 883879  has been merged into this issue.

Sign in to add a comment