New issue
Advanced search Search tips

Issue 626196 link

Starred by 9 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 3
Type: Feature
Launch-M-Target: 70-Stable
Rollout-Type: Default

Blocked on:
issue 381840
issue 599609
issue 877586

Blocking:
issue 500962



Sign in to add a comment

Make document-level wheel listeners passive by default?

Project Member Reported by rbyers@chromium.org, Jul 7 2016

Issue description

Please provide a link to your UX implementation review here before requesting review.
 
The root document scrolller intervention isn't restricted to touch; it is for all scroll blocking handlers. Do we really need this issue?
Oh, it is?   Issue 599609  says specifically "touch scrolling" - I thought we were worried about also doing wheel because we don't have quite as compelling of a story there (eg. no equivalent for touch-action)?  But yes, if we agree to do wheel at the same time, then yes this can be duped in.
Project Member

Comment 3 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
Status: Available (was: Untriaged)
Putting in the backlog. We will revisit this once touchpad latching is enabled. We may not need to do this if the gains there are more than enough.
Project Member

Comment 5 by sheriffbot@chromium.org, Jul 9

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.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: nzolghadr@chromium.org
Owner: sahel@chromium.org
Status: Assigned (was: Untriaged)
Now that we have latching this might be worth revisiting. Sahel, is this something you can take a look and see whether we might gain a considerable improvement in wheel scrolling or not?
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 27

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

commit 667f001d9507c6ba636c93b1a831f333f5f475d8
Author: Sahel Sharify <sahel@chromium.org>
Date: Fri Jul 27 17:37:27 2018

Add use counter for document level wheel event listeners.

This cl adds use counters to find out the ratio of the web pages that
add document level wheel event listeners with passive:false, true, or
unspecified. It also counts the number of document level wheel event
listeners with passive:unspecified that preventDefault wheel events.

This information is useful to investigate the potential performance
benefits and risks of changing the default value of the passive option
to be true for document level wheel/mousewheel event listeners.

Bug: 626196
Change-Id: Ib0ee64b1a5085813d9b70073747ec3f544fbb660
Reviewed-on: https://chromium-review.googlesource.com/1148783
Commit-Queue: Sahel Sharify <sahel@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578695}
[modify] https://crrev.com/667f001d9507c6ba636c93b1a831f333f5f475d8/third_party/blink/public/platform/web_feature.mojom
[modify] https://crrev.com/667f001d9507c6ba636c93b1a831f333f5f475d8/third_party/blink/renderer/core/dom/events/event_target.cc
[modify] https://crrev.com/667f001d9507c6ba636c93b1a831f333f5f475d8/third_party/blink/renderer/core/dom/events/event_target.h
[modify] https://crrev.com/667f001d9507c6ba636c93b1a831f333f5f475d8/third_party/blink/renderer/core/events/wheel_event.cc
[modify] https://crrev.com/667f001d9507c6ba636c93b1a831f333f5f475d8/third_party/blink/renderer/core/events/wheel_event.h
[modify] https://crrev.com/667f001d9507c6ba636c93b1a831f333f5f475d8/tools/metrics/histograms/enums.xml

Labels: -Type-Bug Type-Launch-OWP
Labels: Launch-M-Target-70-Stable
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 13

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

commit 7400e88b12dbf7142a4927778c983d19adaf72fe
Author: Sahel Sharify <sahel@chromium.org>
Date: Mon Aug 13 17:13:21 2018

Document level passive wheel event listeners behind a flag.

With this change wheel event listeners that are registered on a
window, window.document, window.document.body will be treated as passive
if not specified otherwise.

Intent to impl:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/53q-wRk5QkA

Bug: 626196
Change-Id: I1561cd86ddd381e0405c84ba8f48ea7274d8906c
Reviewed-on: https://chromium-review.googlesource.com/1162302
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Sahel Sharify <sahel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582623}
[modify] https://crrev.com/7400e88b12dbf7142a4927778c983d19adaf72fe/chrome/browser/about_flags.cc
[modify] https://crrev.com/7400e88b12dbf7142a4927778c983d19adaf72fe/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/7400e88b12dbf7142a4927778c983d19adaf72fe/chrome/browser/flag_descriptions.h
[add] https://crrev.com/7400e88b12dbf7142a4927778c983d19adaf72fe/content/browser/renderer_host/input/wheel_event_listener_browsertest.cc
[modify] https://crrev.com/7400e88b12dbf7142a4927778c983d19adaf72fe/content/child/runtime_features.cc
[modify] https://crrev.com/7400e88b12dbf7142a4927778c983d19adaf72fe/content/public/common/content_features.cc
[modify] https://crrev.com/7400e88b12dbf7142a4927778c983d19adaf72fe/content/public/common/content_features.h
[modify] https://crrev.com/7400e88b12dbf7142a4927778c983d19adaf72fe/content/test/BUILD.gn
[modify] https://crrev.com/7400e88b12dbf7142a4927778c983d19adaf72fe/third_party/blink/public/platform/web_runtime_features.h
[modify] https://crrev.com/7400e88b12dbf7142a4927778c983d19adaf72fe/third_party/blink/renderer/core/dom/events/event_target.cc
[modify] https://crrev.com/7400e88b12dbf7142a4927778c983d19adaf72fe/third_party/blink/renderer/core/events/wheel_event.cc
[modify] https://crrev.com/7400e88b12dbf7142a4927778c983d19adaf72fe/third_party/blink/renderer/platform/exported/web_runtime_features.cc
[modify] https://crrev.com/7400e88b12dbf7142a4927778c983d19adaf72fe/third_party/blink/renderer/platform/runtime_enabled_features.json5
[modify] https://crrev.com/7400e88b12dbf7142a4927778c983d19adaf72fe/tools/metrics/histograms/enums.xml

