Remove ScrollView::scrollOffset() in preparation for scrollOffset vs. scrollPosition...
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 30 Dec 2015 00:29:36 +0000 (00:29 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 30 Dec 2015 00:29:36 +0000 (00:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=152589

Reviewed by Sam Weinig.

Current code uses scrollOffset vs. scrollPosition interchangeably, and confusingly.
Longer term, I plan to make "scrollPosition" be the value that is relative to the
contents, i.e. affected by scrollOrigin, and "scrollOffset" be the zero-based value
that's used to set scrollbar values.

To prepare for this, remove ScrollView::scrollOffset(), which is just the
scrollPosition as an IntSize.

Add some typedefs in ScrollableArea, which will slowly propagate through the
code as position vs. offset is clarified.

Source/WebCore:

* inspector/InspectorOverlay.cpp:
(WebCore::contentsQuadToCoordinateSystem):
(WebCore::InspectorOverlay::highlightQuad):
(WebCore::localPointToRoot):
* page/FrameView.cpp:
(WebCore::FrameView::scrollOffsetRespectingCustomFixedPosition):
(WebCore::FrameView::topContentInsetDidChange):
(WebCore::FrameView::addTrackedRepaintRect):
(WebCore::FrameView::scrollTo):
(WebCore::FrameView::wheelEvent):
(WebCore::FrameView::setScrollPinningBehavior):
* page/FrameView.h:
* page/SpatialNavigation.cpp:
(WebCore::canScrollInDirection):
(WebCore::rectToAbsoluteCoordinates):
* platform/ScrollView.cpp:
(WebCore::ScrollView::setScrollbarModes):
(WebCore::ScrollView::availableContentSizeChanged):
(WebCore::ScrollView::setContentsSize):
(WebCore::ScrollView::maximumScrollPosition):
(WebCore::ScrollView::minimumScrollPosition):
(WebCore::ScrollView::adjustScrollPositionWithinRange):
(WebCore::ScrollView::documentScrollOffsetRelativeToViewOrigin):
(WebCore::ScrollView::documentScrollOffsetRelativeToScrollableAreaOrigin):
(WebCore::ScrollView::setScrollPosition):
(WebCore::ScrollView::updateScrollbars):
(WebCore::ScrollView::rootViewToTotalContents):
(WebCore::ScrollView::setFrameRect):
(WebCore::ScrollView::scrollbarStyleChanged):
(WebCore::ScrollView::setScrollOrigin):
* platform/ScrollView.h:
(WebCore::ScrollView::convertChildToSelf):
(WebCore::ScrollView::convertSelfToChild):
(WebCore::ScrollView::scrollOffset): Deleted.
* platform/ScrollableArea.cpp:
(WebCore::ScrollableArea::scrollbarIntrusion):
(WebCore::ScrollableArea::scrollPosition):
(WebCore::ScrollableArea::minimumScrollPosition):
(WebCore::ScrollableArea::maximumScrollPosition):
* platform/ScrollableArea.h:
* rendering/RenderBox.cpp:
(WebCore::RenderBox::calculateAutoscrollDirection):
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::scrollPosition):
(WebCore::RenderLayer::minimumScrollPosition):
(WebCore::RenderLayer::maximumScrollPosition):
* rendering/RenderLayer.h:
* rendering/RenderWidget.cpp:
(WebCore::RenderWidget::nodeAtPoint):
* svg/SVGSVGElement.cpp:
(WebCore::SVGSVGElement::localCoordinateSpaceTransform):

Source/WebKit2:

* WebProcess/InjectedBundle/DOM/InjectedBundleRangeHandle.cpp:
(WebKit::InjectedBundleRangeHandle::renderedImage):
* WebProcess/Plugins/PDF/DeprecatedPDFPlugin.h:
* WebProcess/Plugins/PDF/DeprecatedPDFPlugin.mm:
(WebKit::PDFPlugin::scrollPosition):
(WebKit::PDFPlugin::minimumScrollPosition):
(WebKit::PDFPlugin::maximumScrollPosition):
* WebProcess/WebPage/WebFrame.cpp:
(WebKit::WebFrame::scrollOffset):
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::scrollMainFrameIfNotAtMaxScrollPosition):
(WebKit::WebPage::updateMainFrameScrollOffsetPinning):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@194438 268f45cc-cd09-0410-ab3c-d52691b4dbfc

24 files changed:
Source/WebCore/ChangeLog
Source/WebCore/inspector/InspectorOverlay.cpp
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/FrameView.h
Source/WebCore/page/SpatialNavigation.cpp
Source/WebCore/page/win/FrameCGWin.cpp
Source/WebCore/platform/ScrollView.cpp
Source/WebCore/platform/ScrollView.h
Source/WebCore/platform/ScrollableArea.cpp
Source/WebCore/platform/ScrollableArea.h
Source/WebCore/rendering/RenderBox.cpp
Source/WebCore/rendering/RenderLayer.cpp
Source/WebCore/rendering/RenderLayer.h
Source/WebCore/rendering/RenderWidget.cpp
Source/WebCore/svg/SVGSVGElement.cpp
Source/WebKit/win/WebFrame.cpp
Source/WebKit/win/WebView.cpp
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/InjectedBundle/DOM/InjectedBundleRangeHandle.cpp
Source/WebKit2/WebProcess/Plugins/PDF/DeprecatedPDFPlugin.h
Source/WebKit2/WebProcess/Plugins/PDF/DeprecatedPDFPlugin.mm
Source/WebKit2/WebProcess/WebPage/WebFrame.cpp
Source/WebKit2/WebProcess/WebPage/WebPage.cpp
Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm

