Clarify that scrollPositionChangedViaPlatformWidget takes offsets
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 31 Dec 2015 19:35:18 +0000 (19:35 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 31 Dec 2015 19:35:18 +0000 (19:35 +0000)
https://bugs.webkit.org/show_bug.cgi?id=152606

Reviewed by Zalan Bujtas.

scrollPositionChangedViaPlatformWidget actually gets scroll offsets, since the
values we get from AppKit are zero-based, so rename to scrollOffsetChangedViaPlatformWidget().

Change ScrollableArea's setScrollPosition() and requestScrollPositionUpdate() to take
ScrollPositions.

Add a FIXME noting that willRevealEdge events are probably broken in RTL documents.

Source/WebCore:

* dom/Document.cpp:
(WebCore::Document::sendWillRevealEdgeEventsIfNeeded):
* page/FrameView.cpp:
(WebCore::FrameView::setScrollPosition):
(WebCore::FrameView::scrollOffsetChangedViaPlatformWidgetImpl):
(WebCore::FrameView::scrollPositionChanged):
(WebCore::FrameView::requestScrollPositionUpdate):
(WebCore::FrameView::scrollPositionChangedViaPlatformWidgetImpl): Deleted.
* page/FrameView.h:
* platform/ScrollView.cpp:
(WebCore::ScrollView::scrollOffsetChangedViaPlatformWidget):
(WebCore::ScrollView::handleDeferredScrollUpdateAfterContentSizeChange):
(WebCore::ScrollView::scrollTo):
(WebCore::ScrollView::setScrollPosition):
(WebCore::ScrollView::scrollPositionChangedViaPlatformWidget): Deleted.
* platform/ScrollView.h:
(WebCore::ScrollView::scrollOffsetChangedViaPlatformWidgetImpl):
(WebCore::ScrollView::scrollPositionChangedViaPlatformWidgetImpl): Deleted.
* platform/ScrollableArea.cpp:
(WebCore::ScrollableArea::notifyScrollPositionChanged):
* platform/ScrollableArea.h:
(WebCore::ScrollableArea::requestScrollPositionUpdate):

Source/WebKit/mac:

* WebView/WebHTMLView.mm:
(-[WebHTMLView _frameOrBoundsChanged]):

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

Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/FrameView.h
Source/WebCore/platform/ScrollView.cpp
Source/WebCore/platform/ScrollView.h
Source/WebCore/platform/ScrollableArea.cpp
Source/WebCore/platform/ScrollableArea.h
Source/WebKit/mac/ChangeLog
Source/WebKit/mac/WebView/WebHTMLView.mm

index e31ef7b..e0e7863 100644 (file)
@@ -1,3 +1,41 @@
+2015-12-31  Simon Fraser  <simon.fraser@apple.com>
+
+        Clarify that scrollPositionChangedViaPlatformWidget takes offsets
+        https://bugs.webkit.org/show_bug.cgi?id=152606
+
+        Reviewed by Zalan Bujtas.
+
+        scrollPositionChangedViaPlatformWidget actually gets scroll offsets, since the
+        values we get from AppKit are zero-based, so rename to scrollOffsetChangedViaPlatformWidget().
+        
+        Change ScrollableArea's setScrollPosition() and requestScrollPositionUpdate() to take
+        ScrollPositions.
+        
+        Add a FIXME noting that willRevealEdge events are probably broken in RTL documents.
+
+        * dom/Document.cpp:
+        (WebCore::Document::sendWillRevealEdgeEventsIfNeeded):
+        * page/FrameView.cpp:
+        (WebCore::FrameView::setScrollPosition):
+        (WebCore::FrameView::scrollOffsetChangedViaPlatformWidgetImpl):
+        (WebCore::FrameView::scrollPositionChanged):
+        (WebCore::FrameView::requestScrollPositionUpdate):
+        (WebCore::FrameView::scrollPositionChangedViaPlatformWidgetImpl): Deleted.
+        * page/FrameView.h:
+        * platform/ScrollView.cpp:
+        (WebCore::ScrollView::scrollOffsetChangedViaPlatformWidget):
+        (WebCore::ScrollView::handleDeferredScrollUpdateAfterContentSizeChange):
+        (WebCore::ScrollView::scrollTo):
+        (WebCore::ScrollView::setScrollPosition):
+        (WebCore::ScrollView::scrollPositionChangedViaPlatformWidget): Deleted.
+        * platform/ScrollView.h:
+        (WebCore::ScrollView::scrollOffsetChangedViaPlatformWidgetImpl):
+        (WebCore::ScrollView::scrollPositionChangedViaPlatformWidgetImpl): Deleted.
+        * platform/ScrollableArea.cpp:
+        (WebCore::ScrollableArea::notifyScrollPositionChanged):
+        * platform/ScrollableArea.h:
+        (WebCore::ScrollableArea::requestScrollPositionUpdate):
+
 2015-12-31  Zalan Bujtas  <zalan@apple.com>
 
         Simple line layout: Text with stroke width is not positioned correctly.
index fbf4721..b31df4b 100644 (file)
@@ -6098,7 +6098,7 @@ void Document::sendWillRevealEdgeEventsIfNeeded(const IntPoint& oldPosition, con
     // that this is the first moment that we know we crossed the magic line).
     
 #if ENABLE(WILL_REVEAL_EDGE_EVENTS)
-    
+    // FIXME: broken in RTL documents.
     int willRevealBottomNotificationPoint = std::max(0, contentsSize.height() - 2 *  visibleRect.height());
     int willRevealTopNotificationPoint = visibleRect.height();
 
index bcef674..a5f4b82 100644 (file)
@@ -2130,14 +2130,14 @@ void FrameView::scrollElementToRect(const Element& element, const IntRect& rect)
     setScrollPosition(IntPoint(bounds.x() - centeringOffsetX - rect.x(), bounds.y() - centeringOffsetY - rect.y()));
 }
 
-void FrameView::setScrollPosition(const IntPoint& scrollPoint)
+void FrameView::setScrollPosition(const ScrollPosition& scrollPosition)
 {
     TemporaryChange<bool> changeInProgrammaticScroll(m_inProgrammaticScroll, true);
     m_maintainScrollPositionAnchor = nullptr;
     Page* page = frame().page();
     if (page && page->expectsWheelEventTriggers())
         scrollAnimator().setWheelEventTestTrigger(page->testTrigger());
-    ScrollView::setScrollPosition(scrollPoint);
+    ScrollView::setScrollPosition(scrollPosition);
 }
 
 void FrameView::delegatesScrollingDidChange()
@@ -2191,15 +2191,15 @@ void FrameView::didChangeScrollOffset()
     frame().loader().client().didChangeScrollOffset();
 }
 