Project Member

Comment 11 by bugdroid1@chromium.org, Aug 22

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

commit 9187c7d059b7aa6342b39a19c7c32ba5aedd92cf
Author: Sahel Sharify <sahel@chromium.org>
Date: Wed Aug 22 15:08:18 2018

Add PassiveDocumentWheelEventListeners to fieldtrial_testing_config

Please find the intent ot implement here:
https://groups.google.com/a/chromium.org/forum/?hl=en#!topic/blink-dev/53q-wRk5QkA

Bug: 626196
Change-Id: Id77ceb7352629a6a9417617602d330919904ed44
Reviewed-on: https://chromium-review.googlesource.com/1183538
Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Sahel Sharify <sahel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585020}
[modify] https://crrev.com/9187c7d059b7aa6342b39a19c7c32ba5aedd92cf/testing/variations/fieldtrial_testing_config.json

Labels: -OS-All OS-Android OS-Chrome OS-Fuchsia OS-Linux OS-Mac OS-Windows
Explicitly selecting platforms, excluding iOS, per the Finch config.
Thanks
Blockedon: 877586
The change in comment #11 caused a performance regression. See  bug 877586 .
Labels: -Type-Launch-OWP Type-Launch
Labels: MovedFromTypeLaunchOWP
Labels: Rollout-Type-Default
Bulk Edit: Updating Rollout-Type to Default since no Launch/Target Exp bits are set. 
Please update Rollout-Type to Finch and include Launch/Target Exp bits if not intended be launched as default  

Description: Show this description
Description: Show this description
Description: Show this description
Description: Show this description
Description: Show this description
Description: Show this description
Description: Show this description
Labels: -Type-Launch Type-FLT-Launch FLT-Conversion
Stable-M-Target: 70
Automatic generating of FLT Launch data.
Labels: Restrict-View-Google
Labels: FLT-Conversion-1
Updating this issue's FLT-Conversion label.
Please do not modify this value.
Labels: Type-Feature
Labels: -Restrict-View-Google
Bulk updating old OWP-Launch bugs to once again be public. OWP-Launch (previously used as public/transparent blink launch process) was not supposed to have been remapped to Type=Launch.
Labels: Type-FLT-Launch
Preparing this issue to be converted back into the old format.

Will change Type back to 'Feature' after.
Labels: FLT-Conversion
Labels: Type-Feature
Labels: Type-FLT-Launch
Labels: Type-Feature
Project Member

Comment 35 by bugdroid1@chromium.org, Dec 20

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

commit f177dfeb50648a57ab331d9f7619a9ba4c1aeb87
Author: Sahel Sharify <sahel@chromium.org>
Date: Thu Dec 20 19:56:58 2018

WPT for passive document level wheel event listeners.

This cl adds a WPT test for passive document level wheel event listeners
intervention. It also adds a message to let developers know when they call
preventDefault on wheel events that are non-cancelable due to the intervention.

Bug: 626196
Change-Id: I7f51ff4560de7ffa7bf89cab9e5332daa6cb70c0
Reviewed-on: https://chromium-review.googlesource.com/c/1383323
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org>
Commit-Queue: Sahel Sharify <sahel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#618297}
[modify] https://crrev.com/f177dfeb50648a57ab331d9f7619a9ba4c1aeb87/third_party/blink/renderer/core/events/wheel_event.cc
[modify] https://crrev.com/f177dfeb50648a57ab331d9f7619a9ba4c1aeb87/third_party/blink/web_tests/TestExpectations
[add] https://crrev.com/f177dfeb50648a57ab331d9f7619a9ba4c1aeb87/third_party/blink/web_tests/external/wpt/dom/events/document-level-wheel-event-listener-passive-by-default-manual.html
[add] https://crrev.com/f177dfeb50648a57ab331d9f7619a9ba4c1aeb87/third_party/blink/web_tests/external/wpt_automation/dom/events/document-level-wheel-event-listener-passive-by-default-manual-automation.js
[modify] https://crrev.com/f177dfeb50648a57ab331d9f7619a9ba4c1aeb87/third_party/blink/web_tests/resources/testharnessreport.js

Sign in to add a comment