index e5f95f7..f645021 100644 (file)
@@ -1,3 +1,73 @@
+2015-12-29  Simon Fraser  <simon.fraser@apple.com>
+
+        Remove ScrollView::scrollOffset() in preparation for scrollOffset vs. scrollPosition clarification
+        https://bugs.webkit.org/show_bug.cgi?id=152589
+
+        Reviewed by Sam Weinig.
+
+        Current code uses scrollOffset vs. scrollPosition interchangeably, and confusingly.
+        Longer term, I plan to make "scrollPosition" be the value that is relative to the 
+        contents, i.e. affected by scrollOrigin, and "scrollOffset" be the zero-based value
+        that's used to set scrollbar values.
+        
+        To prepare for this, remove ScrollView::scrollOffset(), which is just the
+        scrollPosition as an IntSize.
+        
+        Add some typedefs in ScrollableArea, which will slowly propagate through the
+        code as position vs. offset is clarified.
+
+        * inspector/InspectorOverlay.cpp:
+        (WebCore::contentsQuadToCoordinateSystem):
+        (WebCore::InspectorOverlay::highlightQuad):
+        (WebCore::localPointToRoot):
+        * page/FrameView.cpp:
+        (WebCore::FrameView::scrollOffsetRespectingCustomFixedPosition):
+        (WebCore::FrameView::topContentInsetDidChange):
+        (WebCore::FrameView::addTrackedRepaintRect):
+        (WebCore::FrameView::scrollTo):
+        (WebCore::FrameView::wheelEvent):
+        (WebCore::FrameView::setScrollPinningBehavior):
+        * page/FrameView.h:
+        * page/SpatialNavigation.cpp:
+        (WebCore::canScrollInDirection):
+        (WebCore::rectToAbsoluteCoordinates):
+        * platform/ScrollView.cpp:
+        (WebCore::ScrollView::setScrollbarModes):
+        (WebCore::ScrollView::availableContentSizeChanged):
+        (WebCore::ScrollView::setContentsSize):
+        (WebCore::ScrollView::maximumScrollPosition):
+        (WebCore::ScrollView::minimumScrollPosition):
+        (WebCore::ScrollView::adjustScrollPositionWithinRange):
+        (WebCore::ScrollView::documentScrollOffsetRelativeToViewOrigin):
+        (WebCore::ScrollView::documentScrollOffsetRelativeToScrollableAreaOrigin):
+        (WebCore::ScrollView::setScrollPosition):
+        (WebCore::ScrollView::updateScrollbars):
+        (WebCore::ScrollView::rootViewToTotalContents):
+        (WebCore::ScrollView::setFrameRect):
+        (WebCore::ScrollView::scrollbarStyleChanged):
+        (WebCore::ScrollView::setScrollOrigin):
+        * platform/ScrollView.h:
+        (WebCore::ScrollView::convertChildToSelf):
+        (WebCore::ScrollView::convertSelfToChild):
+        (WebCore::ScrollView::scrollOffset): Deleted.
+        * platform/ScrollableArea.cpp:
+        (WebCore::ScrollableArea::scrollbarIntrusion):
+        (WebCore::ScrollableArea::scrollPosition):
+        (WebCore::ScrollableArea::minimumScrollPosition):
+        (WebCore::ScrollableArea::maximumScrollPosition):
+        * platform/ScrollableArea.h:
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::calculateAutoscrollDirection):
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::scrollPosition):
+        (WebCore::RenderLayer::minimumScrollPosition):
+        (WebCore::RenderLayer::maximumScrollPosition):
+        * rendering/RenderLayer.h:
+        * rendering/RenderWidget.cpp:
+        (WebCore::RenderWidget::nodeAtPoint):
+        * svg/SVGSVGElement.cpp:
+        (WebCore::SVGSVGElement::localCoordinateSpaceTransform):
+
 2015-12-28  Alex Christensen  <achristensen@webkit.org>
 
         Fix Windows build, ostensibly after r194424.
index 074721d..86b05f9 100644 (file)
@@ -72,7 +72,7 @@ static void contentsQuadToCoordinateSystem(const FrameView* mainView, const Fram
     quad.setP4(view->contentsToRootView(roundedIntPoint(quad.p4())));
 
     if (coordinateSystem == InspectorOverlay::CoordinateSystem::View)
-        quad += mainView->scrollOffset();
+        quad += toIntSize(mainView->scrollPosition());
 }
 
 static void contentsQuadToPage(const FrameView* mainView, const FrameView* view, FloatQuad& quad)
