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

Source/web/WebViewImpl.cpp

 /*
  * Copyright (C) 2011, 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 1661 matching lines @@
 
     bool shouldAnchorAndRescaleViewport = settings()->mainFrameResizesAreOrientationChanges()
         && m_size.width && contentsSize().width() && newSize.width != m_size.width && !m_fullscreenController->isFullscreen();
     float oldPageScaleFactor = pageScaleFactor();
     float oldMinimumPageScaleFactor = minimumPageScaleFactor();
 
     m_size = 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())
             view->layout();
 
         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())
         mainFrameImpl()->frameView()->willEndLiveResize();
 
     LocalFrame* frame = mainFrameImpl()->frame();
     WebPluginContainerImpl* pluginContainer = WebLocalFrameImpl::pluginContainerFromFrame(frame);
     if (pluginContainer)
         pluginContainer->willEndLiveResize();
 }
@@ skipping 1193 matching lines @@
 
     page()->frameHost().pinchViewport().setScale(scaleFactor);
     deviceOrPageScaleFactorChanged();
 }
 
 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())
         return;
 
     IntPoint newScrollOffset = origin;
     scaleFactor = clampPageScaleFactorToLimits(scaleFactor);
     newScrollOffset = clampOffsetAtScale(newScrollOffset, scaleFactor);
 
     if (pinchVirtualViewportEnabled())
@@ skipping 1338 matching lines @@
     const PageScaleConstraints& constraints = m_pageScaleConstraintsSet.pageDefinedConstraints();
 
     if (!mainFrameImpl() || !mainFrameImpl()->frameView())
         return false;
 
     return mainFrameImpl()->frameView()->layoutSize().width() == m_size.width
         || (constraints.minimumScale == constraints.maximumScale && constraints.minimumScale != -1);
 }
 
 } // namespace blink