New issue
Advanced search Search tips

Issue 736623 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug



Sign in to add a comment

Touch Events are not dispatched to the correct webview

Reported by nathan.f...@nike.com, Jun 25 2017

Issue description

UserAgent: Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.87 Safari/537.36

Steps to reproduce the problem:
For setup, you'll need a renderer and a webview within it.  The renderer must contain something (we'll say a button) that prevents default.
1. Tap the prevent default button in the webview x times
2. Tap something in the main renderer x+1 times
3. Tap something in the webview x+1 times

What is the expected behavior?
All tap events are sent to the correct context

What went wrong?
In step 2, the first x taps are not dispatched to the outer render
In step 3, the first x taps are not dispatched to the webview

Did this work before? N/A 

Chrome version: 52.0.2743.82  Channel: n/a
OS Version: 6.3
Flash Version: 

Originall reported in 631484.
I have a patch now that I will submit shortly.

This issue is caused by the interaction between the touchscreen_gesture_target_queue_ and prevent default.  The target is calculated on TouchStart, and pushed to the the queue.  It is consumed and popped off on its corresponding GestureTapDown.  However, if the TouchStart is has default behavior prevented, no GestureTapDown will occur, and the minimum queue size is now +1.  The next event that does receive a GestureTapDown will use this item in the queue, even if the surface is not correct--and it's correct surface will be added to the queue for later consumption, by an event that may not have that target.
 

Comment 1 by kenrb@chromium.org, Jun 26 2017

Cc: kenrb@chromium.org wjmaclean@chromium.org dtapu...@chromium.org
My first thought would be that maybe this can be covered by RenderWidgetHostView*::ProcessAckedTouchEvent(), which knows if the touch event had preventDefault called on it, but I think dtapuska@ said something about trying to get rid of event acks.
That API will still exist so it should still be fine. Acks are moving to OnceCallbacks across the IPC channel when we move to mojo. Which deprecates the term really "ACK".
Thank you for the help; I've updated the change accordingly https://chromium-review.googlesource.com/c/547669/
Labels: Needs-Milestone
Cc: jmukthavaram@chromium.org
Labels: Needs-Feedback
Nathan.fairhurst@,

Could you please confirm any work is pending on this issue?
Thanks in advance..!!

Comment 6 by kenrb@chromium.org, Jun 29 2017

Status: Untriaged (was: Unconfirmed)
#5: There is active work on the gerrit CL linked in comment #3.
Components: Platform>Apps>BrowserTag
Owner: wjmaclean@chromium.org
Status: Assigned (was: Untriaged)
Assigning to James to manage the review Nathan has provided.
Project Member

Comment 8 by bugdroid1@chromium.org, Jul 21 2017

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

commit 09a2f88ea04da4884e9d3c3820385c5cc8879c32
Author: Nathan Fairhurst <nathan.fairhurst@nike.com>
Date: Fri Jul 21 21:39:14 2017

Fix an issue where touch events were dispatched to the incorrect webview

Was caused by touchscreen_gesture_target_queue_ + prevent default.  The
webview target was calculated on TouchStart and pushed to the the queue.
It was removed on its corresponding GestureTapDown.  However, if the
TouchStart had default behavior prevented, no GestureTapDown would ever
occur, and the queue would forever have an extra element.

Fixed by converting touchscreen_gesture_target_queue_ to a map with
unique_touch_event_id as keys and gesture target data as values.

Also updates AUTHORS file.

Bug:  736623 ,  739831 
Change-Id: If3bf844e2bcfb9a1ca6d6a56cdc7575e0767e6b6
Reviewed-on: https://chromium-review.googlesource.com/547669
Commit-Queue: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488759}
[modify] https://crrev.com/09a2f88ea04da4884e9d3c3820385c5cc8879c32/AUTHORS
[modify] https://crrev.com/09a2f88ea04da4884e9d3c3820385c5cc8879c32/content/browser/renderer_host/render_widget_host_input_event_router.cc
[modify] https://crrev.com/09a2f88ea04da4884e9d3c3820385c5cc8879c32/content/browser/renderer_host/render_widget_host_input_event_router.h
[modify] https://crrev.com/09a2f88ea04da4884e9d3c3820385c5cc8879c32/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/09a2f88ea04da4884e9d3c3820385c5cc8879c32/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/09a2f88ea04da4884e9d3c3820385c5cc8879c32/content/browser/site_per_process_browsertest.cc
[add] https://crrev.com/09a2f88ea04da4884e9d3c3820385c5cc8879c32/content/test/data/page_with_touch_start_default_prevented.html

Comment 9 by kenrb@chromium.org, Jul 25 2017

Status: Fixed (was: Assigned)
Project Member

Comment 10 by bugdroid1@chromium.org, Jul 26 2017

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

commit b7aaf6ac529ec489923e875c9d8b67c9d2f6b54a
Author: W. James MacLean <wjmaclean@chromium.org>
Date: Wed Jul 26 12:31:34 2017

Fix an issue where touch events were dispatched to the incorrect webview

Was caused by touchscreen_gesture_target_queue_ + prevent default.  The
webview target was calculated on TouchStart and pushed to the the queue.
It was removed on its corresponding GestureTapDown.  However, if the
TouchStart had default behavior prevented, no GestureTapDown would ever
occur, and the queue would forever have an extra element.

Fixed by converting touchscreen_gesture_target_queue_ to a map with
unique_touch_event_id as keys and gesture target data as values.

Also updates AUTHORS file.

TBR=nathan.fairhurst@nike.com

(cherry picked from commit 09a2f88ea04da4884e9d3c3820385c5cc8879c32)

Bug:  736623 ,  739831 
Change-Id: If3bf844e2bcfb9a1ca6d6a56cdc7575e0767e6b6
Reviewed-on: https://chromium-review.googlesource.com/547669
Commit-Queue: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#488759}
Reviewed-on: https://chromium-review.googlesource.com/586747
Cr-Commit-Position: refs/branch-heads/3163@{#54}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/b7aaf6ac529ec489923e875c9d8b67c9d2f6b54a/AUTHORS
[modify] https://crrev.com/b7aaf6ac529ec489923e875c9d8b67c9d2f6b54a/content/browser/renderer_host/render_widget_host_input_event_router.cc
[modify] https://crrev.com/b7aaf6ac529ec489923e875c9d8b67c9d2f6b54a/content/browser/renderer_host/render_widget_host_input_event_router.h
[modify] https://crrev.com/b7aaf6ac529ec489923e875c9d8b67c9d2f6b54a/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/b7aaf6ac529ec489923e875c9d8b67c9d2f6b54a/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/b7aaf6ac529ec489923e875c9d8b67c9d2f6b54a/content/browser/site_per_process_browsertest.cc
[add] https://crrev.com/b7aaf6ac529ec489923e875c9d8b67c9d2f6b54a/content/test/data/page_with_touch_start_default_prevented.html

Sign in to add a comment