Pinch/Zoom Infrastructure & Plumbing CL (Prequel)

Pinch/Zoom Infrastructure & Plumbing CL (Prequel)

This CL supplies the necessary changes to Blink to support
the inner/outer viewport model for pinch-zoom and fixed-
position elements. The specification for these changes is
contained in the document "Layer-based Solution for Pinch
Zoom / Fixed Position".

Specifically, it provides the Blink-side infrastructure to
move scroll layers & scrollbars to using a clip-layer to
specify the max scroll offset and scrolbar positioning, as
opposed to pushing a max_scroll_offset from Blink. This
entails a number of changes to the Web* interface classes.

This CL supports the CC-side cl 
https://codereview.chromium.org/23983047/, and must land 
before that CL.

This code has previous review history at 
https://codereview.chromium.org/24302010/.

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

[wjmaclean, 2014-01-14 19:40:19]

This code is the subset of the CL https://codereview.chromium.org/24302010/ (see
previous review history on that CL) that needs to land before the Chromium CL
https://codereview.chromium.org/23983047.

Please take a look?

[enne (OOO), 2014-01-14 20:05:29]

I get chunk mismatches when trying to look at diffs.  Can you reupload?

Can you also format your description to 72 columns?

[wjmaclean, 2014-01-14 20:39:09]

On 2014/01/14 20:05:29, enne wrote:
> I get chunk mismatches when trying to look at diffs.  Can you reupload?
> 
> Can you also format your description to 72 columns?

Sorry about that, should be fixed now.

[enne (OOO), 2014-01-14 22:43:46]

lgtm, thanks.

[enne (OOO), 2014-01-14 22:43:55]

https://codereview.chromium.org/138453004/diff/260002/Source/platform/graphic...
File Source/platform/graphics/GraphicsLayer.h (right):

https://codereview.chromium.org/138453004/diff/260002/Source/platform/graphic...
Source/platform/graphics/GraphicsLayer.h:305: // Exposed for use in
PinchViewports.cpp.
comment nit: I think this is a little over-specific and could probably be
removed.  What if some other site starts calling it?

[jamesr, 2014-01-14 22:48:58]

https://codereview.chromium.org/138453004/diff/260002/Source/platform/graphic...
File Source/platform/graphics/GraphicsLayer.h (right):

https://codereview.chromium.org/138453004/diff/260002/Source/platform/graphic...
Source/platform/graphics/GraphicsLayer.h:305: // Exposed for use in
PinchViewports.cpp.
On 2014/01/14 22:43:55, enne wrote:
> comment nit: I think this is a little over-specific and could probably be
> removed.  What if some other site starts calling it?

yeah, it doesn't make sense for a comment in /platform/ to talk about classes in
/web/.  Anyone with code search can find out what code is calling something.  If
this needs a comment (and I'm not sure that it does) then the comment needs to
explain the function in terms of concepts exposed in /platform/

It's pretty confusing that this interface has a contentLayer() and a
contentsLayer() that do different things

https://codereview.chromium.org/138453004/diff/260002/Source/web/PinchViewpor...
File Source/web/PinchViewports.cpp (right):

https://codereview.chromium.org/138453004/diff/260002/Source/web/PinchViewpor...
Source/web/PinchViewports.cpp:177:
scrollbarGraphicsLayer->contentsLayer()->setBounds(WebSize(width, height));
don't you actually want to call GraphicsLayer::setContentsRect() here?

[wjmaclean, 2014-01-14 23:55:28]

Revised - thanks!

https://codereview.chromium.org/138453004/diff/260002/Source/platform/graphic...
File Source/platform/graphics/GraphicsLayer.h (right):

https://codereview.chromium.org/138453004/diff/260002/Source/platform/graphic...
Source/platform/graphics/GraphicsLayer.h:305: // Exposed for use in
PinchViewports.cpp.
On 2014/01/14 22:43:55, enne wrote:
> comment nit: I think this is a little over-specific and could probably be
> removed.  What if some other site starts calling it?

Based on James' other comment, I have removed this change.

https://codereview.chromium.org/138453004/diff/260002/Source/web/PinchViewpor...
File Source/web/PinchViewports.cpp (right):

https://codereview.chromium.org/138453004/diff/260002/Source/web/PinchViewpor...
Source/web/PinchViewports.cpp:177:
scrollbarGraphicsLayer->contentsLayer()->setBounds(WebSize(width, height));
On 2014/01/14 22:48:58, jamesr wrote:
> don't you actually want to call GraphicsLayer::setContentsRect() here?

Hmm, yes, that seems better!

[commit-bot: I haz the power, 2014-01-15 00:02:49]

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

[jamesr, 2014-01-15 00:05:39]

lgtm

[commit-bot: I haz the power, 2014-01-15 13:56:23]

Change committed as 165122