Make ViewportScrollCallback an interface and add child and root classes

Make ViewportScrollCallback an interface and add child and root classes

This CL splits the ViewportScrollCallback into two implementations with a common
interface. The RootViewportScrollCallback is used for the one root FrameView on
a page and holds a pointer to the RootFrameViewport which allows it to swap the
root scroller with the layout viewport. It also holds pointers to the
TopControls and Overscroll managers allowing it to perform those actions.

The ChildViewportScrollCallback is used for all other FrameViews. It's
essentially a no-op in that it doesn't perform any non default actions; it
simply scrolls the FrameView as usual. Having this split allows the code to be
general and avoid special casing the root viewport.

BUG=505516
Committed: https://crrev.com/2331411e8433573ca3c6448cf3fcdab82d934649
Cr-Commit-Position: refs/heads/master@{#404195}

[bokan, 2016-07-06 14:39:50]

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

[commit-bot: I haz the power, 2016-07-06 14:40:04]

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-07-06 16:01:47]

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

[commit-bot: I haz the power, 2016-07-06 16:01:48]

Dry run: 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_...)

[bokan, 2016-07-06 17:50:46]

Description was changed from

==========
Make ViewportScrollCallback an interface and add child and root classes

This CL splits the ViewportScrollCallback into two implementations with a common
interface. The RootViewportScrollCallback is used for the one root FrameView on
a page and holds a pointer to the RootFrameViewport which allows it to swap the
root scroller with the layout viewport. It also holds pointers to the
TopControls and Overscroll managers allowing it to perform those actions.

The ChildViewportScrollCallback is used for all other FrameViews. It's
essentially a no-op in that it doesn't perform any non default actions; it
simply scrolls the FrameView as usual. Having this split allows the code to be
general and avoid special casing the root viewport.

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

to

==========
Make ViewportScrollCallback an interface and add child and root classes

This CL splits the ViewportScrollCallback into two implementations with a common
interface. The RootViewportScrollCallback is used for the one root FrameView on
a page and holds a pointer to the RootFrameViewport which allows it to swap the
root scroller with the layout viewport. It also holds pointers to the
TopControls and Overscroll managers allowing it to perform those actions.

The ChildViewportScrollCallback is used for all other FrameViews. It's
essentially a no-op in that it doesn't perform any non default actions; it
simply scrolls the FrameView as usual. Having this split allows the code to be
general and avoid special casing the root viewport.

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

[bokan, 2016-07-06 17:50:46]

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

[tdresser, 2016-07-06 18:28:19]

https://codereview.chromium.org/2128543002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/page/scrolling/ChildViewportScrollCallback.h
(right):

https://codereview.chromium.org/2128543002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/page/scrolling/ChildViewportScrollCallback.h:37:
WeakMember<ScrollableArea> m_scroller;
Should we move this member to the parent class?

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

https://codereview.chromium.org/2128543002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/page/scrolling/RootScrollerController.cpp:15:
#include "core/page/scrolling/RootViewportScrollCallback.h"
Is this used?

https://codereview.chromium.org/2128543002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/page/scrolling/RootScrollerController.h
(right):

https://codereview.chromium.org/2128543002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/page/scrolling/RootScrollerController.h:16: class
RootViewportScrollCallback;
Is this used?

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

https://codereview.chromium.org/2128543002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/page/scrolling/RootViewportScrollCallback.cpp:75:
if (!m_scroller)
Hmmm, there isn't much shared logic between here and the
ChildViewportScrollCallback, but there is some...

Can we factor that logic out somehow?

https://codereview.chromium.org/2128543002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/page/scrolling/RootViewportScrollCallback.h
(right):

https://codereview.chromium.org/2128543002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/page/scrolling/RootViewportScrollCallback.h:15:
class Element;
Unused.

[bokan, 2016-07-06 21:12:05]

ptal

https://codereview.chromium.org/2128543002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/page/scrolling/ChildViewportScrollCallback.h
(right):

https://codereview.chromium.org/2128543002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/page/scrolling/ChildViewportScrollCallback.h:37:
WeakMember<ScrollableArea> m_scroller;
On 2016/07/06 18:28:18, tdresser wrote:
> Should we move this member to the parent class?

It's type will change in RootViewportScrollCallback in the next patch so it
would have to be moved back down.

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

https://codereview.chromium.org/2128543002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/page/scrolling/RootScrollerController.cpp:15:
#include "core/page/scrolling/RootViewportScrollCallback.h"
On 2016/07/06 18:28:18, tdresser wrote:
> Is this used?

Nope, this is a leftover from coding, it should really be
ViewportScrollCallback.h so I put it back in.

