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

Issue 340060 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Mar 2014
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 260732
issue 241964



Sign in to add a comment

Make touch-action: none disable the touch-ACK timeout

Project Member Reported by rbyers@chromium.org, Feb 1 2014

Issue description

Chrome Version       : 34.0.1809.2
OS Version: 5339.2.0

With scrolling disabled (touch-action: none) there's no value in having the touch ACK timeout (since there's no non-main-thread action to do with the events).  We'll mostly get this benefit "for free" with touch-action since it's the touchcancel that happens as a result of scrolling that causes all the problems (see issue 260732).

However, I think we should wire it up to explicitly disable the timeout and add tests to ensure we don't loose this.  First, I want to make sure we've got a good story for people being bit by the timeout problem.  Secondly the timeout could still be an issue in some edge cases (eg. cause unexpected click events).

Jared, feel free to take this bug if you like (or I can do it).

 
Blocking: chromium:241964
Blocking: chromium:260732

Comment 3 by rbyers@chromium.org, Feb 18 2014

I'm not sure if we should really block shipping touch-action ( issue 241964 ) on this or not - seems nice to have.  If it's easy, we should try to get this into M-35.

I think the right way to do this is to have InputRouterImpl::OnSetTouchAction check for the 'none' case and temporarily tell the TouchEventQueue to disable the ACK timeout for the current touch sequence.  We previously allowed the ACK timeout to change as a result of the viewport changing, but I'm not sure if we've tested all the boundary cases (eg. viewport changing when there is a queued touch event - does it get the old timeout policy or the new?).

Comment 4 by jdduke@chromium.org, Feb 18 2014

If the viewport changes to "mobile", the timeout is disabled even if there's a pending touch event.  If it changes to "desktop", the timeout will be enabled for the next potential touchstart or touchmove.  It shouldn't be too crazy to wire in some additional logic for touch-action.

Comment 5 by jdduke@chromium.org, Feb 28 2014

Cc: -jdduke@chromium.org rbyers@chromium.org
Owner: jdduke@chromium.org
I should note that this in itself isn't sufficient to give developers full control over the ACK timeout.  SetTouchAction currently comes only from the renderer main thread, so when the main thread is janky then the timeout may still be fired before the browser knows about the touch-action for the touch.

But eventually we plan to implement impl-thread touch-action hit testing ( issue 318381 ), and then it should be unlikely that the SetTouchAction call will block on the main thread.

Nevertheless, it'll probably still be valuable to disable the timeout.  The SetTouchAction call comes before running any JS handlers for the event, so touch-action: none will still protect the developer from the case when their event handler is slow to complete. 
Project Member

Comment 7 by bugdroid1@chromium.org, Mar 6 2014

------------------------------------------------------------------------
r255304 | jdduke@chromium.org | 2014-03-06T10:48:11.255235Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/input/touch_action_filter.h?r1=255304&r2=255303&pathrev=255304
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/input/touch_event_queue.cc?r1=255304&r2=255303&pathrev=255304
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/input/input_router_impl_unittest.cc?r1=255304&r2=255303&pathrev=255304
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/input/input_router_impl.cc?r1=255304&r2=255303&pathrev=255304
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/input/input_router_impl.h?r1=255304&r2=255303&pathrev=255304

Disable the touch ack timeout for touch-action:none

When the touch-action for the current touch sequence is updated, also update
whether the touch ack time is disabled based on the allowed touch-action. This
provides a straightforward knob for developers to disable the timeout on desktop
sites.

BUG= 340060 

Review URL: https://codereview.chromium.org/183923034
------------------------------------------------------------------------
Status: Fixed
rbyers@ would you care to verify?
Status: Assigned
Hmm, doesn't look like it's quite behaving as we intended.  Repro:
Set "Enable experimental web platform features" in chrome://flags
http://www.rbyers.net/touch-timeout.html
Use the default settings (delay first touchmove, consume touchmove)
Touch and drag on 'normal div', see that you get a touchcancel (timeout firing)
Now touch and drag on the 'touch-action: none' div.
The first time you'll get the touchcancel as well, but if you try again NOW you'll get all the move events.
So the timeout is being disabled for the NEXT touch sequence as we intended, but not for the current one.

If we were delaying the touchstart then I'd expect this (due to lack of impl-thread touch-action hit testing), but this test only delays the touchmove.

Jared, can you take a look please?  I also had touch move absorption enabled in my build, but I don't think it should affect this scenario.
Yeah, I see what's going on, I'll post a fix.
Project Member

Comment 11 by bugdroid1@chromium.org, Mar 7 2014

------------------------------------------------------------------------
r255555 | jdduke@chromium.org | 2014-03-07T08:55:34.340510Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/input/touch_event_queue.h?r1=255555&r2=255554&pathrev=255555
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/input/input_router_impl_unittest.cc?r1=255555&r2=255554&pathrev=255555
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/input/input_router_impl.cc?r1=255555&r2=255554&pathrev=255555
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/input/input_router_impl.h?r1=255555&r2=255554&pathrev=255555
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/input/touch_event_queue_unittest.cc?r1=255555&r2=255554&pathrev=255555
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/input/touch_event_queue.cc?r1=255555&r2=255554&pathrev=255555

[Android] Properly disable the touch ack timeout during a touch sequence

This issue was recently uncovered by the interaction of touch-action:none
when disabling the touch timeout.  In that case, the disable command did not
actually take effect until the following touch sequence (if one was active).
Instead, forcefully disable the timeout/timer for the current touch sequence
when the TouchEventQueue is commanded to do so.

BUG= 340060 

Review URL: https://codereview.chromium.org/188833004
------------------------------------------------------------------------
Status: Fixed
Status: Verified

Sign in to add a comment