- Revision
- 278661
- Author
- [email protected]
- Date
- 2021-06-09 09:02:23 -0700 (Wed, 09 Jun 2021)
Log 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:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (278660 => 278661)
--- trunk/Source/WebCore/ChangeLog 2021-06-09 15:21:39 UTC (rev 278660)
+++ trunk/Source/WebCore/ChangeLog 2021-06-09 16:02:23 UTC (rev 278661)
@@ -1,3 +1,24 @@
+2021-06-09 Simon Fraser <[email protected]>
+
+ 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 <[email protected]>
[Flexbox] FlexItem stays invisible after initial layout
Modified: trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp (278660 => 278661)
--- trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp 2021-06-09 15:21:39 UTC (rev 278660)
+++ trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp 2021-06-09 16:02:23 UTC (rev 278661)
@@ -4689,9 +4689,6 @@
void RenderLayerCompositor::updateScrollingNodeLayers(ScrollingNodeID nodeID, RenderLayer& layer, ScrollingCoordinator& scrollingCoordinator)
{
- auto* scrollableArea = layer.scrollableArea();
- ASSERT(scrollableArea);
-
if (layer.isRenderViewLayer()) {
FrameView& frameView = m_renderView.frameView();
scrollingCoordinator.setNodeLayers(nodeID, { nullptr,
@@ -4699,6 +4696,9 @@
fixedRootBackgroundLayer(), clipLayer(), rootContentsLayer(),
frameView.layerForHorizontalScrollbar(), frameView.layerForVerticalScrollbar() });
} else {
+ auto* scrollableArea = layer.scrollableArea();
+ ASSERT(scrollableArea);
+
auto& backing = *layer.backing();
scrollingCoordinator.setNodeLayers(nodeID, { backing.graphicsLayer(),
backing.scrollContainerLayer(), backing.scrolledContentsLayer(),
Modified: trunk/Source/WebCore/rendering/RenderLayerScrollableArea.cpp (278660 => 278661)
--- trunk/Source/WebCore/rendering/RenderLayerScrollableArea.cpp 2021-06-09 15:21:39 UTC (rev 278660)
+++ trunk/Source/WebCore/rendering/RenderLayerScrollableArea.cpp 2021-06-09 16:02:23 UTC (rev 278661)
@@ -1063,6 +1063,73 @@
return scrollHeight() > roundToInt(m_layer.renderBox()->clientHeight());
}
+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()
{
RenderBox* box = m_layer.renderBox();
@@ -1072,47 +1139,38 @@
if (box->style().appearance() == ListboxPart)
return;
- bool hasHorizontalOverflow = this->hasHorizontalOverflow();
- bool hasVerticalOverflow = this->hasVerticalOverflow();
+ bool hadHorizontalScrollbar = hasHorizontalScrollbar();
+ bool hadVerticalScrollbar = hasVerticalScrollbar();
- // 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);
+ 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();
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())
downcast<RenderFlexibleBox>(parent)->clearCachedMainSizeForChild(*m_layer.renderBox());
@@ -1537,42 +1595,6 @@
return scrollsOverflow() || usesCompositedScrolling();
}
-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)
{
auto& renderer = m_layer.renderer();
Modified: trunk/Source/WebCore/rendering/RenderLayerScrollableArea.h (278660 => 278661)
--- trunk/Source/WebCore/rendering/RenderLayerScrollableArea.h 2021-06-09 15:21:39 UTC (rev 278660)
+++ trunk/Source/WebCore/rendering/RenderLayerScrollableArea.h 2021-06-09 16:02:23 UTC (rev 278661)
@@ -101,9 +101,6 @@
void setHasHorizontalScrollbar(bool);
void setHasVerticalScrollbar(bool);
- Ref<Scrollbar> createScrollbar(ScrollbarOrientation);
- void destroyScrollbar(ScrollbarOrientation);
-
bool requiresScrollPositionReconciliation() const { return m_requiresScrollPositionReconciliation; }
void setRequiresScrollPositionReconciliation(bool requiresReconciliation = true) { m_requiresScrollPositionReconciliation = requiresReconciliation; }
@@ -212,13 +209,9 @@
void updateScrollbarsAfterLayout();
void positionOverflowControls(const IntSize&);
- void clearScrollCorner();
- void clearResizer();
void updateAllScrollbarRelatedStyle();
- void drawPlatformResizerImage(GraphicsContext&, const LayoutRect& resizerCornerRect);
-
LayoutUnit overflowTop() const;
LayoutUnit overflowBottom() const;
LayoutUnit overflowLeft() const;
@@ -230,15 +223,8 @@
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.
void scrollTo(const ScrollPosition&);
void updateCompositingLayersAfterScroll();
@@ -245,10 +231,6 @@
IntSize scrollbarOffset(const Scrollbar&) const;
- void updateScrollableAreaSet(bool hasOverflow);
-
- ScrollOffset clampScrollOffset(const ScrollOffset&) const;
-
void updateLayerPositionsAfterOverflowScroll();
void updateLayerPositionsAfterDocumentScroll();
@@ -260,9 +242,31 @@
#endif
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:
bool m_scrollDimensionsDirty { true };
bool m_inOverflowRelayout { false };
