New issue
Advanced search Search tips

Issue 883508 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Touch move event coordinates incorrect when OOPIF has CSS scale transform

Project Member Reported by halliwell@chromium.org, Sep 12

Issue description

Chrome Version: 68.0.3440.118
OS: ChromeOS

What steps will reproduce the problem?
(1) See output.jsbin.com/vuzefivifa
(2) Blue and red frames are identical, except that blue frame is  (loaded from jsfiddle) and red frame is same-process using srcdoc.
(3) If you touch at left and swipe to right of those frames, you'll see move X coordinate goes to ~1000 for red frame as expected, but only to ~500 for blue frame.

An alternative repro case is at http://output.jsbin.com/lomowed/5, where an <input range> element shows the slider moving way too fast in the blue OOPIF.


 
One other note - I don't think this is reproducible with mouse events - they appear to be getting transformed correctly (and the slider example works properly with mouse input).
Just re-reading my description, (2) meant to say blue frame "is an OOPIF"
Cc: wjmaclean@chromium.org
Labels: -Pri-3 M-71 Pri-2
It's quite likely this occurs due to the fact that the TouchEvents are transformed only by a translation ('delta'), which would be fine for the TouchStart, but wrong for any other TouchEvent not at the exact same position.

https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_host_input_event_router.cc?rcl=6cb8ed9f50e8c23c1fc6968277ffaf1d94bbd2f1&l=718

A fix for this is to instead use TransformPointToCoordSpaceForView 

https://cs.chromium.org/chromium/src/content/browser/frame_host/cross_process_frame_connector.h?rcl=6cb8ed9f50e8c23c1fc6968277ffaf1d94bbd2f1&l=93

... though we had originally intended to do this and ran into reliability problems. We could re-try this though ... it would be good to revisit what the issues where there anyways.

This wouldn't happen to Mouse events since those get transformed individually according to the transformed point computed during targeting (though it could be an issue if mouse capture is locking the mouse event to some other target?).
I can put together a test CL tomorrow to see if my ideas above are correct ...
halliwell@ - I've put up an experimental CL at https://chromium-review.googlesource.com/c/chromium/src/+/1224453 that seems to fix the problem for the OOPIF case ... did you want to give it a spin with your WebView-app?

You might want to try a DCHECK enabled build.
I should note ... my experimental is specific to touch screen gestures, but there's a similar fix that can be enacted for touchpad as well.
I haven't had time to test exhaustively yet, but it does appear to fix the original bug that started this :)
Cc: creis@chromium.org
Note: there is an outstanding issue with Android point transformations that will affect whatever we land to fix this:

https://bugs.chromium.org/p/chromium/issues/detail?id=809122


Cc: -wjmaclean@chromium.org kenrb@chromium.org
Owner: wjmaclean@chromium.org
Status: Assigned (was: Untriaged)
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 4

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

commit 0eb253d276542716a804b6d23090c2f137f1c282
Author: W. James MacLean <wjmaclean@chromium.org>
Date: Thu Oct 04 18:46:53 2018

Make test, BasicSelectionIsolatedIframe, correct.

This CL corrects a deficiency in
TouchSelectionControllerClientAuraSiteIsolationTest.
  BasicSelectionIsolatedIframe
that was discovered in the process of fixing the bug
listed below.

The test was manufacturing its own gesture events,
instead of letting the GestureRecognizer/Provider
framework do it. As a result, these fake gesture
events were routing improperly through
RenderWidgetHostInputEventRouter, and in fact the
test was passing for totally incorrect reasons.

This CL modifies the touch events to be sent through
the Aura event dispatching system, so that proper and
consistent gesture events are inferred from the
touch events.

Bug: 883508
Change-Id: Ic22b402b23fc7ede598821985998d8d3afd31cb1
Reviewed-on: https://chromium-review.googlesource.com/c/1259203
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: James MacLean <wjmaclean@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596780}
[modify] https://crrev.com/0eb253d276542716a804b6d23090c2f137f1c282/content/browser/renderer_host/input/touch_selection_controller_client_aura_browsertest.cc
[modify] https://crrev.com/0eb253d276542716a804b6d23090c2f137f1c282/content/test/data/touch_selection.html

Project Member

Comment 12 by bugdroid1@chromium.org, Oct 4

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

commit c2469abf7b94fa0d0303aa3615899eb4bc3deb36
Author: W. James MacLean <wjmaclean@chromium.org>
Date: Thu Oct 04 20:20:50 2018

Convert coords per touch point with full transforms.

It has long been known that the translation-only model for transforming
point coordinates of Touch and TouchscreenGesture events in
RenderWidgetHostInputEventRouter is insufficient in the case of non-
trivial CSS transformations and OOPIFs; specifically when scale and/or
rotation is involved.

This CL fixes that by using full transforms when they are available via
Viz hit testing.

Bug: 883508
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I53c163b0c942f0eb62d5bee76245003c0f032cfc
Reviewed-on: https://chromium-review.googlesource.com/c/1224453
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: James MacLean <wjmaclean@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596826}
[modify] https://crrev.com/c2469abf7b94fa0d0303aa3615899eb4bc3deb36/content/browser/renderer_host/render_widget_host_input_event_router.cc
[modify] https://crrev.com/c2469abf7b94fa0d0303aa3615899eb4bc3deb36/content/browser/renderer_host/render_widget_host_input_event_router.h
[modify] https://crrev.com/c2469abf7b94fa0d0303aa3615899eb4bc3deb36/content/browser/renderer_host/render_widget_host_view_base.cc
[modify] https://crrev.com/c2469abf7b94fa0d0303aa3615899eb4bc3deb36/content/browser/renderer_host/render_widget_host_view_base.h
[modify] https://crrev.com/c2469abf7b94fa0d0303aa3615899eb4bc3deb36/content/browser/site_per_process_hit_test_browsertest.cc

Sign in to add a comment