Made double-tap zoom work in pinch virtual viewport mode. (Blink-side)

Made double-tap zoom work in pinch virtual viewport mode. (Blink-side)

On the Blink side:
The clamping method used by computeScaleAndScrollForBlockRect now
uses a new method PinchViewport::clampDocumentOffsetAtScale to clamp
the requested viewport offset taking into account the inner and outer
viewports. Added copies of AutoZoom tests that run with virtual-
viewport turned on.

Also added an overload of applyViewportDeltas, called in virtual
viewport mode only, that applies the inner and outer deltas along
page scale in one shot. This makes the viewport layers a special case
again in that their scroll offsets are set through this call, rather
than the standard 'did_scroll' callback of most layers. This is needed
since the scroll offsets may be invalid until the page scale is
applied so this is handled in one method to prevent unintended
clamping. 

On the Compositor side:
Calls the appropriate applyViewportDeltas based on which pinch mode
we're running in. The page scale animation now prefers to scroll the
inner viewport first, scrolling the outer only when the inner has
reached its scroll extent.

Chromium-side: https://codereview.chromium.org/585063002

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

[aelias_OOO_until_Jul13, 2014-09-19 22:39:21]

aelias@chromium.org changed reviewers:
+ aelias@chromium.org

[aelias_OOO_until_Jul13, 2014-09-19 22:39:21]

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

https://codereview.chromium.org/584833003/diff/1/Source/web/WebViewImpl.cpp#n...
Source/web/WebViewImpl.cpp:4165: // When the virtual viewport is enabled,
offsets are already set for us. Don't clamp
We've so far always been able to avoid "don't clamp" methods and I'd prefer to
keep it that way; that kind of approach ends up having subtle assumptions about
the order that things are called in.  It's better to fix these by sending all
parameters that cross-clamp as arguments to one method.  Can we change the Blink
API to pass the scroll offsets for the inner and outer viewport to this method
as well?

[bokan, 2014-09-20 01:14:22]

On 2014/09/19 22:39:21, aelias wrote:
> https://codereview.chromium.org/584833003/diff/1/Source/web/WebViewImpl.cpp
> File Source/web/WebViewImpl.cpp (right):
> 
>
https://codereview.chromium.org/584833003/diff/1/Source/web/WebViewImpl.cpp#n...
> Source/web/WebViewImpl.cpp:4165: // When the virtual viewport is enabled,
> offsets are already set for us. Don't clamp
> We've so far always been able to avoid "don't clamp" methods and I'd prefer to
> keep it that way; that kind of approach ends up having subtle assumptions
about
> the order that things are called in.  It's better to fix these by sending all
> parameters that cross-clamp as arguments to one method.  Can we change the
Blink
> API to pass the scroll offsets for the inner and outer viewport to this method
> as well?

Ok, I've made the change. Unfortunately, I had to go back to special casing the
viewport layers, but I think they *are* a special case and I agree that having
order based assumptions are a bit of a time bomb. If this looks ok I'll clean up
and add tests.

[aelias_OOO_until_Jul13, 2014-09-22 06:45:02]

lgtm

[bokan, 2014-09-23 00:49:04]

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

[bokan, 2014-09-23 00:49:04]

Hi Elliott, could you PTAL for Source/core and Source/platform OWNER? Thanks.

[esprehn, 2014-09-23 00:53:12]

lgtm

https://codereview.chromium.org/584833003/diff/60001/Source/core/frame/PinchV...
File Source/core/frame/PinchViewport.h (right):

https://codereview.chromium.org/584833003/diff/60001/Source/core/frame/PinchV...
Source/core/frame/PinchViewport.h:98: void setScaleAndLocation(float, const
FloatPoint&);
add argument names.

https://codereview.chromium.org/584833003/diff/60001/Source/core/frame/PinchV...
Source/core/frame/PinchViewport.h:119: IntPoint clampDocumentOffsetAtScale(const
IntPoint&, float);
ditto

[bokan, 2014-09-23 12:42:22]

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

[bokan, 2014-09-23 12:42:23]

+jamesr@ for public/web OWNER

https://codereview.chromium.org/584833003/diff/60001/Source/core/frame/PinchV...
File Source/core/frame/PinchViewport.h (right):

https://codereview.chromium.org/584833003/diff/60001/Source/core/frame/PinchV...
Source/core/frame/PinchViewport.h:98: void setScaleAndLocation(float, const
FloatPoint&);
On 2014/09/23 00:53:12, esprehn wrote:
> add argument names.

Done.

https://codereview.chromium.org/584833003/diff/60001/Source/core/frame/PinchV...
Source/core/frame/PinchViewport.h:119: IntPoint clampDocumentOffsetAtScale(const
IntPoint&, float);
On 2014/09/23 00:53:12, esprehn wrote:
> ditto

Done.

[jamesr, 2014-09-23 23:27:27]

lgtm

[bokan, 2014-09-24 13:48:20]

This will need a 3-sided patch, I'll land the interface changes from this CL in
a separate TBR'ed CL

[bokan, 2014-09-29 15:10:26]

The CQ bit was checked by bokan@chromium.org

[commit-bot: I haz the power, 2014-09-29 15:11:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/584833003/100001

[commit-bot: I haz the power, 2014-09-29 15:29:26]

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

[commit-bot: I haz the power, 2014-09-29 15:29:27]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/2...)

[bokan, 2014-09-29 17:59:18]

The CQ bit was checked by bokan@chromium.org

[commit-bot: I haz the power, 2014-09-29 18:00:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/584833003/120001

[commit-bot: I haz the power, 2014-09-29 19:38:45]

Committed patchset #7 (id:120001) as 182860