Issue #1370 - Part 1: Refactor nsFrame to use a separate function for handling `flex-basis`

This removes the need to keep the `flex-basis` handling code of the ComputeSize* functions in sync since they both call the same function now for this purpose.

2 files changed, 43 insertions, 47 deletions

diff --git a/layout/generic/nsFrame.cpp b/layout/generic/nsFrame.cpp
index 14d411e6ab..7a1cd428b1 100644
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -4635,6 +4635,34 @@ nsFrame::GetIntrinsicRatio()
   return AspectRatio();
 }
 
+void
+nsFrame::SetCoordToFlexBasis(bool aIsInlineFlexItem,
+                             const nsStyleCoord* aFlexBasis,
+                             const nsStyleCoord** aInlineStyle,
+                             const nsStyleCoord** aBlockStyle)
+{
+  // Don't bother changing the pointer of the coordinates if the
+  // value of the 'flex-basis' property is set to 'auto'.
+  if (aFlexBasis->GetUnit() == eStyleUnit_Auto) {
+    return;
+  }
+
+  if (aIsInlineFlexItem) {
+    *aInlineStyle = aFlexBasis;
+  } else {
+    // One caveat for vertical flex items: We don't support enumerated
+    // values (e.g. "max-content") for height properties yet. So, if our
+    // computed flex-basis is an enumerated value, we'll just behave as if
+    // it were "auto", which means "use the main-size property after all"
+    // (which is "height", in this case).
+    // NOTE: Once we support intrinsic sizing keywords for "height",
+    // we should remove this check.
+    if (aFlexBasis->GetUnit() != eStyleUnit_Enumerated) {
+      *aBlockStyle = aFlexBasis;
+    }
+  }
+}
+
 /* virtual */
 LogicalSize
 nsFrame::ComputeSize(nsRenderingContext* aRenderingContext,
@@ -4689,35 +4717,14 @@ nsFrame::ComputeSize(nsRenderingContext* aRenderingContext,
                      !(GetStateBits() & NS_FRAME_OUT_OF_FLOW));
   bool isInlineFlexItem = false;
   if (isFlexItem) {
-    // Flex items use their "flex-basis" property in place of their main-size
-    // property (e.g. "width") for sizing purposes, *unless* they have
-    // "flex-basis:auto", in which case they use their main-size property after
-    // all.
     uint32_t flexDirection = GetParent()->StylePosition()->mFlexDirection;
     isInlineFlexItem =
       flexDirection == NS_STYLE_FLEX_DIRECTION_ROW ||
       flexDirection == NS_STYLE_FLEX_DIRECTION_ROW_REVERSE;
 
-    // NOTE: The logic here should match the similar chunk for determining
-    // inlineStyleCoord and blockStyleCoord in
-    // nsFrame::ComputeSizeWithIntrinsicDimensions().
     const nsStyleCoord* flexBasis = &(stylePos->mFlexBasis);
-    if (flexBasis->GetUnit() != eStyleUnit_Auto) {
-      if (isInlineFlexItem) {
-        inlineStyleCoord = flexBasis;
-      } else {
-        // One caveat for vertical flex items: We don't support enumerated
-        // values (e.g. "max-content") for height properties yet. So, if our
-        // computed flex-basis is an enumerated value, we'll just behave as if
-        // it were "auto", which means "use the main-size property after all"
-        // (which is "height", in this case).
-        // NOTE: Once we support intrinsic sizing keywords for "height",
-        // we should remove this check.
-        if (flexBasis->GetUnit() != eStyleUnit_Enumerated) {
-          blockStyleCoord = flexBasis;
-        }
-      }
-    }
+    SetCoordToFlexBasis(isInlineFlexItem, flexBasis,
+                        &inlineStyleCoord, &blockStyleCoord);
   }
 
   // Compute inline-axis size
@@ -4949,31 +4956,10 @@ nsFrame::ComputeSizeWithIntrinsicDimensions(nsRenderingContext*  aRenderingConte
       } else {
         blockStyleCoord = imposedMainSizeStyleCoord.ptr();
       }
-
     } else {
-      // Flex items use their "flex-basis" property in place of their main-size
-      // property (e.g. "width") for sizing purposes, *unless* they have
-      // "flex-basis:auto", in which case they use their main-size property
-      // after all.
-      // NOTE: The logic here should match the similar chunk for determining
-      // inlineStyleCoord and blockStyleCoord in nsFrame::ComputeSize().
       const nsStyleCoord* flexBasis = &(stylePos->mFlexBasis);
-      if (flexBasis->GetUnit() != eStyleUnit_Auto) {
-        if (isInlineFlexItem) {
-          inlineStyleCoord = flexBasis;
-        } else {
-          // One caveat for vertical flex items: We don't support enumerated
-          // values (e.g. "max-content") for height properties yet. So, if our
-          // computed flex-basis is an enumerated value, we'll just behave as if
-          // it were "auto", which means "use the main-size property after all"
-          // (which is "height", in this case).
-          // NOTE: Once we support intrinsic sizing keywords for "height",
-          // we should remove this check.
-          if (flexBasis->GetUnit() != eStyleUnit_Enumerated) {
-            blockStyleCoord = flexBasis;
-          }
-        }
-      }
+      SetCoordToFlexBasis(isInlineFlexItem, flexBasis,
+                          &inlineStyleCoord, &blockStyleCoord);
     }
   }
 
diff --git a/layout/generic/nsFrame.h b/layout/generic/nsFrame.h
index d75555fec2..3110335dd5 100644
--- a/layout/generic/nsFrame.h
+++ b/layout/generic/nsFrame.h
@@ -266,6 +266,16 @@ public:
   virtual mozilla::IntrinsicSize GetIntrinsicSize() override;
   virtual mozilla::AspectRatio GetIntrinsicRatio() override;
 
+  /**
+   * Helper function for determining if flex items should use their
+   * 'flex-basis' property instead of their main-size property, such
+   * as 'width', for sizing purposes.
+   */
+  void SetCoordToFlexBasis(bool aIsInlineFlexItem,
+                           const nsStyleCoord* aFlexBasis,
+                           const nsStyleCoord** aInlineStyle,
+                           const nsStyleCoord** aBlockStyle);
+
   virtual mozilla::LogicalSize
   ComputeSize(nsRenderingContext*         aRenderingContext,
               mozilla::WritingMode        aWM,
@@ -295,7 +305,7 @@ public:
   // Compute tight bounds assuming this frame honours its border, background
   // and outline, its children's tight bounds, and nothing else.
   nsRect ComputeSimpleTightBounds(mozilla::gfx::DrawTarget* aDrawTarget) const;
-  
+
   /**
    * A helper, used by |nsFrame::ComputeSize| (for frames that need to
    * override only this part of ComputeSize), that computes the size