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

Issue 797661 link

Starred by 2 users

Issue metadata

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

Blocking:
issue 796651



Sign in to add a comment

Regression:Unable to close sign in to chrome overlay.

Reported by pranjali...@etouch.net, Dec 26 2017

Issue description

Chrome Version: 65.0.3304.0 (Official Build) 504fd2e8f347bf20638cce9f80ebf065ad017a73-refs/heads/master@{#526164}

OS:Mac Touch bar(10.13.3),Retina(10.12.6)

Steps to reproduce:
1.Launch Chrome ,click on avatar icon and click on sign in to chrome.
2.Now click on close icon and observe.

Actual Result:Unable to close sign in to chrome overlay.
Expected Result:Should be able to close sign in to chrome overlay.


This is a regression issue, broken in M-65 series, Soon update other bisect info

Good Build:65.0.3303.0
Bad Build:65.0.3304.0

 
Actual_result.mov
780 KB Download
Expected_result.mov
971 KB Download
Summary: Regression:Unable to close sign in to chrome overlay. (was: Unable to close sign in to chrome overlay.)
Labels: Needs-Bisect
Status: Untriaged (was: Unconfirmed)
Cc: gov...@chromium.org abdulsyed@chromium.org ligim...@chromium.org
Labels: -Needs-Bisect hasbisect-per-revision ReleaseBlock-Dev
Owner: sadrul@chromium.org
Status: Assigned (was: Untriaged)
Using the per-revision bisect providing the bisect results,
Good build:65.0.3303.0(Revision:526140).
Bad build:65.0.3304.0(Revision:526164).

You are probably looking for a change made after 526153 (known good), but no later than 526154 (first known bad).

CHANGE-LOG URL:
---------------
https://chromium.googlesource.com/chromium/src/+log/d37a9f10c7e32ff40e864a2ff30478fc8a0a1169..8e8ae37d2e02943cadb4d658777866e8e4a12f60

From the CL above, assigning the issue to the concern owner

@sadrul: Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to concern owner.

Reviewed-On: https://chromium-review.googlesource.com/838626
Note : Mac specific issue.Adding Release Block-Dev for this issue.Please remove if not the case.
Labels: RegressedIn-65 Target-65 FoundIn-65
sadrul@ do we have any update in the fix, please let us know.
Cc: sadrul@chromium.org
Owner: tapted@chromium.org
If that particular CL is responsible, then this would show up on other platforms too. But I can't repro on linux. So I suspect it could be something else .. maybe related to views UI on mac?

--> tapted@ for triage.
Cc: tapted@chromium.org
Owner: sadrul@chromium.org
We are not using views on Mac for webui dialogs at this stage, so I don't think so.

On 65.0.3309.0 I could repro on Retina but not on non-retina Mac.  Does it repro on HiDPI Linux?
Cc: gklassen@chromium.org
I tested on hidpi linux, yeah, and could not reproduce the issue.

This maybe related to similar input-targeting issues on mac we are experiencing.
Status: Started (was: Assigned)
Instead of clicking on the x, does the popup close if you click on the top-left corner of the dialog? It does seem to be hi-dpi related as tapted@ suggested, but it's a little bit inconsistent for me right now on linux hi-dpi.
Ah, yep - there's a small region (10x10 DIP) in the top left corner that is clickable on retina mac.

Also interesting: the [x] is shown while the webcontent is still loading -- the whole [x] is clickable then. But once the signin page loads, it stops being clickable. (maybe some layer squashing thing - Issue 788807 ?)
We have scheduled M-65 Dev release tomorrow and this is marked as Dev blocker. Can we get an update and expedite the fix.

Thanks!
Cc: creis@chromium.org
Labels: -ReleaseBlock-Dev ReleaseBlock-Beta
checked with sadrul@, and we can downgrade this to RB-Beta. This is related to site-isolation, and seems like it's affecting high res monitors?
Project Member

Comment 13 by bugdroid1@chromium.org, Jan 5 2018

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

commit 2894c17c9f356b50b4d5e2c7e8b5a455c7fb1d1d
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Fri Jan 05 06:56:09 2018

oopif events: Simplify the view look-up from a FrameSinkId.

Instead of looking up a target based on the process id/routing id, use
the FrameSinkId mapping maintained in the router to find the target view.
This gets rid of an unfortunate dependency on frame_host from
renderer_host. This also makes sure that the targeter keeps walking down
the tree of views in each step.  So a bad renderer cannot re-target the
event to an unrelated view.

BUG= 797362 ,  797661 

Change-Id: Iffb1cbd126fc0afa80cc8184305eb9e8b06f094b
Reviewed-on: https://chromium-review.googlesource.com/851357
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527234}
[modify] https://crrev.com/2894c17c9f356b50b4d5e2c7e8b5a455c7fb1d1d/content/browser/renderer_host/DEPS
[modify] https://crrev.com/2894c17c9f356b50b4d5e2c7e8b5a455c7fb1d1d/content/browser/renderer_host/render_widget_host_input_event_router.cc
[modify] https://crrev.com/2894c17c9f356b50b4d5e2c7e8b5a455c7fb1d1d/content/browser/renderer_host/render_widget_host_input_event_router.h
[modify] https://crrev.com/2894c17c9f356b50b4d5e2c7e8b5a455c7fb1d1d/content/browser/renderer_host/render_widget_targeter.cc
[modify] https://crrev.com/2894c17c9f356b50b4d5e2c7e8b5a455c7fb1d1d/content/browser/renderer_host/render_widget_targeter.h