@@ -278,7 +278,7 @@ void InspectorOverlay::highlightNode(Node* node, const HighlightConfig& highligh
 void InspectorOverlay::highlightQuad(std::unique_ptr<FloatQuad> quad, const HighlightConfig& highlightConfig)
 {
     if (highlightConfig.usePageCoordinates)
-        *quad -= m_page.mainFrame().view()->scrollOffset();
+        *quad -= toIntSize(m_page.mainFrame().view()->scrollPosition());
 
     m_quadHighlightConfig = highlightConfig;
     m_highlightQuad = WTF::move(quad);
@@ -594,7 +594,7 @@ static FloatPoint localPointToRoot(RenderObject* renderer, const FrameView* main
 {
     FloatPoint result = renderer->localToAbsolute(point);
     result = view->contentsToRootView(roundedIntPoint(result));
-    result += mainView->scrollOffset();
+    result += toIntSize(mainView->scrollPosition());
     return result;
 }
 
index 5d43755..9923db7 100644 (file)
@@ -1089,7 +1089,7 @@ LayoutRect FrameView::fixedScrollableAreaBoundsInflatedForScrolling(const Layout
 LayoutSize FrameView::scrollOffsetRespectingCustomFixedPosition() const
 {
 #if PLATFORM(IOS)
-    return useCustomFixedPositionLayoutRect() ? customFixedPositionLayoutRect().location() - LayoutPoint() : scrollOffset();
+    return useCustomFixedPositionLayoutRect() ? customFixedPositionLayoutRect().location() - LayoutPoint() : toLayoutSize(scrollPosition());
 #else
     return scrollOffsetForFixedPosition();
 #endif
@@ -1138,7 +1138,7 @@ void FrameView::topContentInsetDidChange(float newTopContentInset)
     
     layout();
 
-    updateScrollbars(scrollOffset());
+    updateScrollbars(scrollPosition());
     if (renderView->usesCompositing())
         renderView->compositor().frameViewDidChangeSize();
 
@@ -2160,20 +2160,19 @@ void FrameView::setFixedVisibleContentRect(const IntRect& visibleContentRect)
         visibleContentSizeDidChange = true;
     }
 
-    IntSize offset = scrollOffset();
     IntPoint oldPosition = scrollPosition();
     ScrollView::setFixedVisibleContentRect(visibleContentRect);
-    if (offset != scrollOffset()) {
+    IntPoint newPosition = scrollPosition();
+    if (oldPosition != newPosition) {
         updateLayerPositionsAfterScrolling();
         if (frame().settings().acceleratedCompositingForFixedPositionEnabled())
             updateCompositingLayersAfterScrolling();
-        IntPoint newPosition = scrollPosition();
-        scrollAnimator().setCurrentPosition(scrollPosition());
+        scrollAnimator().setCurrentPosition(newPosition);
         scrollPositionChanged(oldPosition, newPosition);
     }
     if (visibleContentSizeDidChange) {
         // Update the scroll-bars to calculate new page-step size.
-        updateScrollbars(scrollOffset());
+        updateScrollbars(scrollPosition());
     }
     didChangeScrollOffset();
 }
@@ -2386,7 +2385,7 @@ void FrameView::addTrackedRepaintRect(const FloatRect& r)
         return;
 
     FloatRect repaintRect = r;
-    repaintRect.move(-scrollOffset());
+    repaintRect.moveBy(-scrollPosition());
     m_trackedRepaintRects.append(repaintRect);
 }
 
@@ -3479,10 +3478,9 @@ bool FrameView::forceUpdateScrollbarsOnMainThreadForPerformanceTesting() const
 
 void FrameView::scrollTo(const IntSize& newOffset)
 {
-    LayoutSize offset = scrollOffset();
     IntPoint oldPosition = scrollPosition();
     ScrollView::scrollTo(newOffset);
-    if (offset != scrollOffset())
+    if (oldPosition != scrollPosition())
         scrollPositionChanged(oldPosition, scrollPosition());
     didChangeScrollOffset();
 }
@@ -4587,11 +4585,10 @@ bool FrameView::wheelEvent(const PlatformWheelEvent& wheelEvent)
 #endif
 
     if (delegatesScrolling()) {
-        IntSize offset = scrollOffset();
-        IntPoint oldPosition = scrollPosition();
-        IntSize newOffset = IntSize(offset.width() - wheelEvent.deltaX(), offset.height() - wheelEvent.deltaY());
-        if (offset != newOffset) {
-            ScrollView::scrollTo(newOffset);
+        ScrollPosition oldPosition = scrollPosition();
+        ScrollPosition newPosition = oldPosition - IntSize(wheelEvent.deltaX(), wheelEvent.deltaY());
+        if (oldPosition != newPosition) {
+            ScrollView::scrollTo(toIntSize(newPosition));
             scrollPositionChanged(oldPosition, scrollPosition());
             didChangeScrollOffset();
         }
@@ -4796,7 +4793,7 @@ void FrameView::setScrollPinningBehavior(ScrollPinningBehavior pinning)
             scrollingCoordinator->setScrollPinningBehavior(pinning);
     }
     
-    updateScrollbars(scrollOffset());
+    updateScrollbars(scrollPosition());
 }
 
 ScrollBehaviorForFixedElements FrameView::scrollBehaviorForFixedElements() const
index b1b5045..22ccd49 100644 (file)
@@ -242,8 +242,8 @@ public:
     virtual void updateCompositingLayersAfterScrolling() override;
     virtual bool requestScrollPositionUpdate(const IntPoint&) override;
     virtual bool isRubberBandInProgress() const override;
-    WEBCORE_EXPORT virtual IntPoint minimumScrollPosition() const override;
-    WEBCORE_EXPORT virtual IntPoint maximumScrollPosition() const override;
+    WEBCORE_EXPORT virtual ScrollPosition minimumScrollPosition() const override;
+    WEBCORE_EXPORT virtual ScrollPosition maximumScrollPosition() const override;
 
     void viewportContentsChanged();
     WEBCORE_EXPORT void resumeVisibleImageAnimationsIncludingSubframes();
index 72b008f..1e5754f 100644 (file)
@@ -483,18 +483,19 @@ bool canScrollInDirection(const Frame* frame, FocusDirection direction)
     if ((direction == FocusDirectionUp || direction == FocusDirectionDown) &&  ScrollbarAlwaysOff == verticalMode)
         return false;
     LayoutSize size = frame->view()->totalContentsSize();
-    LayoutSize offset = frame->view()->scrollOffset();
+    LayoutPoint scrollPosition = frame->view()->scrollPosition();
     LayoutRect rect = frame->view()->unobscuredContentRectIncludingScrollbars();
 
+    // FIXME: wrong in RTL documents.
     switch (direction) {
     case FocusDirectionLeft:
-        return offset.width() > 0;
+        return scrollPosition.x() > 0;
     case FocusDirectionUp:
-        return offset.height() > 0;
+        return scrollPosition.y() > 0;
     case FocusDirectionRight:
-        return rect.width() + offset.width() < size.width();
+        return rect.width() + scrollPosition.x() < size.width();
     case FocusDirectionDown:
-        return rect.height() + offset.height() < size.height();
+        return rect.height() + scrollPosition.y() < size.height();
     default:
         ASSERT_NOT_REACHED();
         return false;
@@ -510,7 +511,7 @@ static LayoutRect rectToAbsoluteCoordinates(Frame* initialFrame, const LayoutRec
             do {
                 rect.move(element->offsetLeft(), element->offsetTop());
             } while ((element = element->offsetParent()));
-            rect.move((-frame->view()->scrollOffset()));
+            rect.moveBy((-frame->view()->scrollPosition()));
         }
     }
     return rect;
index e3bd46f..afdfe40 100644 (file)
@@ -41,8 +41,8 @@ namespace WebCore {
 
 static void drawRectIntoContext(IntRect rect, FrameView* view, GraphicsContext& gc)
 {
-    IntSize offset = view->scrollOffset();
-    rect.move(-offset.width(), -offset.height());
+    IntSize scrollPosition = view->scrollPosition();
+    rect.move(-scrollPosition.x(), -scrollPosition.y());
     rect = view->convertToContainingWindow(rect);
 
     gc.concatCTM(AffineTransform().translate(-rect.x(), -rect.y()));
index f230118..cbca25d 100644 (file)
@@ -165,7 +165,7 @@ void ScrollView::setScrollbarModes(ScrollbarMode horizontalMode, ScrollbarMode v
     if (platformWidget())
         platformSetScrollbarModes();
     else
-        updateScrollbars(scrollOffset());
+        updateScrollbars(scrollPosition());
 }
 
 void ScrollView::scrollbarModes(ScrollbarMode& horizontalMode, ScrollbarMode& verticalMode) const
@@ -366,7 +366,7 @@ void ScrollView::availableContentSizeChanged(AvailableSizeChangeReason reason)
         return;
 
     if (reason != AvailableSizeChangeReason::ScrollbarsChanged)
-        updateScrollbars(scrollOffset());
+        updateScrollbars(scrollPosition());
 }
 
 IntSize ScrollView::contentsSize() const
@@ -382,23 +382,23 @@ void ScrollView::setContentsSize(const IntSize& newSize)
     if (platformWidget())
         platformSetContentsSize();
     else
-        updateScrollbars(scrollOffset());
+        updateScrollbars(scrollPosition());
     updateOverhangAreas();
 }
 
-IntPoint ScrollView::maximumScrollPosition() const
+ScrollPosition ScrollView::maximumScrollPosition() const
 {
     IntPoint maximumOffset(contentsWidth() - visibleWidth() - scrollOrigin().x(), totalContentsSize().height() - visibleHeight() - scrollOrigin().y());
     maximumOffset.clampNegativeToZero();
     return maximumOffset;
 }
 
-IntPoint ScrollView::minimumScrollPosition() const
+ScrollPosition ScrollView::minimumScrollPosition() const
 {
     return IntPoint(-scrollOrigin().x(), -scrollOrigin().y());
 }
 
-IntPoint ScrollView::adjustScrollPositionWithinRange(const IntPoint& scrollPoint) const
+ScrollPosition ScrollView::adjustScrollPositionWithinRange(const ScrollPosition& scrollPoint) const
 {
     if (!constrainsScrollingToContentEdge())
         return scrollPoint;
@@ -408,7 +408,7 @@ IntPoint ScrollView::adjustScrollPositionWithinRange(const IntPoint& scrollPoint
 
 IntSize ScrollView::documentScrollOffsetRelativeToViewOrigin() const
 {
-    return scrollOffset() - IntSize(0, headerHeight() + topContentInset(TopContentInsetType::WebCoreOrPlatformContentInset));
+    return toIntSize(scrollPosition()) - IntSize(0, headerHeight() + topContentInset(TopContentInsetType::WebCoreOrPlatformContentInset));
 }
 
 IntPoint ScrollView::documentScrollPositionRelativeToViewOrigin() const
@@ -419,7 +419,7 @@ IntPoint ScrollView::documentScrollPositionRelativeToViewOrigin() const
 
 IntSize ScrollView::documentScrollOffsetRelativeToScrollableAreaOrigin() const
 {
-    return scrollOffset() - IntSize(0, headerHeight());
+    return toIntSize(scrollPosition()) - IntSize(0, headerHeight());
 }
 
 int ScrollView::scrollSize(ScrollbarOrientation orientation) const
@@ -529,25 +529,25 @@ int ScrollView::scrollPosition(Scrollbar* scrollbar) const
     return 0;
 }
 
-void ScrollView::setScrollPosition(const IntPoint& scrollPoint)
+void ScrollView::setScrollPosition(const ScrollPosition& scrollPosition)
 {
     if (prohibitsScrolling())
         return;
 
     if (platformWidget()) {
-        platformSetScrollPosition(scrollPoint);
+        platformSetScrollPosition(scrollPosition);
         return;
     }
 
-    IntPoint newScrollPosition = !delegatesScrolling() ? adjustScrollPositionWithinRange(scrollPoint) : scrollPoint;
+    IntPoint newScrollPosition = !delegatesScrolling() ? adjustScrollPositionWithinRange(scrollPosition) : scrollPosition;
 
-    if ((!delegatesScrolling() || !inProgrammaticScroll()) && newScrollPosition == scrollPosition())
+    if ((!delegatesScrolling() || !inProgrammaticScroll()) && newScrollPosition == this->scrollPosition())
         return;
 
     if (requestScrollPositionUpdate(newScrollPosition))
         return;
 
-    updateScrollbars(IntSize(newScrollPosition.x(), newScrollPosition.y()));
+    updateScrollbars(newScrollPosition);
 }
 
 bool ScrollView::scroll(ScrollDirection direction, ScrollGranularity granularity)
@@ -582,7 +582,7 @@ IntSize ScrollView::overhangAmount() const
     return stretch;
 }
 
-void ScrollView::updateScrollbars(const IntSize& desiredOffset)
+void ScrollView::updateScrollbars(const ScrollPosition& desiredPosition)
 {
     if (m_inUpdateScrollbars || prohibitsScrolling() || platformWidget() || delegatesScrolling())
         return;
@@ -693,7 +693,7 @@ void ScrollView::updateScrollbars(const IntSize& desiredOffset)
                 // The layout with the new scroll state had no impact on
                 // the document's overall size, so updateScrollbars didn't get called.
                 // Recur manually.
-                updateScrollbars(desiredOffset);
+                updateScrollbars(desiredPosition);
             }
             m_updateScrollbarsPass--;
         }
@@ -760,7 +760,7 @@ void ScrollView::updateScrollbars(const IntSize& desiredOffset)
             invalidateScrollCornerRect(oldScrollCornerRect);
     }
 
-    IntPoint adjustedScrollPosition = IntPoint(desiredOffset);
+    IntPoint adjustedScrollPosition = desiredPosition;
     if (!isRubberBandInProgress())
         adjustedScrollPosition = adjustScrollPositionWithinRange(adjustedScrollPosition);
 
@@ -922,7 +922,7 @@ IntPoint ScrollView::rootViewToTotalContents(const IntPoint& rootViewPoint) cons
 
     IntPoint viewPoint = convertFromRootView(rootViewPoint);
     // Like rootViewToContents(), but ignores headerHeight.
-    return viewPoint + scrollOffset() - IntSize(0, topContentInset(TopContentInsetType::WebCoreOrPlatformContentInset));
+    return viewPoint + toIntSize(scrollPosition()) - IntSize(0, topContentInset(TopContentInsetType::WebCoreOrPlatformContentInset));
 }
 
 IntRect ScrollView::contentsToRootView(const IntRect& contentsRect) const
