Simplify isOverlayScrollbar() logic
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 16 Dec 2015 23:41:05 +0000 (23:41 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 16 Dec 2015 23:41:05 +0000 (23:41 +0000)
https://bugs.webkit.org/show_bug.cgi?id=152357

Reviewed by Beth Dakin.

Replace code that checks for isOverlayScrollbar() explicitly with calls to new
occupiedWidth()/occupiedHeight() functions on Scrollbar, which do the overlay
scrollbar check internally.

Add ScrollableArea::scrollbarIntrusion() which returns an IntSize with the occupiedWidth
and occupiedHeight of any scrollbars, and use it in a few places.

Source/WebCore:

* page/FrameView.cpp:
(WebCore::FrameView::autoSizeIfEnabled):
* platform/ScrollView.cpp:
(WebCore::ScrollView::unscaledVisibleContentSizeIncludingObscuredArea):
(WebCore::ScrollView::calculateOverhangAreasForPainting):
* platform/ScrollableArea.cpp:
(WebCore::ScrollableArea::scrollbarIntrusion):
(WebCore::ScrollableArea::visibleContentRectInternal):
* platform/ScrollableArea.h:
* platform/Scrollbar.cpp:
(WebCore::Scrollbar::occupiedWidth):
(WebCore::Scrollbar::occupiedHeight):
* platform/Scrollbar.h:
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::visibleContentRectInternal):
* rendering/RenderListBox.cpp:
(WebCore::RenderListBox::verticalScrollbarWidth):

Source/WebKit2:

* WebProcess/Plugins/PDF/DeprecatedPDFPlugin.mm:
(WebKit::PDFPlugin::updateScrollbars):
(WebKit::PDFPlugin::maximumScrollPosition):

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

Source/WebCore/ChangeLog
Source/WebCore/page/FrameView.cpp
Source/WebCore/platform/ScrollView.cpp
Source/WebCore/platform/ScrollableArea.cpp
Source/WebCore/platform/ScrollableArea.h
Source/WebCore/platform/Scrollbar.cpp
Source/WebCore/platform/Scrollbar.h
Source/WebCore/rendering/RenderLayer.cpp
Source/WebCore/rendering/RenderListBox.cpp
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/Plugins/PDF/DeprecatedPDFPlugin.mm

index b6f0a65..b006dd3 100644 (file)
@@ -1,3 +1,35 @@
+2015-12-16  Simon Fraser  <simon.fraser@apple.com>
+
+        Simplify isOverlayScrollbar() logic
+        https://bugs.webkit.org/show_bug.cgi?id=152357
+
+        Reviewed by Beth Dakin.
+
+        Replace code that checks for isOverlayScrollbar() explicitly with calls to new
+        occupiedWidth()/occupiedHeight() functions on Scrollbar, which do the overlay
+        scrollbar check internally.
+        
+        Add ScrollableArea::scrollbarIntrusion() which returns an IntSize with the occupiedWidth
+        and occupiedHeight of any scrollbars, and use it in a few places.
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::autoSizeIfEnabled):
+        * platform/ScrollView.cpp:
+        (WebCore::ScrollView::unscaledVisibleContentSizeIncludingObscuredArea):
+        (WebCore::ScrollView::calculateOverhangAreasForPainting):
+        * platform/ScrollableArea.cpp:
+        (WebCore::ScrollableArea::scrollbarIntrusion):
+        (WebCore::ScrollableArea::visibleContentRectInternal):
+        * platform/ScrollableArea.h:
+        * platform/Scrollbar.cpp:
+        (WebCore::Scrollbar::occupiedWidth):
+        (WebCore::Scrollbar::occupiedHeight):
+        * platform/Scrollbar.h:
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::visibleContentRectInternal):
+        * rendering/RenderListBox.cpp:
+        (WebCore::RenderListBox::verticalScrollbarWidth):
+
 2015-12-16  Alex Christensen  <achristensen@webkit.org>
 
         Fix internal Windows build
