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

Issue 880210 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 28
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: Sign-in to chrome page becomes unresponsive after giving print command.

Reported by sanyam.g...@etouch.net, Sep 4

Issue description

Chrome Version: 71.0.3542.0 (Official Build) Revision	4097c6595e73c799040bc3a99bcfcc6adb178386-refs/branch-heads/3542@{#1} (32/64-bit)

OS: Windows(7,8,8.1,10)and Linux(14.04 LTS)

What steps will reproduce the problem?
1. Launch chrome, navigate to NTP and give print command by pressing 'ctrl+p'
2. Now click on avatar icon (while print preview overlay is open).
3. Click on 'sign in' from avatar overlay and observe.

Actual Result  : Sign-in to chrome page becomes unresponsive.
Expected Result: Sign-in to chrome page should not become unresponsive

This is regression issue broken in ‘M-70’ and will soon update the other info:

Good build: 70.0.3536.0(Revision: 587137)
Bad build : 70.0.3537.0(Revision: 587302)

Note: Issue is not reproducible on  Mac (10.12.6, 10.13.1, 10.13.6, 10.14) OS
 
Actual_Behaviour.mp4
790 KB View Download
Expected_Behaviour.mp4
630 KB View Download
Labels: hasbisect
Owner: nicolaso@chromium.org
Status: Assigned (was: Unconfirmed)
Update: 

Change-Log URL:
https://chromium.googlesource.com/chromium/src/+/de05f8f9dc649b306c6ba098c45977ed84894960

Suspecting: r587249 ?

@nicolaso: 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: 
1. Provided suspect through 'Change-log' URL because unable to perform bisect using 'per-revision' bisect script and 'Chromium bisect' script. 
2. Tried performing 'per revision' bisect on multiple Windows and Linux machines but unable to perform the same since getting following errors: 
   (a) Error message on Windows and Linux OS : Unable to find locale data files 
3. Unable to provide the bisect using Chromium script also as unable to sign-in to Chromium builds.

Thank you.
Thank you for the bug report.

I was able to reproduce this by cloning from the master branch.

However, I cannot reproduce when I check out de05f8f9dc649b306c6ba098c45977ed84894960 (r587249) and build Chromium (with API keys).

Based on this, I don't think my patch introduced this regression...
nicolaso: Can you try bisecting with tools/bisect-builds.py to help find the cause? You'll need to set the API keys at run time instead of at build time. Let me know if you need help.
Cc: a...@chromium.org
Re-bisected with API keys to https://chromium.googlesource.com/chromium/src/+log/578be39046de0eb8ad2c103b3a98dda76b5651b2..6aa5a8a398d978aca7cb31ecf40afd764323d8da

I'm guessing this is https://crrev.com/c/1194484, which is the fix for  https://crbug.com/863582 . This prevented interacting with PDFs behind constrained web dialogs (like Print Preview). Maybe it also disabled interacting with this sign-in input somehow. cc-ing avi@.
+avi@

Ran the bisect script to identify the culprit CL. It looks like [1] is fine, but I can reproduce the bug on [2].

avi@, since you are the author of [2], do you think your change could have introduced this bug?

[1] https://chromium.googlesource.com/chromium/src/+/09a292e587ef9c97e9bf19e316b47061f62d98be
[2] https://chromium.googlesource.com/chromium/src/+/738ea19831a4395bff3066a8ce994482ac8b44d6
( I should really refresh the page before hitting 'send' >_> )

Anyways, looks like I arrived at the same results as rbpotter@ did
I'm OOO for 2 weeks or so.

This sounds related to what I've done, but if widgets in an unrelated WebContents are going unresponsive, this is surprising and makes me think that something deeper is wrong.
I cannot reproduce this as the steps don't work for me.

When I click the sign-in button, in the repro video that causes the print dialog to close, but on the ToT I'm working on (67025f5a3a66d54d511f8f8a1b97a5b6ad30dd7b) that doesn't cause the print dialog to close, and when I manually close the dialog things work.
I accidentally ran into another way to reproduce this the other day, which seems to still work in current Canary (on Win10):
(1) Open Print Preview
(2) In the omnibox, type chrome://settings/searchEngines + hit enter to navigate
(3) Try to interact with the search field for the subpage, located below the blue header next to the "Manage Search Engines" label.
 Issue 884636  has been merged into this issue.
This seems to be some kind of race that I'm not winning on my custom builds.

I have a suspicion for a fix based on the code.
Project Member

Comment 12 by bugdroid1@chromium.org, Sep 25

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

commit f7e423bf9327e5069d1c462c1da43e487932b606
Author: Avi Drissman <avi@chromium.org>
Date: Tue Sep 25 22:51:57 2018

Make sure to unblock a WebContents during a mass dialog closure.

BUG= 880210 

Change-Id: I44e80bc0866fef5370936db3f2cb4d3eacf620f2
Reviewed-on: https://chromium-review.googlesource.com/1241094
Reviewed-by: Leonard Grey <lgrey@chromium.org>
Reviewed-by: Mike Wittman <wittman@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594135}
[modify] https://crrev.com/f7e423bf9327e5069d1c462c1da43e487932b606/components/web_modal/web_contents_modal_dialog_manager.cc
[modify] https://crrev.com/f7e423bf9327e5069d1c462c1da43e487932b606/components/web_modal/web_contents_modal_dialog_manager.h