@@ -1037,7 +1037,7 @@ void ScrollView::setFrameRect(const IntRect& newRect)
     Widget::setFrameRect(newRect);
     frameRectsChanged();
 
-    updateScrollbars(scrollOffset());
+    updateScrollbars(scrollPosition());
     
     if (!m_useFixedLayout && oldRect.size() != newRect.size())
         availableContentSizeChanged(AvailableSizeChangeReason::AreaSizeChanged);
@@ -1155,7 +1155,7 @@ void ScrollView::scrollbarStyleChanged(ScrollbarStyle newStyle, bool forceUpdate
     if (!forceUpdate)
         return;
 
-    updateScrollbars(scrollOffset());
+    updateScrollbars(scrollPosition());
     positionScrollbarLayers();
 }
 
@@ -1480,7 +1480,7 @@ void ScrollView::setScrollOrigin(const IntPoint& origin, bool updatePositionAtAl
     
     // Update if the scroll origin changes, since our position will be different if the content size did not change.
     if (updatePositionAtAll && updatePositionSynchronously)
-        updateScrollbars(scrollOffset());
+        updateScrollbars(scrollPosition());
 }
 
 void ScrollView::styleDidChange()
index f46e017..85a04e7 100644 (file)
@@ -225,12 +225,13 @@ public:
     virtual void setContentsSize(const IntSize&);
 
     // Functions for querying the current scrolled position (both as a point, a size, or as individual X and Y values).
