Bug 1374540 part 3: Refactor nsFrame::ComputeSize methods to handle the two "used flex-basis of content" scenarios with a consistent codepath. r?mats
This patch doesn't change behavior.
It simply makes us share code/data for two different cases that both ended up
producing mainAxisCoord->GetUnit() == eStyleUnit_Auto. Now, they'll *both* use
the same static nsStyleCoord to represent this "auto" value.
Originally, in one of these cases ("flex-basis:auto;[main-size-property]:auto),
we left the mainAxisCoord untouched. Now we'll point it at this dummy 'auto'
value. Either way we end up with mainAxisCoord->GetUnit() == eStyleUnit_Auto,
so the behavior doesn't change.
The next patch in this series will make further changes to one of these spots,
as noted in the "XXXdholbert" code-comment included here.
MozReview-Commit-ID: 5ClfbNHuKhO
--- a/layout/generic/nsFlexContainerFrame.cpp
+++ b/layout/generic/nsFlexContainerFrame.cpp
@@ -4424,16 +4424,32 @@ nsFlexContainerFrame::IsItemInlineAxisMa
flexDirection == NS_STYLE_FLEX_DIRECTION_ROW ||
flexDirection == NS_STYLE_FLEX_DIRECTION_ROW_REVERSE;
// aFrame's inline axis is its flex container's main axis IFF the above
// questions have the same answer.
return flexContainerIsRowOriented == itemInlineAxisIsParallelToParent;
}
+/* static */
+bool
+nsFlexContainerFrame::IsUsedFlexBasisContent(const nsStyleCoord* aFlexBasis,
+ const nsStyleCoord* aMainSize)
+{
+ // We have a used flex-basis of 'content' if flex-basis explicitly has that
+ // value, OR if flex-basis is 'auto' (deferring to the main-size property)
+ // and the main-size property is also 'auto'.
+ // See https://drafts.csswg.org/css-flexbox-1/#valdef-flex-basis-auto
+ return
+ (aFlexBasis->GetUnit() == eStyleUnit_Enumerated &&
+ aFlexBasis->GetIntValue() == NS_STYLE_FLEX_BASIS_CONTENT) ||
+ (aFlexBasis->GetUnit() == eStyleUnit_Auto &&
+ aMainSize->GetUnit() == eStyleUnit_Auto);
+}
+
void
nsFlexContainerFrame::DoFlexLayout(nsPresContext* aPresContext,
ReflowOutput& aDesiredSize,
const ReflowInput& aReflowInput,
nsReflowStatus& aStatus,
nscoord aContentBoxMainSize,
nscoord aAvailableBSizeForContent,
nsTArray<StrutInfo>& aStruts,
--- a/layout/generic/nsFlexContainerFrame.h
+++ b/layout/generic/nsFlexContainerFrame.h
@@ -7,16 +7,18 @@
/* rendering object for CSS "display: flex" and "display: -webkit-box" */
#ifndef nsFlexContainerFrame_h___
#define nsFlexContainerFrame_h___
#include "nsContainerFrame.h"
#include "mozilla/UniquePtr.h"
+class nsStyleCoord;
+
namespace mozilla {
template <class T> class LinkedList;
class LogicalPoint;
} // namespace mozilla
nsContainerFrame* NS_NewFlexContainerFrame(nsIPresShell* aPresShell,
mozilla::ComputedStyle* aStyle);
@@ -207,16 +209,27 @@ public:
* equivalent & more optimal.)
*
* @param aFrame a flex item (must return true from IsFlexItem)
* @return true iff aFrame's inline axis is the same as (i.e. not orthogonal
* to) its flex container's main axis. Otherwise, false.
*/
static bool IsItemInlineAxisMainAxis(nsIFrame* aFrame);
+ /**
+ * Returns true iff the given computed 'flex-basis' & main-size property
+ * values collectively represent a used flex-basis of 'content'.
+ * See https://drafts.csswg.org/css-flexbox-1/#valdef-flex-basis-auto
+ *
+ * @param aFlexBasis the computed 'flex-basis' for a flex item.
+ * @param aMainSize the computed main-size property for a flex item.
+ */
+ static bool IsUsedFlexBasisContent(const nsStyleCoord* aFlexBasis,
+ const nsStyleCoord* aMainSize);
+
protected:
// Protected constructor & destructor
explicit nsFlexContainerFrame(ComputedStyle* aStyle)
: nsContainerFrame(aStyle, kClassID)
, mBaselineFromLastReflow(NS_INTRINSIC_WIDTH_UNKNOWN)
, mLastBaselineFromLastReflow(NS_INTRINSIC_WIDTH_UNKNOWN)
{}
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -5638,38 +5638,34 @@ nsFrame::ComputeSize(gfxContext*
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.
flexMainAxis = nsFlexContainerFrame::IsItemInlineAxisMainAxis(this) ?
eLogicalAxisInline : eLogicalAxisBlock;
- // NOTE: The logic here should match the similar chunk for determining
- // inlineStyleCoord and blockStyleCoord in
- // nsFrame::ComputeSizeWithIntrinsicDimensions().
+ // NOTE: The logic here should match the similar chunk for updating
+ // mainAxisCoord in nsFrame::ComputeSizeWithIntrinsicDimensions().
const nsStyleCoord* flexBasis = &(stylePos->mFlexBasis);
- if (flexBasis->GetUnit() != eStyleUnit_Auto) {
- // Replace our main-axis styleCoord pointer with a different one,
- // depending on our flex-basis value.
- auto& mainAxisCoord = (flexMainAxis == eLogicalAxisInline
- ? inlineStyleCoord : blockStyleCoord);
- if (flexBasis->GetUnit() == eStyleUnit_Enumerated &&
- flexBasis->GetIntValue() == NS_STYLE_FLEX_BASIS_CONTENT) {
- // We have 'flex-basis: content', which is equivalent to
- // 'flex-basis:auto; {main-size}: auto'. So, just swap in a dummy
- // 'auto' value to use for the main size property:
- static const nsStyleCoord autoStyleCoord(eStyleUnit_Auto);
- mainAxisCoord = &autoStyleCoord;
- } else {
- // For all other flex-basis values, we just swap in the flex-basis
- // itself for the main-size property here:
- mainAxisCoord = flexBasis;
- }
- }
+ auto& mainAxisCoord = (flexMainAxis == eLogicalAxisInline
+ ? inlineStyleCoord : blockStyleCoord);
+
+ if (nsFlexContainerFrame::IsUsedFlexBasisContent(flexBasis,
+ mainAxisCoord)) {
+ // XXXdholbert Technically we shouldn't be using 'auto' here. A later
+ // patch will convert this to max-content instead.
+ static const nsStyleCoord autoStyleCoord(eStyleUnit_Auto);
+ mainAxisCoord = &autoStyleCoord;
+ } else if (flexBasis->GetUnit() != eStyleUnit_Auto) {
+ // For all other non-'auto' flex-basis values, we just swap in the
+ // flex-basis itself for the main-size property.
+ mainAxisCoord = flexBasis;
+ } // else: flex-basis is 'auto', which is deferring to some explicit value
+ // in mainAxisCoord. So we proceed w/o touching mainAxisCoord.
}
// Compute inline-axis size
if (inlineStyleCoord->GetUnit() != eStyleUnit_Auto) {
result.ISize(aWM) =
ComputeISizeValue(aRenderingContext, aCBSize.ISize(aWM),
boxSizingAdjust.ISize(aWM), boxSizingToMarginEdgeISize,
*inlineStyleCoord, aFlags);
@@ -5894,38 +5890,44 @@ nsFrame::ComputeSizeWithIntrinsicDimensi
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().
+ // NOTE: The logic here should match the similar chunk for updating
+ // mainAxisCoord in nsFrame::ComputeSize().
const nsStyleCoord* flexBasis = &(stylePos->mFlexBasis);
- if (flexBasis->GetUnit() != eStyleUnit_Auto) {
- // Replace our main-axis styleCoord pointer with a different one,
- // depending on our flex-basis value.
- auto& mainAxisCoord = (flexMainAxis == eLogicalAxisInline
- ? inlineStyleCoord : blockStyleCoord);
-
- if (flexBasis->GetUnit() == eStyleUnit_Enumerated &&
- flexBasis->GetIntValue() == NS_STYLE_FLEX_BASIS_CONTENT) {
- // We have 'flex-basis: content', which is equivalent to
- // 'flex-basis:auto; {main-size}: auto'. So, just swap in a dummy
- // 'auto' value to use for the main size property:
- static const nsStyleCoord autoStyleCoord(eStyleUnit_Auto);
- mainAxisCoord = &autoStyleCoord;
- } else {
- // For all other flex-basis values, we just swap in the flex-basis
- // itself for the main-size property here:
- mainAxisCoord = flexBasis;
- }
- }
+ auto& mainAxisCoord = (flexMainAxis == eLogicalAxisInline
+ ? inlineStyleCoord : blockStyleCoord);
+
+ if (nsFlexContainerFrame::IsUsedFlexBasisContent(flexBasis,
+ mainAxisCoord)) {
+ // If we get here, we're resolving the flex base size for a flex item,
+ // and we fall into the flexbox spec section 9.2 step 3, substep C (if
+ // we have a definite cross size) or E (if not). And specifically:
+ //
+ // * If we have a definite cross size, we're supposed to resolve our
+ // main-size based on that and our intrinsic ratio.
+ // * Otherwise, we're supposed to produce our max-content size.
+ //
+ // Conveniently, we can handle both of those scenarios (regardless of
+ // which substep we fall into) by using the 'auto' keyword for our
+ // main-axis coordinate here. (This makes sense, because the spec is
+ // effectively trying to produce the 'auto' sizing behavior).
+ static const nsStyleCoord autoStyleCoord(eStyleUnit_Auto);
+ mainAxisCoord = &autoStyleCoord;
+ } else if (flexBasis->GetUnit() != eStyleUnit_Auto) {
+ // For all other non-'auto' flex-basis values, we just swap in the
+ // flex-basis itself for the main-size property.
+ mainAxisCoord = flexBasis;
+ } // else: flex-basis is 'auto', which is deferring to some explicit
+ // value in mainAxisCoord. So we proceed w/o touching mainAxisCoord.
}
}
// 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
// Note: throughout the following section of the function, I avoid