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(),
-                                 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.
     if (mainFrameImpl() && mainFrameImpl()->frame()->isLocalFrame())
     {
         // Avoids unnecessary invalidations while various bits of state in TextAutosizer are updated.
         TextAutosizer::DeferUpdatePageInfo deferUpdatePageInfo(page());
         performResize();
     } else {
         performResize();
     }
 
     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));
+            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())
         mainFrameImpl()->frameView()->willEndLiveResize();
 
     LocalFrame* frame = mainFrameImpl()->frame();
     WebPluginContainerImpl* pluginContainer = WebLocalFrameImpl::pluginContainerFromFrame(frame);
     if (pluginContainer)
         pluginContainer->willEndLiveResize();
 }
@@ skipping 1168 matching lines @@
 WebFloatPoint WebViewImpl::pinchViewportOffset() const
 {
     ASSERT(page());
 
     if (!pinchVirtualViewportEnabled())
         return WebFloatPoint();
 
     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());
 
     scaleFactor = clampPageScaleFactorToLimits(scaleFactor);
     if (scaleFactor == pageScaleFactor())
         return;
 
     // TODO(bokan): Old-style pinch path. Remove when we're migrated to
     // virtual viewport pinch.
@@ skipping 22 matching lines @@
     newScrollOffset = clampOffsetAtScale(newScrollOffset, scaleFactor);
 
     if (pinchVirtualViewportEnabled())
         setPageScaleFactor(scaleFactor);
         // Note, we don't set the offset in the new path. This method is going
         // away for the new pinch model so that's ok.
     else
         page()->setPageScaleFactor(scaleFactor, newScrollOffset);
 }
 
-
 float WebViewImpl::deviceScaleFactor() const
 {
     if (!page())
         return 1;
 
     return page()->deviceScaleFactor();
 }
 
 void WebViewImpl::setDeviceScaleFactor(float scaleFactor)
 {
@@ skipping 1320 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