Changeset 194184 in webkit

Timestamp:

Dec 16, 2015 3:41:05 PM (8 years ago)

Author:

Simon Fraser

Message:

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.

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):

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/page/FrameView.cpp (modified) (2 diffs)
• WebCore/platform/ScrollView.cpp (modified) (5 diffs)
• WebCore/platform/ScrollableArea.cpp (modified) (2 diffs)
• WebCore/platform/ScrollableArea.h (modified) (1 diff)
• WebCore/platform/Scrollbar.cpp (modified) (1 diff)
• WebCore/platform/Scrollbar.h (modified) (1 diff)
• WebCore/rendering/RenderLayer.cpp (modified) (1 diff)
• WebCore/rendering/RenderListBox.cpp (modified) (1 diff)
• WebKit2/ChangeLog (modified) (1 diff)
• WebKit2/WebProcess/Plugins/PDF/DeprecatedPDFPlugin.mm (modified) (4 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/page/FrameView.cpp

@@ -3263 +3263 @@
             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
@@ -3272 +3271 @@
             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

trunk/Source/WebCore/platform/ScrollView.cpp

@@ -276 +276 @@
 #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;
-    }
-
-    return IntSize(width() - verticalScrollbarWidth, height() - horizontalScrollbarHeight).expandedTo(IntSize());
+    IntSize scrollbarSpace;
+    if (scrollbarInclusion == ExcludeScrollbars)
+        scrollbarSpace = scrollbarIntrusion();
+
+    return IntSize(width() - scrollbarSpace.width(), height() - scrollbarSpace.height()).expandedTo(IntSize());
 }
     
@@ -1250 +1244 @@
 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();
@@ -1259 +1250 @@
         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());
     }
 
@@ -1271 +1262 @@
     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())
@@ -1280 +1271 @@
         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());

trunk/Source/WebCore/platform/ScrollableArea.cpp

@@ -517 +517 @@
 #endif // PLATFORM(IOS)
 
+IntSize ScrollableArea::scrollbarIntrusion() const
+{
+    return IntSize(
+        verticalScrollbar() ? verticalScrollbar()->occupiedWidth() : 0,
+        horizontalScrollbar() ? horizontalScrollbar()->occupiedHeight() : 0);
+}
+
 IntPoint ScrollableArea::scrollPosition() const
 {
@@ -583 +590 @@
     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();
     }
 

trunk/Source/WebCore/platform/ScrollableArea.h

@@ -174 +174 @@
         return scrollbar->Widget::convertFromContainingView(parentPoint);
     }
+    
+    WEBCORE_EXPORT IntSize scrollbarIntrusion() const;
 
     virtual Scrollbar* horizontalScrollbar() const { return 0; }

trunk/Source/WebCore/platform/Scrollbar.cpp

@@ -98 +98 @@
 }
 
+int Scrollbar::occupiedWidth() const
+{
+    return isOverlayScrollbar() ? 0 : width();
+}
+
+int Scrollbar::occupiedHeight() const
+{
+    return isOverlayScrollbar() ? 0 : height();
+}
+
 void Scrollbar::offsetDidChange()
 {

trunk/Source/WebCore/platform/Scrollbar.h

@@ -70 +70 @@
     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; }

trunk/Source/WebCore/rendering/RenderLayer.cpp

@@ -2748 +2748 @@
 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;
-    }
-    
-    return IntRect(scrollPosition(), IntSize(std::max(0, m_layerSize.width() - verticalScrollbarWidth), std::max(0, m_layerSize.height() - horizontalScrollbarHeight)));
+    IntSize scrollbarSpace;
+    if (showsOverflowControls() && scrollbarInclusion == IncludeScrollbars)
+        scrollbarSpace = scrollbarIntrusion();
+    
+    return IntRect(scrollPosition(), IntSize(std::max(0, m_layerSize.width() - scrollbarSpace.width()), std::max(0, m_layerSize.height() - scrollbarSpace.height())));
 }
 

trunk/Source/WebCore/rendering/RenderListBox.cpp

@@ -631 +631 @@
 int RenderListBox::verticalScrollbarWidth() const
 {
-    return m_vBar && !m_vBar->isOverlayScrollbar() ? m_vBar->width() : 0;
+    return m_vBar ? m_vBar->occupiedWidth() : 0;
 }
 

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebKit2/WebProcess/Plugins/PDF/DeprecatedPDFPlugin.mm

@@ -588 +588 @@
         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();
@@ -595 +594 @@
     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)
@@ -603 +602 @@
     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)
@@ -797 +796 @@
 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;
-
-    IntPoint maximumOffset(m_pdfDocumentSize.width() - m_size.width() + verticalScrollbarWidth, m_pdfDocumentSize.height() - m_size.height() + horizontalScrollbarHeight);
+    IntSize scrollbarSpace = scrollbarIntrusion();
+
+    IntPoint maximumOffset(m_pdfDocumentSize.width() - m_size.width() + scrollbarSpace.width(), m_pdfDocumentSize.height() - m_size.height() + scrollbarSpace.height());
     maximumOffset.clampNegativeToZero();
     return maximumOffset;