[css-flex] Remove death code paths when evaluating percentage resolution
authorsvillar@igalia.com <svillar@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 1 Jul 2020 10:36:43 +0000 (10:36 +0000)
committersvillar@igalia.com <svillar@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 1 Jul 2020 10:36:43 +0000 (10:36 +0000)
https://bugs.webkit.org/show_bug.cgi?id=213809

Reviewed by Manuel Rego Casasnovas.

Both crossSizeForPercentageResolution() and mainSizeForPercentageResolution() are only called from
childLogicalHeightForPercentageResolution(). The former is called whenever hasOrthogonalFlow(child)
is false and the latter when it's true. However crossSizeForPercentageResolution() has a path for
hasOrthogonalFlow(child)==true which is impossible to reach. The same happens to
mainSizeForPercentageResolution() which has a path for hasOrthogonalFlow(child)==false which is
also impossible to reach.

Remove both death code paths and replace them by assertions. We're also making both methods
private since are not meant to be used from the outside.

* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::crossSizeForPercentageResolution): Remove death code path.
(WebCore::RenderFlexibleBox::mainSizeForPercentageResolution): Ditto.
* rendering/RenderFlexibleBox.h: Make both calls private.

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

Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderFlexibleBox.cpp
Source/WebCore/rendering/RenderFlexibleBox.h

index 842269b..2eb346b 100644 (file)
@@ -1,3 +1,25 @@
+2020-06-30  Sergio Villar Senin  <svillar@igalia.com>
+
+        [css-flex] Remove death code paths when evaluating percentage resolution
+        https://bugs.webkit.org/show_bug.cgi?id=213809
+
+        Reviewed by Manuel Rego Casasnovas.
+
+        Both crossSizeForPercentageResolution() and mainSizeForPercentageResolution() are only called from
+        childLogicalHeightForPercentageResolution(). The former is called whenever hasOrthogonalFlow(child)
+        is false and the latter when it's true. However crossSizeForPercentageResolution() has a path for
+        hasOrthogonalFlow(child)==true which is impossible to reach. The same happens to
+        mainSizeForPercentageResolution() which has a path for hasOrthogonalFlow(child)==false which is
+        also impossible to reach.
+
+        Remove both death code paths and replace them by assertions. We're also making both methods
+        private since are not meant to be used from the outside.
+
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::RenderFlexibleBox::crossSizeForPercentageResolution): Remove death code path.
+        (WebCore::RenderFlexibleBox::mainSizeForPercentageResolution): Ditto.
+        * rendering/RenderFlexibleBox.h: Make both calls private.
+
 2020-06-30  Sam Weinig  <weinig@apple.com>
 
         Split Color serialization out of Color classes
index acafdfd..2547b75 100644 (file)
@@ -1161,13 +1161,12 @@ LayoutUnit RenderFlexibleBox::adjustChildSizeForMinAndMax(const RenderBox& child
     
 Optional<LayoutUnit> RenderFlexibleBox::crossSizeForPercentageResolution(const RenderBox& child)
 {
+    ASSERT(!hasOrthogonalFlow(child));
     if (alignmentForChild(child) != ItemPosition::Stretch)
         return WTF::nullopt;
 
     // Here we implement https://drafts.csswg.org/css-flexbox/#algo-stretch
-    if (hasOrthogonalFlow(child) && child.hasOverrideContentLogicalWidth())
-        return child.overrideContentLogicalWidth();
-    if (!hasOrthogonalFlow(child) && child.hasOverrideContentLogicalHeight())
+    if (child.hasOverrideContentLogicalHeight())
         return child.overrideContentLogicalHeight();
     
     // We don't currently implement the optimization from
@@ -1181,15 +1180,14 @@ Optional<LayoutUnit> RenderFlexibleBox::crossSizeForPercentageResolution(const R
 
 Optional<LayoutUnit> RenderFlexibleBox::mainSizeForPercentageResolution(const RenderBox& child)
 {
+    ASSERT(hasOrthogonalFlow(child));
     // This function implements section 9.8. Definite and Indefinite Sizes, case 2) of the flexbox spec.
     // If the flex container has a definite main size the flex item post-flexing main size is also treated
     // as definite. We make up a percentage to check whether we have a definite size.
     if (!mainAxisLengthIsDefinite(child, Length(0, Percent)))
         return WTF::nullopt;
 
-    if (hasOrthogonalFlow(child))
-        return child.hasOverrideContentLogicalHeight() ? Optional<LayoutUnit>(child.overrideContentLogicalHeight()) : WTF::nullopt;
-    return child.hasOverrideContentLogicalWidth() ? Optional<LayoutUnit>(child.overrideContentLogicalWidth()) : WTF::nullopt;
+    return child.hasOverrideContentLogicalHeight() ? Optional<LayoutUnit>(child.overrideContentLogicalHeight()) : WTF::nullopt;
 }
 
 Optional<LayoutUnit> RenderFlexibleBox::childLogicalHeightForPercentageResolution(const RenderBox& child)
index 15afbb7..c4b0373 100644 (file)
@@ -69,8 +69,6 @@ public:
 
     virtual bool isFlexibleBoxImpl() const { return false; };
     
-    Optional<LayoutUnit> crossSizeForPercentageResolution(const RenderBox&);
-    Optional<LayoutUnit> mainSizeForPercentageResolution(const RenderBox&);
     Optional<LayoutUnit> childLogicalHeightForPercentageResolution(const RenderBox&);
     
     void clearCachedMainSizeForChild(const RenderBox& child);
@@ -155,7 +153,9 @@ private:
     Overflow mainAxisOverflowForChild(const RenderBox& child) const;
     Overflow crossAxisOverflowForChild(const RenderBox& child) const;
     void cacheChildMainSize(const RenderBox& child);
-    
+    Optional<LayoutUnit> crossSizeForPercentageResolution(const RenderBox&);
+    Optional<LayoutUnit> mainSizeForPercentageResolution(const RenderBox&);
+
     void layoutFlexItems(bool relayoutChildren);
     LayoutUnit autoMarginOffsetInMainAxis(const Vector<FlexItem>&, LayoutUnit& availableFreeSpace);
     void updateAutoMarginsInMainAxis(RenderBox& child, LayoutUnit autoMarginOffset);