-    virtual IntPoint scrollPosition() const override { return visibleContentRect(LegacyIOSDocumentVisibleRect).location(); }
-    IntSize scrollOffset() const { return toIntSize(visibleContentRect(LegacyIOSDocumentVisibleRect).location()); } // Gets the scrolled position as an IntSize. Convenient for adding to other sizes.
-    virtual IntPoint maximumScrollPosition() const override; // The maximum position we can be scrolled to.
-    virtual IntPoint minimumScrollPosition() const override; // The minimum position we can be scrolled to.
+    virtual ScrollPosition scrollPosition() const override { return visibleContentRect(LegacyIOSDocumentVisibleRect).location(); }
+
+    virtual ScrollPosition maximumScrollPosition() const override; // The maximum position we can be scrolled to.
+    virtual ScrollPosition minimumScrollPosition() const override; // The minimum position we can be scrolled to.
+
     // Adjust the passed in scroll position to keep it between the minimum and maximum positions.
-    IntPoint adjustScrollPositionWithinRange(const IntPoint&) const; 
+    ScrollPosition adjustScrollPositionWithinRange(const ScrollPosition&) const;
     int scrollX() const { return scrollPosition().x(); }
     int scrollY() const { return scrollPosition().y(); }
 
@@ -267,7 +268,7 @@ public:
     IntPoint cachedScrollPosition() const { return m_cachedScrollPosition; }
 
     // Functions for scrolling the view.
