Improve mobile device rotation behavior.

Source/WebKit/chromium/src/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 181 matching lines @@
 // The following constants control parameters for automated scaling of webpages
 // (such as due to a double tap gesture or find in page etc.). These are
 // experimentally determined.
 static const int touchPointPadding = 32;
 static const int nonUserInitiatedPointPadding = 11;
 static const float minScaleDifference = 0.01f;
 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;
 static const int minReadableCaretHeight = 18;
 static const float minScaleChangeToTriggerZoom = 1.05f;
 static const float leftBoxRatio = 0.3f;
 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
 // those in Apple's port in WebKit/WebKit/WebView/WebView.mm
 const double WebView::textSizeMultiplierRatio = 1.2;
 const double WebView::minTextSizeMultiplier = 0.5;
 const double WebView::maxTextSizeMultiplier = 3.0;
 const float WebView::minPageScaleFactor = 0.25f;
 const float WebView::maxPageScaleFactor = 4.0f;
 
@@ skipping 1441 matching lines @@
 {
     if (m_shouldAutoResize || m_size == newSize)
         return;
 
     FrameView* view = mainFrameImpl()->frameView();
     if (!view)
         return;
 
     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
 
     WebDevToolsAgentPrivate* agentPrivate = devToolsAgentPrivate();
     if (agentPrivate)
         agentPrivate->webViewResized(newSize);
     if (!agentPrivate || !agentPrivate->metricsOverridden()) {
         WebFrameImpl* webFrame = mainFrameImpl();
         if (webFrame->frameView())
             webFrame->frameView()->resize(m_size);
     }
 
 #if ENABLE(VIEWPORT)
     if (settings()->viewportEnabled()) {
         // Relayout immediately to recalculate the minimum scale limit.
         if (view->needsLayout())
             view->layout();
 
         // When the device rotates:

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

This comment is out of date, could you delete/update?

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

Done.

         // - If the page width is unchanged, then zoom by new width/old width
         //   such as to keep the same content horizontally onscreen.
         // - If the page width stretches proportionally to the change in
         //   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
 
     sendResizeEventAndRepaint();
 }
 
 void WebViewImpl::willEndLiveResize()
 {
     if (mainFrameImpl() && mainFrameImpl()->frameView())
         mainFrameImpl()->frameView()->willEndLiveResize();
@@ skipping 2558 matching lines @@
 #endif
 
 bool WebViewImpl::shouldDisableDesktopWorkarounds()
 {
     ViewportArguments arguments = mainFrameImpl()->frame()->document()->viewportArguments();
     return arguments.width == ViewportArguments::ValueDeviceWidth || !arguments.userZoom
         || (arguments.minZoom == arguments.maxZoom && arguments.minZoom != ViewportArguments::ValueAuto);
 }
 
 } // namespace WebKit