Implement ScrollableArea::scrollOffset()
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 17 Jun 2019 00:33:32 +0000 (00:33 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 17 Jun 2019 00:33:32 +0000 (00:33 +0000)
https://bugs.webkit.org/show_bug.cgi?id=198895

Reviewed by Antti Koivisto.

Source/WebCore:

Remove from ScrollableArea the following:
    virtual int scrollSize(ScrollbarOrientation) const = 0;
    virtual int scrollOffset(ScrollbarOrientation) const = 0;
and instead implement ScrollOffset scrollOffset() const.

Also make scrollPosition() pure virtual, avoiding the reverse dependency where
this base class implementation got values from scrollbars.

scrollSize(ScrollbarOrientation) was only used by ScrollAnimatorIOS and we can
do the same computation via min/max scroll positions.

RenderListBox and PopupMenuWin need implementations of scrollPosition().

Remove some PLATFORM(IOS_FAMILY) #ifdefs from ScrollableArea for code that compiles
on all platforms.

* page/FrameView.h:
* platform/ScrollView.cpp:
(WebCore::ScrollView::overhangAmount const):
(WebCore::ScrollView::scrollSize const): Deleted.
(WebCore::ScrollView::scrollOffset const): Deleted.
* platform/ScrollView.h:
* platform/ScrollableArea.cpp:
(WebCore::ScrollableArea::isPinnedVerticallyInDirection const):
(WebCore::ScrollableArea::scrollOffset const):
(WebCore::ScrollableArea::scrollPosition const): Deleted.
* platform/ScrollableArea.h:
(WebCore::offsetForOrientation):
(WebCore::ScrollableArea::isHorizontalScrollerPinnedToMinimumPosition const):
(WebCore::ScrollableArea::isHorizontalScrollerPinnedToMaximumPosition const):
(WebCore::ScrollableArea::isVerticalScrollerPinnedToMinimumPosition const):
(WebCore::ScrollableArea::isVerticalScrollerPinnedToMaximumPosition const):
(WebCore::ScrollableArea::tiledBacking const): Deleted.
* platform/Scrollbar.cpp:
(WebCore::Scrollbar::Scrollbar):
(WebCore::Scrollbar::offsetDidChange):
* platform/ios/ScrollAnimatorIOS.mm:
(WebCore::ScrollAnimatorIOS::handleTouchEvent):
* platform/win/PopupMenuWin.cpp:
(WebCore::PopupMenuWin::scrollPosition const):
(WebCore::PopupMenuWin::wndProc):
(WebCore::PopupMenuWin::scrollSize const): Deleted.
(WebCore::PopupMenuWin::scrollOffset const): Deleted.
* platform/win/PopupMenuWin.h:
(WebCore::PopupMenuWin::scrollOffset const): Deleted.
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::scrollSize const): Deleted.
(WebCore::RenderLayer::scrollOffset const): Deleted.
* rendering/RenderLayer.h:
* rendering/RenderListBox.cpp:
(WebCore::RenderListBox::scrollPosition const):
(WebCore::RenderListBox::scrollSize const): Deleted.
(WebCore::RenderListBox::scrollOffset const): Deleted.
* rendering/RenderListBox.h:

Source/WebKit:

* UIProcess/win/WebPopupMenuProxyWin.cpp:
(WebKit::PopupMenuWin::scrollPosition const):
(WebKit::WebPopupMenuProxyWin::onKeyDown): Just use m_scrollOffset.
(WebKit::WebPopupMenuProxyWin::scrollSize const): Deleted.
* UIProcess/win/WebPopupMenuProxyWin.h: Remove the one-axis scrollOffset()
* WebProcess/Plugins/PDF/PDFPlugin.h:
* WebProcess/Plugins/PDF/PDFPlugin.mm:
(WebKit::PDFPlugin::scrollSize const): Deleted.
(WebKit::PDFPlugin::scrollOffset const): Deleted.
* WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm:
(WebKit::RemoteLayerTreeDrawingArea::updateScrolledExposedRect):

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