-void FrameView::scrollPositionChangedViaPlatformWidgetImpl(const IntPoint& oldPosition, const IntPoint& newPosition)
+void FrameView::scrollOffsetChangedViaPlatformWidgetImpl(const ScrollOffset& oldOffset, const ScrollOffset& newOffset)
 {
     updateLayerPositionsAfterScrolling();
     updateCompositingLayersAfterScrolling();
     repaintSlowRepaintObjects();
-    scrollPositionChanged(oldPosition, newPosition);
+    scrollPositionChanged(scrollPositionFromOffset(oldOffset), scrollPositionFromOffset(newOffset));
 }
 
-void FrameView::scrollPositionChanged(const IntPoint& oldPosition, const IntPoint& newPosition)
+void FrameView::scrollPositionChanged(const ScrollPosition& oldPosition, const ScrollPosition& newPosition)
 {
     Page* page = frame().page();
     auto throttlingDelay = page ? page->chrome().client().eventThrottlingDelay() : std::chrono::milliseconds::zero();
@@ -2351,7 +2351,7 @@ bool FrameView::isRubberBandInProgress() const
     return false;
 }
 
-bool FrameView::requestScrollPositionUpdate(const IntPoint& position)
+bool FrameView::requestScrollPositionUpdate(const ScrollPosition& position)
 {
 #if ENABLE(ASYNC_SCROLLING)
     if (TiledBacking* tiledBacking = this->tiledBacking())
index b41a5bd..a439a07 100644 (file)
@@ -237,10 +237,10 @@ public:
 #if USE(COORDINATED_GRAPHICS)
     virtual void setFixedVisibleContentRect(const IntRect&) override;
 #endif
-    WEBCORE_EXPORT virtual void setScrollPosition(const IntPoint&) override;
+    WEBCORE_EXPORT virtual void setScrollPosition(const ScrollPosition&) override;
     virtual void updateLayerPositionsAfterScrolling() override;
     virtual void updateCompositingLayersAfterScrolling() override;
-    virtual bool requestScrollPositionUpdate(const IntPoint&) override;
+    virtual bool requestScrollPositionUpdate(const ScrollPosition&) override;
     virtual bool isRubberBandInProgress() const override;
     WEBCORE_EXPORT virtual ScrollPosition minimumScrollPosition() const override;
     WEBCORE_EXPORT virtual ScrollPosition maximumScrollPosition() const override;
@@ -597,7 +597,7 @@ private:
 
     virtual bool shouldDeferScrollUpdateAfterContentSizeChange() override;
 
-    virtual void scrollPositionChangedViaPlatformWidgetImpl(const IntPoint& oldPosition, const IntPoint& newPosition) override;
+    virtual void scrollOffsetChangedViaPlatformWidgetImpl(const ScrollOffset& oldOffset, const ScrollOffset& newOffset) override;
 
     void applyOverflowToViewport(const RenderElement&, ScrollbarMode& hMode, ScrollbarMode& vMode);
     void applyPaginationToViewport();
@@ -658,7 +658,7 @@ private:
     bool updateEmbeddedObjects();
     void updateEmbeddedObject(RenderEmbeddedObject&);
     void scrollToAnchor();
-    void scrollPositionChanged(const IntPoint& oldPosition, const IntPoint& newPosition);
+    void scrollPositionChanged(const ScrollPosition& oldPosition, const ScrollPosition& newPosition);
     void scrollableAreaSetChanged();
     void sendScrollEvent();
 
index 264a5ca..7597bf0 100644 (file)
@@ -444,35 +444,35 @@ void ScrollView::setScrollOffset(const IntPoint& offset)
     scrollTo(scrollPositionFromOffset(constrainedOffset));
 }
 