Blocking: 796651
Project Member

Comment 15 by bugdroid1@chromium.org, Jan 5 2018

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

commit 6539b7337c5fd07a870113fca70b47a9dac23838
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Fri Jan 05 23:02:08 2018

oopif events: Return the correct FrameSinkId.

The RenderFrameProxy has the correct FrameSinkId for the embedded oopif.
So return that directly as the hit-testing result, instead of
constructing it from the frame's routing id.

BUG= 797661 

Change-Id: I9e6e263f6929b5ce53baeef5c8093bd9de052626
Reviewed-on: https://chromium-review.googlesource.com/852817
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Lucas Gadani <lfg@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527421}
[modify] https://crrev.com/6539b7337c5fd07a870113fca70b47a9dac23838/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/6539b7337c5fd07a870113fca70b47a9dac23838/content/renderer/input/input_target_client_impl.cc
[modify] https://crrev.com/6539b7337c5fd07a870113fca70b47a9dac23838/content/renderer/input/render_widget_input_handler.cc
[modify] https://crrev.com/6539b7337c5fd07a870113fca70b47a9dac23838/content/renderer/input/render_widget_input_handler.h
[modify] https://crrev.com/6539b7337c5fd07a870113fca70b47a9dac23838/content/renderer/render_frame_proxy.h
[modify] https://crrev.com/6539b7337c5fd07a870113fca70b47a9dac23838/content/renderer/render_widget.cc
[modify] https://crrev.com/6539b7337c5fd07a870113fca70b47a9dac23838/content/renderer/render_widget.h
[modify] https://crrev.com/6539b7337c5fd07a870113fca70b47a9dac23838/content/renderer/render_widget_browsertest.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Jan 8 2018

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

commit 2d72b38b601cb38a10384e8d32045ec8b0042845
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Mon Jan 08 05:57:13 2018

oopif events: Scale the location only when appropriate.

Some platforms use zooming to implement device-scale-factor (e.g. on
linux, windows). It is necessary to convert the hit-test point from DIP
to physical-pixel space only on those platforms. In other platforms
(e.g. on mac, android), it is not necessary to apply the device-scale
factor before hit-testing in blink.

BUG= 797661 , 793018

Change-Id: If5252d598f29c007d967d3c35eddac049abfc989
Reviewed-on: https://chromium-review.googlesource.com/853161
Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527577}
[modify] https://crrev.com/2d72b38b601cb38a10384e8d32045ec8b0042845/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/2d72b38b601cb38a10384e8d32045ec8b0042845/content/renderer/input/render_widget_input_handler.cc

Project Member

Comment 17 by bugdroid1@chromium.org, Jan 8 2018

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

commit bc55716b7ce8d4a19eea2b4b9d62bf4ad72e7a6b
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Mon Jan 08 14:46:38 2018

oopif events: Fix a couple of issues with async event targeting.

. The renderer already applies the device-scale-factor during hit-test.
  So avoid doing any scaling on the browser side.
. When querying a nested client for targeting, make sure the location
  is correctly transformed to that view's coordinate space.

BUG= 797661 , 793018

Change-Id: I65d74ad49fb25d1a9cb05745e2a2f8d3ac26763e
Reviewed-on: https://chromium-review.googlesource.com/848033
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527632}
[modify] https://crrev.com/bc55716b7ce8d4a19eea2b4b9d62bf4ad72e7a6b/content/browser/renderer_host/render_widget_targeter.cc
[modify] https://crrev.com/bc55716b7ce8d4a19eea2b4b9d62bf4ad72e7a6b/content/browser/site_per_process_browsertest.cc
[add] https://crrev.com/bc55716b7ce8d4a19eea2b4b9d62bf4ad72e7a6b/content/test/data/frame_tree/page_with_content_overlap_positioned_frame.html
[modify] https://crrev.com/bc55716b7ce8d4a19eea2b4b9d62bf4ad72e7a6b/testing/buildbot/filters/viz.content_browsertests.filter

Status: Fixed (was: Started)
This should now be fixed.

Sign in to add a comment