Improve mobile device rotation behavior.

Source/WebKit/chromium/src/WebViewImpl.cpp

Index: Source/WebKit/chromium/src/WebViewImpl.cpp
diff --git a/Source/WebKit/chromium/src/WebViewImpl.cpp b/Source/WebKit/chromium/src/WebViewImpl.cpp
index dd732f5136c940f128c2b12c2e3d106a5fc0eb30..4328582d8eb92c0d62af294e52bc1e845a229a2b 100644
--- a/Source/WebKit/chromium/src/WebViewImpl.cpp
+++ b/Source/WebKit/chromium/src/WebViewImpl.cpp
@@ -199,6 +199,7 @@ static const float doubleTapZoomContentDefaultMargin = 5;
 static const float doubleTapZoomContentMinimumMargin = 2;
 static const double doubleTapZoomAnimationDurationInSeconds = 0.25;
 static const float doubleTapZoomAlreadyLegibleRatio = 1.2f;
+static const float viewportAnchorRelativeEpsilon = 0.1f;
 
 // Constants for zooming in on a focused text field.
 static const double scrollAndScaleAnimationDurationInSeconds = 0.2;
@@ -209,6 +210,113 @@ static const int caretPadding = 10;
 
 namespace WebKit {
 
+#if ENABLE(VIEWPORT)
+namespace {
+// ViewportAnchor provides a way to anchor a viewport origin to a DOM node.
+// In particular, the user supplies the current viewport (in CSS coordinates)
+// and an anchor point (in view coordinates, e.g., (0, 0) == viewport origin,
+// (0.5, 0) == viewport top center). The anchor point tracks the underlying DOM
+// node; as the node moves or the view is resized, the viewport anchor maintains
+// its orientation relative to the node, and the viewport origin maintains its
+// orientation relative to the anchor.
+class ViewportAnchor {
+public:
+    ViewportAnchor(EventHandler* eventHandler)
+        : m_eventHandler(eventHandler)
+    {
+        ASSERT(m_eventHandler);

[aelias_OOO_until_Jul13, 2013/04/08 04:24:52]

This type of assertion is not an idiom in WebKit, please remove.  We'll just hit
the null pointer dereference later anyway.

[jdduke (slow), 2013/04/10 16:58:19]

Done.

+    }
+
+    void setAnchor(const IntRect& viewRect, const FloatSize& anchorInViewCoords)
+    {
+        m_viewRect = viewRect;
+        m_anchorNode.clear();
+        m_anchorNodeBounds = LayoutRect();
+        m_anchorInNodeCoords = FloatSize();
+        m_anchorInViewCoords = anchorInViewCoords;
+
+        if (viewRect.isEmpty())
+            return;
+
+        // Preserve origins at the absolute screen origin
+        if (viewRect.location() == IntPoint::zero())
+            return;
+
+        FloatSize anchorOffset = FloatSize(viewRect.size());

[aelias_OOO_until_Jul13, 2013/04/08 04:24:52]

= viewRect.size()

[jdduke (slow), 2013/04/10 16:58:19]

Done.

+        anchorOffset.scale(anchorInViewCoords.width(), anchorInViewCoords.height());
+        const FloatPoint anchorPoint = FloatPoint(viewRect.location()) + anchorOffset;
+
+        Node* node = findNonEmptyAnchorNode(flooredIntPoint(anchorPoint), viewRect, m_eventHandler);
+        if (!node)
+            return;
+
+        m_anchorNode = node;
+        m_anchorNodeBounds = node->Node::boundingBox();

[aelias_OOO_until_Jul13, 2013/04/08 04:24:52]

node->boundingBox().  The Node:: you copied from somewhere was needed at one
point to work around a bug, but no longer.

[jdduke (slow), 2013/04/10 16:58:19]

Done.

+
+        m_anchorInNodeCoords = anchorPoint - m_anchorNodeBounds.location();
+        m_anchorInNodeCoords.scale(1.f / m_anchorNodeBounds.width(), 1.f / m_anchorNodeBounds.height());

[aelias_OOO_until_Jul13, 2013/04/08 04:24:52]

Delete .f

[jdduke (slow), 2013/04/10 16:58:19]

Without the .f, int / LayoutUnit performs integer division!? Spent all morning
looking for this bug =/ I'll use a FloatRect to avoid any confusion here...

+    }
+
+    // Note: No guarantees are made on the validity of the returned point.

[aelias_OOO_until_Jul13, 2013/04/08 04:24:52]

This comment is too scary and not very useful.  Can you switch to saying "The
returned point may not be clamped to document bounds" since I think that's what
you mean?

[jdduke (slow), 2013/04/10 16:58:19]

Done.

+    //       It is up to the caller to ensure the point is properly bounded for
+    //       the relevant use-case.
+    IntPoint computeOrigin(const IntSize& currentViewSize) const
+    {
+        if (!m_anchorNode || !m_anchorNode->inDocument())
+            return m_viewRect.location();
+
+        const LayoutRect currentNodeBounds = m_anchorNode->Node::boundingBox();

[aelias_OOO_until_Jul13, 2013/04/08 04:24:52]

m_anchorNode->boundingBox()

[jdduke (slow), 2013/04/10 16:58:19]

Done.

+        if (m_anchorNodeBounds == currentNodeBounds)
+            return m_viewRect.location();
+
+        // Compute the new anchor point relative to the node position
+        FloatSize anchorOffsetFromNode = currentNodeBounds.size();
+        anchorOffsetFromNode.scale(m_anchorInNodeCoords.width(), m_anchorInNodeCoords.height());
+        FloatPoint anchorPoint = currentNodeBounds.location() + anchorOffsetFromNode;
+
+        // Compute the new origin point relative to the new anchor point
+        FloatSize anchorOffsetFromOrigin = FloatSize(currentViewSize);

[aelias_OOO_until_Jul13, 2013/04/08 04:24:52]

= currentViewSize;

[jdduke (slow), 2013/04/10 16:58:19]

Done.

+        anchorOffsetFromOrigin.scale(m_anchorInViewCoords.width(), m_anchorInViewCoords.height());
+        return flooredIntPoint(anchorPoint - anchorOffsetFromOrigin);
+    }
+
+private:
+    ViewportAnchor();
+    ViewportAnchor& operator=(ViewportAnchor&);

[aelias_OOO_until_Jul13, 2013/04/08 04:24:52]

Not a common idiom to have a private operator=, please delete.

[jdduke (slow), 2013/04/10 16:58:19]

Done.

+
+    static Node* findNonEmptyAnchorNode(const IntPoint& point, const IntRect& viewRect, EventHandler* eventHandler)
+    {
+        Node* node = eventHandler->hitTestResultAtPoint(point).innerNode();
+
+        // If the node bounding box contains the view, make a single attempt to
+        // find a smaller node; the larger the node bounds, the greater the
+        // variability under resize.
+        if (node && node->Node::pixelSnappedBoundingBox().contains(viewRect)) {

[aelias_OOO_until_Jul13, 2013/04/08 04:24:52]

node->boundingBox() ?

[jdduke (slow), 2013/04/10 16:58:19]

Done.

+            IntSize pointOffset = viewRect.size();
+            pointOffset.scale(viewportAnchorRelativeEpsilon);
+            IntPoint shiftedPoint = point + pointOffset;

[aelias_OOO_until_Jul13, 2013/04/08 04:24:52]

Please delete this local variable.

[jdduke (slow), 2013/04/10 16:58:19]

Done.

+            node = eventHandler->hitTestResultAtPoint(shiftedPoint).innerNode();
+        }
+
+        while (node && node->Node::boundingBox().isEmpty())

[aelias_OOO_until_Jul13, 2013/04/08 04:24:52]

Switch to node->boundingBox().

[jdduke (slow), 2013/04/10 16:58:19]

Done.

+            node = node->parentNode();
+
+        return node;
+    }
+
+    EventHandler* m_eventHandler;
+
+    IntRect m_viewRect;
+
+    RefPtr<Node> m_anchorNode;
+    LayoutRect m_anchorNodeBounds;
+
+    FloatSize m_anchorInViewCoords;
+    FloatSize m_anchorInNodeCoords;
+};
+}
+#endif // ENABLE(VIEWPORT)
+
 // Change the text zoom level by kTextSizeMultiplierRatio each time the user
 // zooms text in or out (ie., change by 20%).  The min and max values limit
 // text zoom to half and 3x the original text size.  These three values match
@@ -1670,12 +1778,17 @@ void WebViewImpl::resize(const WebSize& newSize)
 
     WebSize oldSize = m_size;
     float oldPageScaleFactor = pageScaleFactor();
-    IntSize oldScrollOffset = view->scrollOffset();
+    float oldMinimumPageScaleFactor = m_minimumPageScaleFactor;
+    int oldContentsWidth = contentsSize().width();
     int oldFixedLayoutWidth = fixedLayoutSize().width;
 
     m_size = newSize;
 
 #if ENABLE(VIEWPORT)
+    ViewportAnchor viewportAnchor(mainFrameImpl()->frame()->eventHandler());
+    if (settings()->viewportEnabled())
+        viewportAnchor.setAnchor(view->visibleContentRect(), FloatSize(0.5f, 0));

[aelias_OOO_until_Jul13, 2013/04/08 04:24:52]

FloatSize(0.5f, 0) looks like a magic number, please put in a constant.

[jdduke (slow), 2013/04/10 16:58:19]

Done.

[jdduke (slow), 2013/04/10 16:58:19]

Done.

+
     ViewportArguments viewportArguments = mainFrameImpl()->frame()->document()->viewportArguments();
     m_page->chrome()->client()->dispatchViewportPropertiesDidChange(viewportArguments);
 #endif
@@ -1702,14 +1815,26 @@ void WebViewImpl::resize(const WebSize& newSize)
         //   screen width, then don't zoom at all (assuming the content has
         //   scaled uniformly, then the same content will be horizontally
         //   onscreen).
-        //   - If the page width partially stretches, then zoom partially to
+        // - If the page width partially stretches, then zoom partially to
         //   make up the difference.
+        // - If the resized content triggers a change in minimum scale limit,
+        //   zoom relative to the change in content width.
         // In all cases try to keep the same content at the top of the screen.
+        IntSize viewportSize = view->visibleContentRect().size();
         float viewportWidthRatio = !oldSize.width ? 1 : newSize.width / (float) oldSize.width;
         float fixedLayoutWidthRatio = !oldFixedLayoutWidth ? 1 : fixedLayoutSize().width / (float) oldFixedLayoutWidth;
-        float scaleMultiplier = viewportWidthRatio / fixedLayoutWidthRatio;
-        if (scaleMultiplier != 1)
-            setPageScaleFactor(oldPageScaleFactor * scaleMultiplier, WebPoint(oldScrollOffset.width(), oldScrollOffset.height()));
+        float contentsWidthRatio = !oldContentsWidth ? 1 : contentsSize().width() / (float) oldContentsWidth;
+        float scaleMultiplier = m_minimumPageScaleFactor == oldMinimumPageScaleFactor ?

[aelias_OOO_until_Jul13, 2013/04/08 04:24:52]

Too much for ternary operator, please make into an if statement.

[jdduke (slow), 2013/04/10 16:58:19]

Done.

+            viewportWidthRatio / fixedLayoutWidthRatio : viewportWidthRatio / contentsWidthRatio;
+        if (scaleMultiplier != 1) {
+            float newPageScaleFactor = oldPageScaleFactor * scaleMultiplier;
+            viewportSize.scale(pageScaleFactor() / newPageScaleFactor);
+            IntPoint scrollOffsetAtNewScale = viewportAnchor.computeOrigin(viewportSize);
+            setPageScaleFactor(newPageScaleFactor, scrollOffsetAtNewScale);
+        } else {
+            IntPoint scrollOffsetAtNewScale = clampOffsetAtScale(viewportAnchor.computeOrigin(viewportSize), pageScaleFactor());
+            updateMainFrameScrollPosition(scrollOffsetAtNewScale, false);
+        }
     }
 #endif