[css-flexbox] Move flex item preferred width computation specifics to…

… RenderFlexibleBox class

https://bugs.webkit.org/show_bug.cgi?id=226822

Reviewed by Alan Bujtas.

RenderBlock had some specific code for flex items that cleared the overriding sizes before
computing the {min|max}PreferredWidths and then restored them afterwards. That is done to
properly compute flex items intrinsic sizes. That code is flexbox specific so we better move
it to RenderFlexibleBox. In order to do that a new virtual method was added to RenderBlock which
just calls minPreferredLogicalWidth() and maxPreferredLogicalWidth() for every block except
flexbox containers.

In the case of flexbox containers, it wraps those calls with a RAII class that properly
clears the overriding sizes on instantiation and clears them on destruction. Now that the RAII
class is available we use it also for another existing code path that requires to
temporarily set an overriding size in a given scope.

No need for new tests as there is no change in behaviour.

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::computeChildIntrinsicLogicalWidths const): New virtual method.
(WebCore::RenderBlock::computeChildPreferredLogicalWidths const): Call computeChildIntrinsicLogicalWidths().
* rendering/RenderBlock.h:
* rendering/RenderFlexibleBox.cpp:
(WebCore::OverridingSizesScope::OverridingSizesScope): New RAII class to set/clear overriding sizes in a scope.
(WebCore::OverridingSizesScope::~OverridingSizesScope):
(WebCore::OverridingSizesScope::setOrClearOverridingSize):
(WebCore::RenderFlexibleBox::computeChildIntrinsicLogicalWidths const): Redefinition for flexbox containers.
(WebCore::RenderFlexibleBox::childIntrinsicLogicalHeight const): Removed constness from attribute.
(WebCore::RenderFlexibleBox::childIntrinsicLogicalWidth): Ditto.
(WebCore::RenderFlexibleBox::crossAxisIntrinsicExtentForChild): Ditto.
* rendering/RenderFlexibleBox.h:

Canonical link: https://commits.webkit.org/239147@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@279268 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog

@@ -1,3 +1,38 @@
+2021-06-09  Sergio Villar Senin  <svillar@igalia.com>
+
+        [css-flexbox] Move flex item preferred width computation specifics to RenderFlexibleBox class
+        https://bugs.webkit.org/show_bug.cgi?id=226822
+
+        Reviewed by Alan Bujtas.
+
+        RenderBlock had some specific code for flex items that cleared the overriding sizes before
+        computing the {min|max}PreferredWidths and then restored them afterwards. That is done to
+        properly compute flex items intrinsic sizes. That code is flexbox specific so we better move
+        it to RenderFlexibleBox. In order to do that a new virtual method was added to RenderBlock which
+        just calls minPreferredLogicalWidth() and maxPreferredLogicalWidth() for every block except
+        flexbox containers.
+
+        In the case of flexbox containers, it wraps those calls with a RAII class that properly
+        clears the overriding sizes on instantiation and clears them on destruction. Now that the RAII
+        class is available we use it also for another existing code path that requires to
+        temporarily set an overriding size in a given scope.
+
+        No need for new tests as there is no change in behaviour.
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::computeChildIntrinsicLogicalWidths const): New virtual method.
+        (WebCore::RenderBlock::computeChildPreferredLogicalWidths const): Call computeChildIntrinsicLogicalWidths().
+        * rendering/RenderBlock.h:
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::OverridingSizesScope::OverridingSizesScope): New RAII class to set/clear overriding sizes in a scope.
+        (WebCore::OverridingSizesScope::~OverridingSizesScope):
+        (WebCore::OverridingSizesScope::setOrClearOverridingSize):
+        (WebCore::RenderFlexibleBox::computeChildIntrinsicLogicalWidths const): Redefinition for flexbox containers.
+        (WebCore::RenderFlexibleBox::childIntrinsicLogicalHeight const): Removed constness from attribute.
+        (WebCore::RenderFlexibleBox::childIntrinsicLogicalWidth): Ditto.
+        (WebCore::RenderFlexibleBox::crossAxisIntrinsicExtentForChild): Ditto.
+        * rendering/RenderFlexibleBox.h:
+
 2021-06-24  Antoine Quint  <graouts@webkit.org>
 
         [Model] Create a sandbox extension for a temporary directory to store model resources

Source/WebCore/rendering/RenderBlock.cpp

@@ -2383,6 +2383,12 @@ void RenderBlock::computeBlockPreferredLogicalWidths(LayoutUnit& minLogicalWidth
     maxLogicalWidth = std::max(floatLeftWidth + floatRightWidth, maxLogicalWidth);
 }
 
