[RLS] Don't create PaintLayerCompositor layers.

[RLS] Don't create PaintLayerCompositor layers.

In root layer scrolling mode, the main GraphicsLayer in the LayoutView's
CompositedLayerMapping is now connected directly to the VisualViewport scroll
layer (in the main frame), or to the layer of an owning LayoutIFrame.

BUG=698464
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2728273002
Cr-Commit-Position: refs/heads/master@{#455954}
Committed: https://chromium.googlesource.com/chromium/src/+/13e9d344a202ab2d800c64f9c3f0beef605651d9

[skobes, 2017-03-04 20:03:43]

Description was changed from

==========
[RLS] Don't create PaintLayerCompositor layers.

BUG=698464
==========

to

==========
[RLS] Don't create PaintLayerCompositor layers.

BUG=698464
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[skobes, 2017-03-04 20:08:35]

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

[commit-bot: I haz the power, 2017-03-04 20:08:45]

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

[skobes, 2017-03-04 20:34:45]

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

[commit-bot: I haz the power, 2017-03-04 20:34:59]

Dry run: 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-03-04 21:15:06]

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

[commit-bot: I haz the power, 2017-03-04 21:15:06]

Dry run: Try jobs failed on following builders:
  linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_layout_te...)

[skobes, 2017-03-04 22:07:58]

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

[commit-bot: I haz the power, 2017-03-04 22:08:08]

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

[skobes, 2017-03-04 22:08:28]

Patchset #1 (id:1) has been deleted

[skobes, 2017-03-04 22:08:43]

Patchset #1 (id:20001) has been deleted

[skobes, 2017-03-04 22:33:43]

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

[commit-bot: I haz the power, 2017-03-04 22:33:48]

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

[skobes, 2017-03-04 22:36:42]

Patchset #1 (id:40001) has been deleted

[skobes, 2017-03-04 23:39:12]

skobes@chromium.org changed reviewers:
+ bokan@chromium.org, chrishtr@chromium.org, szager@chromium.org

[skobes, 2017-03-04 23:46:55]

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

[commit-bot: I haz the power, 2017-03-04 23:47:04]

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

[skobes, 2017-03-04 23:47:16]

Patchset #1 (id:60001) has been deleted

[commit-bot: I haz the power, 2017-03-05 01:27:31]

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

[commit-bot: I haz the power, 2017-03-05 01:27:32]

Dry run: This issue passed the CQ dry run.

[bokan, 2017-03-06 15:42:30]

https://codereview.chromium.org/2728273002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp
(left):

https://codereview.chromium.org/2728273002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp:1186:
if (!m_rootContentLayer)
It seems like the existence of rootContentLayer was previously used to tell if
the PLC has been initialised. Was this just an optimisation that doesn't affect
correctness in all these cases? Does the PLC under RLS no longer maintain any
real state that we don't have to ensure it's been initialised?

https://codereview.chromium.org/2728273002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp
(right):

https://codereview.chromium.org/2728273002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp:362:
return isMainFrame() ? visualViewport().scrollLayer() : nullptr;
Is it true that !rootContentLayer implies rootLayerScrolling? If so, please add
a DCHECK.

https://codereview.chromium.org/2728273002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp:1274:
else if (m_rootContentLayer)
Just some questions for my own benefit: how does this work for the non-RLS case?
When is there a rootContentLayer but no overflowControlsHostLayer? If there's no
overflowControlsHostLayer, does this just disconnect the RootContentLayer from
the FrameScroll layer or is there no FrameScroll layer in that case either?

[skobes, 2017-03-06 21:21:25]

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

[commit-bot: I haz the power, 2017-03-06 21:21:47]

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

[szager1, 2017-03-06 22:07:47]

https://codereview.chromium.org/2728273002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp
(right):

https://codereview.chromium.org/2728273002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp:1160:
// TODO(skobes): When root layer scrolling is enabled, we should not even
Remove this comment.

https://codereview.chromium.org/2728273002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp:1234:
DisableCompositingQueryAsserts disabler;
Why is this now necessary?

[skobes, 2017-03-06 22:58:14]

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

[commit-bot: I haz the power, 2017-03-06 22:58:44]

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

[skobes, 2017-03-06 23:00:25]

https://codereview.chromium.org/2728273002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp
(left):

https://codereview.chromium.org/2728273002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp:1186:
if (!m_rootContentLayer)
On 2017/03/06 15:42:30, bokan wrote:
> It seems like the existence of rootContentLayer was previously used to tell if
> the PLC has been initialised. Was this just an optimisation that doesn't
affect
> correctness in all these cases? Does the PLC under RLS no longer maintain any
> real state that we don't have to ensure it's been initialised?

PLC still tracks its state in m_rootLayerAttachment.  I think the
!m_rootContentLayer checks were unnecessary.

https://codereview.chromium.org/2728273002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp
(right):

https://codereview.chromium.org/2728273002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp:362:
return isMainFrame() ? visualViewport().scrollLayer() : nullptr;
On 2017/03/06 15:42:30, bokan wrote:
> Is it true that !rootContentLayer implies rootLayerScrolling? If so, please
add
> a DCHECK.

