New issue
Advanced search Search tips

Issue 626020 link

Starred by 2 users

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Investigate whether transforms affect certain coordinate space translations

Project Member Reported by kenrb@chromium.org, Jul 6 2016

Issue description

There are places in the browser where we move coordinates from one frame's space to another using just offsets (e.g. CrossProcessFrameConnector::BubbleScrollEvent(), BrowserPluginGuest::ResendEventToEmbedder()).

It might be better to use coordinate transformation methods in RenderWidgetHostViewBase for those, although I have seen places in the renderer where this is straightforward translation to move between frame coordinate spaces (which may or may not also be buggy).

This warrants investigation to understand that better and see if we should change it.
 
Project Member

Comment 1 by sheriffbot@chromium.org, Jul 7 2017

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label.

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

Comment 2 by bugdroid1@chromium.org, Mar 26 2018

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

commit 8961ab574285a3907e22c60eb78eda0d82904e50
Author: Kevin McNee <mcnee@chromium.org>
Date: Mon Mar 26 23:53:11 2018

OOPIF: Transform point of resent gesture event instead of applying offset.

When bubbling scroll events to ancestor views, we adjust the event's
position by applying the parent's offset. This is incorrect if an OOPIF
is subject to CSS scaling, since the point in the child is scaled. We
now use a transformation function which accounts for scaling.

We were also assuming that the event was being resent to the parent. This
is true for GestureScrollBegin events, but for the remaining events in the
sequence, we may bubble directly to an ancestor, so those events have the
coordinates for the wrong view. We now defer the transformation of the
coordinates to the target view's coordinate space to the
RenderWidgetHostInputEventRouter which knows which view is the target.

Bug: 626020,  817392 
Change-Id: I86331f7bfc5fe7dd9e9032beb16ebb75b579f5b1
Reviewed-on: https://chromium-review.googlesource.com/978647
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545890}
[modify] https://crrev.com/8961ab574285a3907e22c60eb78eda0d82904e50/content/browser/frame_host/cross_process_frame_connector.cc
[modify] https://crrev.com/8961ab574285a3907e22c60eb78eda0d82904e50/content/browser/renderer_host/render_widget_host_input_event_router.cc
[modify] https://crrev.com/8961ab574285a3907e22c60eb78eda0d82904e50/content/browser/renderer_host/render_widget_host_input_event_router.h
[modify] https://crrev.com/8961ab574285a3907e22c60eb78eda0d82904e50/content/browser/site_per_process_hit_test_browsertest.cc
[add] https://crrev.com/8961ab574285a3907e22c60eb78eda0d82904e50/content/test/data/frame_tree/page_with_positioned_scaled_frame.html
[modify] https://crrev.com/8961ab574285a3907e22c60eb78eda0d82904e50/testing/buildbot/filters/viz.content_browsertests.filter

Project Member

Comment 3 by bugdroid1@chromium.org, Mar 29 2018

Labels: merge-merged-3359
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5857d8c65e779b222549d5794f28941300952f5a

commit 5857d8c65e779b222549d5794f28941300952f5a
Author: Kevin McNee <mcnee@chromium.org>
Date: Thu Mar 29 16:28:25 2018

Merge to M66: OOPIF: Transform point of resent gesture event instead of applying offset.

Note that this CL differs from the original as the representation of
coordinates in WebGestureEvent has changed between M66 and ToT.

OOPIF: Transform point of resent gesture event instead of applying offset.

When bubbling scroll events to ancestor views, we adjust the event's
position by applying the parent's offset. This is incorrect if an OOPIF
is subject to CSS scaling, since the point in the child is scaled. We
now use a transformation function which accounts for scaling.

We were also assuming that the event was being resent to the parent. This
is true for GestureScrollBegin events, but for the remaining events in the
sequence, we may bubble directly to an ancestor, so those events have the
coordinates for the wrong view. We now defer the transformation of the
coordinates to the target view's coordinate space to the
RenderWidgetHostInputEventRouter which knows which view is the target.

(cherry picked from commit 8961ab574285a3907e22c60eb78eda0d82904e50)

Bug: 626020,  817392 
Change-Id: I86331f7bfc5fe7dd9e9032beb16ebb75b579f5b1
Reviewed-on: https://chromium-review.googlesource.com/978647
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: Nasko Oskov <nasko@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#545890}
Reviewed-on: https://chromium-review.googlesource.com/983756
Cr-Commit-Position: refs/branch-heads/3359@{#496}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/5857d8c65e779b222549d5794f28941300952f5a/content/browser/frame_host/cross_process_frame_connector.cc
[modify] https://crrev.com/5857d8c65e779b222549d5794f28941300952f5a/content/browser/renderer_host/render_widget_host_input_event_router.cc
[modify] https://crrev.com/5857d8c65e779b222549d5794f28941300952f5a/content/browser/renderer_host/render_widget_host_input_event_router.h
[modify] https://crrev.com/5857d8c65e779b222549d5794f28941300952f5a/content/browser/site_per_process_hit_test_browsertest.cc
[add] https://crrev.com/5857d8c65e779b222549d5794f28941300952f5a/content/test/data/frame_tree/page_with_positioned_scaled_frame.html
[modify] https://crrev.com/5857d8c65e779b222549d5794f28941300952f5a/testing/buildbot/filters/viz.content_browsertests.filter

Sign in to add a comment