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 3 users
Status: Fixed
Owner:
█ kenjibaheux@chromium.org
Last visit > 30 days ago
Closed: Nov 2016
Cc:
█ yiyaoliu@chromium.org
rbyers@chromium.org
dtapu...@chromium.org
tdres...@chromium.org
█ kenjibaheux@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: ----
Type: Launch-OWP
Launch-Accessibility: NA
Launch-Exp-Leadership: ----
Launch-Leadership: ----
Launch-Legal: NotReviewed
Launch-M-Approved: 55-Dev , 56-Dev , 56-Beta , 56-Stable
Launch-M-Target: 55-Dev , 56-Dev , 56-Beta , 56-Stable
Launch-Privacy: NotReviewed
Launch-Security: NotReviewed
Launch-Test: NotReviewed
Launch-UI: NA

Blocked on:
issue 627104
issue 639227


Sign in to add a comment
link

Issue 625675: Passive Document Level Event Listeners Intervention

Reported by dtapu...@chromium.org, Jul 4 2016 Project Member 
Feature description:
Root Document Scroller Passive Event Listener Intervention is the forcing of 'passive' to default to true for root level DOM elements.


Eng owner: dtapuska@chromium.org
Product owner: kenjibaheux@chromium.org 

Mocks: N/A
Design doc (send to chrome-design-docs@): https://docs.google.com/document/d/1II7oSIpd8pK91V5kEM3tDLKcIj398jOJn8Niqy6_loI/edit

Finch/experimentation: See Design Doc.


Accessibility survey: The accessibility survey is included in a review bug
that is automatically filed when you update this bug to Launch-Status-
Review-Requested. Please answer all questions there.

Legal survey: Email ctanaka@ (for non-Chrome OS) or scottgerwin@ and
jlchen@ (for Chrome OS) to request a legal review.

Privacy survey: The privacy survey is included in a review bug that is
automatically filed when you update this bug to Launch-Status-Review-
Requested. Please answer all questions there.

<b>Security survey: <https://goto.google.com/chrome-security-questions></b>

<b>Test survey: <https://goto.google.com/chrome-test-questions></b>

UI survey: Email chrome-ui-review@ (for non-Chrome OS) or chromeos-ui-
review@ (for Chrome OS) to request a UI review.

Comment 1 by kenjibaheux@chromium.org, Jul 5 2016

Labels: -Launch-UI-NotReviewed -Launch-Accessibility-NotReviewed Launch-Accessibility-NA Launch-UI-NA
Not a UI facing change => NA.
Should not impact A11Y => NA.

Dave, what's the earliest milestone we can aim for?
I'm assuming that we'll do a finch experiment to observe the benefit and any breakage.

Does the following plan look reasonable?
 1. Canary/Dev: 50/50
 2. Beta: 50/50
 3. Stable: 1%
 4. Stable: launch

Which metrics do we expect to see moving?
 - Event.Latency.BlockingTime.TouchStartDefaultAllowed ?
 - ?

Comment 2 by kenjibaheux@chromium.org, Jul 5 2016 

Nevermind, the design doc had the info I was looking for:

"We expect to see a significant reduction in input latency due to this intervention, reflected in the input team’s end to end scroll latency metrics. In particular, we expect to see a drop of over 10% from the 99th percentile of Event.Latency.TouchToFirstScrollUpdateSwapBegin on Nexus 5."


"We also expect to see at least a 10% decrease in the percentage of events which are cancellable but not cancelled, which will be reflected in the Event.PassiveListeners metric."


Experiment: "50% of users in Canary/Dev" was mentioned.

Comment 3 by kenjibaheux@chromium.org, Jul 5 2016

Owner: kenjibaheux@chromium.org
Self assigning: I'll take care of the launch process.

Comment 4 by dtapu...@chromium.org, Jul 6 2016 

I have a change ready to go for the canary/dev finch server configuration. It is just waiting the chromium side change to land so that I can put a proper min version number in the change.

Will let you know when I get it up for review.

Comment 5 by bugdroid1@chromium.org, Jul 6 2016

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

commit acc2fb2eaf19f06817b8ef90b64b195d032a662a
Author: dtapuska <dtapuska@chromium.org>
Date: Wed Jul 06 23:26:23 2016

Add option to force document level passive event listeners as a runtime feature.

This approach removes the value from the override setting and allows it
to be set as a runtime feature.

Since field trial variations aren't supported in the renderer and only
the browser process adding an additional runtime enabled feature seems
the easiest approach forward.

BUG= 625675 

