Move TopDocumentRootScrollerController to a separate object on FrameHost

Move TopDocumentRootScrollerController to a separate object on FrameHost

It contains a number of methods that don't conceptually belong on
RootScrollerController. This is also a natural step towards making this feature
work with out of process iframes.

The main changes in this CL are:

  -TopDocumentRootScrollerController is no longer a subclass of
   RootScrollerController

  -TopDocumentRootScrollerController is now owned by FrameHost

  -All the methods on RootScrollerController that really only belonged on
   the Page level object have been removed.

The class will be renamed in a followup CL to be more representative of these
changes.

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

Committed: https://crrev.com/23cd9757ba798fa41b4d8dd80eb8457da6a9fa9e
Cr-Commit-Position: refs/heads/master@{#418868}

[bokan, 2016-08-29 20:41:56]

Description was changed from

==========
Move TopDocumentRootScrollerController to a separate object on FrameHost

It contains a number of methods that don't conceptually belong on
RootScrollerController. This is also a natural step towards making this feature
work with out of process iframes.

The main changes in this CL are:

  -TopDocumentRootScrollerController is no longer a subclass of
   RootScrollerController

  -TopDocumentRootScrollerController is now owned by FrameHost

  -All the methods on RootScrollerController that really only belonged on
   the Page level object have been removed.

The class will be renamed in a followup CL to be more representative of these
changes.

BUG=505516
==========

to

==========
Move TopDocumentRootScrollerController to a separate object on FrameHost

It contains a number of methods that don't conceptually belong on
RootScrollerController. This is also a natural step towards making this feature
work with out of process iframes.

The main changes in this CL are:

  -TopDocumentRootScrollerController is no longer a subclass of
   RootScrollerController

  -TopDocumentRootScrollerController is now owned by FrameHost

  -All the methods on RootScrollerController that really only belonged on
   the Page level object have been removed.

The class will be renamed in a followup CL to be more representative of these
changes.

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

[bokan, 2016-08-29 21:17:03]

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

[bokan, 2016-08-29 21:17:57]

bokan@chromium.org changed reviewers:
+ tdresser@chromium.org

[bokan, 2016-08-29 21:17:57]

This CL finally feels like everything is conceptually right. I'll rename the
TopDocument class in a followup patch just for ease of review.

[commit-bot: I haz the power, 2016-08-29 21:55:09]

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, 2016-08-30 03:41:04]

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

[commit-bot: I haz the power, 2016-08-30 03:41:05]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  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...)
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[tdresser, 2016-08-30 15:05:30]

LGTM, I agree this feels better.

https://codereview.chromium.org/2285253003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/FrameHost.h (right):

https://codereview.chromium.org/2285253003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameHost.h:146:
Member<TopDocumentRootScrollerController> m_globalRootScrollerController;
Should this be const?

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

https://codereview.chromium.org/2285253003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp:473:
TopDocumentRootScrollerController& globalRootScrollerController =
const?

https://codereview.chromium.org/2285253003/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/page/scrolling/TopDocumentRootScrollerController.cpp
(right):

https://codereview.chromium.org/2285253003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/scrolling/TopDocumentRootScrollerController.cpp:126:
RootFrameViewport& rfv)
Might as well write out the whole:
rootFrameViewport

https://codereview.chromium.org/2285253003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/RootScrollerTest.cpp (right):

https://codereview.chromium.org/2285253003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/RootScrollerTest.cpp:514:
TopDocumentRootScrollerController& mainController =
Can this be const?

[bokan, 2016-08-30 16:13:39]

This was dependent on a previous patch that made some changes in clipping and
PaintLayerCompositor that are mostly unrelated. I'll try and swap them and land
this first if rearranging the commits isn't to gnarly.

https://codereview.chromium.org/2285253003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/FrameHost.h (right):

https://codereview.chromium.org/2285253003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameHost.h:146:
Member<TopDocumentRootScrollerController> m_globalRootScrollerController;
On 2016/08/30 15:05:30, tdresser wrote:
> Should this be const?

Done.

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

https://codereview.chromium.org/2285253003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp:473:
TopDocumentRootScrollerController& globalRootScrollerController =
On 2016/08/30 15:05:30, tdresser wrote:
> const?

Done.

https://codereview.chromium.org/2285253003/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/page/scrolling/TopDocumentRootScrollerController.cpp
(right):

https://codereview.chromium.org/2285253003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/scrolling/TopDocumentRootScrollerController.cpp:126:
RootFrameViewport& rfv)
On 2016/08/30 15:05:30, tdresser wrote:
> Might as well write out the whole:
> rootFrameViewport

Done.

https://codereview.chromium.org/2285253003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/RootScrollerTest.cpp (right):

https://codereview.chromium.org/2285253003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/RootScrollerTest.cpp:514:
TopDocumentRootScrollerController& mainController =
On 2016/08/30 15:05:30, tdresser wrote:
> Can this be const?

Done.

[bokan, 2016-08-30 17:14:51]

On 2016/08/30 16:13:39, bokan wrote:
> This was dependent on a previous patch that made some changes in clipping and
> PaintLayerCompositor that are mostly unrelated. I'll try and swap them and
land
> this first if rearranging the commits isn't to gnarly.

Actually, I'll wait and see if that patch can land in near-term as is it'll be
simpler to just wait.

[bokan, 2016-09-15 12:24:46]

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

[commit-bot: I haz the power, 2016-09-15 12:25:03]

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, 2016-09-15 15:04:30]

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

[commit-bot: I haz the power, 2016-09-15 15:04:31]

Dry run: This issue passed the CQ dry run.

[bokan, 2016-09-15 15:05:52]

The CQ bit was checked by bokan@chromium.org

[bokan, 2016-09-15 15:05:52]

The patchset sent to the CQ was uploaded after l-g-t-m from
tdresser@chromium.org
Link to the patchset: https://codereview.chromium.org/2285253003/#ps100001
(title: "Rebase")

[commit-bot: I haz the power, 2016-09-15 15:06:18]

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, 2016-09-15 15:11:24]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-09-15 15:13:55]

Description was changed from

==========
Move TopDocumentRootScrollerController to a separate object on FrameHost

It contains a number of methods that don't conceptually belong on
RootScrollerController. This is also a natural step towards making this feature
work with out of process iframes.

The main changes in this CL are:

  -TopDocumentRootScrollerController is no longer a subclass of
   RootScrollerController

  -TopDocumentRootScrollerController is now owned by FrameHost

  -All the methods on RootScrollerController that really only belonged on
   the Page level object have been removed.

The class will be renamed in a followup CL to be more representative of these
changes.

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

to

==========
Move TopDocumentRootScrollerController to a separate object on FrameHost

It contains a number of methods that don't conceptually belong on
RootScrollerController. This is also a natural step towards making this feature
work with out of process iframes.

The main changes in this CL are:

  -TopDocumentRootScrollerController is no longer a subclass of
   RootScrollerController

  -TopDocumentRootScrollerController is now owned by FrameHost

  -All the methods on RootScrollerController that really only belonged on
   the Page level object have been removed.

The class will be renamed in a followup CL to be more representative of these
changes.

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

Committed: https://crrev.com/23cd9757ba798fa41b4d8dd80eb8457da6a9fa9e
Cr-Commit-Position: refs/heads/master@{#418868}
==========

[commit-bot: I haz the power, 2016-09-15 15:13:57]

Patchset 6 (id:??) landed as
https://crrev.com/23cd9757ba798fa41b4d8dd80eb8457da6a9fa9e
Cr-Commit-Position: refs/heads/master@{#418868}