Change ScrollView::scrollTo() to take a ScrollPosition
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 31 Dec 2015 18:50:10 +0000 (18:50 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 31 Dec 2015 18:50:10 +0000 (18:50 +0000)
https://bugs.webkit.org/show_bug.cgi?id=152607

Reviewed by Zalan Bujtas.

Make it more explicit that ScrollView::scrollTo() takes a ScrollPosition, and
change the name and type of the m_scrollOffset member variable.

* page/FrameView.cpp:
(WebCore::FrameView::scrollTo):
(WebCore::FrameView::wheelEvent):
* page/FrameView.h:
* platform/ScrollView.cpp:
(WebCore::ScrollView::unobscuredContentRectInternal):
(WebCore::ScrollView::setScrollOffset):
(WebCore::ScrollView::scrollTo):
* platform/ScrollView.h:

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

Source/WebCore/ChangeLog
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/FrameView.h
Source/WebCore/platform/ScrollView.cpp
Source/WebCore/platform/ScrollView.h
Source/WebCore/platform/ios/ScrollViewIOS.mm

index 0b56076..d818821 100644 (file)
@@ -1,3 +1,23 @@
+2015-12-30  Simon Fraser  <simon.fraser@apple.com>
+
+        Change ScrollView::scrollTo() to take a ScrollPosition
+        https://bugs.webkit.org/show_bug.cgi?id=152607
+
+        Reviewed by Zalan Bujtas.
+
+        Make it more explicit that ScrollView::scrollTo() takes a ScrollPosition, and
+        change the name and type of the m_scrollOffset member variable.
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::scrollTo):
+        (WebCore::FrameView::wheelEvent):
+        * page/FrameView.h:
+        * platform/ScrollView.cpp:
+        (WebCore::ScrollView::unobscuredContentRectInternal):
+        (WebCore::ScrollView::setScrollOffset):
+        (WebCore::ScrollView::scrollTo):
+        * platform/ScrollView.h:
+
 2015-12-30  Brady Eidson  <beidson@apple.com>
 
         Modern IDB: Only fire blocked events after all open connections have handled their versionchange events.
index 89e130c..bcef674 100644 (file)
@@ -3474,10 +3474,10 @@ bool FrameView::forceUpdateScrollbarsOnMainThreadForPerformanceTesting() const
     return page && page->settings().forceUpdateScrollbarsOnMainThreadForPerformanceTesting();
 }
 