20 files changed:
Source/WebCore/ChangeLog
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/WebCore/platform/Scrollbar.cpp
Source/WebCore/platform/ios/ScrollAnimatorIOS.mm
Source/WebCore/platform/win/PopupMenuWin.cpp
Source/WebCore/platform/win/PopupMenuWin.h
Source/WebCore/rendering/RenderLayer.cpp
Source/WebCore/rendering/RenderLayer.h
Source/WebCore/rendering/RenderListBox.cpp
Source/WebCore/rendering/RenderListBox.h
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/win/WebPopupMenuProxyWin.cpp
Source/WebKit/UIProcess/win/WebPopupMenuProxyWin.h
Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.h
Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm
Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm

index beb707a..ed397f4 100644 (file)
@@ -1,3 +1,65 @@
+2019-06-16  Simon Fraser  <simon.fraser@apple.com>
+
+        Implement ScrollableArea::scrollOffset()
+        https://bugs.webkit.org/show_bug.cgi?id=198895
+
+        Reviewed by Antti Koivisto.
+
+        Remove from ScrollableArea the following:
+            virtual int scrollSize(ScrollbarOrientation) const = 0;
+            virtual int scrollOffset(ScrollbarOrientation) const = 0;
+        and instead implement ScrollOffset scrollOffset() const.
+
+        Also make scrollPosition() pure virtual, avoiding the reverse dependency where
+        this base class implementation got values from scrollbars.
+        
+        scrollSize(ScrollbarOrientation) was only used by ScrollAnimatorIOS and we can
+        do the same computation via min/max scroll positions.
+        
+        RenderListBox and PopupMenuWin need implementations of scrollPosition().
+        
+        Remove some PLATFORM(IOS_FAMILY) #ifdefs from ScrollableArea for code that compiles
+        on all platforms.
+
+        * page/FrameView.h:
+        * platform/ScrollView.cpp:
+        (WebCore::ScrollView::overhangAmount const):
+        (WebCore::ScrollView::scrollSize const): Deleted.
+        (WebCore::ScrollView::scrollOffset const): Deleted.
+        * platform/ScrollView.h:
+        * platform/ScrollableArea.cpp:
+        (WebCore::ScrollableArea::isPinnedVerticallyInDirection const):
+        (WebCore::ScrollableArea::scrollOffset const):
+        (WebCore::ScrollableArea::scrollPosition const): Deleted.
+        * platform/ScrollableArea.h:
+        (WebCore::offsetForOrientation):
+        (WebCore::ScrollableArea::isHorizontalScrollerPinnedToMinimumPosition const):
+        (WebCore::ScrollableArea::isHorizontalScrollerPinnedToMaximumPosition const):
+        (WebCore::ScrollableArea::isVerticalScrollerPinnedToMinimumPosition const):
+        (WebCore::ScrollableArea::isVerticalScrollerPinnedToMaximumPosition const):
+        (WebCore::ScrollableArea::tiledBacking const): Deleted.
+        * platform/Scrollbar.cpp:
+        (WebCore::Scrollbar::Scrollbar):
+        (WebCore::Scrollbar::offsetDidChange):
+        * platform/ios/ScrollAnimatorIOS.mm:
+        (WebCore::ScrollAnimatorIOS::handleTouchEvent):
+        * platform/win/PopupMenuWin.cpp:
+        (WebCore::PopupMenuWin::scrollPosition const):
+        (WebCore::PopupMenuWin::wndProc):
+        (WebCore::PopupMenuWin::scrollSize const): Deleted.
+        (WebCore::PopupMenuWin::scrollOffset const): Deleted.
+        * platform/win/PopupMenuWin.h:
+        (WebCore::PopupMenuWin::scrollOffset const): Deleted.
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::scrollSize const): Deleted.
+        (WebCore::RenderLayer::scrollOffset const): Deleted.
+        * rendering/RenderLayer.h:
+        * rendering/RenderListBox.cpp:
+        (WebCore::RenderListBox::scrollPosition const):
+        (WebCore::RenderListBox::scrollSize const): Deleted.
+        (WebCore::RenderListBox::scrollOffset const): Deleted.
+        * rendering/RenderListBox.h:
+
 2019-06-16  Zalan Bujtas  <zalan@apple.com>
 
         Address Sam's post-landing review of r246234.
