Initial draft - modify ViewportAnchor to know about both inner and outer viewports.

Source/web/WebViewImpl.cpp

Index: Source/web/WebViewImpl.cpp
diff --git a/Source/web/WebViewImpl.cpp b/Source/web/WebViewImpl.cpp
index a170b30c711d430597a550f8a410233e7b41e766..b9642e142328c4ed749b90f97adf8c4ff0251acb 100644
--- a/Source/web/WebViewImpl.cpp
+++ b/Source/web/WebViewImpl.cpp
@@ -1679,8 +1679,10 @@ void WebViewImpl::resize(const WebSize& newSize)
 
     ViewportAnchor viewportAnchor(&localFrameRootTemporary()->frame()->eventHandler());
     if (shouldAnchorAndRescaleViewport) {
-        viewportAnchor.setAnchor(view->visibleContentRect(),
-                                 FloatSize(viewportAnchorXCoord, viewportAnchorYCoord));
+        viewportAnchor.setAnchor(
+            localFrameRootTemporary()->frameView()->visibleContentRect(),

[bokan, 2014/09/12 20:53:07]

Just use the local 'view' as before.

[timav, 2014/09/12 23:33:35]

Acknowledged.

[timav, 2014/09/15 20:04:35]

Done.

+            visibleRectInDocument(),
+            FloatSize(viewportAnchorXCoord, viewportAnchorYCoord));
     }
 
     // FIXME: TextAutosizer does not yet support out-of-process frames.
@@ -1700,15 +1702,34 @@ void WebViewImpl::resize(const WebSize& newSize)
 
         if (shouldAnchorAndRescaleViewport) {
             float newPageScaleFactor = oldPageScaleFactor / oldMinimumPageScaleFactor * minimumPageScaleFactor();
-            IntSize scaledViewportSize = newSize;
-            scaledViewportSize.scale(1 / newPageScaleFactor);
-            setPageScaleFactor(newPageScaleFactor, viewportAnchor.computeOrigin(scaledViewportSize));
+            newPageScaleFactor = clampPageScaleFactorToLimits(newPageScaleFactor);
+
+            IntSize pinchViewportSize = newSize;
+            pinchViewportSize.scale(1 / newPageScaleFactor);
+
+            IntSize mainFrameSize = localFrameRootTemporary()->frameView()->visibleContentRect().size();

[bokan, 2014/09/12 20:53:07]

ditto here, localFrameRootTemporary()->frameView() --> view

[timav, 2014/09/12 23:33:35]

Acknowledged.

[timav, 2014/09/15 20:04:35]

Done.

+
+            IntPoint mainFrameOrigin;
+            FloatPoint pinchViewportOrigin;
+            viewportAnchor.computeOrigins(mainFrameSize, pinchViewportSize, &mainFrameOrigin, &pinchViewportOrigin);
+            scrollAndRescaleViewports(newPageScaleFactor, mainFrameOrigin, pinchViewportOrigin);
         }
     }
 
     sendResizeEventAndRepaint();
 }
 
+IntRect WebViewImpl::visibleRectInDocument() const
+{
+    if (pinchVirtualViewportEnabled()) {
+        // Inner viewport in the document coordinates
+        return enclosedIntRect(page()->frameHost().pinchViewport().visibleRectInDocument());
+    }
+
+    // Outer viewport in the document coordinates
+    return localFrameRootTemporary()->frameView()->visibleContentRect();
+}
+
 void WebViewImpl::willEndLiveResize()
 {
     if (mainFrameImpl() && mainFrameImpl()->frameView())
@@ -2897,6 +2918,39 @@ WebFloatPoint WebViewImpl::pinchViewportOffset() const
     return page()->frameHost().pinchViewport().visibleRect().location();
 }
 
+void WebViewImpl::scrollAndRescaleViewports(float scaleFactor,
+    const IntPoint & mainFrameOrigin,
+    const FloatPoint & pinchViewportOrigin)
+{
+    // Old way
+    if (!pinchVirtualViewportEnabled()) {
+        setPageScaleFactor(scaleFactor, mainFrameOrigin);
+        return;
+    }
+
+    if (!page())
+        return;
+
+    if (!mainFrameImpl())
+        return;
+
+    FrameView * view = mainFrameImpl()->frameView();
+    if (!view)
+        return;
+
+    // Order is important: pinch viewport location is clamped based on
+    // main frame scroll position and pinch viewport scale.

[bokan, 2014/09/12 20:53:07]

The inner viewport is always set relative to the outer viewport so it doesn't
matter if you set the inner or outer first. Any time the outer viewport moves,
the inner viewport implicitly moves with it (it maintains its relative offset to
the outer viewport). However, anytime we scale or move the inner viewport, or
resize the outer, we clamp to make sure it remains bounded by the outer, so in
general we do need to set the scale before the inner offset. Though if the
offset was calculated to take the final scale into account (which I think you do
now) that shouldn't matter either.

Additionally, we have to consider clamping of outer viewport here. If
mainFrameOrigin causes the outer viewport to exceed the document bounds, it'll
be clamped so the inner viewport wont end up at the desired offset. You'll need
to check the difference between mainFrameOrigin and view->scrollOffset() after
setting it and apply the difference to the inner viewport.

[timav, 2014/09/12 23:33:35]

I think you are right for the outer viewport, i.e. ScrollView.
The scale affects the value of PinchViewport::maximumScrollPosition(): the
larger the scale, the smaller is the visibleRect() and the more you are allowed
to scroll. In my experiments the scale went approx 2.5 -> 4.0. When the proper
scale was not set before setLocation(), the latter was erroneously clamped. This
was the bug I was referring to.
Yes, I can imagine that view->setScrollOffset() is clamped and implicitly pulls
the inner viewport, but maybe the inner could have stayed where it was in the
doc (i.e. still within the outer).
I was tempted to do this correction in ViewportAchor::computeOrigins(), see my
comment for this method.

[bokan, 2014/09/15 15:36:35]

You're right, I misspoke, the scale needs to be applied before the offset for
this reason.
Yes, I think it's best to do all the corrections in ViewportAnchor. See my
comments there.

[timav, 2014/09/15 20:04:35]

Done.

+
+    // Set outer viewport position.
+    view->setScrollOffset(mainFrameOrigin);
+
+    // Set the scale.
+    setPageScaleFactor(scaleFactor);
+
+    // Set inner viewport position.
+    page()->frameHost().pinchViewport().setLocation(pinchViewportOrigin);
+}
+
 void WebViewImpl::setPageScaleFactor(float scaleFactor)
 {
     ASSERT(page());
@@ -2939,7 +2993,6 @@ void WebViewImpl::setPageScaleFactor(float scaleFactor, const WebPoint& origin)
         page()->setPageScaleFactor(scaleFactor, newScrollOffset);
 }
 
-
 float WebViewImpl::deviceScaleFactor() const
 {
     if (!page())