https://codereview.chromium.org/2128543002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/page/scrolling/RootScrollerController.h
(right):

https://codereview.chromium.org/2128543002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/page/scrolling/RootScrollerController.h:16: class
RootViewportScrollCallback;
On 2016/07/06 18:28:18, tdresser wrote:
> Is this used?

Nope, removed. As well as OverscrollController and TopControls.

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

https://codereview.chromium.org/2128543002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/page/scrolling/RootViewportScrollCallback.cpp:75:
if (!m_scroller)
On 2016/07/06 18:28:18, tdresser wrote:
> Hmmm, there isn't much shared logic between here and the
> ChildViewportScrollCallback, but there is some...
> 
> Can we factor that logic out somehow?

Done. I moved the "native scroll" portion out into a protected method in the
ancestor class and rearranged some of the remaining logic in here.

https://codereview.chromium.org/2128543002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/page/scrolling/RootViewportScrollCallback.h
(right):

https://codereview.chromium.org/2128543002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/page/scrolling/RootViewportScrollCallback.h:15:
class Element;
On 2016/07/06 18:28:19, tdresser wrote:
> Unused.

Done.

[tdresser, 2016-07-07 13:43:26]

LGTM

[bokan, 2016-07-07 16:34:53]

The CQ bit was checked by bokan@chromium.org

[bokan, 2016-07-07 16:34:53]

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/2128543002/#ps40001
(title: "Rebase")

[commit-bot: I haz the power, 2016-07-07 16:36:02]

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-07-07 18:56:04]

Description was changed from

==========
Make ViewportScrollCallback an interface and add child and root classes

This CL splits the ViewportScrollCallback into two implementations with a common
interface. The RootViewportScrollCallback is used for the one root FrameView on
a page and holds a pointer to the RootFrameViewport which allows it to swap the
root scroller with the layout viewport. It also holds pointers to the
TopControls and Overscroll managers allowing it to perform those actions.

The ChildViewportScrollCallback is used for all other FrameViews. It's
essentially a no-op in that it doesn't perform any non default actions; it
simply scrolls the FrameView as usual. Having this split allows the code to be
general and avoid special casing the root viewport.

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

to

==========
Make ViewportScrollCallback an interface and add child and root classes

This CL splits the ViewportScrollCallback into two implementations with a common
interface. The RootViewportScrollCallback is used for the one root FrameView on
a page and holds a pointer to the RootFrameViewport which allows it to swap the
root scroller with the layout viewport. It also holds pointers to the
TopControls and Overscroll managers allowing it to perform those actions.

The ChildViewportScrollCallback is used for all other FrameViews. It's
essentially a no-op in that it doesn't perform any non default actions; it
simply scrolls the FrameView as usual. Having this split allows the code to be
general and avoid special casing the root viewport.

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

[commit-bot: I haz the power, 2016-07-07 18:56:06]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-07-07 18:56:21]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-07 18:59:28]

Description was changed from

==========
Make ViewportScrollCallback an interface and add child and root classes

This CL splits the ViewportScrollCallback into two implementations with a common
interface. The RootViewportScrollCallback is used for the one root FrameView on
a page and holds a pointer to the RootFrameViewport which allows it to swap the
root scroller with the layout viewport. It also holds pointers to the
TopControls and Overscroll managers allowing it to perform those actions.

The ChildViewportScrollCallback is used for all other FrameViews. It's
essentially a no-op in that it doesn't perform any non default actions; it
simply scrolls the FrameView as usual. Having this split allows the code to be
general and avoid special casing the root viewport.

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

to

==========
Make ViewportScrollCallback an interface and add child and root classes

This CL splits the ViewportScrollCallback into two implementations with a common
interface. The RootViewportScrollCallback is used for the one root FrameView on
a page and holds a pointer to the RootFrameViewport which allows it to swap the
root scroller with the layout viewport. It also holds pointers to the
TopControls and Overscroll managers allowing it to perform those actions.

The ChildViewportScrollCallback is used for all other FrameViews. It's
essentially a no-op in that it doesn't perform any non default actions; it
simply scrolls the FrameView as usual. Having this split allows the code to be
general and avoid special casing the root viewport.

BUG=505516

Committed: https://crrev.com/2331411e8433573ca3c6448cf3fcdab82d934649
Cr-Commit-Position: refs/heads/master@{#404195}
==========

[commit-bot: I haz the power, 2016-07-07 18:59:29]

Patchset 3 (id:??) landed as
https://crrev.com/2331411e8433573ca3c6448cf3fcdab82d934649
Cr-Commit-Position: refs/heads/master@{#404195}