Done.

https://codereview.chromium.org/2728273002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp:1160:
// TODO(skobes): When root layer scrolling is enabled, we should not even
On 2017/03/06 22:07:47, szager1 wrote:
> Remove this comment.

Done.

https://codereview.chromium.org/2728273002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp:1234:
DisableCompositingQueryAsserts disabler;
On 2017/03/06 22:07:47, szager1 wrote:
> Why is this now necessary?

Done.

https://codereview.chromium.org/2728273002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp:1274:
else if (m_rootContentLayer)
On 2017/03/06 15:42:29, bokan wrote:
> Just some questions for my own benefit: how does this work for the non-RLS
case?
> When is there a rootContentLayer but no overflowControlsHostLayer? If there's
no
> overflowControlsHostLayer, does this just disconnect the RootContentLayer from
> the FrameScroll layer or is there no FrameScroll layer in that case either?

Good question.  It looks like this branch is a historical artifact - it's no
longer possible to have m_rootContentLayer without m_overflowControlsHostLayer. 
Updated.

[bokan, 2017-03-06 23:09:44]

lgtm (though I'm not an expert in PLC so please wait for other reviewers)

[commit-bot: I haz the power, 2017-03-07 00:34:57]

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

[commit-bot: I haz the power, 2017-03-07 00:34:58]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[skobes, 2017-03-07 23:05:10]

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

[commit-bot: I haz the power, 2017-03-07 23:06:26]

Dry run: 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-03-08 01:10:49]

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

[commit-bot: I haz the power, 2017-03-08 01:10:50]

Dry run: This issue passed the CQ dry run.

[skobes, 2017-03-08 02:32:19]

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

[commit-bot: I haz the power, 2017-03-08 02:33:49]

Dry run: 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-03-08 02:36:07]

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

[commit-bot: I haz the power, 2017-03-08 02:36:07]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[skobes, 2017-03-08 02:49:03]

Description was changed from

==========
[RLS] Don't create PaintLayerCompositor layers.

BUG=698464
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
[RLS] Don't create PaintLayerCompositor layers.

In root layer scrolling mode, the main GraphicsLayer in the LayoutView's
CompositedLayerMapping is now connected directly to the VisualViewport scroll
layer (in the main frame), or to the layer of an owning LayoutIFrame.

BUG=698464
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[skobes, 2017-03-08 02:52:08]

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

[commit-bot: I haz the power, 2017-03-08 02:52:45]

Dry run: 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-03-08 03:16:20]

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

[commit-bot: I haz the power, 2017-03-08 03:16:21]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[skobes, 2017-03-08 07:07:16]

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

[commit-bot: I haz the power, 2017-03-08 07:07:31]

Dry run: 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-03-08 08:17:33]

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

[commit-bot: I haz the power, 2017-03-08 08:17:34]

Dry run: This issue passed the CQ dry run.

[skobes, 2017-03-09 17:49:58]

szager: ping :)

[szager1, 2017-03-09 20:38:53]

lgtm

[skobes, 2017-03-09 21:20:05]

The CQ bit was checked by skobes@chromium.org

[skobes, 2017-03-09 21:20:06]

The patchset sent to the CQ was uploaded after l-g-t-m from bokan@chromium.org,
szager@chromium.org
Link to the patchset: https://codereview.chromium.org/2728273002/#ps200001
(title: "rebase")

[commit-bot: I haz the power, 2017-03-09 21:20:29]

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-03-09 23:57:03]

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

[commit-bot: I haz the power, 2017-03-09 23:57:04]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[skobes, 2017-03-10 00:32:07]

The CQ bit was checked by skobes@chromium.org

[commit-bot: I haz the power, 2017-03-10 00:33:09]

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-03-10 01:40:39]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1489105927029730,
"parent_rev": "589c696ecd13c722eb3b7eeef6eb43791978a687", "commit_rev":
"13e9d344a202ab2d800c64f9c3f0beef605651d9"}

[commit-bot: I haz the power, 2017-03-10 01:41:52]

Description was changed from

==========
[RLS] Don't create PaintLayerCompositor layers.

In root layer scrolling mode, the main GraphicsLayer in the LayoutView's
CompositedLayerMapping is now connected directly to the VisualViewport scroll
layer (in the main frame), or to the layer of an owning LayoutIFrame.

BUG=698464
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
[RLS] Don't create PaintLayerCompositor layers.

In root layer scrolling mode, the main GraphicsLayer in the LayoutView's
CompositedLayerMapping is now connected directly to the VisualViewport scroll
layer (in the main frame), or to the layer of an owning LayoutIFrame.

BUG=698464
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2728273002
Cr-Commit-Position: refs/heads/master@{#455954}
Committed:
https://chromium.googlesource.com/chromium/src/+/13e9d344a202ab2d800c64f9c3f0...
==========

[commit-bot: I haz the power, 2017-03-10 01:41:53]

Committed patchset #7 (id:200001) as
https://chromium.googlesource.com/chromium/src/+/13e9d344a202ab2d800c64f9c3f0...