Review-Url: https://codereview.chromium.org/2125683002
Cr-Commit-Position: refs/heads/master@{#403972}

[modify] https://crrev.com/acc2fb2eaf19f06817b8ef90b64b195d032a662a/chrome/app/generated_resources.grd
[modify] https://crrev.com/acc2fb2eaf19f06817b8ef90b64b195d032a662a/chrome/browser/about_flags.cc
[modify] https://crrev.com/acc2fb2eaf19f06817b8ef90b64b195d032a662a/content/child/runtime_features.cc
[modify] https://crrev.com/acc2fb2eaf19f06817b8ef90b64b195d032a662a/content/public/common/content_features.cc
[modify] https://crrev.com/acc2fb2eaf19f06817b8ef90b64b195d032a662a/content/public/common/content_features.h
[modify] https://crrev.com/acc2fb2eaf19f06817b8ef90b64b195d032a662a/content/renderer/render_view_impl.cc
[modify] https://crrev.com/acc2fb2eaf19f06817b8ef90b64b195d032a662a/third_party/WebKit/Source/core/events/AddEventListenerOptionsDefaults.h
[modify] https://crrev.com/acc2fb2eaf19f06817b8ef90b64b195d032a662a/third_party/WebKit/Source/core/events/EventTarget.cpp
[modify] https://crrev.com/acc2fb2eaf19f06817b8ef90b64b195d032a662a/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in
[modify] https://crrev.com/acc2fb2eaf19f06817b8ef90b64b195d032a662a/third_party/WebKit/Source/web/AssertMatchingEnums.cpp
[modify] https://crrev.com/acc2fb2eaf19f06817b8ef90b64b195d032a662a/third_party/WebKit/Source/web/WebRuntimeFeatures.cpp
[modify] https://crrev.com/acc2fb2eaf19f06817b8ef90b64b195d032a662a/third_party/WebKit/public/web/WebRuntimeFeatures.h
[modify] https://crrev.com/acc2fb2eaf19f06817b8ef90b64b195d032a662a/third_party/WebKit/public/web/WebSettings.h
[modify] https://crrev.com/acc2fb2eaf19f06817b8ef90b64b195d032a662a/tools/metrics/histograms/histograms.xml

Comment 6 by bugdroid1@chromium.org, Jul 8 2016

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

commit cba808f5a6b5b553da23c5612418ba6e8ce486a8
Author: dtapuska <dtapuska@chromium.org>
Date: Fri Jul 08 09:40:38 2016

Limit PassiveDocumentEventListeners to touch and make it experimental

Make sure we don't change the default for wheel listeners for document
elements so let the feature only work on touchstart and touchmove
listeners.

Enable it as experimental as well.

BUG= 625675 

Review-Url: https://codereview.chromium.org/2127163002
Cr-Commit-Position: refs/heads/master@{#404332}

[modify] https://crrev.com/cba808f5a6b5b553da23c5612418ba6e8ce486a8/chrome/app/generated_resources.grd
[modify] https://crrev.com/cba808f5a6b5b553da23c5612418ba6e8ce486a8/content/child/runtime_features.cc
[modify] https://crrev.com/cba808f5a6b5b553da23c5612418ba6e8ce486a8/third_party/WebKit/LayoutTests/fast/events/touch/compositor-touch-hit-rects-global.html
[modify] https://crrev.com/cba808f5a6b5b553da23c5612418ba6e8ce486a8/third_party/WebKit/LayoutTests/fast/events/touch/resources/compositor-touch-hit-rects-iframe-doc.html
[modify] https://crrev.com/cba808f5a6b5b553da23c5612418ba6e8ce486a8/third_party/WebKit/LayoutTests/fast/events/touch/resources/compositor-touch-hit-rects.js
[modify] https://crrev.com/cba808f5a6b5b553da23c5612418ba6e8ce486a8/third_party/WebKit/LayoutTests/fast/events/touch/touch-event-cancelable.html
[modify] https://crrev.com/cba808f5a6b5b553da23c5612418ba6e8ce486a8/third_party/WebKit/LayoutTests/fast/events/touch/touch-target-move-documents.html
[modify] https://crrev.com/cba808f5a6b5b553da23c5612418ba6e8ce486a8/third_party/WebKit/LayoutTests/fullscreen/compositor-touch-hit-rects-fullscreen-video-controls.html
[modify] https://crrev.com/cba808f5a6b5b553da23c5612418ba6e8ce486a8/third_party/WebKit/LayoutTests/inspector-protocol/input/emulateTouchFromMouseEvent.html
[modify] https://crrev.com/cba808f5a6b5b553da23c5612418ba6e8ce486a8/third_party/WebKit/Source/core/events/EventTarget.cpp
[modify] https://crrev.com/cba808f5a6b5b553da23c5612418ba6e8ce486a8/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in
[modify] https://crrev.com/cba808f5a6b5b553da23c5612418ba6e8ce486a8/third_party/WebKit/Source/web/tests/FrameThrottlingTest.cpp
[modify] https://crrev.com/cba808f5a6b5b553da23c5612418ba6e8ce486a8/third_party/WebKit/Source/web/tests/data/touch-event-handler.html

Comment 8 by dtapu...@chromium.org, Jul 11 2016

Blockedon: 627104

Comment 9 by bugdroid1@chromium.org, Jul 13 2016

Project Member 
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/38e3bf349d518922038c1918bd76480bd2924fc4

commit 38e3bf349d518922038c1918bd76480bd2924fc4
Author: dtapuska <dtapuska@chromium.org>
Date: Wed Jul 13 19:53:39 2016

Revert of Enable PassiveDocumentEventListeners on waterfall. (patchset #2 id:20001 of https://codereview.chromium.org/2119403002/ )

Reason for revert:
Revert this for now. We are seeing cc:scheduler changes as a result of this and we need to understand them better and I'm not going to be in the office the next two days.

Original issue's description:
> Enable PassiveDocumentEventListeners on waterfall.
>
> Enable passive document event listeners finch experimentl on waterfall.
>
> BUG= 625675 
> R=rkaplow@chromium.org
>
> Committed: https://crrev.com/19233086ad7e2363e7e1d14582dc6a5428816169
> Cr-Commit-Position: refs/heads/master@{#404351}

TBR=rkaplow@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 625675 

Review-Url: https://codereview.chromium.org/2149673002
Cr-Commit-Position: refs/heads/master@{#405240}

[modify] https://crrev.com/38e3bf349d518922038c1918bd76480bd2924fc4/testing/variations/fieldtrial_testing_config_android.json
[modify] https://crrev.com/38e3bf349d518922038c1918bd76480bd2924fc4/testing/variations/fieldtrial_testing_config_chromeos.json
[modify] https://crrev.com/38e3bf349d518922038c1918bd76480bd2924fc4/testing/variations/fieldtrial_testing_config_linux.json
[modify] https://crrev.com/38e3bf349d518922038c1918bd76480bd2924fc4/testing/variations/fieldtrial_testing_config_mac.json
[modify] https://crrev.com/38e3bf349d518922038c1918bd76480bd2924fc4/testing/variations/fieldtrial_testing_config_win.json

Comment 10 by bugdroid1@chromium.org, Jul 18 2016

Project Member 
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7a44b9eb2df493236df608d21681b563dbd74539

commit 7a44b9eb2df493236df608d21681b563dbd74539
Author: dtapuska <dtapuska@chromium.org>
Date: Mon Jul 18 15:47:21 2016

Add UMA metrics for root scroller intervention to track forcing passive breakage.

Calculate the number of event listener invocations that we didn't actually
end up allowing preventDefault to actually execute. The denominator
in this metric is the total number of invocations that we force passive
on.

BUG= 625675 

Review-Url: https://codereview.chromium.org/2137483003
Cr-Commit-Position: refs/heads/master@{#406002}

[modify] https://crrev.com/7a44b9eb2df493236df608d21681b563dbd74539/third_party/WebKit/Source/core/core.gypi
[add] https://crrev.com/7a44b9eb2df493236df608d21681b563dbd74539/third_party/WebKit/Source/core/events/AddEventListenerOptionsResolved.cpp
[add] https://crrev.com/7a44b9eb2df493236df608d21681b563dbd74539/third_party/WebKit/Source/core/events/AddEventListenerOptionsResolved.h
[modify] https://crrev.com/7a44b9eb2df493236df608d21681b563dbd74539/third_party/WebKit/Source/core/events/Event.cpp
[modify] https://crrev.com/7a44b9eb2df493236df608d21681b563dbd74539/third_party/WebKit/Source/core/events/Event.h
[modify] https://crrev.com/7a44b9eb2df493236df608d21681b563dbd74539/third_party/WebKit/Source/core/events/EventListenerMap.cpp
[modify] https://crrev.com/7a44b9eb2df493236df608d21681b563dbd74539/third_party/WebKit/Source/core/events/EventListenerMap.h
[modify] https://crrev.com/7a44b9eb2df493236df608d21681b563dbd74539/third_party/WebKit/Source/core/events/EventTarget.cpp
[modify] https://crrev.com/7a44b9eb2df493236df608d21681b563dbd74539/third_party/WebKit/Source/core/events/EventTarget.h
[add] https://crrev.com/7a44b9eb2df493236df608d21681b563dbd74539/third_party/WebKit/Source/core/events/EventTargetTest.cpp
[modify] https://crrev.com/7a44b9eb2df493236df608d21681b563dbd74539/third_party/WebKit/Source/core/events/RegisteredEventListener.h
[modify] https://crrev.com/7a44b9eb2df493236df608d21681b563dbd74539/third_party/WebKit/Source/core/svg/SVGElement.cpp
[modify] https://crrev.com/7a44b9eb2df493236df608d21681b563dbd74539/third_party/WebKit/Source/modules/mediastream/MediaStream.cpp
[modify] https://crrev.com/7a44b9eb2df493236df608d21681b563dbd74539/third_party/WebKit/Source/modules/mediastream/MediaStream.h
[modify] https://crrev.com/7a44b9eb2df493236df608d21681b563dbd74539/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerGlobalScope.cpp
[modify] https://crrev.com/7a44b9eb2df493236df608d21681b563dbd74539/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerGlobalScope.h
[modify] https://crrev.com/7a44b9eb2df493236df608d21681b563dbd74539/tools/metrics/histograms/histograms.xml

Comment 11 by bugdroid1@chromium.org, Jul 27 2016

Project Member 
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/23914fdde83b0b7f763f34f00ff9fbba5d1a7611

commit 23914fdde83b0b7f763f34f00ff9fbba5d1a7611
Author: dtapuska <dtapuska@chromium.org>
Date: Wed Jul 27 16:49:51 2016

Turn off root scroller intervention on key_silk_cases page.

The http://s.codepen.io/befamous/fullpage/pFsqb?scroll pageset
seems to call preventDefault on every touchmove so to not cause
a regression with our root scroller intervention the page really
should articulate that it needs touch-action:none.

BUG= 625675 

Review-Url: https://codereview.chromium.org/2181413005
Cr-Commit-Position: refs/heads/master@{#408154}

[modify] https://crrev.com/23914fdde83b0b7f763f34f00ff9fbba5d1a7611/tools/perf/page_sets/key_silk_cases.py

Comment 13 by tdres...@chromium.org, Aug 15 2016

Labels: Hotlist-Input-Dev
rbyers@ found this case that breaks with this intervention:
http://m.soundtransit.org/schedules?direction=outbound#40_100479

"Click on "view map", can't really pan the map vertically - scrolls instead.  I expect some breakage like this and it's still probably the right choice, but we need to keep an eye on it and publish good guidance."

(https://bugs.chromium.org/p/chromium/issues/detail?id=599609#c23)

Comment 14 by tdres...@chromium.org, Aug 18 2016

Labels: Hotlist-Interventions

Comment 15 by dtapu...@chromium.org, Aug 18 2016

Summary: Passive Document Level Event Listeners Intervention Experiment (was: Root Document Scrollers Passive Event Listener Intervention Experiment)

Comment 16 by kenjibaheux@chromium.org, Aug 19 2016

Blockedon: 639227
For the benefits of the community, I made a public bug: https://bugs.chromium.org/p/chromium/issues/detail?id=639227

CL and general updates should be posted against it so that external folks can track progress.

Comment 17 by dtapu...@chromium.org, Nov 14 2016

Labels: -Type-Launch Launch-M-Approved-55-Dev Launch-M-Approved-56-Dev Launch-M-Approved-56-Beta Launch-M-Approved-56-Stable Launch-M-Target-55-Dev Launch-M-Target-56-Dev Launch-M-Target-56-Beta Type-Launch-OWP
Approved Intent to Intervene:

https://groups.google.com/a/chromium.org/d/msg/blink-dev/BW3qrkisqIs/v5Au-HVTAwAJ

Comment 19 by dtapu...@chromium.org, Nov 18 2016

Status: Fixed (was: Assigned)

Comment 20 by dtapu...@chromium.org, Nov 18 2016

Labels: Launch-M-Target-56-Stable

Comment 21 by rbyers@chromium.org, Aug 10 2017

Summary: Passive Document Level Event Listeners Intervention (was: Passive Document Level Event Listeners Intervention Experiment)
WebKit bug: https://bugs.webkit.org/show_bug.cgi?id=175346
►

Sign in to add a comment