Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(9)

Issue 784453003: Initial scroll-blocks-on compositor integration (Closed)

Created:
6 years ago by Rick Byers
Modified:
5 years, 4 months ago
Reviewers:
Ian Vollick, Nico, skobes
CC:
blink-reviews, krit, blink-reviews-rendering, Rik, jbroman, zoltan1, blink-reviews-css, pdr+renderingwatchlist_chromium.org, eae+blinkwatch, ed+blinkwatch_opera.com, leviw+renderwatch, danakj, blink-layers+watch_chromium.org, dglazkov+blink, pdr+graphicswatchlist_chromium.org, apavlov+blink_chromium.org, jchaffraix+rendering, darktears, f(malita), Stephen Chennney, rwlbuis
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Initial scroll-blocks-on compositor integration Causes elements with scroll-blocks-on set to be promoted to a composited layer and plumbs the value through to WebLayer. By default (for compatibility) we must block on start-touch and wheel-event. In order to allow that to be explicitly disabled we apply it using a user-agent stylesheet. We're careful not to create any additional RenderLayers (and so composited layers) when only these defaults are present. Consumed by cc in https://codereview.chromium.org/784463002/ Promotes scroll-blocks-on support to "experimental" status. Note that a number of things are required before we'd consider shipping this feature, including a mitigation mechanism for super-janky pages (scroll block timeout and failure notification). Intent to implement: https://groups.google.com/a/chromium.org/forum/#!topic/input-dev/N3YrsrHus-k Test/demo page: http://rbyers.net/scroll-blocks-on.html BUG=347272 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=188230

Patch Set 1 #

Patch Set 2 : Add user agent stylesheet rule #

Patch Set 3 : Merge with trunk #

Patch Set 4 : Fixes and tests #

Patch Set 5 : Add squashing test #

Patch Set 6 : Remove unneeded TODO #

Total comments: 15

Patch Set 7 : CR feedback and fix most test failures #

Patch Set 8 : Merge with trunk (no other changes) #

Patch Set 9 : Fix remaining test failures #

Patch Set 10 : Fix merge with trunk (no actual changes) #

Patch Set 11 : Minor cleanup #

Patch Set 12 : Merge with trunk (no actual changes) #

Patch Set 13 : Fix test failures #

Patch Set 14 : Attempt to make iframe test results stable across platforms #

Patch Set 15 : merge with trunk (no actual changes) #

Patch Set 16 : Eliminate scrollbars from iframe test for cross-platform output consistency #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+574 lines, -104 lines) Patch
A LayoutTests/compositing/layer-creation/scroll-blocks-on-default.html View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +20 lines, -0 lines 0 comments Download
A LayoutTests/compositing/layer-creation/scroll-blocks-on-default-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +20 lines, -0 lines 0 comments Download
A LayoutTests/compositing/layer-creation/scroll-blocks-on-document.html View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +14 lines, -0 lines 0 comments Download
A LayoutTests/compositing/layer-creation/scroll-blocks-on-document-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +14 lines, -0 lines 0 comments Download
A LayoutTests/compositing/layer-creation/scroll-blocks-on-explicit.html View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +26 lines, -0 lines 0 comments Download
A LayoutTests/compositing/layer-creation/scroll-blocks-on-explicit-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +26 lines, -0 lines 0 comments Download
A LayoutTests/compositing/layer-creation/scroll-blocks-on-iframe.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +38 lines, -0 lines 0 comments Download
A LayoutTests/compositing/layer-creation/scroll-blocks-on-iframe-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +108 lines, -0 lines 0 comments Download
A LayoutTests/compositing/layer-creation/scroll-blocks-on-script.html View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +19 lines, -0 lines 0 comments Download
A + LayoutTests/compositing/layer-creation/scroll-blocks-on-script-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +5 lines, -5 lines 0 comments Download
A LayoutTests/compositing/layer-creation/scroll-blocks-on-svg.svg View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +18 lines, -0 lines 0 comments Download
A LayoutTests/compositing/layer-creation/scroll-blocks-on-svg-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +14 lines, -0 lines 0 comments Download
A + LayoutTests/compositing/squashing/no-squashing-for-scroll-blocks-on.html View 1 2 3 4 1 chunk +4 lines, -4 lines 0 comments Download
A + LayoutTests/compositing/squashing/no-squashing-for-scroll-blocks-on-expected.txt View 1 2 3 4 2 chunks +4 lines, -2 lines 0 comments Download
M LayoutTests/inspector/elements/styles/styles-new-API-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +3 lines, -0 lines 0 comments Download
M LayoutTests/inspector/layers/layer-compositing-reasons-expected.txt View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M Source/core/css/CSSPrimitiveValueMappings.h View 1 2 2 chunks +7 lines, -6 lines 0 comments Download
M Source/core/css/CSSProperties.in View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M Source/core/css/RenderStyleCSSValueMapping.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +5 lines, -5 lines 0 comments Download
M Source/core/css/html.css View 1 2 3 4 5 6 7 8 1 chunk +4 lines, -0 lines 0 comments Download
M Source/core/css/resolver/StyleResolver.cpp View 1 2 3 4 5 6 7 8 9 10 1 chunk +5 lines, -1 line 0 comments Download
M Source/core/css/svg.css View 1 2 3 4 5 6 1 chunk +2 lines, -1 line 0 comments Download
M Source/core/dom/Document.cpp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +5 lines, -1 line 0 comments Download
M Source/core/rendering/RenderBox.h View 1 2 3 4 5 6 7 1 chunk +1 line, -11 lines 0 comments Download
M Source/core/rendering/RenderBox.cpp View 1 2 3 4 5 6 7 1 chunk +24 lines, -0 lines 0 comments Download
M Source/core/rendering/compositing/CompositedLayerMapping.h View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/rendering/compositing/CompositedLayerMapping.cpp View 1 2 3 3 chunks +16 lines, -0 lines 0 comments Download
M Source/core/rendering/compositing/CompositingReasonFinder.h View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/rendering/compositing/CompositingReasonFinder.cpp View 1 2 3 4 5 6 7 8 9 10 4 chunks +50 lines, -1 line 0 comments Download
M Source/core/rendering/compositing/CompositingRequirementsUpdater.cpp View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/style/RenderStyle.h View 1 2 3 4 5 6 7 8 9 10 11 4 chunks +5 lines, -3 lines 0 comments Download
M Source/core/rendering/style/RenderStyleConstants.h View 1 2 3 1 chunk +0 lines, -11 lines 0 comments Download
M Source/core/rendering/style/StyleRareNonInheritedData.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/testing/Internals.idl View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +1 line, -0 lines 0 comments Download
M Source/platform/RuntimeEnabledFeatures.in View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M Source/platform/graphics/CompositingReasons.h View 2 chunks +49 lines, -46 lines 0 comments Download
M Source/platform/graphics/CompositingReasons.cpp View 1 chunk +3 lines, -0 lines 0 comments Download
M Source/platform/graphics/GraphicsLayer.h View 1 2 3 3 chunks +4 lines, -0 lines 0 comments Download
M Source/platform/graphics/GraphicsLayer.cpp View 1 2 3 3 chunks +21 lines, -1 line 0 comments Download
M Source/platform/graphics/GraphicsLayerClient.h View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
M public/platform/WebLayer.h View 2 chunks +5 lines, -0 lines 1 comment Download
A public/platform/WebScrollBlocksOn.h View 1 chunk +25 lines, -0 lines 0 comments Download