This fix is from inspection, as I can't repro. For those who have repro-ed, can you test on a canary that has this patch to verify? Thanks! 
Update :
Rechecked the above issue on Windows (7,8,8.1,10) and Linux (14.04 LTS) using latest Canary build #71.0.3562.0 and the issue is still reproducible.

Kindly refer the attached screen cast.

Thank you..!!
Canary_Behaviour.mp4
740 KB View Download
rbpotter: that seems like a focus issue, though the call that should be focusing is the same one that isn't getting called re event routing.

I'm still looking at this.
OK, I know what's going on, though not entirely what changed.

What's happening when navigating away from the page with the print dialog:

1. The print dialog is closed.
2. WebContentsModalDialogManager::BlockWebContentsInteraction(false) is called.
3. That calls SetWebContentsBlocked(false) on the delegate, which is Browser.
4. Browser::SetWebContentsBlocked() sees that the WebContents is being unblocked and is in front, so it calls WebContents::Focus().
5. WebContentsImpl::Focus() calls WebContentsView::Focus() on the view.
6. Linux and Windows use WebContentsViewAura::Focus(), which (unlike WebContentsViewMac::Focus() and thus the platform difference!) delegates to WebContentsViewDelegate::Focus().
7. The delegate for Chromium is ChromeWebContentsViewDelegateViews, and its Focus() call turns around and calls Focus() on the focus helper.
8. The ChromeWebContentsViewFocusHelper::Focus() call has:
    const web_modal::WebContentsModalDialogManager* manager =
        web_modal::WebContentsModalDialogManager::FromWebContents(web_contents_);
    if (manager && manager->IsDialogActive())
      manager->FocusTopmostDialog();
   The WebContentsModalDialogManager indeed thinks that there is a dialog active, and "activates" it, causing the page to not properly take focus.

I don't know where in this chain my change made things different, but this is the chain right now. The question I'm going to answer is "after step 1, can we make sure that the dialog manager returns false from IsDialogActive()?"
OK, it's not quite as simple as #16 makes it out to be.

Commenting out the FocusTopmostDialog() call in ChromeWebContentsViewFocusHelper::Focus() makes the problem go away, but the problem is that it's not called by Browser when the dialog goes away. It's called when *someone else* calls Focus().

I'm still not clear about what's going on.
Project Member

Comment 18 by bugdroid1@chromium.org, Sep 28

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

commit a0549215f29c2a0b574f323222a043ecebde6a68
Author: Avi Drissman <avi@chromium.org>
Date: Fri Sep 28 18:43:45 2018

Revert "Make sure to unblock a WebContents during a mass dialog closure."

This reverts commit f7e423bf9327e5069d1c462c1da43e487932b606.

Reason for revert:

This was a speculative fix, thinking that perhaps WillClose() wasn't getting called. But it is, and all the correct calls are being made (see https://bugs.chromium.org/p/chromium/issues/detail?id=880210#c16) so this code only complicates things.

Original change's description:
> Make sure to unblock a WebContents during a mass dialog closure.
> 
> BUG= 880210 
> 
> Change-Id: I44e80bc0866fef5370936db3f2cb4d3eacf620f2
> Reviewed-on: https://chromium-review.googlesource.com/1241094
> Reviewed-by: Leonard Grey <lgrey@chromium.org>
> Reviewed-by: Mike Wittman <wittman@chromium.org>
> Commit-Queue: Avi Drissman <avi@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#594135}

TBR=avi@chromium.org,wittman@chromium.org,lgrey@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  880210 
Change-Id: Ibbd669649add66e0bc4052eb6b04c845ffb40f38
Reviewed-on: https://chromium-review.googlesource.com/1249389
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Leonard Grey <lgrey@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595166}
[modify] https://crrev.com/a0549215f29c2a0b574f323222a043ecebde6a68/components/web_modal/web_contents_modal_dialog_manager.cc
[modify] https://crrev.com/a0549215f29c2a0b574f323222a043ecebde6a68/components/web_modal/web_contents_modal_dialog_manager.h

Project Member

Comment 19 by bugdroid1@chromium.org, Sep 28

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

commit d1596214f9f76bbc890cd792076c9d00d54bf8cb
Author: Avi Drissman <avi@chromium.org>
Date: Fri Sep 28 19:05:19 2018

Fix focus issues with web contents modal dialogs.

BUG= 880210 
TEST=as in bug

Change-Id: I9f305084bf9f6b146d0323baeabc4437466857df
Reviewed-on: https://chromium-review.googlesource.com/1252241
Reviewed-by: Leonard Grey <lgrey@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595171}
[modify] https://crrev.com/d1596214f9f76bbc890cd792076c9d00d54bf8cb/chrome/browser/ui/views/tab_contents/chrome_web_contents_view_focus_helper.cc

Status: Fixed (was: Assigned)
That should fix it; please verify.
Cc: msrchandra@chromium.org
 Issue 529234  has been merged into this issue.

Sign in to add a comment