index 1387121..72de9d4 100644 (file)
@@ -3262,8 +3262,7 @@ void FrameView::autoSizeIfEnabled()
             RefPtr<Scrollbar> localHorizontalScrollbar = horizontalScrollbar();
             if (!localHorizontalScrollbar)
                 localHorizontalScrollbar = createScrollbar(HorizontalScrollbar);
-            if (!localHorizontalScrollbar->isOverlayScrollbar())
-                newSize.setHeight(newSize.height() + localHorizontalScrollbar->height());
+            newSize.expand(0, localHorizontalScrollbar->occupiedHeight());
 
             // Don't bother checking for a vertical scrollbar because the width is at
             // already greater the maximum.
@@ -3271,8 +3270,7 @@ void FrameView::autoSizeIfEnabled()
             RefPtr<Scrollbar> localVerticalScrollbar = verticalScrollbar();
             if (!localVerticalScrollbar)
                 localVerticalScrollbar = createScrollbar(VerticalScrollbar);
-            if (!localVerticalScrollbar->isOverlayScrollbar())
-                newSize.setWidth(newSize.width() + localVerticalScrollbar->width());
+            newSize.expand(localVerticalScrollbar->occupiedWidth(), 0);
 
             // Don't bother checking for a horizontal scrollbar because the height is
             // already greater the maximum.
index 1906ba3..2e7a498 100644 (file)
@@ -275,17 +275,11 @@ IntSize ScrollView::unscaledVisibleContentSizeIncludingObscuredArea(VisibleConte
         return m_fixedVisibleContentRect.size();
 #endif
 
-    int verticalScrollbarWidth = 0;
-    int horizontalScrollbarHeight = 0;
-
-    if (scrollbarInclusion == ExcludeScrollbars) {
-        if (Scrollbar* verticalBar = verticalScrollbar())
-            verticalScrollbarWidth = !verticalBar->isOverlayScrollbar() ? verticalBar->width() : 0;
-        if (Scrollbar* horizontalBar = horizontalScrollbar())
-            horizontalScrollbarHeight = !horizontalBar->isOverlayScrollbar() ? horizontalBar->height() : 0;
-    }
+    IntSize scrollbarSpace;
+    if (scrollbarInclusion == ExcludeScrollbars)
+        scrollbarSpace = scrollbarIntrusion();
 
-    return IntSize(width() - verticalScrollbarWidth, height() - horizontalScrollbarHeight).expandedTo(IntSize());
+    return IntSize(width() - scrollbarSpace.width(), height() - scrollbarSpace.height()).expandedTo(IntSize());
 }
     
 IntSize ScrollView::unscaledUnobscuredVisibleContentSize(VisibleContentRectIncludesScrollbars scrollbarInclusion) const