index 390301f..bf94012 100644 (file)
@@ -153,7 +153,7 @@ public:
     WEBCORE_EXPORT GraphicsLayer* graphicsLayerForPlatformWidget(PlatformWidget);
     WEBCORE_EXPORT void scheduleLayerFlushAllowingThrottling();
 
-    WEBCORE_EXPORT TiledBacking* tiledBacking() const final;
+    WEBCORE_EXPORT TiledBacking* tiledBacking() const;
 
     ScrollingNodeID scrollingNodeID() const override;
     ScrollableArea* scrollableAreaForScrollLayerID(uint64_t) const;
index 61bb518..b126a9c 100644 (file)
@@ -378,19 +378,6 @@ ScrollPosition ScrollView::documentScrollPositionRelativeToScrollableAreaOrigin(
     return scrollPosition() - IntSize(0, headerHeight());
 }
 
-int ScrollView::scrollSize(ScrollbarOrientation orientation) const
-{
-    // If no scrollbars are present, it does not indicate content is not be scrollable.
-    if (!m_horizontalScrollbar && !m_verticalScrollbar && !prohibitsScrolling()) {
-        IntSize scrollSize = m_contentsSize - visibleContentRect(LegacyIOSDocumentVisibleRect).size();
-        scrollSize.clampNegativeToZero();
-        return orientation == HorizontalScrollbar ? scrollSize.width() : scrollSize.height();
-    }
-
-    Scrollbar* scrollbar = ((orientation == HorizontalScrollbar) ? m_horizontalScrollbar : m_verticalScrollbar).get();
-    return scrollbar ? (scrollbar->totalSize() - scrollbar->visibleSize()) : 0;
-}
-
 void ScrollView::notifyPageThatContentAreaWillPaint() const
 {
 }
@@ -474,19 +461,6 @@ void ScrollView::completeUpdatesAfterScrollTo(const IntSize& scrollDelta)
     updateCompositingLayersAfterScrolling();
 }
 
-int ScrollView::scrollOffset(ScrollbarOrientation orientation) const
-{
-    ScrollOffset offset = scrollOffsetFromPosition(scrollPosition());
-
-    if (orientation == HorizontalScrollbar)
-        return offset.x();
-
-    if (orientation == VerticalScrollbar)
-        return offset.y();
-
-    return 0;
-}
-
 void ScrollView::setScrollPosition(const ScrollPosition& scrollPosition)
 {
     LOG_WITH_STREAM(Scrolling, stream << "ScrollView::setScrollPosition " << scrollPosition);
@@ -528,7 +502,7 @@ IntSize ScrollView::overhangAmount() const
     IntSize stretch;
 
     // FIXME: use maximumScrollOffset()
-    ScrollOffset scrollOffset = scrollOffsetFromPosition(scrollPosition());
+    ScrollOffset scrollOffset = this->scrollOffset();
     if (scrollOffset.y() < 0)
         stretch.setHeight(scrollOffset.y());
     else if (totalContentsSize().height() && scrollOffset.y() > totalContentsSize().height() - visibleHeight())
index ff82c27..03eb5d1 100644 (file)
@@ -69,8 +69,6 @@ public:
     using Widget::weakPtrFactory;
 
     // ScrollableArea functions.
-    int scrollSize(ScrollbarOrientation) const final;
-    int scrollOffset(ScrollbarOrientation) const final;
     WEBCORE_EXPORT void setScrollOffset(const ScrollOffset&) final;
     bool isScrollCornerVisible() const final;
     void scrollbarStyleChanged(ScrollbarStyle, bool forceUpdate) override;
index a98e5da..c8882ee 100644 (file)
@@ -572,7 +572,6 @@ void ScrollableArea::serviceScrollAnimations()
         scrollAnimator->serviceScrollAnimations();
 }
 
-#if PLATFORM(IOS_FAMILY)
 bool ScrollableArea::isPinnedInBothDirections(const IntSize& scrollDelta) const
 {
     return isPinnedHorizontallyInDirection(scrollDelta.width()) && isPinnedVerticallyInDirection(scrollDelta.height());
@@ -595,7 +594,6 @@ bool ScrollableArea::isPinnedVerticallyInDirection(int verticalScrollDelta) cons
         return true;
     return false;
 }
