Changeset 278661 in webkit

Timestamp:

Jun 9, 2021, 9:02:23 AM (5 years ago)

Author:

Simon Fraser

Message:

Clean up scrollbar creation code in RenderLayerScrollableArea
​https://bugs.webkit.org/show_bug.cgi?id=226805

Reviewed by Alan Bujtas.

Share code between updateScrollbarsAfterStyleChange() and updateScrollbarsAfterLayout() which
had a lot of common logic. updateScrollbarPresenceAndState() takes two optionals, indicating
whether information about overflow is available (which is only the case after layout).

Also make lots of member function declarations private in RenderLayerScrollableArea.

• rendering/RenderLayerCompositor.cpp:

(WebCore::RenderLayerCompositor::updateScrollingNodeLayers):

• rendering/RenderLayerScrollableArea.cpp:

(WebCore::RenderLayerScrollableArea::updateScrollbarPresenceAndState):
(WebCore::RenderLayerScrollableArea::updateScrollbarsAfterStyleChange):
(WebCore::RenderLayerScrollableArea::updateScrollbarsAfterLayout):

• rendering/RenderLayerScrollableArea.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• rendering/RenderLayerCompositor.cpp (modified) (2 diffs)
• rendering/RenderLayerScrollableArea.cpp (modified) (4 diffs)
• rendering/RenderLayerScrollableArea.h (modified) (5 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-06-09  Simon Fraser  <simon.fraser@apple.com>
+
+        Clean up scrollbar creation code in RenderLayerScrollableArea
+        https://bugs.webkit.org/show_bug.cgi?id=226805
+
+        Reviewed by Alan Bujtas.
+
+        Share code between updateScrollbarsAfterStyleChange() and updateScrollbarsAfterLayout() which
+        had a lot of common logic. updateScrollbarPresenceAndState() takes two optionals, indicating
+        whether information about overflow is available (which is only the case after layout).
+
+        Also make lots of member function declarations private in RenderLayerScrollableArea.
+
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::updateScrollingNodeLayers):
+        * rendering/RenderLayerScrollableArea.cpp:
+        (WebCore::RenderLayerScrollableArea::updateScrollbarPresenceAndState):
+        (WebCore::RenderLayerScrollableArea::updateScrollbarsAfterStyleChange):
+        (WebCore::RenderLayerScrollableArea::updateScrollbarsAfterLayout):
+        * rendering/RenderLayerScrollableArea.h:
+
 2021-06-09  Alan Bujtas  <zalan@apple.com>
 

trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp

@@ -4690 +4690 @@
 void RenderLayerCompositor::updateScrollingNodeLayers(ScrollingNodeID nodeID, RenderLayer& layer, ScrollingCoordinator& scrollingCoordinator)
 {
-    auto* scrollableArea = layer.scrollableArea();
-    ASSERT(scrollableArea);
-
     if (layer.isRenderViewLayer()) {
         FrameView& frameView = m_renderView.frameView();
@@ -4700 +4697 @@
             frameView.layerForHorizontalScrollbar(), frameView.layerForVerticalScrollbar() });
     } else {
+        auto* scrollableArea = layer.scrollableArea();
+        ASSERT(scrollableArea);
+
         auto& backing = *layer.backing();
         scrollingCoordinator.setNodeLayers(nodeID, { backing.graphicsLayer(),

trunk/Source/WebCore/rendering/RenderLayerScrollableArea.cpp

@@ -1064 +1064 @@
 }
 
+void RenderLayerScrollableArea::updateScrollbarPresenceAndState(std::optional<bool> hasHorizontalOverflow, std::optional<bool> hasVerticalOverflow)
+{
+    auto* box = m_layer.renderBox();
+    ASSERT(box);
+
+    enum class ScrollbarState {
+        NoScrollbar,
+        Enabled,
+        Disabled
+    };
+
+    auto scrollbarForAxis = [&](ScrollbarOrientation orientation) -> RefPtr<Scrollbar>& {
+        return orientation == ScrollbarOrientation::HorizontalScrollbar ? m_hBar : m_vBar;
+    };
+
+    auto stateForScrollbar = [&](ScrollbarOrientation orientation, std::optional<bool> hasOverflow, ScrollbarState nonScrollableState) {
+        if (hasOverflow)
+            return *hasOverflow ? ScrollbarState::Enabled : nonScrollableState;
+        
+        // If we don't have information about overflow (because we haven't done layout yet), just return the current state of the scrollbar.
+        auto existingScrollbar = scrollbarForAxis(orientation);
+        return (existingScrollbar && existingScrollbar->enabled()) ? ScrollbarState::Enabled : nonScrollableState;
+    };
+
+    auto stateForScrollbarOnAxis = [&](ScrollbarOrientation orientation, std::optional<bool> hasOverflow) {
+        if (box->hasAlwaysPresentScrollbar(orientation))
+            return stateForScrollbar(orientation, hasOverflow, ScrollbarState::Disabled);
+
+        if (box->hasAutoScrollbar(orientation))
+            return stateForScrollbar(orientation, hasOverflow, ScrollbarState::NoScrollbar);
+
+        return ScrollbarState::NoScrollbar;
+    };
+
+    auto horizontalBarState = stateForScrollbarOnAxis(ScrollbarOrientation::HorizontalScrollbar, hasHorizontalOverflow);
+    setHasHorizontalScrollbar(horizontalBarState != ScrollbarState::NoScrollbar);
+    if (horizontalBarState != ScrollbarState::NoScrollbar)
+        m_hBar->setEnabled(horizontalBarState == ScrollbarState::Enabled);
+
+    auto verticalBarState = stateForScrollbarOnAxis(ScrollbarOrientation::VerticalScrollbar, hasVerticalOverflow);
+    setHasVerticalScrollbar(verticalBarState != ScrollbarState::NoScrollbar);
+    if (verticalBarState != ScrollbarState::NoScrollbar)
+        m_vBar->setEnabled(verticalBarState == ScrollbarState::Enabled);
+}
+
+void RenderLayerScrollableArea::updateScrollbarsAfterStyleChange(const RenderStyle* oldStyle)
+{
+    // Overflow is a box concept.
+    RenderBox* box = m_layer.renderBox();
+    if (!box)
+        return;
+
+    // List box parts handle the scrollbars by themselves so we have nothing to do.
+    if (box->style().appearance() == ListboxPart)
+        return;
+
+    bool hadVerticalScrollbar = hasVerticalScrollbar();
+    updateScrollbarPresenceAndState();
+    bool hasVerticalScrollbar = this->hasVerticalScrollbar();
+
+    if (hadVerticalScrollbar != hasVerticalScrollbar || (hasVerticalScrollbar && oldStyle && oldStyle->shouldPlaceVerticalScrollbarOnLeft() != box->style().shouldPlaceVerticalScrollbarOnLeft()))
+        computeScrollOrigin();
+
+    if (!m_scrollDimensionsDirty)
+        updateScrollableAreaSet(hasScrollableHorizontalOverflow() || hasScrollableVerticalOverflow());
+}
+
 void RenderLayerScrollableArea::updateScrollbarsAfterLayout()
 {
@@ -1073 +1140 @@
         return;
 
-    bool hasHorizontalOverflow = this->hasHorizontalOverflow();
-    bool hasVerticalOverflow = this->hasVerticalOverflow();
-
-    // If overflow requires a scrollbar, then we just need to enable or disable.
-    auto& renderer = m_layer.renderer();
-    if (m_hBar && box->hasAlwaysPresentScrollbar(ScrollbarOrientation::HorizontalScrollbar))
-        m_hBar->setEnabled(hasHorizontalOverflow);
-    if (m_vBar && box->hasAlwaysPresentScrollbar(ScrollbarOrientation::VerticalScrollbar))
-        m_vBar->setEnabled(hasVerticalOverflow);
+    bool hadHorizontalScrollbar = hasHorizontalScrollbar();
+    bool hadVerticalScrollbar = hasVerticalScrollbar();
+
+    updateScrollbarPresenceAndState(hasHorizontalOverflow(), hasVerticalOverflow());
 
     // Scrollbars with auto behavior may need to lay out again if scrollbars got added or removed.
-    bool autoHorizontalScrollBarChanged = box->hasAutoScrollbar(ScrollbarOrientation::HorizontalScrollbar) && (hasHorizontalScrollbar() != hasHorizontalOverflow);
-    bool autoVerticalScrollBarChanged = box->hasAutoScrollbar(ScrollbarOrientation::VerticalScrollbar) && (hasVerticalScrollbar() != hasVerticalOverflow);
+    bool autoHorizontalScrollBarChanged = box->hasAutoScrollbar(ScrollbarOrientation::HorizontalScrollbar) && (hadHorizontalScrollbar != hasHorizontalScrollbar());
+    bool autoVerticalScrollBarChanged = box->hasAutoScrollbar(ScrollbarOrientation::VerticalScrollbar) && (hadVerticalScrollbar != hasVerticalScrollbar());
 
     if (autoHorizontalScrollBarChanged || autoVerticalScrollBarChanged) {
-        if (box->hasAutoScrollbar(ScrollbarOrientation::HorizontalScrollbar))
-            setHasHorizontalScrollbar(hasHorizontalOverflow);
-        if (box->hasAutoScrollbar(ScrollbarOrientation::VerticalScrollbar))
-            setHasVerticalScrollbar(hasVerticalOverflow);
-
         if (autoVerticalScrollBarChanged && shouldPlaceVerticalScrollbarOnLeft())
             computeScrollOrigin();
@@ -1098 +1155 @@
         m_layer.updateSelfPaintingLayer();
 
+        auto& renderer = m_layer.renderer();
         renderer.repaint();
 
         if (renderer.style().overflowX() == Overflow::Auto || renderer.style().overflowY() == Overflow::Auto) {
             if (!m_inOverflowRelayout) {
-                m_inOverflowRelayout = true;
+                SetForScope<bool> inOverflowRelayoutScope(m_inOverflowRelayout, true);
                 renderer.setNeedsLayout(MarkOnlyThis);
                 if (is<RenderBlock>(renderer)) {
-                    RenderBlock& block = downcast<RenderBlock>(renderer);
+                    auto& block = downcast<RenderBlock>(renderer);
                     block.scrollbarsChanged(autoHorizontalScrollBarChanged, autoVerticalScrollBarChanged);
                     block.layoutBlock(true);
                 } else
                     renderer.layout();
-                m_inOverflowRelayout = false;
             }
         }
 
+        // FIXME: This does not belong here.
         RenderObject* parent = renderer.parent();
         if (parent && parent->isFlexibleBox() && renderer.isBox())
@@ -1538 +1596 @@
 }
 
-void RenderLayerScrollableArea::updateScrollbarsAfterStyleChange(const RenderStyle* oldStyle)
-{
-    // Overflow are a box concept.
-    RenderBox* box = m_layer.renderBox();
-    if (!box)
-        return;
-
-    // List box parts handle the scrollbars by themselves so we have nothing to do.
-    if (box->style().appearance() == ListboxPart)
-        return;
-
-    Overflow overflowX = box->style().overflowX();
-    Overflow overflowY = box->style().overflowY();
-
-    // To avoid doing a relayout in updateScrollbarsAfterLayout, we try to keep any automatic scrollbar that was already present.
-    bool hadVerticalScrollbar = m_vBar;
-    bool needsHorizontalScrollbar = (m_hBar && box->hasAutoScrollbar(ScrollbarOrientation::HorizontalScrollbar)) || box->hasAlwaysPresentScrollbar(ScrollbarOrientation::HorizontalScrollbar);
-    bool needsVerticalScrollbar = (m_vBar && box->hasAutoScrollbar(ScrollbarOrientation::VerticalScrollbar)) || box->hasAlwaysPresentScrollbar(ScrollbarOrientation::VerticalScrollbar);
-    setHasHorizontalScrollbar(needsHorizontalScrollbar);
-    setHasVerticalScrollbar(needsVerticalScrollbar);
-
-    if (hadVerticalScrollbar != needsVerticalScrollbar || (needsVerticalScrollbar && oldStyle && box->style().shouldPlaceVerticalScrollbarOnLeft() != oldStyle->shouldPlaceVerticalScrollbarOnLeft()))
-        computeScrollOrigin();
-
-    // With non-overlay overflow:scroll, scrollbars are always visible but may be disabled.
-    // When switching to another value, we need to re-enable them (see bug 11985).
-    if (m_hBar && needsHorizontalScrollbar && oldStyle && oldStyle->overflowX() == Overflow::Scroll && overflowX != Overflow::Scroll)
-        m_hBar->setEnabled(true);
-
-    if (m_vBar && needsVerticalScrollbar && oldStyle && oldStyle->overflowY() == Overflow::Scroll && overflowY != Overflow::Scroll)
-        m_vBar->setEnabled(true);
-
-    if (!m_scrollDimensionsDirty)
-        updateScrollableAreaSet(hasScrollableHorizontalOverflow() || hasScrollableVerticalOverflow());
-}
-
 void RenderLayerScrollableArea::updateScrollableAreaSet(bool hasOverflow)
 {

trunk/Source/WebCore/rendering/RenderLayerScrollableArea.h

@@ -102 +102 @@
     void setHasVerticalScrollbar(bool);
 
-    Ref<Scrollbar> createScrollbar(ScrollbarOrientation);
-    void destroyScrollbar(ScrollbarOrientation);
-
     bool requiresScrollPositionReconciliation() const { return m_requiresScrollPositionReconciliation; }
     void setRequiresScrollPositionReconciliation(bool requiresReconciliation = true) { m_requiresScrollPositionReconciliation = requiresReconciliation; }
@@ -213 +210 @@
 
     void positionOverflowControls(const IntSize&);
-    void clearScrollCorner();
-    void clearResizer();
 
     void updateAllScrollbarRelatedStyle();
-
-    void drawPlatformResizerImage(GraphicsContext&, const LayoutRect& resizerCornerRect);
 
     LayoutUnit overflowTop() const;
@@ -231 +224 @@
     bool scrollingMayRevealBackground() const;
 
-    void computeScrollDimensions();
-    void computeScrollOrigin();
     void computeHasCompositedScrollableOverflow();
-
-    bool hasHorizontalOverflow() const;
-    bool hasVerticalOverflow() const;
-
-    bool showsOverflowControls() const;
 
     // NOTE: This should only be called by the overridden setScrollOffset from ScrollableArea.
@@ -246 +232 @@
     IntSize scrollbarOffset(const Scrollbar&) const;
 
-    void updateScrollableAreaSet(bool hasOverflow);
-
-    ScrollOffset clampScrollOffset(const ScrollOffset&) const;
-
     void updateLayerPositionsAfterOverflowScroll();
     void updateLayerPositionsAfterDocumentScroll();
@@ -261 +243 @@
 
 private:
+    bool hasHorizontalOverflow() const;
+    bool hasVerticalOverflow() const;
+
+    bool showsOverflowControls() const;
+
+    ScrollOffset clampScrollOffset(const ScrollOffset&) const;
+
+    void computeScrollDimensions();
+    void computeScrollOrigin();
+
+    void updateScrollableAreaSet(bool hasOverflow);
+
     void updateScrollCornerStyle();
     void updateResizerStyle();
+
+    void drawPlatformResizerImage(GraphicsContext&, const LayoutRect& resizerCornerRect);
+
+    Ref<Scrollbar> createScrollbar(ScrollbarOrientation);
+    void destroyScrollbar(ScrollbarOrientation);
+
+    void clearScrollCorner();
+    void clearResizer();
+
+    void updateScrollbarPresenceAndState(std::optional<bool> hasHorizontalOverflow = std::nullopt, std::optional<bool> hasVerticalOverflow = std::nullopt);
 
 private: