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

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug

Blocked on:
issue 489802

Blocking:
issue 599609



Sign in to add a comment

Non-passive touchend listeners shouldn't block scroll

Project Member Reported by rbyers@chromium.org, Apr 9 2016

Issue description

Repro:
 - Use http://rbyers.net/scroll-latency.html
 - Enable 'periodic jank' to 500ms
 - Notice that touch scrolling can be slow to start, but is fixed by enabling 'passive'.
 - Switch the touch listener type to touchend
 - Try scrolling without 'passive', it's also janky

There's no good reason to block scroll start as a result of non-passive touchend listeners (since touchend doesn't block scroll).  I believe this happens because the touch hit rects are a union of all touch listeners.

We've discussed this as outstanding work, but I couldn't find any bug tracking it.

This is bad because it means sites wishing to cancel just touchend (eg. to avoid the mouse events) can't avoid scroll blocking.  Also the touch scroll intervention ( issue 599609 ) can probably only safely effect touchstart and touchmove listners - making touchend events uncancelable on tap would break fastclick libraries (get double click handling).

One possible simple fix is to not include touchend (and touchcancel?) listeners in the touch hit rects at all, and always send touchend events to blink (regardless of listener status).  Since the touchend during a scroll is uncancelable, I don't see a big down side to this.  I guess there's some risk of tap latency being higher when tapping on a location without a touch handler (but on a page that has a touch handler somewhere) - two trips to blink instead of just one.  But I expect that would be minor.

Marking this Pri-1 for M52 as I think it's a pre-requisite to doing any sort of touch scrolling intervention.  Thoughts?
 
Cc: aelias@chromium.org mustaq@chromium.org
Blockedon: 489802
This is related to issue 464721.

Rick's proposed fix sounds like a good interim solution, but eventually (blocked on slimming paint V2?) I think we should consider having finer grained information about where touch event listeners of each type are. 

The first step there would be gathering data about how big the potential benefit would be.

Comment 4 by rbyers@chromium.org, Apr 11 2016

Just to clarify, you're saying we should go ahead and make a change like I propose now without concrete data (since it's more about potential future benefit then any benefit we can measure right now).  Then use metrics to prioritize doing better?

Do you think there's some risk that the change I propose would regress any real-world scenario?  Should we try to be proactive about measuring the risk or just try and see if our metrics move?
Sorry, yes, that's what meant.

I don't think there's significant risk that this would regress a real world scenario. If the main thread is blocked, then we'll delay generation of the click event, but we'd end up delaying the processing of the click event anyways.

The other option here is to do correct rect-based touchend listener tracking. I doubt it would be worth the effort currently. jdduke@ started looking at that here (https://codereview.chromium.org/1055683003/).
Status: Fixed (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 14 2016

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

commit e1504c1965e9ee3f757bda6f8b357781b66fbf22
Author: dtapuska <dtapuska@chromium.org>
Date: Thu Apr 14 19:56:24 2016

Non passive touch end or touch cancel listeners should not block scroll.

Ensure that touch end or touch cancel listeners don't block scroll by
removing these listeners from the touch hit test regions.

When no touch listener is encountered for TouchStart examine the touch
end/cancel listeners and determine if the touch start needs to be
dispatched non-blocking.

BUG= 601990 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review URL: https://codereview.chromium.org/1884863003

Cr-Commit-Position: refs/heads/master@{#387399}

[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/cc/input/event_listener_properties.h
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/cc/proto/layer_tree_host.proto
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/cc/trees/layer_tree_host_unittest.cc
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/content/browser/renderer_host/input/non_blocking_event_browsertest.cc
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/content/renderer/gpu/render_widget_compositor.cc
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/LayoutTests/fast/events/event-listener-on-attribute-inside-shadow-dom-expected.txt
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/LayoutTests/fast/events/event-listener-on-attribute-inside-shadow-dom.html
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/LayoutTests/fast/events/touch/resources/compositor-touch-hit-rects.js
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/LayoutTests/fast/events/touch/touch-action-touch-handlers.html
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/LayoutTests/fast/events/touch/touch-handler-count-expected.txt
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/LayoutTests/fast/events/touch/touch-handler-count.html
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/LayoutTests/fast/events/touch/touch-input-element-change-documents-expected.txt
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/LayoutTests/fast/events/touch/touch-input-element-change-documents.html
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/LayoutTests/fast/events/touch/touch-target-removed-crash.html
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/LayoutTests/plugins/re-request-touch-events-crash-expected.txt
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/LayoutTests/plugins/re-request-touch-events-crash.html
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/Source/core/frame/EventHandlerRegistry.cpp
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/Source/core/frame/EventHandlerRegistry.h
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/Source/core/html/HTMLInputElement.cpp
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/Source/core/input/EventHandler.cpp
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/Source/core/layout/LayoutObject.cpp
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/Source/core/loader/EmptyClients.h
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/Source/core/loader/FrameLoader.cpp
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/Source/core/page/ChromeClient.h
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/Source/core/testing/Internals.cpp
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/Source/core/testing/Internals.h
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/Source/core/testing/Internals.idl
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/Source/web/ChromeClientImpl.cpp
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/Source/web/ChromeClientImpl.h
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/Source/web/WebPagePopupImpl.cpp
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/Source/web/WebPluginContainerImpl.cpp
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/Source/web/tests/ScrollingCoordinatorTest.cpp
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/Source/web/tests/WebViewTest.cpp
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/third_party/WebKit/public/platform/WebEventListenerProperties.h
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/ui/events/blink/input_handler_proxy.cc
[modify] https://crrev.com/e1504c1965e9ee3f757bda6f8b357781b66fbf22/ui/events/blink/input_handler_proxy_unittest.cc

Labels: Merge-Request-51

Comment 9 by tin...@google.com, Apr 20 2016

Labels: -Merge-Request-51 Merge-Approved-51 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M51 (branch: 2704)
Please merge your change to M51 branch 2704 before 5:00 PM PST so we can take it for today's M51 Beta candidate cut. Thank you.
Project Member

Comment 11 by bugdroid1@chromium.org, Apr 20 2016

Labels: -merge-approved-51 merge-merged-2704
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6cf24acafd8d428d56d724bbf6f4f0a460973bec

commit 6cf24acafd8d428d56d724bbf6f4f0a460973bec
Author: Dave Tapuska <dtapuska@chromium.org>
Date: Wed Apr 20 18:26:13 2016

Non passive touch end or touch cancel listeners should not block scroll.

Ensure that touch end or touch cancel listeners don't block scroll by
removing these listeners from the touch hit test regions.

When no touch listener is encountered for TouchStart examine the touch
end/cancel listeners and determine if the touch start needs to be
dispatched non-blocking.

BUG= 601990 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review URL: https://codereview.chromium.org/1884863003

Cr-Commit-Position: refs/heads/master@{#387399}
(cherry picked from commit e1504c1965e9ee3f757bda6f8b357781b66fbf22)

Review URL: https://codereview.chromium.org/1895303007 .

Cr-Commit-Position: refs/branch-heads/2704@{#148}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/cc/input/event_listener_properties.h
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/cc/proto/layer_tree_host.proto
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/cc/trees/layer_tree_host_unittest.cc
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/content/browser/renderer_host/input/non_blocking_event_browsertest.cc
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/content/renderer/gpu/render_widget_compositor.cc
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/LayoutTests/fast/events/event-listener-on-attribute-inside-shadow-dom-expected.txt
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/LayoutTests/fast/events/event-listener-on-attribute-inside-shadow-dom.html
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/LayoutTests/fast/events/touch/resources/compositor-touch-hit-rects.js
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/LayoutTests/fast/events/touch/touch-action-touch-handlers.html
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/LayoutTests/fast/events/touch/touch-handler-count-expected.txt
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/LayoutTests/fast/events/touch/touch-handler-count.html
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/LayoutTests/fast/events/touch/touch-input-element-change-documents-expected.txt
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/LayoutTests/fast/events/touch/touch-input-element-change-documents.html
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/LayoutTests/fast/events/touch/touch-target-removed-crash.html
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/LayoutTests/plugins/re-request-touch-events-crash-expected.txt
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/LayoutTests/plugins/re-request-touch-events-crash.html
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/Source/core/frame/EventHandlerRegistry.cpp
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/Source/core/frame/EventHandlerRegistry.h
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/Source/core/html/HTMLInputElement.cpp
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/Source/core/input/EventHandler.cpp
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/Source/core/layout/LayoutObject.cpp
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/Source/core/loader/EmptyClients.h
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/Source/core/loader/FrameLoader.cpp
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/Source/core/page/ChromeClient.h
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/Source/core/testing/Internals.cpp
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/Source/core/testing/Internals.h
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/Source/core/testing/Internals.idl
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/Source/web/ChromeClientImpl.cpp
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/Source/web/ChromeClientImpl.h
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/Source/web/WebPagePopupImpl.cpp
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/Source/web/WebPluginContainerImpl.cpp
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/Source/web/tests/ScrollingCoordinatorTest.cpp
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/Source/web/tests/WebViewTest.cpp
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/third_party/WebKit/public/platform/WebEventListenerProperties.h
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/ui/events/blink/input_handler_proxy.cc
[modify] https://crrev.com/6cf24acafd8d428d56d724bbf6f4f0a460973bec/ui/events/blink/input_handler_proxy_unittest.cc

Sign in to add a comment