[RootLayerScrolls] Annotate non-fast-scroll regions to correct layer

[RootLayerScrolls] Annotate non-fast-scroll regions to correct layer

This doesn't change how the non-fast-scroll regions are generated;
it just affects where they are annotated for the compositor.

With this change, all of the scrollingcoordinator/ tests need to
ensure that they overflow the top frame, to ensure that the root
layer has a scrolling contents layer.

BUG=417782
R=skobes@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2651793012
Cr-Commit-Position: refs/heads/master@{#447243}
Committed: https://chromium.googlesource.com/chromium/src/+/b58d93befd050fa4f397f7d4be064818378b10a2

[szager1, 2017-01-27 19:54:47]

Description was changed from

==========
[RootLayerScrolls] Annotate non-fast-scroll regions to correct layer

This doesn't change how the non-fast-scroll regions are generated;
it just affects where they are annotated for the compositor.

With this change, all of the scrollingcoordinator/ tests need to
ensure that they overflow the top frame, to ensure that the root
layer has a scrolling contents layer.

BUG=417782
R=skobes@chromium.org
==========

to

==========
[RootLayerScrolls] Annotate non-fast-scroll regions to correct layer

This doesn't change how the non-fast-scroll regions are generated;
it just affects where they are annotated for the compositor.

With this change, all of the scrollingcoordinator/ tests need to
ensure that they overflow the top frame, to ensure that the root
layer has a scrolling contents layer.

BUG=417782
R=skobes@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[szager1, 2017-01-27 19:58:07]

The CQ bit was checked by szager@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-27 19:58:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[skobes, 2017-01-27 20:14:18]

https://codereview.chromium.org/2651793012/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/page/Page.cpp (right):

https://codereview.chromium.org/2651793012/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/page/Page.cpp:185: ->nonFastScrollableRects();
Could this just read
layoutViewportScrollableArea()->layerForScrolling()->platformLayer()->nonFastScrollableRegion()?

That would match what ScrollingCoordinator already does on the write path, and
avoid the need for nonFastScrollableRects plumbing in ScrollableArea, FrameView,
PLSA, and PaintLayer.

(The plumbing not only adds complexity but is actively misleading because it
makes you think we support non-fast-scrollable regions with per-scroller
granularity, when in fact they will return an empty ClientRectList for anything
other than the main frame's layout viewport.)

[commit-bot: I haz the power, 2017-01-27 22:13:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-27 22:13:56]

Dry run: This issue passed the CQ dry run.

[szager1, 2017-01-27 22:32:49]

https://codereview.chromium.org/2651793012/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/page/Page.cpp (right):

https://codereview.chromium.org/2651793012/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/page/Page.cpp:185: ->nonFastScrollableRects();
On 2017/01/27 20:14:18, skobes wrote:
> Could this just read
>
layoutViewportScrollableArea()->layerForScrolling()->platformLayer()->nonFastScrollableRegion()?
> 
> That would match what ScrollingCoordinator already does on the write path, and
> avoid the need for nonFastScrollableRects plumbing in ScrollableArea,
FrameView,
> PLSA, and PaintLayer.
> 
> (The plumbing not only adds complexity but is actively misleading because it
> makes you think we support non-fast-scrollable regions with per-scroller
> granularity, when in fact they will return an empty ClientRectList for
anything
> other than the main frame's layout viewport.)

layerForScrolling()
 layer()->compositedLayerMapping()
   DCHECK(isAllowedToQueryCompositingState())
BOOM!

All of the new plumbing is just to avoid that DCHECK.  I'm open to any
reasonable solution.  Maybe compositedLayerMappingForTesting?  I dunno.

[skobes, 2017-01-27 22:35:50]

On 2017/01/27 22:32:49, szager1 wrote:
> All of the new plumbing is just to avoid that DCHECK.  I'm open to any
> reasonable solution.  Maybe compositedLayerMappingForTesting?  I dunno.

Add this line:

DisableCompositingQueryAsserts disabler;

[szager1, 2017-01-30 23:58:47]

The CQ bit was checked by szager@chromium.org

[szager1, 2017-01-30 23:59:00]

The CQ bit was unchecked by szager@chromium.org

[szager1, 2017-01-30 23:59:15]

On 2017/01/27 22:35:50, skobes wrote:
> On 2017/01/27 22:32:49, szager1 wrote:
> > All of the new plumbing is just to avoid that DCHECK.  I'm open to any
> > reasonable solution.  Maybe compositedLayerMappingForTesting?  I dunno.
> 
> Add this line:
> 
> DisableCompositingQueryAsserts disabler;

Done, PTAL.

[skobes, 2017-01-31 01:02:55]

lgtm

[szager1, 2017-01-31 14:45:00]

The CQ bit was checked by szager@chromium.org

[commit-bot: I haz the power, 2017-01-31 14:45:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-31 16:32:59]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1485873900273930,
"parent_rev": "a43bc5031e4786cc8158656d1ca0d6dda4ae1e69", "commit_rev":
"b58d93befd050fa4f397f7d4be064818378b10a2"}

[commit-bot: I haz the power, 2017-01-31 16:33:31]

Description was changed from

==========
[RootLayerScrolls] Annotate non-fast-scroll regions to correct layer

This doesn't change how the non-fast-scroll regions are generated;
it just affects where they are annotated for the compositor.

With this change, all of the scrollingcoordinator/ tests need to
ensure that they overflow the top frame, to ensure that the root
layer has a scrolling contents layer.

BUG=417782
R=skobes@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
[RootLayerScrolls] Annotate non-fast-scroll regions to correct layer

This doesn't change how the non-fast-scroll regions are generated;
it just affects where they are annotated for the compositor.

With this change, all of the scrollingcoordinator/ tests need to
ensure that they overflow the top frame, to ensure that the root
layer has a scrolling contents layer.

BUG=417782
R=skobes@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2651793012
Cr-Commit-Position: refs/heads/master@{#447243}
Committed:
https://chromium.googlesource.com/chromium/src/+/b58d93befd050fa4f397f7d4be06...
==========

[commit-bot: I haz the power, 2017-01-31 16:33:32]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/b58d93befd050fa4f397f7d4be06...