@@ -1249,28 +1243,25 @@ void ScrollView::paint(GraphicsContext& context, const IntRect& rect)
 
 void ScrollView::calculateOverhangAreasForPainting(IntRect& horizontalOverhangRect, IntRect& verticalOverhangRect)
 {
-    int verticalScrollbarWidth = (verticalScrollbar() && !verticalScrollbar()->isOverlayScrollbar())
-        ? verticalScrollbar()->width() : 0;
-    int horizontalScrollbarHeight = (horizontalScrollbar() && !horizontalScrollbar()->isOverlayScrollbar())
-        ? horizontalScrollbar()->height() : 0;
+    IntSize scrollbarSpace = scrollbarIntrusion();
 
     int physicalScrollY = scrollPosition().y() + scrollOrigin().y();
     if (physicalScrollY < 0) {
         horizontalOverhangRect = frameRect();
         horizontalOverhangRect.setHeight(-physicalScrollY);
-        horizontalOverhangRect.setWidth(horizontalOverhangRect.width() - verticalScrollbarWidth);
+        horizontalOverhangRect.setWidth(horizontalOverhangRect.width() - scrollbarSpace.width());
     } else if (totalContentsSize().height() && physicalScrollY > totalContentsSize().height() - visibleHeight()) {
         int height = physicalScrollY - (totalContentsSize().height() - visibleHeight());
         horizontalOverhangRect = frameRect();
-        horizontalOverhangRect.setY(frameRect().maxY() - height - horizontalScrollbarHeight);
+        horizontalOverhangRect.setY(frameRect().maxY() - height - scrollbarSpace.height());
         horizontalOverhangRect.setHeight(height);
-        horizontalOverhangRect.setWidth(horizontalOverhangRect.width() - verticalScrollbarWidth);
+        horizontalOverhangRect.setWidth(horizontalOverhangRect.width() - scrollbarSpace.width());
     }
 
     int physicalScrollX = scrollPosition().x() + scrollOrigin().x();
     if (physicalScrollX < 0) {
         verticalOverhangRect.setWidth(-physicalScrollX);
-        verticalOverhangRect.setHeight(frameRect().height() - horizontalOverhangRect.height() - horizontalScrollbarHeight);
+        verticalOverhangRect.setHeight(frameRect().height() - horizontalOverhangRect.height() - scrollbarSpace.height());
         verticalOverhangRect.setX(frameRect().x());
         if (horizontalOverhangRect.y() == frameRect().y())
             verticalOverhangRect.setY(frameRect().y() + horizontalOverhangRect.height());
@@ -1279,8 +1270,8 @@ void ScrollView::calculateOverhangAreasForPainting(IntRect& horizontalOverhangRe
     } else if (contentsWidth() && physicalScrollX > contentsWidth() - visibleWidth()) {
         int width = physicalScrollX - (contentsWidth() - visibleWidth());
         verticalOverhangRect.setWidth(width);
-        verticalOverhangRect.setHeight(frameRect().height() - horizontalOverhangRect.height() - horizontalScrollbarHeight);
-        verticalOverhangRect.setX(frameRect().maxX() - width - verticalScrollbarWidth);
+        verticalOverhangRect.setHeight(frameRect().height() - horizontalOverhangRect.height() - scrollbarSpace.height());
+        verticalOverhangRect.setX(frameRect().maxX() - width - scrollbarSpace.width());
         if (horizontalOverhangRect.y() == frameRect().y())
             verticalOverhangRect.setY(frameRect().y() + horizontalOverhangRect.height());
         else
index fb540d5..3bcc25d 100644 (file)
@@ -516,6 +516,13 @@ bool ScrollableArea::isPinnedVerticallyInDirection(int verticalScrollDelta) cons
 }
 #endif // PLATFORM(IOS)
 
+IntSize ScrollableArea::scrollbarIntrusion() const
+{
+    return IntSize(
+        verticalScrollbar() ? verticalScrollbar()->occupiedWidth() : 0,
+        horizontalScrollbar() ? horizontalScrollbar()->occupiedHeight() : 0);
+}
+
 IntPoint ScrollableArea::scrollPosition() const
 {
     int x = horizontalScrollbar() ? horizontalScrollbar()->value() : 0;
@@ -582,9 +589,9 @@ IntRect ScrollableArea::visibleContentRectInternal(VisibleContentRectIncludesScr
 
     if (scrollbarInclusion == IncludeScrollbars) {
         if (Scrollbar* verticalBar = verticalScrollbar())
-            verticalScrollbarWidth = !verticalBar->isOverlayScrollbar() ? verticalBar->width() : 0;
+            verticalScrollbarWidth = verticalBar->occupiedWidth();
         if (Scrollbar* horizontalBar = horizontalScrollbar())
-            horizontalScrollbarHeight = !horizontalBar->isOverlayScrollbar() ? horizontalBar->height() : 0;
+            horizontalScrollbarHeight = horizontalBar->occupiedHeight();
     }
 
     return IntRect(scrollPosition().x(),
index f8ebeef..be20b64 100644 (file)
@@ -173,6 +173,8 @@ public:
     {
         return scrollbar->Widget::convertFromContainingView(parentPoint);
     }
+    
+    WEBCORE_EXPORT IntSize scrollbarIntrusion() const;
 
     virtual Scrollbar* horizontalScrollbar() const { return 0; }
     virtual Scrollbar* verticalScrollbar() const { return 0; }
index b3414b7..c498457 100644 (file)
@@ -97,6 +97,16 @@ Scrollbar::~Scrollbar()
     theme().unregisterScrollbar(*this);
 }
 
+int Scrollbar::occupiedWidth() const
+{
+    return isOverlayScrollbar() ? 0 : width();
+}
+
+int Scrollbar::occupiedHeight() const
+{
+    return isOverlayScrollbar() ? 0 : height();
+}
+
 void Scrollbar::offsetDidChange()
 {
     float position = static_cast<float>(m_scrollableArea.scrollPosition(this));
index e24a207..8242510 100644 (file)
@@ -69,6 +69,9 @@ public:
     int totalSize() const { return m_totalSize; }
     int maximum() const { return m_totalSize - m_visibleSize; }
     ScrollbarControlSize controlSize() const { return m_controlSize; }
+    
+    int occupiedWidth() const;
+    int occupiedHeight() const;
 
     int lineStep() const { return m_lineStep; }
     int pageStep() const { return m_pageStep; }
index 279bcc0..f30527a 100644 (file)
@@ -2747,14 +2747,11 @@ IntPoint RenderLayer::maximumScrollPosition() const
 
 IntRect RenderLayer::visibleContentRectInternal(VisibleContentRectIncludesScrollbars scrollbarInclusion, VisibleContentRectBehavior) const
 {
-    int verticalScrollbarWidth = 0;
-    int horizontalScrollbarHeight = 0;
-    if (showsOverflowControls() && scrollbarInclusion == IncludeScrollbars) {
-        verticalScrollbarWidth = (verticalScrollbar() && !verticalScrollbar()->isOverlayScrollbar()) ? verticalScrollbar()->width() : 0;
-        horizontalScrollbarHeight = (horizontalScrollbar() && !horizontalScrollbar()->isOverlayScrollbar()) ? horizontalScrollbar()->height() : 0;
-    }
+    IntSize scrollbarSpace;
+    if (showsOverflowControls() && scrollbarInclusion == IncludeScrollbars)
+        scrollbarSpace = scrollbarIntrusion();
     
-    return IntRect(scrollPosition(), IntSize(std::max(0, m_layerSize.width() - verticalScrollbarWidth), std::max(0, m_layerSize.height() - horizontalScrollbarHeight)));
+    return IntRect(scrollPosition(), IntSize(std::max(0, m_layerSize.width() - scrollbarSpace.width()), std::max(0, m_layerSize.height() - scrollbarSpace.height())));
 }
 
 IntSize RenderLayer::overhangAmount() const
index 8ac4810..e0337f4 100644 (file)
@@ -630,7 +630,7 @@ LayoutUnit RenderListBox::itemHeight() const
 
 int RenderListBox::verticalScrollbarWidth() const
 {
-    return m_vBar && !m_vBar->isOverlayScrollbar() ? m_vBar->width() : 0;
+    return m_vBar ? m_vBar->occupiedWidth() : 0;
 }
 
 // FIXME: We ignore padding in the vertical direction as far as these values are concerned, since that's
index 7c69a5d..fbdec6b 100644 (file)
@@ -1,3 +1,21 @@
+2015-12-16  Simon Fraser  <simon.fraser@apple.com>
+
+        Simplify isOverlayScrollbar() logic
+        https://bugs.webkit.org/show_bug.cgi?id=152357
+
+        Reviewed by Beth Dakin.
+
+        Replace code that checks for isOverlayScrollbar() explicitly with calls to new
+        occupiedWidth()/occupiedHeight() functions on Scrollbar, which do the overlay
+        scrollbar check internally.
+        
+        Add ScrollableArea::scrollbarIntrusion() which returns an IntSize with the occupiedWidth
+        and occupiedHeight of any scrollbars, and use it in a few places.
+
+        * WebProcess/Plugins/PDF/DeprecatedPDFPlugin.mm:
+        (WebKit::PDFPlugin::updateScrollbars):
+        (WebKit::PDFPlugin::maximumScrollPosition):
+
 2015-12-16  Alex Christensen  <achristensen@webkit.org>
 
         Build fix when using NETWORK_SESSION after r193972.
index 3f1cbcd..104b7d7 100644 (file)
@@ -587,14 +587,13 @@ void PDFPlugin::updateScrollbars()
     } else if (m_size.height() < m_pdfDocumentSize.height())
         m_verticalScrollbar = createScrollbar(VerticalScrollbar);
 
-    int horizontalScrollbarHeight = (m_horizontalScrollbar && !m_horizontalScrollbar->isOverlayScrollbar()) ? m_horizontalScrollbar->height() : 0;
-    int verticalScrollbarWidth = (m_verticalScrollbar && !m_verticalScrollbar->isOverlayScrollbar()) ? m_verticalScrollbar->width() : 0;
+    IntSize scrollbarSpace = scrollbarIntrusion();
 
     int pageStep = m_pageBoxes.isEmpty() ? 0 : m_pageBoxes[0].height();
 
     if (m_horizontalScrollbar) {
         m_horizontalScrollbar->setSteps(Scrollbar::pixelsPerLineStep(), pageStep);
-        m_horizontalScrollbar->setProportion(m_size.width() - verticalScrollbarWidth, m_pdfDocumentSize.width());
+        m_horizontalScrollbar->setProportion(m_size.width() - scrollbarSpace.width(), m_pdfDocumentSize.width());
         IntRect scrollbarRect(pluginView()->x(), pluginView()->y() + m_size.height() - m_horizontalScrollbar->height(), m_size.width(), m_horizontalScrollbar->height());
         if (m_verticalScrollbar)
             scrollbarRect.contract(m_verticalScrollbar->width(), 0);
@@ -602,7 +601,7 @@ void PDFPlugin::updateScrollbars()
     }
     if (m_verticalScrollbar) {
         m_verticalScrollbar->setSteps(Scrollbar::pixelsPerLineStep(), pageStep);
-        m_verticalScrollbar->setProportion(m_size.height() - horizontalScrollbarHeight, m_pdfDocumentSize.height());
+        m_verticalScrollbar->setProportion(m_size.height() - scrollbarSpace.height(), m_pdfDocumentSize.height());
         IntRect scrollbarRect(IntRect(pluginView()->x() + m_size.width() - m_verticalScrollbar->width(), pluginView()->y(), m_verticalScrollbar->width(), m_size.height()));
         if (m_horizontalScrollbar)
             scrollbarRect.contract(0, m_horizontalScrollbar->height());
@@ -796,10 +795,9 @@ IntPoint PDFPlugin::minimumScrollPosition() const
 
 IntPoint PDFPlugin::maximumScrollPosition() const
 {
-    int horizontalScrollbarHeight = (m_horizontalScrollbar && !m_horizontalScrollbar->isOverlayScrollbar()) ? m_horizontalScrollbar->height() : 0;
-    int verticalScrollbarWidth = (m_verticalScrollbar && !m_verticalScrollbar->isOverlayScrollbar()) ? m_verticalScrollbar->width() : 0;
+    IntSize scrollbarSpace = scrollbarIntrusion();
 
-    IntPoint maximumOffset(m_pdfDocumentSize.width() - m_size.width() + verticalScrollbarWidth, m_pdfDocumentSize.height() - m_size.height() + horizontalScrollbarHeight);
+    IntPoint maximumOffset(m_pdfDocumentSize.width() - m_size.width() + scrollbarSpace.width(), m_pdfDocumentSize.height() - m_size.height() + scrollbarSpace.height());
     maximumOffset.clampNegativeToZero();
     return maximumOffset;
 }