Revert of Revert of Initial draft - modify ViewportAnchor to know abo…

…ut both inner and outer viewports. (patchset #1 id:1 of https://codereview.chromium.org/597113002/) Reason for revert: Reapplying, reverting this one shouldn't be necessary. Original issue's description: > Revert of Initial draft - modify ViewportAnchor to know about both inner and outer viewports. (patchset #11 id:200001 of https://codereview.chromium.org/556703005/) > > Reason for revert: > The new test is failing on mac. Reverting for now so it can be investigated. > > [ FAILED ] PinchViewportTest.TestResizeAfterHorizontalScroll (39 ms) > [1471/1471] PinchViewportTest.TestResizeAfterHorizontalScroll (39 ms) > Retrying 2 tests (retry #3) > [ RUN ] PinchViewportTest.TestResizeAfterVerticalScroll > ../../third_party/WebKit/Source/web/tests/PinchViewportTest.cpp:251: Failure > Value of: (pinchViewport.visibleRect().size()).width() > Actual: 45.94595 > Expected: (FloatSize(50, 25)).width() > Which is: 50 > ../../third_party/WebKit/Source/web/tests/PinchViewportTest.cpp:251: Failure > Value of: (pinchViewport.visibleRect().size()).height() > Actual: 22.972975 > Expected: (FloatSize(50, 25)).height() > Which is: 25 > ../../third_party/WebKit/Source/web/tests/PinchViewportTest.cpp:253: Failure > Value of: (frame()->view()->scrollPosition()).y() > Actual: 638 > Expected: (IntPoint(0, 625)).y() > Which is: 625 > ../../third_party/WebKit/Source/web/tests/PinchViewportTest.cpp:254: Failure > Value of: (pinchViewport.location()).y() > Actual: 62 > Expected: (FloatPoint(0, 75)).y() > Which is: 75 > > Original issue's description: > > Fix pinch virtual viewport position after resize. > > > > Associate the viewport anchor with the inner viewport. > > Adjust the inner and outer viewport positions after resize > > such that they both remain in their allowed range and the > > inner viewport origin scales proportionally within the > > outer viewport. > > > > As a small cleanup, made the method ScrollView::scrollTo() protected. > > > > BUG=364108 > > > > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=182365 > > [email protected],[email protected],[email protected],[email protected],[email protected] > NOTREECHECKS=true > NOTRY=true > BUG=364108 > > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=182580 [email protected],[email protected],[email protected],[email protected],[email protected] NOTREECHECKS=true NOTRY=true BUG=364108 Review URL: https://codereview.chromium.org/597683007 git-svn-id: svn://svn.chromium.org/blink/trunk@182586 bbb929c8-8fbe-4397-9dbb-9b2b20218538
yoavweiss · Sep 24, 2014 · aca131c · aca131c
1 parent db83e1f
commit aca131c
Show file tree

Hide file tree

Showing 8 changed files with 310 additions and 29 deletions.
diff --git a/Source/core/frame/PinchViewport.h b/Source/core/frame/PinchViewport.h
@@ -80,6 +80,7 @@ class PinchViewport FINAL : public GraphicsLayerClient, public ScrollableArea {
     // coordinates are in partial CSS pixels.
     void setLocation(const FloatPoint&);
     void move(const FloatPoint&);
+    FloatPoint location() const { return m_offset; } // used in unit tests
 
     // Sets the size of the inner viewport when unscaled in CSS pixels.
     // This will be clamped to the size of the outer viewport (the main frame).

diff --git a/Source/platform/scroll/ScrollView.h b/Source/platform/scroll/ScrollView.h
@@ -55,9 +55,6 @@ class PLATFORM_EXPORT ScrollView : public Widget, public ScrollableArea {
 
     virtual void notifyPageThatContentAreaWillPaint() const;
 
-    // NOTE: This should only be called by the overriden setScrollOffset from ScrollableArea.
-    virtual void scrollTo(const IntSize& newOffset);
-
     // The window that hosts the ScrollView. The ScrollView will communicate scrolls and repaints to the
     // host window in the window's coordinate space.
     virtual HostWindow* hostWindow() const = 0;
@@ -259,6 +256,9 @@ class PLATFORM_EXPORT ScrollView : public Widget, public ScrollableArea {
 protected:
     ScrollView();
 
+    // NOTE: This should only be called by the overriden setScrollOffset from ScrollableArea.
+    virtual void scrollTo(const IntSize& newOffset);
+
     virtual void contentRectangleForPaintInvalidation(const IntRect&);
     virtual void paintContents(GraphicsContext*, const IntRect& damageRect) = 0;
 

diff --git a/Source/web/ViewportAnchor.cpp b/Source/web/ViewportAnchor.cpp
@@ -35,6 +35,7 @@
 #include "core/dom/Node.h"
 #include "core/page/EventHandler.h"
 #include "core/rendering/HitTestResult.h"
+#include "platform/scroll/ScrollView.h"
 
 namespace blink {
 
@@ -68,31 +69,76 @@ Node* findNonEmptyAnchorNode(const IntPoint& point, const IntRect& viewRect, Eve
     return node;
 }
 
+void moveToEncloseRect(IntRect& outer, const FloatRect& inner)
+{
+    IntPoint minimumPosition = ceiledIntPoint(inner.location() + inner.size() - FloatSize(outer.size()));
+    IntPoint maximumPosition = flooredIntPoint(inner.location());
+
+    IntPoint outerOrigin = outer.location();
+    outerOrigin = outerOrigin.expandedTo(minimumPosition);
+    outerOrigin = outerOrigin.shrunkTo(maximumPosition);
+
+    outer.setLocation(outerOrigin);
+}
+
+void moveIntoRect(FloatRect& inner, const IntRect& outer)
+{
+    FloatPoint minimumPosition = FloatPoint(outer.location());
+    FloatPoint maximumPosition = minimumPosition + outer.size() - inner.size();
+
+    // Adjust maximumPosition to the nearest lower integer because
+    // PinchViewport::maximumScrollPosition() does the same.
+    // The value of minumumPosition is already adjusted since it is
+    // constructed from an integer point.
+    maximumPosition = flooredIntPoint(maximumPosition);
+
+    FloatPoint innerOrigin = inner.location();
+    innerOrigin = innerOrigin.expandedTo(minimumPosition);
+    innerOrigin = innerOrigin.shrunkTo(maximumPosition);
+
+    inner.setLocation(innerOrigin);
+}
+
 } // namespace
 
 ViewportAnchor::ViewportAnchor(EventHandler* eventHandler)
     : m_eventHandler(eventHandler) { }
 
-void ViewportAnchor::setAnchor(const IntRect& viewRect, const FloatSize& anchorInViewCoords)
+void ViewportAnchor::setAnchor(const IntRect& outerViewRect, const IntRect& innerViewRect,
+    const FloatSize& anchorInInnerViewCoords)
 {
-    m_viewRect = viewRect;
+    // Preserve the inner viewport position in document in case we won't find the anchor
+    m_pinchViewportInDocument = innerViewRect.location();
+
     m_anchorNode.clear();
     m_anchorNodeBounds = LayoutRect();
     m_anchorInNodeCoords = FloatSize();
-    m_anchorInViewCoords = anchorInViewCoords;
+    m_anchorInInnerViewCoords = anchorInInnerViewCoords;
+    m_normalizedPinchViewportOffset = FloatSize();
 
-    if (viewRect.isEmpty())
+    if (innerViewRect.isEmpty())
         return;
 
     // Preserve origins at the absolute screen origin
-    if (viewRect.location() == IntPoint::zero())
+    if (innerViewRect.location() == IntPoint::zero())
         return;
 
-    FloatSize anchorOffset = viewRect.size();
-    anchorOffset.scale(anchorInViewCoords.width(), anchorInViewCoords.height());
-    const FloatPoint anchorPoint = FloatPoint(viewRect.location()) + anchorOffset;
+    // Inner rectangle should be within the outer one.
+    ASSERT(outerViewRect.contains(innerViewRect));
+
+    // Outer rectangle is used as a scale, we need positive width and height.
+    ASSERT(!outerViewRect.isEmpty());
 
-    Node* node = findNonEmptyAnchorNode(flooredIntPoint(anchorPoint), viewRect, m_eventHandler);
+    m_normalizedPinchViewportOffset = innerViewRect.location() - outerViewRect.location();
+
+    // Normalize by the size of the outer rect
+    m_normalizedPinchViewportOffset.scale(1.0 / outerViewRect.width(), 1.0 / outerViewRect.height());
+
+    FloatSize anchorOffset = innerViewRect.size();
+    anchorOffset.scale(anchorInInnerViewCoords.width(), anchorInInnerViewCoords.height());
+    const FloatPoint anchorPoint = FloatPoint(innerViewRect.location()) + anchorOffset;
+
+    Node* node = findNonEmptyAnchorNode(flooredIntPoint(anchorPoint), innerViewRect, m_eventHandler);
     if (!node)
         return;
 
@@ -102,24 +148,49 @@ void ViewportAnchor::setAnchor(const IntRect& viewRect, const FloatSize& anchorI
     m_anchorInNodeCoords.scale(1.f / m_anchorNodeBounds.width(), 1.f / m_anchorNodeBounds.height());
 }
 
-IntPoint ViewportAnchor::computeOrigin(const IntSize& currentViewSize) const
+void ViewportAnchor::computeOrigins(const ScrollView& scrollView, const FloatSize& innerSize,
+    IntPoint& mainFrameOffset, FloatPoint& pinchViewportOffset) const
+{
+    IntSize outerSize = scrollView.visibleContentRect().size();
+
+    // Compute the viewport origins in CSS pixels relative to the document.
+    FloatSize absPinchViewportOffset = m_normalizedPinchViewportOffset;
+    absPinchViewportOffset.scale(outerSize.width(), outerSize.height());
+
+    FloatPoint innerOrigin = getInnerOrigin(innerSize);
+    FloatPoint outerOrigin = innerOrigin - absPinchViewportOffset;
+
+    IntRect outerRect = IntRect(flooredIntPoint(outerOrigin), outerSize);
+    FloatRect innerRect = FloatRect(innerOrigin, innerSize);
+
+    moveToEncloseRect(outerRect, innerRect);
+
+    outerRect.setLocation(scrollView.adjustScrollPositionWithinRange(outerRect.location()));
+
+    moveIntoRect(innerRect, outerRect);
+
+    mainFrameOffset = outerRect.location();
+    pinchViewportOffset = FloatPoint(innerRect.location() - outerRect.location());
+}
+
+FloatPoint ViewportAnchor::getInnerOrigin(const FloatSize& innerSize) const
 {
     if (!m_anchorNode || !m_anchorNode->inDocument())
-        return m_viewRect.location();
+        return m_pinchViewportInDocument;
 
     const LayoutRect currentNodeBounds = m_anchorNode->boundingBox();
     if (m_anchorNodeBounds == currentNodeBounds)
-        return m_viewRect.location();
+        return m_pinchViewportInDocument;
 
     // 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 = currentViewSize;
-    anchorOffsetFromOrigin.scale(m_anchorInViewCoords.width(), m_anchorInViewCoords.height());
-    return flooredIntPoint(anchorPoint - anchorOffsetFromOrigin);
+    FloatSize anchorOffsetFromOrigin = innerSize;
+    anchorOffsetFromOrigin.scale(m_anchorInInnerViewCoords.width(), m_anchorInInnerViewCoords.height());
+    return anchorPoint - anchorOffsetFromOrigin;
 }
 
 } // namespace blink
diff --git a/Source/web/ViewportAnchor.h b/Source/web/ViewportAnchor.h
@@ -43,6 +43,7 @@ namespace blink {
 class EventHandler;
 class IntSize;
 class Node;
+class ScrollView;
 
 // ViewportAnchor provides a way to anchor a viewport origin to a DOM node.
 // In particular, the user supplies the current viewport (in CSS coordinates)
@@ -56,22 +57,31 @@ class ViewportAnchor {
 public:
     explicit ViewportAnchor(EventHandler*);
 
-    void setAnchor(const IntRect& viewRect, const FloatSize& anchorInViewCoords);
+    void setAnchor(const IntRect& outerViewRect, const IntRect& innerViewRect, const FloatSize& anchorInViewCoords);
 
-    IntPoint computeOrigin(const IntSize& currentViewSize) const;
+    void computeOrigins(const ScrollView&, const FloatSize& innerSize,
+        IntPoint& mainFrameOffset, FloatPoint& pinchViewportOffset) const;
+
+private:
+    FloatPoint getInnerOrigin(const FloatSize& innerSize) const;
 
 private:
     RawPtrWillBeMember<EventHandler> m_eventHandler;
 
-    IntRect m_viewRect;
+    // Inner viewport origin in the reference frame of the document in CSS pixels
+    FloatPoint m_pinchViewportInDocument;
+
+    // Inner viewport origin in the reference frame of the outer viewport
+    // normalized to the outer viewport size.
+    FloatSize m_normalizedPinchViewportOffset;
 
     RefPtrWillBeMember<Node> m_anchorNode;
     LayoutRect m_anchorNodeBounds;
 
-    FloatSize m_anchorInViewCoords;
+    FloatSize m_anchorInInnerViewCoords;
     FloatSize m_anchorInNodeCoords;
 };
 
 } // namespace blink
 
- #endif
+#endif
diff --git a/Source/web/WebViewImpl.cpp b/Source/web/WebViewImpl.cpp
@@ -1705,8 +1705,10 @@ void WebViewImpl::resize(const WebSize& newSize)
 
     ViewportAnchor viewportAnchor(&localFrameRootTemporary()->frame()->eventHandler());
     if (shouldAnchorAndRescaleViewport) {
-        viewportAnchor.setAnchor(view->visibleContentRect(),
-                                 FloatSize(viewportAnchorXCoord, viewportAnchorYCoord));
+        viewportAnchor.setAnchor(
+            view->visibleContentRect(),
+            visibleRectInDocument(),
+            FloatSize(viewportAnchorXCoord, viewportAnchorYCoord));
     }
 
     // FIXME: TextAutosizer does not yet support out-of-process frames.
@@ -1726,15 +1728,33 @@ 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));
+            newPageScaleFactor = clampPageScaleFactorToLimits(newPageScaleFactor);
+
+            FloatSize pinchViewportSize = FloatSize(newSize);
+            pinchViewportSize.scale(1 / newPageScaleFactor);
+
+            IntPoint mainFrameOrigin;
+            FloatPoint pinchViewportOrigin;
+            viewportAnchor.computeOrigins(*view, 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())
@@ -2940,6 +2960,36 @@ WebFloatPoint WebViewImpl::pinchViewportOffset() const
     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.
+
+    view->setScrollOffset(mainFrameOrigin);
+
+    setPageScaleFactor(scaleFactor);
+
+    page()->frameHost().pinchViewport().setLocation(pinchViewportOrigin);
+}
+
 void WebViewImpl::setPageScaleFactor(float scaleFactor)
 {
     ASSERT(page());
@@ -2982,7 +3032,6 @@ void WebViewImpl::setPageScaleFactor(float scaleFactor, const WebPoint& origin)
         page()->setPageScaleFactor(scaleFactor, newScrollOffset);
 }
 
-
 float WebViewImpl::deviceScaleFactor() const
 {
     if (!page())

diff --git a/Source/web/WebViewImpl.h b/Source/web/WebViewImpl.h
@@ -505,6 +505,10 @@ class WebViewImpl FINAL : public WebView
     // prevent external usage
     virtual void setPageScaleFactor(float scaleFactor, const WebPoint& origin) OVERRIDE;
 
+    void scrollAndRescaleViewports(float scaleFactor, const IntPoint& mainFrameOrigin, const FloatPoint& pinchViewportOrigin);
+
+    IntRect visibleRectInDocument() const;
+
     float legibleScale() const;
     void refreshPageScaleFactorAfterLayout();
     void resumeTreeViewCommits();