-#endif // PLATFORM(IOS_FAMILY)
 
 int ScrollableArea::horizontalScrollbarIntrusion() const
 {
@@ -612,12 +610,9 @@ IntSize ScrollableArea::scrollbarIntrusion() const
     return { horizontalScrollbarIntrusion(), verticalScrollbarIntrusion() };
 }
 
-ScrollPosition ScrollableArea::scrollPosition() const
+ScrollOffset ScrollableArea::scrollOffset() 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);
+    return scrollOffsetFromPosition(scrollPosition());
 }
 
 ScrollPosition ScrollableArea::minimumScrollPosition() const
index fde607f..d7438c1 100644 (file)
@@ -48,6 +48,18 @@ typedef IntPoint ScrollPosition;
 // scrollOffset() is the value used by scrollbars (min is 0,0), and should never have negative components.
 typedef IntPoint ScrollOffset;
 
+
+inline int offsetForOrientation(ScrollOffset offset, ScrollbarOrientation orientation)
+{
+    switch (orientation) {
+    case HorizontalScrollbar: return offset.x();
+    case VerticalScrollbar: return offset.y();
+    }
+    ASSERT_NOT_REACHED();
+    return 0;
+}
+
+
 class ScrollableArea : public CanMakeWeakPtr<ScrollableArea> {
 public:
     WEBCORE_EXPORT bool scroll(ScrollDirection, ScrollGranularity, float multiplier = 1);
@@ -157,8 +169,6 @@ public:
     ScrollAnimator* existingScrollAnimator() const { return m_scrollAnimator.get(); }
 
     virtual bool isActive() const = 0;
-    virtual int scrollSize(ScrollbarOrientation) const = 0;
-    virtual int scrollOffset(ScrollbarOrientation) const = 0;
     WEBCORE_EXPORT virtual void invalidateScrollbar(Scrollbar&, const IntRect&);
     virtual bool isScrollCornerVisible() const = 0;
     virtual IntRect scrollCornerRect() const = 0;
@@ -196,7 +206,7 @@ public:
     const IntPoint& scrollOrigin() const { return m_scrollOrigin; }
     bool scrollOriginChanged() const { return m_scrollOriginChanged; }
 
-    virtual ScrollPosition scrollPosition() const;
+    virtual ScrollPosition scrollPosition() const = 0;
     virtual ScrollPosition minimumScrollPosition() const;
     virtual ScrollPosition maximumScrollPosition() const;
 
@@ -205,6 +215,8 @@ public:
         return position.constrainedBetween(minimumScrollPosition(), maximumScrollPosition());
     }
 
+    WEBCORE_EXPORT ScrollOffset scrollOffset() const;
+
     ScrollOffset maximumScrollOffset() const;
 
     WEBCORE_EXPORT ScrollPosition scrollPositionFromOffset(ScrollOffset) const;
@@ -298,18 +310,14 @@ public:
     virtual bool scheduleAnimation() { return false; }
     void serviceScrollAnimations();
 
-#if PLATFORM(IOS_FAMILY)
-    bool isHorizontalScrollerPinnedToMinimumPosition() const { return !horizontalScrollbar() || scrollOffset(HorizontalScrollbar) <= 0; }
-    bool isHorizontalScrollerPinnedToMaximumPosition() const { return !horizontalScrollbar() || scrollOffset(HorizontalScrollbar) >= maximumScrollOffset().x(); }
-    bool isVerticalScrollerPinnedToMinimumPosition() const { return !verticalScrollbar() || scrollOffset(VerticalScrollbar) <= 0; }
-    bool isVerticalScrollerPinnedToMaximumPosition() const { return !verticalScrollbar() || scrollOffset(VerticalScrollbar) >= maximumScrollOffset().y(); }
+    bool isHorizontalScrollerPinnedToMinimumPosition() const { return !horizontalScrollbar() || scrollOffset().x() <= 0; }
+    bool isHorizontalScrollerPinnedToMaximumPosition() const { return !horizontalScrollbar() || scrollOffset().x() >= maximumScrollOffset().x(); }
+    bool isVerticalScrollerPinnedToMinimumPosition() const { return !verticalScrollbar() || scrollOffset().y() <= 0; }
+    bool isVerticalScrollerPinnedToMaximumPosition() const { return !verticalScrollbar() || scrollOffset().y() >= maximumScrollOffset().y(); }
 
     bool isPinnedInBothDirections(const IntSize&) const; 
     bool isPinnedHorizontallyInDirection(int horizontalScrollDelta) const; 
     bool isPinnedVerticallyInDirection(int verticalScrollDelta) const;