-void FrameView::scrollTo(const IntSize& newOffset)
+void FrameView::scrollTo(const ScrollPosition& newPosition)
 {
     IntPoint oldPosition = scrollPosition();
-    ScrollView::scrollTo(newOffset);
+    ScrollView::scrollTo(newPosition);
     if (oldPosition != scrollPosition())
         scrollPositionChanged(oldPosition, scrollPosition());
     didChangeScrollOffset();
@@ -4586,7 +4586,7 @@ bool FrameView::wheelEvent(const PlatformWheelEvent& wheelEvent)
         ScrollPosition oldPosition = scrollPosition();
         ScrollPosition newPosition = oldPosition - IntSize(wheelEvent.deltaX(), wheelEvent.deltaY());
         if (oldPosition != newPosition) {
-            ScrollView::scrollTo(toIntSize(newPosition));
+            ScrollView::scrollTo(newPosition);
             scrollPositionChanged(oldPosition, scrollPosition());
             didChangeScrollOffset();
         }
index a056f2c..b41a5bd 100644 (file)
@@ -624,7 +624,7 @@ private:
 
     // ScrollableArea interface
     virtual void invalidateScrollbarRect(Scrollbar*, const IntRect&) override;
-    virtual void scrollTo(const IntSize&) override;
+    virtual void scrollTo(const ScrollPosition&) override;
     virtual void setVisibleScrollerThumbRect(const IntRect&) override;
     virtual ScrollableArea* enclosingScrollableArea() const override;
     virtual IntRect scrollableAreaBoundingBox(bool* = nullptr) const override;
index 9762b00..264a5ca 100644 (file)
@@ -264,7 +264,7 @@ IntRect ScrollView::unobscuredContentRectInternal(VisibleContentRectIncludesScro
 {
     FloatSize visibleContentSize = unscaledUnobscuredVisibleContentSize(scrollbarInclusion);
     visibleContentSize.scale(1 / visibleContentScaleFactor());
-    return IntRect(IntPoint(m_scrollOffset), expandedIntSize(visibleContentSize));
+    return IntRect(m_scrollPosition, expandedIntSize(visibleContentSize));
 }
 
 IntSize ScrollView::unscaledVisibleContentSizeIncludingObscuredArea(VisibleContentRectIncludesScrollbars scrollbarInclusion) const
@@ -441,8 +441,7 @@ void ScrollView::setScrollOffset(const IntPoint& offset)
     if (constrainsScrollingToContentEdge())
         constrainedOffset = constrainedOffset.constrainedBetween(IntPoint(), maximumScrollOffset());
 
-    ScrollPosition newPosition = scrollPositionFromOffset(constrainedOffset);
-    scrollTo(toIntSize(newPosition));
+    scrollTo(scrollPositionFromOffset(constrainedOffset));
 }
 
 void ScrollView::scrollPositionChangedViaPlatformWidget(const IntPoint& oldPosition, const IntPoint& newPosition)
@@ -476,19 +475,20 @@ void ScrollView::handleDeferredScrollUpdateAfterContentSizeChange()
     m_deferredScrollPositions = nullptr;
 }
 
-void ScrollView::scrollTo(const IntSize& newOffset)
+void ScrollView::scrollTo(const ScrollPosition& newPosition)
 {
-    IntSize scrollDelta = newOffset - m_scrollOffset;
+    IntSize scrollDelta = newPosition - m_scrollPosition;
     if (scrollDelta.isZero())
         return;
-    m_scrollOffset = newOffset;
+
+    m_scrollPosition = newPosition;
 
     if (scrollbarsSuppressed())
         return;
 
 #if USE(COORDINATED_GRAPHICS)
     if (delegatesScrolling()) {
-        requestScrollPositionUpdate(IntPoint(newOffset));
+        requestScrollPositionUpdate(newPosition);
         return;
     }
 #endif
index 8c70f5f..c0e38eb 100644 (file)
@@ -74,7 +74,7 @@ public:
     virtual void notifyPageThatContentAreaWillPaint() const;
 
     // NOTE: This should only be called by the overriden setScrollOffset from ScrollableArea.
-    virtual void scrollTo(const IntSize& newOffset);
+    virtual void scrollTo(const ScrollPosition&);
 
     // The window thats hosts the ScrollView. The ScrollView will communicate scrolls and repaints to the
     // host window in the window's coordinate space.
@@ -450,7 +450,7 @@ private:
 #else
     IntRect m_fixedVisibleContentRect;
 #endif
-    IntSize m_scrollOffset; // FIXME: Would rather store this as a position, but we will wait to make this change until more code is shared.
+    ScrollPosition m_scrollPosition;
     IntPoint m_cachedScrollPosition;
     IntSize m_fixedLayoutSize;
     IntSize m_contentsSize;
index 38f1398..2aba23e 100644 (file)
@@ -108,8 +108,8 @@ IntRect ScrollView::unobscuredContentRect(VisibleContentRectIncludesScrollbars)
     }
 
     if (!m_unobscuredContentSize.isEmpty()) {
-        // FIXME: is this correct in RTL documents?
-        return IntRect(scrollOrigin() + m_scrollOffset, roundedIntSize(m_unobscuredContentSize));
+        // FIXME: This seems wrong but gives the right answer, probably because m_scrollPosition is wrong.
+        return IntRect(m_scrollPosition + scrollOrigin(), roundedIntSize(m_unobscuredContentSize));
     }
 
     return unobscuredContentRectInternal();