Bug 1449838 part 2: Morph 'usingFlexBasisForISize' variable in nsFrame::ComputeSize* methods, to better reflect reality. r=mats
authorDaniel Holbert <dholbert@cs.stanford.edu>
Thu, 29 Mar 2018 14:49:40 -0700
changeset 410785 99e609ea0c47dbbc36713f4c9d3a471efb2bc906
parent 410784 70c80f12ce8e9bacfcf2c14c555014de6f8ec17b
child 410786 bf285f58e643f3f283fcfe0c53c419dae49709f0
push id33739
push usernbeleuzu@mozilla.com
push dateFri, 30 Mar 2018 21:47:45 +0000
treeherdermozilla-central@10c662d8416e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmats
bugs1449838
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1449838 part 2: Morph 'usingFlexBasisForISize' variable in nsFrame::ComputeSize* methods, to better reflect reality. r=mats This patch doesn't affect behavior; it just adjusts a variable for clarity. Really, this variable tracks which axis (inline vs. block) is the main axis (if we're a flex item). So, this patch morphs the variable to more directly track that. The variable's old name 'usingFlexBasisForISize' was confusing, because even when it was set to 'true', we might not *really* be using the flex-basis in place of the ISize property. In particular: when we have 'flex-basis:auto', we don't use 'flex-basis' in place of the ISize/BSize property -- rather, that indicates that 'flex-basis' is *deferring* to the main-axis size property. Hopefully the new name/type makes it clearer what we're actually tracking. MozReview-Commit-ID: ITkb4zuEwgQ
layout/generic/nsFrame.cpp
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -5652,35 +5652,35 @@ nsFrame::ComputeSize(gfxContext*        
       // When resolving justify/align-self below, we want to use the grid
       // container's justify/align-items value and WritingMode.
       alignCB = grandParent;
     }
   }
   bool isFlexItem = parentFrame && parentFrame->IsFlexContainerFrame() &&
     !parentFrame->HasAnyStateBits(NS_STATE_FLEX_IS_EMULATING_LEGACY_BOX) &&
     !HasAnyStateBits(NS_FRAME_OUT_OF_FLOW);