-void ScrollView::scrollPositionChangedViaPlatformWidget(const IntPoint& oldPosition, const IntPoint& newPosition)
+void ScrollView::scrollOffsetChangedViaPlatformWidget(const ScrollOffset& oldOffset, const ScrollOffset& newOffset)
 {
     // We should not attempt to actually modify (paint) platform widgets if the layout phase
     // is not complete. Instead, defer the scroll event until the layout finishes.
     if (shouldDeferScrollUpdateAfterContentSizeChange()) {
         // We only care about the most recent scroll position change request
-        m_deferredScrollPositions = std::make_unique<std::pair<IntPoint, IntPoint>>(std::make_pair(oldPosition, newPosition));
+        m_deferredScrollOffsets = std::make_unique<std::pair<ScrollOffset, ScrollOffset>>(std::make_pair(oldOffset, newOffset));
         return;
     }
 
-    scrollPositionChangedViaPlatformWidgetImpl(oldPosition, newPosition);
+    scrollOffsetChangedViaPlatformWidgetImpl(oldOffset, newOffset);
 }
 
 void ScrollView::handleDeferredScrollUpdateAfterContentSizeChange()
 {
     ASSERT(!shouldDeferScrollUpdateAfterContentSizeChange());
 
-    if (!m_deferredScrollDelta && !m_deferredScrollPositions)
+    if (!m_deferredScrollDelta && !m_deferredScrollOffsets)
         return;
 
-    ASSERT(static_cast<bool>(m_deferredScrollDelta) != static_cast<bool>(m_deferredScrollPositions));
+    ASSERT(static_cast<bool>(m_deferredScrollDelta) != static_cast<bool>(m_deferredScrollOffsets));
 
     if (m_deferredScrollDelta)
         completeUpdatesAfterScrollTo(*m_deferredScrollDelta);
-    else if (m_deferredScrollPositions)
-        scrollPositionChangedViaPlatformWidgetImpl(m_deferredScrollPositions->first, m_deferredScrollPositions->second);
+    else if (m_deferredScrollOffsets)
+        scrollOffsetChangedViaPlatformWidgetImpl(m_deferredScrollOffsets->first, m_deferredScrollOffsets->second);
     
     m_deferredScrollDelta = nullptr;
-    m_deferredScrollPositions = nullptr;
+    m_deferredScrollOffsets = nullptr;
 }
 
 void ScrollView::scrollTo(const ScrollPosition& newPosition)
@@ -529,7 +529,7 @@ void ScrollView::setScrollPosition(const ScrollPosition& scrollPosition)
         return;
     }
 
-    IntPoint newScrollPosition = !delegatesScrolling() ? adjustScrollPositionWithinRange(scrollPosition) : scrollPosition;
+    ScrollPosition newScrollPosition = !delegatesScrolling() ? adjustScrollPositionWithinRange(scrollPosition) : scrollPosition;
 
     if ((!delegatesScrolling() || !inProgrammaticScroll()) && newScrollPosition == this->scrollPosition())
         return;
index c0e38eb..7ea30a2 100644 (file)
@@ -375,7 +375,7 @@ public:
 
     virtual bool isScrollView() const override { return true; }
 