-    virtual void setScrollPosition(const IntPoint&);
+    virtual void setScrollPosition(const ScrollPosition&);
     void scrollBy(const IntSize& s) { return setScrollPosition(scrollPosition() + s); }
 
     // This function scrolls by lines, pages or pixels.
@@ -331,7 +332,7 @@ public:
     {
         IntPoint newPoint = point;
         if (!isScrollViewScrollbar(child))
-            newPoint = point - scrollOffset();
+            newPoint = point - toIntSize(scrollPosition());
         newPoint.moveBy(child->location());
         return newPoint;
     }
@@ -340,7 +341,7 @@ public:
     {
         IntPoint newPoint = point;
         if (!isScrollViewScrollbar(child))
-            newPoint = point + scrollOffset();
+            newPoint = point + toIntSize(scrollPosition());
         newPoint.moveBy(-child->location());
         return newPoint;
     }
@@ -409,7 +410,7 @@ protected:
     virtual bool isFlippedDocument() const { return false; }
 
     // Called to update the scrollbars to accurately reflect the state of the view.
-    void updateScrollbars(const IntSize& desiredOffset);
+    void updateScrollbars(const ScrollPosition& desiredPosition);
 
     float platformTopContentInset() const;
     void platformSetTopContentInset(float);
index 3bcc25d..be5afa7 100644 (file)
@@ -518,24 +518,26 @@ bool ScrollableArea::isPinnedVerticallyInDirection(int verticalScrollDelta) cons
 
 IntSize ScrollableArea::scrollbarIntrusion() const
 {
-    return IntSize(
+    return {
         verticalScrollbar() ? verticalScrollbar()->occupiedWidth() : 0,
-        horizontalScrollbar() ? horizontalScrollbar()->occupiedHeight() : 0);
+        horizontalScrollbar() ? horizontalScrollbar()->occupiedHeight() : 0
+    };
 }
 
-IntPoint ScrollableArea::scrollPosition() const
+ScrollPosition ScrollableArea::scrollPosition() const
 {
+    // FIXME: This relationship seems to be inverted. Scrollbars should be 'view', not 'model', and should get their values from us.
     int x = horizontalScrollbar() ? horizontalScrollbar()->value() : 0;
     int y = verticalScrollbar() ? verticalScrollbar()->value() : 0;
     return IntPoint(x, y);
 }
 
-IntPoint ScrollableArea::minimumScrollPosition() const
+ScrollPosition ScrollableArea::minimumScrollPosition() const
 {
     return IntPoint();
 }
 
-IntPoint ScrollableArea::maximumScrollPosition() const
+ScrollPosition ScrollableArea::maximumScrollPosition() const
 {
     return IntPoint(totalContentsSize().width() - visibleWidth(), totalContentsSize().height() - visibleHeight());
 }
index be20b64..7fdafa3 100644 (file)
@@ -41,6 +41,11 @@ class ScrollAnimator;
 class GraphicsLayer;
 class TiledBacking;
 