-  // This bool only gets set (and used) if isFlexItem is true.
-  // It indicates (for flex items) whether we're using their flex-basis for the
-  // item's own ISize property vs. for its BSize property.
-  bool usingFlexBasisForISize = false;
+  // This variable only gets set (and used) if isFlexItem is true.  It
+  // indicates which axis (in this frame's own WM) corresponds to its
+  // flex container's main axis.
+  LogicalAxis flexMainAxis = eLogicalAxisInline; // (init to make valgrind happy)
   if (isFlexItem) {
     // Flex items use their "flex-basis" property in place of their main-size
     // property for sizing purposes, *unless* they have "flex-basis:auto", in
     // which case they use their main-size property after all.
-    usingFlexBasisForISize =
-      nsFlexContainerFrame::IsItemInlineAxisMainAxis(this);
+    flexMainAxis = nsFlexContainerFrame::IsItemInlineAxisMainAxis(this) ?
+      eLogicalAxisInline : eLogicalAxisBlock;
 
     // 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) {
       // Override whichever styleCoord is in flex container's main axis:
-      (usingFlexBasisForISize ? inlineStyleCoord : blockStyleCoord) =
-        flexBasis;
+      (flexMainAxis == eLogicalAxisInline ?
+       inlineStyleCoord : blockStyleCoord) = flexBasis;
     }
   }
 
   // Compute inline-axis size
   if (inlineStyleCoord->GetUnit() != eStyleUnit_Auto) {
     result.ISize(aWM) =
       ComputeISizeValue(aRenderingContext, aCBSize.ISize(aWM),
                         boxSizingAdjust.ISize(aWM), boxSizingToMarginEdgeISize,
@@ -5711,28 +5711,28 @@ nsFrame::ComputeSize(gfxContext*        
   }
 
   // Flex items ignore their min & max sizing properties in their
   // flex container's main-axis.  (Those properties get applied later in
   // the flexbox algorithm.)
   const nsStyleCoord& maxISizeCoord = stylePos->MaxISize(aWM);
   nscoord maxISize = NS_UNCONSTRAINEDSIZE;
   if (maxISizeCoord.GetUnit() != eStyleUnit_None &&
-      !(isFlexItem && usingFlexBasisForISize)) {
+      !(isFlexItem && flexMainAxis == eLogicalAxisInline)) {
     maxISize =
       ComputeISizeValue(aRenderingContext, aCBSize.ISize(aWM),
                         boxSizingAdjust.ISize(aWM), boxSizingToMarginEdgeISize,
                         maxISizeCoord, aFlags);
     result.ISize(aWM) = std::min(maxISize, result.ISize(aWM));
   }
 
   const nsStyleCoord& minISizeCoord = stylePos->MinISize(aWM);
   nscoord minISize;
   if (minISizeCoord.GetUnit() != eStyleUnit_Auto &&
-      !(isFlexItem && usingFlexBasisForISize)) {
+      !(isFlexItem && flexMainAxis == eLogicalAxisInline)) {
     minISize =
       ComputeISizeValue(aRenderingContext, aCBSize.ISize(aWM),
                         boxSizingAdjust.ISize(aWM), boxSizingToMarginEdgeISize,
                         minISizeCoord, aFlags);
   } else if (MOZ_UNLIKELY(aFlags & eIApplyAutoMinSize)) {
     // This implements "Implied Minimum Size of Grid Items".
     // https://drafts.csswg.org/css-grid/#min-size-auto
     minISize = std::min(maxISize, GetMinISize(aRenderingContext));
@@ -5799,28 +5799,28 @@ nsFrame::ComputeSize(gfxContext*        
       }
     }
   }
 
   const nsStyleCoord& maxBSizeCoord = stylePos->MaxBSize(aWM);
 
   if (result.BSize(aWM) != NS_UNCONSTRAINEDSIZE) {
     if (!nsLayoutUtils::IsAutoBSize(maxBSizeCoord, aCBSize.BSize(aWM)) &&
-        !(isFlexItem && !usingFlexBasisForISize)) {
+        !(isFlexItem && flexMainAxis == eLogicalAxisBlock)) {
       nscoord maxBSize =
         nsLayoutUtils::ComputeBSizeValue(aCBSize.BSize(aWM),
                                          boxSizingAdjust.BSize(aWM),
                                          maxBSizeCoord);
       result.BSize(aWM) = std::min(maxBSize, result.BSize(aWM));
     }
 
     const nsStyleCoord& minBSizeCoord = stylePos->MinBSize(aWM);
 
     if (!nsLayoutUtils::IsAutoBSize(minBSizeCoord, aCBSize.BSize(aWM)) &&
-        !(isFlexItem && !usingFlexBasisForISize)) {
+        !(isFlexItem && flexMainAxis == eLogicalAxisBlock)) {
       nscoord minBSize =
         nsLayoutUtils::ComputeBSizeValue(aCBSize.BSize(aWM),
                                          boxSizingAdjust.BSize(aWM),
                                          minBSizeCoord);
       result.BSize(aWM) = std::max(minBSize, result.BSize(aWM));
     }
   }
 
@@ -5871,58 +5871,58 @@ nsFrame::ComputeSizeWithIntrinsicDimensi
   const nsStyleCoord* inlineStyleCoord = &stylePos->ISize(aWM);
   const nsStyleCoord* blockStyleCoord = &stylePos->BSize(aWM);
   auto* parentFrame = GetParent();
   const bool isGridItem = parentFrame && parentFrame->IsGridContainerFrame() &&
     !HasAnyStateBits(NS_FRAME_OUT_OF_FLOW);
   const bool isFlexItem = parentFrame && parentFrame->IsFlexContainerFrame() &&
     !parentFrame->HasAnyStateBits(NS_STATE_FLEX_IS_EMULATING_LEGACY_BOX) &&
     !HasAnyStateBits(NS_FRAME_OUT_OF_FLOW);
-  // This bool only gets set (and used) if isFlexItem is true.
-  // It indicates (for flex items) whether we're using their flex-basis for the
-  // item's own ISize property vs. for its BSize property.
-  bool usingFlexBasisForISize = false;
+  // This variable only gets set (and used) if isFlexItem is true.  It
+  // indicates which axis (in this frame's own WM) corresponds to its
+  // flex container's main axis.
+  LogicalAxis flexMainAxis = eLogicalAxisInline; // (init to make valgrind happy)
   Maybe<nsStyleCoord> imposedMainSizeStyleCoord;
 
   // If this is a flex item, and we're measuring its cross size after flexing
   // to resolve its main size, then we need to use the resolved main size
   // that the container provides to us *instead of* the main-size coordinate
   // from our style struct. (Otherwise, we'll be using an irrelevant value in
   // the aspect-ratio calculations below.)
   if (isFlexItem) {
-    usingFlexBasisForISize =
-      nsFlexContainerFrame::IsItemInlineAxisMainAxis(this);
+    flexMainAxis = nsFlexContainerFrame::IsItemInlineAxisMainAxis(this) ?
+      eLogicalAxisInline : eLogicalAxisBlock;
 
     // If FlexItemMainSizeOverride frame-property is set, then that means the
     // flex container is imposing a main-size on this flex item for it to use
     // as its size in the container's main axis.
     bool didImposeMainSize;
     nscoord imposedMainSize =
       GetProperty(nsIFrame::FlexItemMainSizeOverride(), &didImposeMainSize);
     if (didImposeMainSize) {
       imposedMainSizeStyleCoord.emplace(imposedMainSize,
                                         nsStyleCoord::CoordConstructor);
-      if (usingFlexBasisForISize) {
+      if (flexMainAxis == eLogicalAxisInline) {
         inlineStyleCoord = imposedMainSizeStyleCoord.ptr();
       } 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) {
         // Override whichever styleCoord is in flex container's main axis:
-        (usingFlexBasisForISize ? inlineStyleCoord : blockStyleCoord) =
-          flexBasis;
+        (flexMainAxis == eLogicalAxisInline ?
+         inlineStyleCoord : blockStyleCoord) = flexBasis;
       }
     }
   }
 
   // Handle intrinsic sizes and their interaction with
   // {min-,max-,}{width,height} according to the rules in
   // http://www.w3.org/TR/CSS21/visudet.html#min-max-widths
 
@@ -6024,32 +6024,32 @@ nsFrame::ComputeSizeWithIntrinsicDimensi
       aFlags = ComputeSizeFlags(aFlags &
                                 ~ComputeSizeFlags::eIClampMarginBoxMinSize);
     }
   }
 
   const nsStyleCoord& maxISizeCoord = stylePos->MaxISize(aWM);
 
   if (maxISizeCoord.GetUnit() != eStyleUnit_None &&
-      !(isFlexItem && usingFlexBasisForISize)) {
+      !(isFlexItem && flexMainAxis == eLogicalAxisInline)) {
     maxISize = ComputeISizeValue(aRenderingContext,
                  aCBSize.ISize(aWM), boxSizingAdjust.ISize(aWM),
                  boxSizingToMarginEdgeISize, maxISizeCoord, aFlags);
   } else {
     maxISize = nscoord_MAX;
   }
 
   // NOTE: Flex items ignore their min & max sizing properties in their
   // flex container's main-axis.  (Those properties get applied later in
   // the flexbox algorithm.)
 
   const nsStyleCoord& minISizeCoord = stylePos->MinISize(aWM);
 
   if (minISizeCoord.GetUnit() != eStyleUnit_Auto &&
-      !(isFlexItem && usingFlexBasisForISize)) {
+      !(isFlexItem && flexMainAxis == eLogicalAxisInline)) {
     minISize = ComputeISizeValue(aRenderingContext,
                  aCBSize.ISize(aWM), boxSizingAdjust.ISize(aWM),
                  boxSizingToMarginEdgeISize, minISizeCoord, aFlags);
   } else {
     // Treat "min-width: auto" as 0.
     // NOTE: Technically, "auto" is supposed to behave like "min-content" on
     // flex items. However, we don't need to worry about that here, because
     // flex items' min-sizes are intentionally ignored until the flex
@@ -6093,27 +6093,27 @@ nsFrame::ComputeSizeWithIntrinsicDimensi
       aFlags = ComputeSizeFlags(aFlags &
                                 ~ComputeSizeFlags::eBClampMarginBoxMinSize);
     }
   }
 
   const nsStyleCoord& maxBSizeCoord = stylePos->MaxBSize(aWM);
 
   if (!nsLayoutUtils::IsAutoBSize(maxBSizeCoord, aCBSize.BSize(aWM)) &&
-      !(isFlexItem && !usingFlexBasisForISize)) {
+      !(isFlexItem && flexMainAxis == eLogicalAxisBlock)) {
     maxBSize = nsLayoutUtils::ComputeBSizeValue(aCBSize.BSize(aWM),
                   boxSizingAdjust.BSize(aWM), maxBSizeCoord);
   } else {
     maxBSize = nscoord_MAX;
   }
 
   const nsStyleCoord& minBSizeCoord = stylePos->MinBSize(aWM);
 
   if (!nsLayoutUtils::IsAutoBSize(minBSizeCoord, aCBSize.BSize(aWM)) &&
-      !(isFlexItem && !usingFlexBasisForISize)) {
+      !(isFlexItem && flexMainAxis == eLogicalAxisBlock)) {
     minBSize = nsLayoutUtils::ComputeBSizeValue(aCBSize.BSize(aWM),
                   boxSizingAdjust.BSize(aWM), minBSizeCoord);
   } else {
     minBSize = 0;
   }
 
   NS_ASSERTION(aCBSize.ISize(aWM) != NS_UNCONSTRAINEDSIZE,
                "Our containing block must not have unconstrained inline-size!");