-    WEBCORE_EXPORT void scrollPositionChangedViaPlatformWidget(const IntPoint& oldPosition, const IntPoint& newPosition);
+    WEBCORE_EXPORT void scrollOffsetChangedViaPlatformWidget(const ScrollOffset& oldOffset, const ScrollOffset& newOffset);
 
 protected:
     ScrollView();
@@ -418,7 +418,7 @@ protected:
 
     virtual bool shouldDeferScrollUpdateAfterContentSizeChange() { return false; }
 
-    virtual void scrollPositionChangedViaPlatformWidgetImpl(const IntPoint&, const IntPoint&) { }
+    virtual void scrollOffsetChangedViaPlatformWidgetImpl(const ScrollOffset&, const ScrollOffset&) { }
 
 private:
     virtual IntRect visibleContentRectInternal(VisibleContentRectIncludesScrollbars, VisibleContentRectBehavior) const override;
@@ -456,7 +456,7 @@ private:
     IntSize m_contentsSize;
 
     std::unique_ptr<IntSize> m_deferredScrollDelta; // Needed for WebKit scrolling
-    std::unique_ptr<std::pair<IntPoint, IntPoint>> m_deferredScrollPositions; // Needed for platform widget scrolling
+    std::unique_ptr<std::pair<ScrollOffset, ScrollOffset>> m_deferredScrollOffsets; // Needed for platform widget scrolling
 
     bool m_scrollbarsSuppressed;
 
index fc90bd5..d433422 100644 (file)
@@ -145,7 +145,7 @@ void ScrollableArea::scrollToOffsetWithoutAnimation(ScrollbarOrientation orienta
         scrollToOffsetWithoutAnimation(FloatPoint(scrollAnimator().currentPosition().x(), offset));
 }
 
-void ScrollableArea::notifyScrollPositionChanged(const IntPoint& position)
+void ScrollableArea::notifyScrollPositionChanged(const ScrollPosition& position)
 {
     scrollPositionChanged(position);
     scrollAnimator().setCurrentPosition(position);
index 715b90f..3c71cc8 100644 (file)
@@ -54,12 +54,12 @@ public:
 
     // Should be called when the scroll position changes externally, for example if the scroll layer position
     // is updated on the scrolling thread and we need to notify the main thread.
-    WEBCORE_EXPORT void notifyScrollPositionChanged(const IntPoint&);
+    WEBCORE_EXPORT void notifyScrollPositionChanged(const ScrollPosition&);
 
     // Allows subclasses to handle scroll position updates themselves. If this member function
     // returns true, the scrollable area won't actually update the scroll position and instead
     // expect it to happen sometime in the future.
-    virtual bool requestScrollPositionUpdate(const IntPoint&) { return false; }
+    virtual bool requestScrollPositionUpdate(const ScrollPosition&) { return false; }
 
     WEBCORE_EXPORT bool handleWheelEvent(const PlatformWheelEvent&);
 
index 997145b..5fcf3a3 100644 (file)
@@ -1,3 +1,21 @@
+2015-12-31  Simon Fraser  <simon.fraser@apple.com>
+
+        Clarify that scrollPositionChangedViaPlatformWidget takes offsets
+        https://bugs.webkit.org/show_bug.cgi?id=152606
+
+        Reviewed by Zalan Bujtas.
+
+        scrollPositionChangedViaPlatformWidget actually gets scroll offsets, since the
+        values we get from AppKit are zero-based, so rename to scrollOffsetChangedViaPlatformWidget().
+        
+        Change ScrollableArea's setScrollPosition() and requestScrollPositionUpdate() to take
+        ScrollPositions.
+        
+        Add a FIXME noting that willRevealEdge events are probably broken in RTL documents.
+
+        * WebView/WebHTMLView.mm:
+        (-[WebHTMLView _frameOrBoundsChanged]):
+
 2015-12-22  Andy Estes  <aestes@apple.com>
 
         [CF] Replace CFNetwork-related WebKitSystemInterface calls with SPI
index 2e47f6d..6c7ed41 100644 (file)
@@ -1675,7 +1675,7 @@ static NSURL* uniqueURLWithRelativePart(NSString *relativePart)
         if (Frame* coreFrame = core([self _frame])) {
             if (FrameView* coreView = coreFrame->view()) {
                 _private->inScrollPositionChanged = YES;
-                coreView->scrollPositionChangedViaPlatformWidget(IntPoint(_private->lastScrollPosition), IntPoint(origin));
+                coreView->scrollOffsetChangedViaPlatformWidget(IntPoint(_private->lastScrollPosition), IntPoint(origin));
                 _private->inScrollPositionChanged = NO;
             }
         }