+// scrollPosition is in content coordinates (0,0 is at scrollOrigin), so may have negative components.
+typedef IntPoint ScrollPosition;
+// scrollOffset() is the value used by scrollbars (min is 0,0), and should never have negative components.
+typedef IntPoint ScrollOffset;
+
 class ScrollableArea {
 public:
     WEBCORE_EXPORT bool scroll(ScrollDirection, ScrollGranularity, float multiplier = 1);
@@ -179,9 +184,10 @@ public:
     virtual Scrollbar* horizontalScrollbar() const { return 0; }
     virtual Scrollbar* verticalScrollbar() const { return 0; }
 
-    virtual IntPoint scrollPosition() const;
-    virtual IntPoint minimumScrollPosition() const;
-    virtual IntPoint maximumScrollPosition() const;
+    virtual ScrollPosition scrollPosition() const;
+    virtual ScrollPosition minimumScrollPosition() const;
+    virtual ScrollPosition maximumScrollPosition() const;
+
     WEBCORE_EXPORT virtual bool scrolledToTop() const;
     WEBCORE_EXPORT virtual bool scrolledToBottom() const;
     WEBCORE_EXPORT virtual bool scrolledToLeft() const;
index c11e10e..0673519 100644 (file)
@@ -919,7 +919,7 @@ bool RenderBox::canAutoscroll() const
 IntSize RenderBox::calculateAutoscrollDirection(const IntPoint& windowPoint) const
 {
     IntRect box(absoluteBoundingBoxRect());
-    box.move(view().frameView().scrollOffset());
+    box.moveBy(view().frameView().scrollPosition());
     IntRect windowBox = view().frameView().contentsToWindow(box);
 
     IntPoint windowAutoscrollPoint = windowPoint;
index e8dea63..0fa34f0 100644 (file)
@@ -2740,17 +2740,17 @@ int RenderLayer::scrollPosition(Scrollbar* scrollbar) const
     return 0;
 }
 
-IntPoint RenderLayer::scrollPosition() const
+ScrollPosition RenderLayer::scrollPosition() const
 {
-    return IntPoint(m_scrollOffset);
+    return ScrollPosition(m_scrollOffset);
 }
 
-IntPoint RenderLayer::minimumScrollPosition() const
+ScrollPosition RenderLayer::minimumScrollPosition() const
 {
     return -scrollOrigin();
 }
 
-IntPoint RenderLayer::maximumScrollPosition() const
+ScrollPosition RenderLayer::maximumScrollPosition() const
 {
     // FIXME: m_scrollSize may not be up-to-date if m_scrollDimensionsDirty is true.
     return -scrollOrigin() + roundedIntSize(m_scrollSize) - visibleContentRectIncludingScrollbars(ContentsVisibleRect).size();
index a773ecb..664492d 100644 (file)
@@ -864,9 +864,9 @@ private:
     virtual IntPoint convertFromContainingViewToScrollbar(const Scrollbar*, const IntPoint&) const override;
     virtual int scrollSize(ScrollbarOrientation) const override;
     virtual void setScrollOffset(const IntPoint&) override;
-    virtual IntPoint scrollPosition() const override;
-    virtual IntPoint minimumScrollPosition() const override;
-    virtual IntPoint maximumScrollPosition() const override;
+    virtual ScrollPosition scrollPosition() const override;
+    virtual ScrollPosition minimumScrollPosition() const override;
+    virtual ScrollPosition maximumScrollPosition() const override;
     virtual IntRect visibleContentRectInternal(VisibleContentRectIncludesScrollbars, VisibleContentRectBehavior) const override;
     virtual IntSize visibleSize() const override;
     virtual IntSize contentsSize() const override;
index cc9b8ce..f99ed4e 100644 (file)
@@ -350,7 +350,7 @@ bool RenderWidget::nodeAtPoint(const HitTestRequest& request, HitTestResult& res
         RenderView& childRoot = *childFrameView.renderView();
 
         LayoutPoint adjustedLocation = accumulatedOffset + location();
-        LayoutPoint contentOffset = LayoutPoint(borderLeft() + paddingLeft(), borderTop() + paddingTop()) - childFrameView.scrollOffset();
+        LayoutPoint contentOffset = LayoutPoint(borderLeft() + paddingLeft(), borderTop() + paddingTop()) - toIntSize(childFrameView.scrollPosition());
         HitTestLocation newHitTestLocation(locationInContainer, -adjustedLocation - contentOffset);
         HitTestRequest newHitTestRequest(request.type() | HitTestRequest::ChildFrameHitTest);
         HitTestResult childFrameResult(newHitTestLocation);
index 62e9dd1..0c54034 100644 (file)
@@ -416,9 +416,9 @@ AffineTransform SVGSVGElement::localCoordinateSpaceTransform(SVGLocatable::CTMSc
 
             // Respect scroll offset.
             if (FrameView* view = document().view()) {
-                LayoutSize scrollOffset = view->scrollOffset();
-                scrollOffset.scale(zoomFactor);
-                transform.translate(-scrollOffset.width(), -scrollOffset.height());
+                LayoutPoint scrollPosition = view->scrollPosition();
+                scrollPosition.scale(zoomFactor, zoomFactor);
+                transform.translate(-scrollPosition.x(), -scrollPosition.y());
             }
         }
     }
index ceb18dc..ca9b0e0 100644 (file)
@@ -901,7 +901,7 @@ HRESULT WebFrame::scrollOffset(_Out_ SIZE* offset)
     if (!view)
         return E_FAIL;
 
-    *offset = view->scrollOffset();
+    *offset = toIntSize(view->scrollPosition());
     return S_OK;
 }
 
index 8a0ab23..158fc56 100644 (file)
@@ -3789,7 +3789,7 @@ HRESULT WebView::rectsForTextMatches(_COM_Outptr_opt_ IEnumTextMatches** pmatche
         if (Document* document = frame->document()) {
             IntRect visibleRect = frame->view()->visibleContentRect();
             Vector<FloatRect> frameRects = document->markers().renderedRectsForMarkers(DocumentMarker::TextMatch);
-            IntPoint frameOffset(-frame->view()->scrollOffset().width(), -frame->view()->scrollOffset().height());
+            IntPoint frameOffset = -frame->view()->scrollPosition();
             frameOffset = frame->view()->convertToContainingWindow(frameOffset);
 
             Vector<FloatRect>::iterator end = frameRects.end();
@@ -3835,7 +3835,7 @@ HRESULT WebView::selectionRect(_Inout_ RECT* rc)
 
     IntRect ir = enclosingIntRect(frame.selection().selectionBounds());
     ir = frame.view()->convertToContainingWindow(ir);
-    ir.move(-frame.view()->scrollOffset().width(), -frame.view()->scrollOffset().height());
+    ir.moveBy(-frame.view()->scrollPosition());
 
     float scaleFactor = deviceScaleFactor();
     rc->left = ir.x() * scaleFactor;
index f36cc19..c5c8d00 100644 (file)
@@ -1,3 +1,34 @@
+2015-12-29  Simon Fraser  <simon.fraser@apple.com>
+
+        Remove ScrollView::scrollOffset() in preparation for scrollOffset vs. scrollPosition clarification
+        https://bugs.webkit.org/show_bug.cgi?id=152589
+
+        Reviewed by Sam Weinig.
+
+        Current code uses scrollOffset vs. scrollPosition interchangeably, and confusingly.
+        Longer term, I plan to make "scrollPosition" be the value that is relative to the 
+        contents, i.e. affected by scrollOrigin, and "scrollOffset" be the zero-based value
+        that's used to set scrollbar values.
+        
+        To prepare for this, remove ScrollView::scrollOffset(), which is just the
+        scrollPosition as an IntSize.
+        
+        Add some typedefs in ScrollableArea, which will slowly propagate through the
+        code as position vs. offset is clarified.
+
+        * WebProcess/InjectedBundle/DOM/InjectedBundleRangeHandle.cpp:
+        (WebKit::InjectedBundleRangeHandle::renderedImage):
+        * WebProcess/Plugins/PDF/DeprecatedPDFPlugin.h:
+        * WebProcess/Plugins/PDF/DeprecatedPDFPlugin.mm:
+        (WebKit::PDFPlugin::scrollPosition):
+        (WebKit::PDFPlugin::minimumScrollPosition):
+        (WebKit::PDFPlugin::maximumScrollPosition):
+        * WebProcess/WebPage/WebFrame.cpp:
+        (WebKit::WebFrame::scrollOffset):
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::scrollMainFrameIfNotAtMaxScrollPosition):
+        (WebKit::WebPage::updateMainFrameScrollOffsetPinning):
+
 2015-12-25  Andy Estes  <aestes@apple.com>
 
         Stop moving local objects in return statements
index 65fbe73..8a94514 100644 (file)
@@ -129,7 +129,7 @@ PassRefPtr<WebImage> InjectedBundleRangeHandle::renderedImage(SnapshotOptions op
     graphicsContext->scale(FloatSize(scaleFactor, scaleFactor));
 
     paintRect.move(frameView->frameRect().x(), frameView->frameRect().y());
-    paintRect.move(-frameView->scrollOffset());
+    paintRect.moveBy(-frameView->scrollPosition());
 
     graphicsContext->translate(-paintRect.x(), -paintRect.y());
 
index d255a2f..d29ac94 100644 (file)
@@ -197,9 +197,9 @@ private:
     virtual bool isActive() const override;
     virtual bool isScrollCornerVisible() const override { return false; }
     virtual int scrollPosition(WebCore::Scrollbar*) const override;
-    virtual WebCore::IntPoint scrollPosition() const override;
-    virtual WebCore::IntPoint minimumScrollPosition() const override;
-    virtual WebCore::IntPoint maximumScrollPosition() const override;
+    virtual WebCore::ScrollPosition scrollPosition() const override;
+    virtual WebCore::ScrollPosition minimumScrollPosition() const override;
+    virtual WebCore::ScrollPosition maximumScrollPosition() const override;
     virtual WebCore::IntSize visibleSize() const override { return m_size; }
     virtual WebCore::IntSize contentsSize() const override { return m_pdfDocumentSize; }
     virtual WebCore::Scrollbar* horizontalScrollbar() const override { return m_horizontalScrollbar.get(); }
index 104b7d7..9467459 100644 (file)
@@ -783,17 +783,17 @@ int PDFPlugin::scrollPosition(Scrollbar* scrollbar) const
     return 0;
 }
 
-IntPoint PDFPlugin::scrollPosition() const
+ScrollPosition PDFPlugin::scrollPosition() const
 {
     return IntPoint(m_scrollOffset.width(), m_scrollOffset.height());
 }
 
-IntPoint PDFPlugin::minimumScrollPosition() const
+ScrollPosition PDFPlugin::minimumScrollPosition() const
 {
     return IntPoint();
 }
 
-IntPoint PDFPlugin::maximumScrollPosition() const
+ScrollPosition PDFPlugin::maximumScrollPosition() const
 {
     IntSize scrollbarSpace = scrollbarIntrusion();
 
index ec05baa..91530a7 100644 (file)
@@ -568,7 +568,7 @@ IntSize WebFrame::scrollOffset() const
     if (!view)
         return IntSize();
 
-    return view->scrollOffset();
+    return toIntSize(view->scrollPosition());
 }
 
 bool WebFrame::hasHorizontalScrollbar() const
index cee722d..8d9af8a 100644 (file)
@@ -1316,8 +1316,8 @@ void WebPage::scrollMainFrameIfNotAtMaxScrollPosition(const IntSize& scrollOffse
 {
     FrameView* frameView = m_page->mainFrame().view();
 
-    IntPoint scrollPosition = frameView->scrollPosition();
-    IntPoint maximumScrollPosition = frameView->maximumScrollPosition();
+    ScrollPosition scrollPosition = frameView->scrollPosition();
+    ScrollPosition maximumScrollPosition = frameView->maximumScrollPosition();
 
     // If the current scroll position in a direction is the max scroll position 
     // we don't want to scroll at all.
@@ -3504,9 +3504,9 @@ void WebPage::addMIMETypeWithCustomContentProvider(const String& mimeType)
 void WebPage::updateMainFrameScrollOffsetPinning()
 {
     Frame& frame = m_page->mainFrame();
-    IntPoint scrollPosition = frame.view()->scrollPosition();
-    IntPoint maximumScrollPosition = frame.view()->maximumScrollPosition();
-    IntPoint minimumScrollPosition = frame.view()->minimumScrollPosition();
+    ScrollPosition scrollPosition = frame.view()->scrollPosition();
+    ScrollPosition maximumScrollPosition = frame.view()->maximumScrollPosition();
+    ScrollPosition minimumScrollPosition = frame.view()->minimumScrollPosition();
 
     bool isPinnedToLeftSide = (scrollPosition.x() <= minimumScrollPosition.x());
     bool isPinnedToRightSide = (scrollPosition.x() >= maximumScrollPosition.x());
index 3872d09..fae27ad 100644 (file)
@@ -2585,7 +2585,7 @@ void WebPage::dynamicViewportSizeUpdate(const FloatSize& minimumLayoutSize, cons
     IntSize oldContentSize = frameView.contentsSize();
     float oldPageScaleFactor = m_page->pageScaleFactor();
 
-    m_dynamicSizeUpdateHistory.add(std::make_pair(oldContentSize, oldPageScaleFactor), IntPoint(frameView.scrollOffset()));
+    m_dynamicSizeUpdateHistory.add(std::make_pair(oldContentSize, oldPageScaleFactor), frameView.scrollPosition());
 
     RefPtr<Node> oldNodeAtCenter;
     double visibleHorizontalFraction = 1;