-#endif
-
-    virtual TiledBacking* tiledBacking() const { return nullptr; }
 
     // True if scrolling happens by moving compositing layers.
     virtual bool usesCompositedScrolling() const { return false; }
index 25f8af1..3969155 100644 (file)
@@ -69,7 +69,7 @@ Scrollbar::Scrollbar(ScrollableArea& scrollableArea, ScrollbarOrientation orient
     int thickness = theme().scrollbarThickness(controlSize);
     Widget::setFrameRect(IntRect(0, 0, thickness, thickness));
 
-    m_currentPos = static_cast<float>(m_scrollableArea.scrollOffset(m_orientation));
+    m_currentPos = static_cast<float>(offsetForOrientation(m_scrollableArea.scrollOffset(), m_orientation));
 }
 
 Scrollbar::~Scrollbar()
@@ -91,7 +91,7 @@ int Scrollbar::occupiedHeight() const
 
 void Scrollbar::offsetDidChange()
 {
-    float position = static_cast<float>(m_scrollableArea.scrollOffset(m_orientation));
+    float position = static_cast<float>(offsetForOrientation(m_scrollableArea.scrollOffset(), m_orientation));
     if (position == m_currentPos)
         return;
 
index 4957468..01f0a9b 100644 (file)
@@ -103,8 +103,9 @@ bool ScrollAnimatorIOS::handleTouchEvent(const PlatformTouchEvent& touchEvent)
         determineScrollableAreaForTouchSequence(touchDelta);
 
     if (!m_committedToScrollAxis) {
-        bool horizontallyScrollable = m_scrollableArea.scrollSize(HorizontalScrollbar);
-        bool verticallyScrollable = m_scrollableArea.scrollSize(VerticalScrollbar);
+        auto scrollSize = m_scrollableArea.maximumScrollPosition() - m_scrollableArea.minimumScrollPosition();
+        bool horizontallyScrollable = scrollSize.width();
+        bool verticallyScrollable = scrollSize.height();
 
         if (!horizontallyScrollable && !verticallyScrollable)
             return false;
index 522a340..7c7bfdf 100644 (file)
@@ -679,14 +679,9 @@ void PopupMenuWin::paint(const IntRect& damageRect, HDC hdc)
     ::BitBlt(localDC, damageRect.x(), damageRect.y(), damageRect.width(), damageRect.height(), m_DC.get(), damageRect.x(), damageRect.y(), SRCCOPY);
 }
 
-int PopupMenuWin::scrollSize(ScrollbarOrientation orientation) const
+ScrollPosition PopupMenuWin::scrollPosition() const
 {
-    return ((orientation == VerticalScrollbar) && m_scrollbar) ? (m_scrollbar->totalSize() - m_scrollbar->visibleSize()) : 0;
-}
-
-int PopupMenuWin::scrollOffset(ScrollbarOrientation) const
-{
-    return m_scrollOffset;
+    return { 0, m_scrollOffset };
 }
 
 void PopupMenuWin::setScrollOffset(const IntPoint& offset)
@@ -892,9 +887,9 @@ LRESULT PopupMenuWin::wndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lPa
                     focusLast();
                     break;
                 case VK_PRIOR:
-                    if (focusedIndex() != scrollOffset()) {
+                    if (focusedIndex() != m_scrollOffset) {
                         // Set the selection to the first visible item
-                        int firstVisibleItem = scrollOffset();
+                        int firstVisibleItem = m_scrollOffset;
                         up(focusedIndex() - firstVisibleItem);
                     } else {
                         // The first visible item is selected, so move the selection back one page
@@ -902,7 +897,7 @@ LRESULT PopupMenuWin::wndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lPa
                     }
                     break;
                 case VK_NEXT: {
-                    int lastVisibleItem = scrollOffset() + visibleItems() - 1;
+                    int lastVisibleItem = m_scrollOffset + visibleItems() - 1;
                     if (focusedIndex() != lastVisibleItem) {
                         // Set the selection to the last visible item
                         down(lastVisibleItem - focusedIndex());
index 363a0a3..3529b8f 100644 (file)
@@ -77,8 +77,6 @@ private:
     void setWasClicked(bool b = true) { m_wasClicked = b; }
     bool wasClicked() const { return m_wasClicked; }
 
-    int scrollOffset() const { return m_scrollOffset; }
-
     bool scrollToRevealSelection();
 
     void incrementWheelDelta(int delta);
@@ -89,8 +87,7 @@ private:
     void setScrollbarCapturingMouse(bool b) { m_scrollbarCapturingMouse = b; }
 
     // ScrollableArea
-    int scrollSize(ScrollbarOrientation) const override;
-    int scrollOffset(ScrollbarOrientation) const override;
+    ScrollPosition scrollPosition() const override;
     void setScrollOffset(const IntPoint&) override;
     void invalidateScrollbarRect(Scrollbar&, const IntRect&) override;
     void invalidateScrollCornerRect(const IntRect&) override { }
index 6b6e8b0..736c841 100644 (file)
@@ -2898,28 +2898,11 @@ void RenderLayer::resize(const PlatformMouseEvent& evt, const LayoutSize& oldOff
     // FIXME (Radar 4118564): We should also autoscroll the window as necessary to keep the point under the cursor in view.
 }
 
-int RenderLayer::scrollSize(ScrollbarOrientation orientation) const
-{
-    Scrollbar* scrollbar = ((orientation == HorizontalScrollbar) ? m_hBar : m_vBar).get();
-    return scrollbar ? (scrollbar->totalSize() - scrollbar->visibleSize()) : 0;
-}
-
 void RenderLayer::setScrollOffset(const ScrollOffset& offset)
 {
     scrollTo(scrollPositionFromOffset(offset));
 }
 
-int RenderLayer::scrollOffset(ScrollbarOrientation orientation) const
-{
-    if (orientation == HorizontalScrollbar)
-        return scrollOffset().x();
-
-    if (orientation == VerticalScrollbar)
-        return scrollOffset().y();
-
-    return 0;
-}
-
 ScrollingNodeID RenderLayer::scrollingNodeID() const
 {
     if (!isComposited())
index 5e07bc7..8cc8758 100644 (file)
@@ -442,8 +442,6 @@ public:
     void setPostLayoutScrollPosition(Optional<ScrollPosition>);
     void applyPostLayoutScrollPositionIfNeeded();
 
-    ScrollOffset scrollOffset() const { return scrollOffsetFromPosition(m_scrollPosition); }
-
     void availableContentSizeChanged(AvailableSizeChangeReason) override;
 
     // "absoluteRect" is in scaled document coordinates.
@@ -1073,8 +1071,6 @@ private:
 
     bool shouldBeSelfPaintingLayer() const;
 
-    int scrollOffset(ScrollbarOrientation) const override;
-    
     // ScrollableArea interface
     void invalidateScrollbarRect(Scrollbar&, const IntRect&) override;
     void invalidateScrollCornerRect(const IntRect&) override;
@@ -1085,7 +1081,6 @@ private:
     IntRect convertFromContainingViewToScrollbar(const Scrollbar&, const IntRect&) const override;
     IntPoint convertFromScrollbarToContainingView(const Scrollbar&, const IntPoint&) const override;
     IntPoint convertFromContainingViewToScrollbar(const Scrollbar&, const IntPoint&) const override;
-    int scrollSize(ScrollbarOrientation) const override;
     void setScrollOffset(const ScrollOffset&) override;
     ScrollingNodeID scrollingNodeID() const override;
 
index a696bf1..6b3c121 100644 (file)
@@ -645,14 +645,9 @@ void RenderListBox::valueChanged(unsigned listIndex)
     selectElement().dispatchFormControlChangeEvent();
 }
 
-int RenderListBox::scrollSize(ScrollbarOrientation orientation) const
+ScrollPosition RenderListBox::scrollPosition() const
 {
-    return ((orientation == VerticalScrollbar) && m_vBar) ? (m_vBar->totalSize() - m_vBar->visibleSize()) : 0;
-}
-
-int RenderListBox::scrollOffset(ScrollbarOrientation) const
-{
-    return m_indexOffset;
+    return { 0, m_indexOffset };
 }
 
 ScrollPosition RenderListBox::minimumScrollPosition() const
index 3cd1697..65d9226 100644 (file)
@@ -112,11 +112,12 @@ private:
     bool nodeAtPoint(const HitTestRequest&, HitTestResult&, const HitTestLocation& locationInContainer, const LayoutPoint& accumulatedOffset, HitTestAction) override;
 
     // ScrollableArea interface.
-    int scrollSize(ScrollbarOrientation) const override;
-    int scrollOffset(ScrollbarOrientation) const override;
     void setScrollOffset(const ScrollOffset&) override;
+
+    ScrollPosition scrollPosition() const override;
     ScrollPosition minimumScrollPosition() const override;
     ScrollPosition maximumScrollPosition() const override;
+
     void invalidateScrollbarRect(Scrollbar&, const IntRect&) override;
     bool isActive() const override;
     bool isScrollCornerVisible() const override { return false; } // We don't support resize on list boxes yet. If we did these would have to change.
index 56bb488..4e2369d 100644 (file)
@@ -1,3 +1,22 @@
+2019-06-16  Simon Fraser  <simon.fraser@apple.com>
+
+        Implement ScrollableArea::scrollOffset()
+        https://bugs.webkit.org/show_bug.cgi?id=198895
+
+        Reviewed by Antti Koivisto.
+
+        * UIProcess/win/WebPopupMenuProxyWin.cpp:
+        (WebKit::PopupMenuWin::scrollPosition const):
+        (WebKit::WebPopupMenuProxyWin::onKeyDown): Just use m_scrollOffset.
+        (WebKit::WebPopupMenuProxyWin::scrollSize const): Deleted.
+        * UIProcess/win/WebPopupMenuProxyWin.h: Remove the one-axis scrollOffset()
+        * WebProcess/Plugins/PDF/PDFPlugin.h:
+        * WebProcess/Plugins/PDF/PDFPlugin.mm:
+        (WebKit::PDFPlugin::scrollSize const): Deleted.
+        (WebKit::PDFPlugin::scrollOffset const): Deleted.
+        * WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm:
+        (WebKit::RemoteLayerTreeDrawingArea::updateScrolledExposedRect):
+
 2019-06-15  Youenn Fablet  <youenn@apple.com>
 
         WebProcessPool::clearWebProcessHasUploads cannot assume its given processIdentifier is valid
index 980e0b7..0a8d8c4 100644 (file)
@@ -441,9 +441,9 @@ void WebPopupMenuProxyWin::invalidateItem(int index)
     ::InvalidateRect(m_popup, &r, TRUE);
 }
 
-int WebPopupMenuProxyWin::scrollSize(ScrollbarOrientation orientation) const
+ScrollPosition WebPopupMenuProxyWin::scrollPosition() const
 {
-    return ((orientation == VerticalScrollbar) && m_scrollbar) ? (m_scrollbar->totalSize() - m_scrollbar->visibleSize()) : 0;
+    return { 0, m_scrollOffset };
 }
 
 void WebPopupMenuProxyWin::setScrollOffset(const IntPoint& offset)
@@ -554,9 +554,9 @@ LRESULT WebPopupMenuProxyWin::onKeyDown(HWND hWnd, UINT message, WPARAM wParam,
         focusLast();
         break;
     case VK_PRIOR:
-        if (focusedIndex() != scrollOffset(VerticalScrollbar)) {
+        if (focusedIndex() != m_scrollOffset) {
             // Set the selection to the first visible item
-            int firstVisibleItem = scrollOffset(VerticalScrollbar);
+            int firstVisibleItem = m_scrollOffset;
             up(focusedIndex() - firstVisibleItem);
         } else {
             // The first visible item is selected, so move the selection back one page
@@ -564,7 +564,7 @@ LRESULT WebPopupMenuProxyWin::onKeyDown(HWND hWnd, UINT message, WPARAM wParam,
         }
         break;
     case VK_NEXT: {
-        int lastVisibleItem = scrollOffset(VerticalScrollbar) + visibleItems() - 1;
+        int lastVisibleItem = m_scrollOffset + visibleItems() - 1;
         if (focusedIndex() != lastVisibleItem) {
             // Set the selection to the last visible item
             down(lastVisibleItem - focusedIndex());
@@ -921,7 +921,6 @@ void WebPopupMenuProxyWin::focusLast()
     }
 }
 
-
 void WebPopupMenuProxyWin::incrementWheelDelta(int delta)
 {
     m_wheelDelta += delta;
index 4694c47..64739b3 100644 (file)
@@ -58,9 +58,8 @@ private:
     WebCore::Scrollbar* scrollbar() const { return m_scrollbar.get(); }
 
     // ScrollableArea
-    int scrollSize(WebCore::ScrollbarOrientation) const override;
+    WebCore::ScrollPosition scrollPosition() const override;
     void setScrollOffset(const WebCore::IntPoint&) override;
-    int scrollOffset(WebCore::ScrollbarOrientation) const override { return m_scrollOffset; }
 
     void invalidateScrollbarRect(WebCore::Scrollbar&, const WebCore::IntRect&) override;
     void invalidateScrollCornerRect(const WebCore::IntRect&) override { }
index 6862e30..d41503b 100644 (file)
@@ -202,10 +202,8 @@ private:
     void invalidateScrollbarRect(WebCore::Scrollbar&, const WebCore::IntRect&) final;
     void invalidateScrollCornerRect(const WebCore::IntRect&) final;
     WebCore::IntPoint lastKnownMousePosition() const final { return m_lastMousePositionInPluginCoordinates; }
-    int scrollSize(WebCore::ScrollbarOrientation) const final;
     bool isActive() const final;
     bool isScrollCornerVisible() const final { return false; }
-    int scrollOffset(WebCore::ScrollbarOrientation) const final;
     WebCore::ScrollPosition scrollPosition() const final;
     WebCore::ScrollPosition minimumScrollPosition() const final;
     WebCore::ScrollPosition maximumScrollPosition() const final;
index 1588bd0..03a1f04 100644 (file)
@@ -863,12 +863,6 @@ IntRect PDFPlugin::scrollableAreaBoundingBox(bool*) const
     return pluginView()->frameRect();
 }
 
-int PDFPlugin::scrollSize(ScrollbarOrientation orientation) const
-{
-    Scrollbar* scrollbar = ((orientation == HorizontalScrollbar) ? m_horizontalScrollbar : m_verticalScrollbar).get();
-    return scrollbar ? (scrollbar->totalSize() - scrollbar->visibleSize()) : 0;
-}
-
 bool PDFPlugin::isActive() const
 {
     if (Frame* coreFrame = m_frame->coreFrame()) {
@@ -889,18 +883,6 @@ bool PDFPlugin::forceUpdateScrollbarsOnMainThreadForPerformanceTesting() const
     return false;
 }
 
-int PDFPlugin::scrollOffset(ScrollbarOrientation orientation) const
-{
-    if (orientation == HorizontalScrollbar)
-        return m_scrollOffset.width();
-
-    if (orientation == VerticalScrollbar)
-        return m_scrollOffset.height();
-
-    ASSERT_NOT_REACHED();
-    return 0;
-}
-
 ScrollPosition PDFPlugin::scrollPosition() const
 {
     return IntPoint(m_scrollOffset.width(), m_scrollOffset.height());
index 4e4762c..5e50e0b 100644 (file)
@@ -253,10 +253,8 @@ void RemoteLayerTreeDrawingArea::updateScrolledExposedRect()
     m_scrolledViewExposedRect = m_viewExposedRect;
 
 #if !PLATFORM(IOS_FAMILY)
-    if (m_viewExposedRect) {
-        ScrollOffset scrollOffset = frameView->scrollOffsetFromPosition(frameView->scrollPosition());
-        m_scrolledViewExposedRect.value().moveBy(scrollOffset);
-    }
+    if (m_viewExposedRect)
+        m_scrolledViewExposedRect.value().moveBy(frameView->scrollOffset());
 #endif
 
     frameView->setViewExposedRect(m_scrolledViewExposedRect);