Create Pinch Virtual Viewport scrollbar layers in Blink.

Create Pinch Virtual Viewport scrollbar layers in Blink.

This CL creates solid color scrollbar layers for the Inner Viewport and
attaches them.

BUG=

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=157718

[wjmaclean, 2013-09-06 17:37:45]

Small CL to use ScrollingCoordinator to create the scrollbar layers required by
the pinch virtual viewport.

[enne (OOO), 2013-09-10 17:04:43]

This looks good in general, but I have some questions about
registerPinchViewportLayers that I left on
https://codereview.chromium.org/23922006/.

[wjmaclean, 2013-09-11 17:30:17]

PTAL - not quite done, pending decision on what to do about connecting the clip
layer in Blink.

[enne (OOO), 2013-09-11 20:22:13]

This seems fine to me.  The clip layer could go in a separate patch?

[wjmaclean, 2013-09-12 18:17:38]

I've added the Blink-side interface for setting the clip layer in this CL, as
well as adjusted when the viewport layers are registered.

[wjmaclean, 2013-09-12 18:17:52]

On 2013/09/12 18:17:38, wjmaclean wrote:
> I've added the Blink-side interface for setting the clip layer in this CL, as
> well as adjusted when the viewport layers are registered.

PTAL?

[enne (OOO), 2013-09-12 18:51:21]

https://codereview.chromium.org/23464035/diff/13001/Source/web/WebViewImpl.cpp
File Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/23464035/diff/13001/Source/web/WebViewImpl.cp...
Source/web/WebViewImpl.cpp:3827:
rootScrollLayer->platformLayer()->setScrollClipLayer(m_rootLayer);
I don't think the root layer is the clip layer for the root scroll layer.  The
clip layer is one of the children, right?  Can you do this where the clip layer
/ root scroll layer is created rather than here?  The same comment applies to
PinchViewports::registerViewportLayersWithTreeView.

(I also think it's a little bit indirect to store the clip layer on a scroll
layer since it's really information for the scrollbars that just happen to be on
that scrollbar.  I think this is workable though; it's just some design
awkwardness, I think.)

https://codereview.chromium.org/23464035/diff/13001/public/platform/WebLayer.h
File public/platform/WebLayer.h (right):

https://codereview.chromium.org/23464035/diff/13001/public/platform/WebLayer....
public/platform/WebLayer.h:166: // FIXME: Remove default implementation when
CC-side implementation lands.
Or just land the cc side first?

[wjmaclean, 2013-09-12 19:01:56]

https://codereview.chromium.org/23464035/diff/13001/Source/web/WebViewImpl.cpp
File Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/23464035/diff/13001/Source/web/WebViewImpl.cp...
Source/web/WebViewImpl.cpp:3827:
rootScrollLayer->platformLayer()->setScrollClipLayer(m_rootLayer);
On 2013/09/12 18:51:21, enne wrote:
> I don't think the root layer is the clip layer for the root scroll layer.  The
> clip layer is one of the children, right?

Oops, you're right ... I'll fix this.

>  Can you do this where the clip layer
> / root scroll layer is created rather than here?

This happens in RenderLayerCompositor, which doesn't (at present) ever seem to
deal with the WebLayer interface ... is it OK to add it there?

>  The same comment applies to
> PinchViewports::registerViewportLayersWithTreeView.

This is doable.
 
> (I also think it's a little bit indirect to store the clip layer on a scroll
> layer since it's really information for the scrollbars that just happen to be
on
> that scrollbar.  I think this is workable though; it's just some design
> awkwardness, I think.)

I thought if I gave the clip layer info to the scroll layer, then the scrollbar
could just accept values computed by the scroll layer, and never have to worry
about where they come from. The scroll layer can do one set of computations for
one or two scrollbars in the same call. I thought this seemed a bit cleaner, but
if it becomes unworkable it can change. :-)

https://codereview.chromium.org/23464035/diff/13001/public/platform/WebLayer.h
File public/platform/WebLayer.h (right):

https://codereview.chromium.org/23464035/diff/13001/public/platform/WebLayer....
public/platform/WebLayer.h:166: // FIXME: Remove default implementation when
CC-side implementation lands.
On 2013/09/12 18:51:21, enne wrote:
> Or just land the cc side first?

OK, I had just thought this way it would be one less CL before I could start
cc-side development using the clip layer.

[wjmaclean, 2013-09-12 20:11:09]

I've removed the setClipLayer() stuff from this CL, as it perhaps needs some
more thought about how best to wire it. That can happen during debvelopment of
the wiring CLs to some.

[enne (OOO), 2013-09-12 20:12:59]

lgtm

[commit-bot: I haz the power, 2013-09-12 20:22:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wjmaclean@chromium.org/23464035/14006

[jamesr, 2013-09-12 20:35:03]

lgtm

https://codereview.chromium.org/23464035/diff/14006/Source/web/WebViewImpl.cpp
File Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/23464035/diff/14006/Source/web/WebViewImpl.cp...
Source/web/WebViewImpl.cpp:34: #include "core/rendering/RenderLayerCompositor.h"
could you put this with the rest of the core/... includes starting around line
95? i think this should go at about line 159

[wjmaclean, 2013-09-12 20:42:54]

https://codereview.chromium.org/23464035/diff/14006/Source/web/WebViewImpl.cpp
File Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/23464035/diff/14006/Source/web/WebViewImpl.cp...
Source/web/WebViewImpl.cpp:34: #include "core/rendering/RenderLayerCompositor.h"
On 2013/09/12 20:35:03, jamesr wrote:
> could you put this with the rest of the core/... includes starting around line
> 95? i think this should go at about line 159

Done.

[commit-bot: I haz the power, 2013-09-12 20:43:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wjmaclean@chromium.org/23464035/14007

[commit-bot: I haz the power, 2013-09-13 01:36:56]

Change committed as 157718