+void RenderBlock::computeChildIntrinsicLogicalWidths(RenderObject& child, LayoutUnit& minPreferredLogicalWidth, LayoutUnit& maxPreferredLogicalWidth) const
+{
+    minPreferredLogicalWidth = child.minPreferredLogicalWidth();
+    maxPreferredLogicalWidth = child.maxPreferredLogicalWidth();
+}
+
 void RenderBlock::computeChildPreferredLogicalWidths(RenderObject& child, LayoutUnit& minPreferredLogicalWidth, LayoutUnit& maxPreferredLogicalWidth) const
 {
     if (child.isBox() && child.isHorizontalWritingMode() != isHorizontalWritingMode()) {
@@ -2403,32 +2409,7 @@ void RenderBlock::computeChildPreferredLogicalWidths(RenderObject& child, Layout
         return;
     }
 
-    // The preferred widths of flexbox children should never depend on overriding sizes. They should
-    // always be computed without regard for any overrides that are present.
-    std::optional<LayoutUnit> overridingHeight;
-    std::optional<LayoutUnit> overridingWidth;
-
-    if (child.isBox()) {
-        auto& box = downcast<RenderBox>(child);
-        if (box.isFlexItem()) {
-            if (box.hasOverridingLogicalHeight())
-                overridingHeight = std::optional<LayoutUnit>(box.overridingLogicalHeight());
-            if (box.hasOverridingLogicalWidth())
-                overridingWidth = std::optional<LayoutUnit>(box.overridingLogicalWidth());
-            box.clearOverridingContentSize();
-        }
-    }
-
-    minPreferredLogicalWidth = child.minPreferredLogicalWidth();
-    maxPreferredLogicalWidth = child.maxPreferredLogicalWidth();
-
-    if (child.isBox()) {
-        auto& box = downcast<RenderBox>(child);
-        if (overridingHeight)
-            box.setOverridingLogicalHeight(overridingHeight.value());
-        if (overridingWidth)
-            box.setOverridingLogicalWidth(overridingWidth.value());
-    }
+    computeChildIntrinsicLogicalWidths(child, minPreferredLogicalWidth, maxPreferredLogicalWidth);
 
     // For non-replaced blocks if the inline size is min|max-content or a definite
     // size the min|max-content contribution is that size plus border, padding and

Source/WebCore/rendering/RenderBlock.h

@@ -426,6 +426,8 @@ class RenderBlock : public RenderBox {
 
     void blockWillBeDestroyed();
 
+    virtual void computeChildIntrinsicLogicalWidths(RenderObject&, LayoutUnit& minPreferredLogicalWidth, LayoutUnit& maxPreferredLogicalWidth) const;
+
 private:
     static RenderPtr<RenderBlock> createAnonymousBlockWithStyleAndDisplay(Document&, const RenderStyle&, DisplayType);
 

Source/WebCore/rendering/RenderFlexibleBox.cpp

@@ -153,6 +153,77 @@ void RenderFlexibleBox::computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidt
     addScrollbarWidth();
 }
 
+// RAII class which defines a scope in which overriding sizes of a box are either:
+//   1) replaced by other size in one axis if size is specified
+//   2) cleared in both axis if size == std::nullopt
+//
+// In any case the previous overriding sizes are restored on destruction (in case of
+// not having a previous value it's simply cleared).
+class OverridingSizesScope {
+public:
+    enum class Axis {
+        Inline,
+        Block,
+        Both
+    };
+
+    OverridingSizesScope(RenderBox& box, Axis axis, std::optional<LayoutUnit> size = std::nullopt)
+        : m_box(box)
+        , m_axis(axis)
+    {
+        ASSERT(!size || (axis != Axis::Both));
+        if (axis == Axis::Both || axis == Axis::Inline) {
+            if (box.hasOverridingLogicalWidth())
+                m_overridingWidth = box.overridingLogicalWidth();
+            setOrClearOverridingSize(size, Axis::Inline);
+        }
+        if (axis == Axis::Both || axis == Axis::Block) {
+            if (box.hasOverridingLogicalHeight())
+                m_overridingHeight = box.overridingLogicalHeight();
+            setOrClearOverridingSize(size, Axis::Block);
+        }
+    }
+    ~OverridingSizesScope()
+    {
+        if (m_axis == Axis::Both || m_axis == Axis::Inline)
+            setOrClearOverridingSize(m_overridingWidth, Axis::Inline);
+
+        if (m_axis == Axis::Both || m_axis == Axis::Block)
+            setOrClearOverridingSize(m_overridingHeight, Axis::Block);
+    }
+
+private:
+    void setOrClearOverridingSize(std::optional<LayoutUnit> size, Axis axis)
+    {
+        ASSERT(axis != Axis::Both);
+        if (size) {
+            if (axis == Axis::Inline)
+                m_box.setOverridingLogicalWidth(*size);
+            else
+                m_box.setOverridingLogicalHeight(*size);
+            return;
+        }
+        if (axis == Axis::Inline)
+            m_box.clearOverridingLogicalWidth();
+        else
+            m_box.clearOverridingLogicalHeight();
+    }
+
+    RenderBox& m_box;
+    Axis m_axis;
+    std::optional<LayoutUnit> m_overridingWidth;
+    std::optional<LayoutUnit> m_overridingHeight;
+};
+
+void RenderFlexibleBox::computeChildIntrinsicLogicalWidths(RenderObject& childObject, LayoutUnit& minPreferredLogicalWidth, LayoutUnit& maxPreferredLogicalWidth) const
+{
+    ASSERT(childObject.isBox());
+    RenderBox& child = downcast<RenderBox>(childObject);
+
+    OverridingSizesScope cleanOverridingSizesScope(child, OverridingSizesScope::Axis::Both);
+    RenderBlock::computeChildIntrinsicLogicalWidths(childObject, minPreferredLogicalWidth, maxPreferredLogicalWidth);
+}
+
 LayoutUnit RenderFlexibleBox::baselinePosition(FontBaseline, bool, LineDirectionMode direction, LinePositionMode) const
 {
     auto baseline = firstLineBaseline();
@@ -476,7 +547,7 @@ void RenderFlexibleBox::clearCachedChildIntrinsicContentLogicalHeight(const Rend
     m_intrinsicContentLogicalHeights.remove(&child);
 }
 
-LayoutUnit RenderFlexibleBox::childIntrinsicLogicalHeight(const RenderBox& child) const
+LayoutUnit RenderFlexibleBox::childIntrinsicLogicalHeight(RenderBox& child) const
 {
     // This should only be called if the logical height is the cross size
     ASSERT(mainAxisIsChildInlineAxis(child));
@@ -488,27 +559,22 @@ LayoutUnit RenderFlexibleBox::childIntrinsicLogicalHeight(const RenderBox& child
     return child.logicalHeight();
 }
 
-LayoutUnit RenderFlexibleBox::childIntrinsicLogicalWidth(const RenderBox& child)
+LayoutUnit RenderFlexibleBox::childIntrinsicLogicalWidth(RenderBox& child)
 {
     // This should only be called if the logical width is the cross size
     ASSERT(!mainAxisIsChildInlineAxis(child));
     if (childCrossSizeIsDefinite(child, child.style().logicalWidth()))
         return child.logicalWidth();
 
-    std::optional<LayoutUnit> childOverridingWidth;
-    if (child.hasOverridingLogicalWidth()) {
-        // Temporarily clear potential overrides to compute the logical width otherwise it'll return the override size.
-        childOverridingWidth = child.overridingLogicalWidth();
-        const_cast<RenderBox*>(&child)->clearOverridingLogicalWidth();
-    }
     LogicalExtentComputedValues values;
-    child.computeLogicalWidthInFragment(values);
-    if (childOverridingWidth)
-        const_cast<RenderBox*>(&child)->setOverridingLogicalWidth(*childOverridingWidth);
+    {
+        OverridingSizesScope cleanOverridingWidthScope(child, OverridingSizesScope::Axis::Inline);
+        child.computeLogicalWidthInFragment(values);
+    }
     return values.m_extent;
 }
 
-LayoutUnit RenderFlexibleBox::crossAxisIntrinsicExtentForChild(const RenderBox& child)
+LayoutUnit RenderFlexibleBox::crossAxisIntrinsicExtentForChild(RenderBox& child)
 {
     return mainAxisIsChildInlineAxis(child) ? childIntrinsicLogicalHeight(child) : childIntrinsicLogicalWidth(child);
 }

Source/WebCore/rendering/RenderFlexibleBox.h

@@ -89,6 +89,7 @@ class RenderFlexibleBox : public RenderBlock {
 
 protected:
     void computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidth, LayoutUnit& maxLogicalWidth) const override;
+    void computeChildIntrinsicLogicalWidths(RenderObject&, LayoutUnit& minLogicalWidth, LayoutUnit& maxLogicalWidth) const override;
 
     bool shouldResetChildLogicalHeightBeforeLayout(const RenderBox&) const override { return m_shouldResetChildLogicalHeightBeforeLayout; }
 
@@ -116,9 +117,9 @@ class RenderFlexibleBox : public RenderBlock {
     Length crossSizeLengthForChild(SizeType, const RenderBox&) const;
     bool shouldApplyMinSizeAutoForChild(const RenderBox&) const;
     LayoutUnit crossAxisExtentForChild(const RenderBox& child) const;
-    LayoutUnit crossAxisIntrinsicExtentForChild(const RenderBox& child);
-    LayoutUnit childIntrinsicLogicalHeight(const RenderBox& child) const;
-    LayoutUnit childIntrinsicLogicalWidth(const RenderBox& child);
+    LayoutUnit crossAxisIntrinsicExtentForChild(RenderBox& child);
+    LayoutUnit childIntrinsicLogicalHeight(RenderBox& child) const;
+    LayoutUnit childIntrinsicLogicalWidth(RenderBox& child);
     LayoutUnit mainAxisExtentForChild(const RenderBox& child) const;
     LayoutUnit mainAxisContentExtentForChildIncludingScrollbar(const RenderBox& child) const;
     LayoutUnit crossAxisExtent() const;