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..ce94184b7d05e807117039973402fb3b81655a55 100644
--- a/Source/web/WebViewImpl.cpp
+++ b/Source/web/WebViewImpl.cpp
@@ -1679,20 +1679,19 @@ void WebViewImpl::resize(const WebSize& newSize)
 
     ViewportAnchor viewportAnchor(&localFrameRootTemporary()->frame()->eventHandler());
     if (shouldAnchorAndRescaleViewport) {
-        viewportAnchor.setAnchor(view->visibleContentRect(),
+        // TODO: should we pass FloatRects instead of IntRects?
+        viewportAnchor.setAnchor(visibleRectInDocument(), outerViewportRectInDocument(),
                                  FloatSize(viewportAnchorXCoord, viewportAnchorYCoord));
     }
 
     // FIXME: TextAutosizer does not yet support out-of-process frames.
-    if (mainFrameImpl() && mainFrameImpl()->frame()->isLocalFrame())
-    {
+    if (mainFrameImpl() && mainFrameImpl()->frame()->isLocalFrame()) {
         // Avoids unnecessary invalidations while various bits of state in TextAutosizer are updated.
         TextAutosizer::DeferUpdatePageInfo deferUpdatePageInfo(page());
-        performResize();
-    } else {
-        performResize();
     }
 
+    performResize();

[bokan, 2014/09/10 15:03:59]

Why did this get moved? TextAutosizer::DeferUpdatePageInfo sets/unsets some
global state in its constructor/destructor so this is not equivalent.

[timav, 2014/09/10 18:22:40]

Oops. Thank you, I did not catch that. Will undo.

+
     if (settings()->viewportEnabled() && !m_fixedLayoutSizeLock) {
         // Relayout immediately to recalculate the minimum scale limit.
         if (view->needsLayout())
@@ -1700,15 +1699,40 @@ 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));
+            IntSize innerViewportSize = newSize;
+            innerViewportSize.scale(1 / newPageScaleFactor);
+
+            // in CSS pixel relative to the document
+            IntPoint innerOrigin = viewportAnchor.computeInnerViewportOrigin(innerViewportSize);

[bokan, 2014/09/10 15:03:59]

Can you please rename as follows (here and other places in this CL):

innerOrigin -> pinchViewportOrigin
outerOrigin -> mainFrameOrigin

On CC they're referred to by inner/outer, in Blink they're pinchViewport and
mainFrame. Hopefully we'll soon converge and rename to one set everywhere but
for now we should at least maintain consistency within local context.

+
+            // in CSS pixel relative to the document
+            IntPoint outerOrigin = viewportAnchor.computeOuterViewportOrigin(
+                innerOrigin, outerViewportRectInDocument().size());
+
+            scrollAndRescaleViewports(newPageScaleFactor, innerOrigin, outerOrigin);
         }
     }
 
     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();
+}
+
+IntRect WebViewImpl::outerViewportRectInDocument() const

[aelias_OOO_until_Jul13, 2014/09/10 01:46:25]

This method doesn't seem to save much code duplication so please delete it.

[timav, 2014/09/10 18:22:40]

Acknowledged.

+{
+    // Same as the inner viewport if the virtual viewport is disabled
+    return localFrameRootTemporary()->frameView()->visibleContentRect();
+}
+
 void WebViewImpl::willEndLiveResize()
 {
     if (mainFrameImpl() && mainFrameImpl()->frameView())
@@ -2922,6 +2946,36 @@ void WebViewImpl::setMainFrameScrollOffset(const WebPoint& origin)
     updateMainFrameScrollPosition(origin, false);
 }
 
+void WebViewImpl::scrollAndRescaleViewports(float scaleFactor,
+    const IntPoint & innerViewportOrigin,

[aelias_OOO_until_Jul13, 2014/09/10 01:46:25]

Please make these arguments:
const IntPoint& outerViewportOffset, 
const FloatPoint& pinchViewportOffset

then you don't need to do any conversions at all in this method.

[timav, 2014/09/10 18:22:40]

Acknowledged.

+    const IntPoint & outerViewportOrigin)
+{
+    // Old way
+    if (!pinchVirtualViewportEnabled()) {
+        setPageScaleFactor(scaleFactor, innerViewportOrigin);
+        return;
+    }
+
+    if (!page())
+        return;
+
+    if (!mainFrameImpl())
+        return;
+
+    FrameView * view = mainFrameImpl()->frameView();
+    if (!view)
+        return;
+
+    // Set outer viewport position.
+    view->scrollTo(toIntSize(outerViewportOrigin));

[bokan, 2014/09/10 15:03:58]

Use WebViewImpl::updateMainFrameScrollPosition(IntPoint, false) instead. Or
view->setScrollOffset(). ScrollView::scrollTo has a comment not to call from
outside setScrollOffset and looks like it doesn't clamp to scroll bounds (try
going landscape to portrait when fully vertically scrolled). Something,
something, footguns...

[timav, 2014/09/10 18:22:40]

Yes, I missed that comment. The ScrollView::scrollTo() is public though, from
the comment I would assume it should have been protected...
The view->setScrollOffset() seems simpler, I will use it unless there is a
compelling reason to use WebViewImpl::updateMainFrameScrollPosition()

[bokan, 2014/09/10 18:26:40]

Yup, highly easy to misuse. If you don't mind, go ahead and make that protected
in ScrollView (and FrameView). Thanks.

+
+    // Set inner viewport position.
+    IntSize innerViewportInOuter = innerViewportOrigin - outerViewportOrigin;
+    page()->frameHost().pinchViewport().setLocation(IntPoint(innerViewportInOuter));
+
+    setPageScaleFactor(scaleFactor);
+}
+
 void WebViewImpl::setPageScaleFactor(float scaleFactor, const WebPoint& origin)
 {
     if (!page())