Messages

Total messages: 16 (4 generated)
Ian Vollick
This looks pretty reasonable (esp the plumbing), but I was hoping you could clarify something ...
5 years, 11 months ago (2015-01-06 04:13:51 UTC) #3
skobes
https://codereview.chromium.org/784453003/diff/100001/Source/core/css/resolver/StyleResolver.cpp File Source/core/css/resolver/StyleResolver.cpp (right): https://codereview.chromium.org/784453003/diff/100001/Source/core/css/resolver/StyleResolver.cpp#newcode515 Source/core/css/resolver/StyleResolver.cpp:515: documentStyle->setScrollBlocksOn(WebScrollBlocksOnStartTouch | WebScrollBlocksOnWheelEvent); If you have this default in ...
5 years, 11 months ago (2015-01-07 18:49:25 UTC) #4
Rick Byers
https://codereview.chromium.org/784453003/diff/100001/LayoutTests/compositing/layer-creation/scroll-blocks-on-default.html File LayoutTests/compositing/layer-creation/scroll-blocks-on-default.html (right): https://codereview.chromium.org/784453003/diff/100001/LayoutTests/compositing/layer-creation/scroll-blocks-on-default.html#newcode17 LayoutTests/compositing/layer-creation/scroll-blocks-on-default.html:17: document.getElementById('output').innerText += window.internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_SCROLL_BLOCKS_ON); On 2015/01/06 04:13:51, vollick wrote: ...
5 years, 11 months ago (2015-01-08 21:08:11 UTC) #5
Rick Byers
> But this whole renderview vs. html style thing is getting pretty unwieldy, and > ...
5 years, 11 months ago (2015-01-09 02:29:06 UTC) #6
Ian Vollick
Thanks. That addresses my concerns and nits. I'll defer to skobes at this point. https://codereview.chromium.org/784453003/diff/100001/LayoutTests/compositing/layer-creation/scroll-blocks-on-default.html ...
5 years, 11 months ago (2015-01-09 15:17:33 UTC) #7
Rick Byers
On 2015/01/09 15:17:33, vollick wrote: > Thanks. That addresses my concerns and nits. I'll defer ...
5 years, 11 months ago (2015-01-09 16:05:13 UTC) #8
skobes
lgtm
5 years, 11 months ago (2015-01-09 17:47:58 UTC) #9
Ian Vollick
On 2015/01/09 17:47:58, skobes wrote: > lgtm lgtm.
5 years, 11 months ago (2015-01-12 15:32:10 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/784453003/290001
5 years, 11 months ago (2015-01-12 16:01:50 UTC) #12
commit-bot: I haz the power
Committed patchset #16 (id:290001) as https://src.chromium.org/viewvc/blink?view=rev&revision=188230
5 years, 11 months ago (2015-01-12 16:06:13 UTC) #13
Nico
https://codereview.chromium.org/784453003/diff/290001/public/platform/WebLayer.h File public/platform/WebLayer.h (right): https://codereview.chromium.org/784453003/diff/290001/public/platform/WebLayer.h#newcode190 public/platform/WebLayer.h:190: // FIXME: Make pure once cc is updated. crbug.com/347272 ...
5 years, 4 months ago (2015-07-28 19:22:44 UTC) #15
Rick Byers
5 years, 4 months ago (2015-07-28 19:25:58 UTC) #16
Message was sent while issue was closed.
On 2015/07/28 19:22:44, Nico wrote:
>
https://codereview.chromium.org/784453003/diff/290001/public/platform/WebLayer.h
> File public/platform/WebLayer.h (right):
> 
>
https://codereview.chromium.org/784453003/diff/290001/public/platform/WebLaye...
> public/platform/WebLayer.h:190: // FIXME: Make pure once cc is updated. 
> crbug.com/347272
> can this be done now?

Yep, I've got a CL in progress that includes updating this.  Sorry for the
delay.

Powered by Google App Engine
This is Rietveld 408576698