Bug 1030952 part 3: Add a frame property to allow flex container to impose a different main-size on a flex item for aspect ratio calculations. r=mats
authorDaniel Holbert <dholbert@cs.stanford.edu>
Thu, 28 Apr 2016 20:17:02 -0700
changeset 357703 f73c4127855cee67f38e081e0d75df911d30f400
parent 357702 69dd11091fd1bcd64eadd2d0140700a1d90742db
child 357704 3e962c342017ee9fa1aa45faf4769c582344e6cd
push id16816
push userbmo:gasolin@mozilla.com
push dateFri, 29 Apr 2016 03:33:20 +0000
reviewersmats
bugs1030952
milestone49.0a1
Bug 1030952 part 3: Add a frame property to allow flex container to impose a different main-size on a flex item for aspect ratio calculations. r=mats MozReview-Commit-ID: HZylbVhO1Iv
layout/base/nsLayoutUtils.cpp
layout/generic/nsFlexContainerFrame.cpp
layout/generic/nsIFrame.h
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -5334,43 +5334,68 @@ nsLayoutUtils::ComputeSizeWithIntrinsicD
   // If we're a flex item, we'll compute our size a bit differently.
   bool isVertical = aWM.IsVertical();
   const nsStyleCoord* inlineStyleCoord = &stylePos->ISize(aWM);
   const nsStyleCoord* blockStyleCoord = &stylePos->BSize(aWM);
 
   bool isFlexItem = aFrame->IsFlexItem();
   bool isInlineFlexItem = false;
 
+  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) {
-    // 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 =
       aFrame->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::ComputeSize().
-    const nsStyleCoord* flexBasis = &(stylePos->mFlexBasis);
-    if (flexBasis->GetUnit() != eStyleUnit_Auto) {
+    // 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.
+    FrameProperties props = aFrame->Properties();
+    bool didImposeMainSize;
+    nscoord imposedMainSize =
+      props.Get(nsIFrame::FlexItemMainSizeOverride(), &didImposeMainSize);
+    if (didImposeMainSize) {
+      imposedMainSizeStyleCoord.emplace(imposedMainSize,
+                                        nsStyleCoord::CoordConstructor);
       if (isInlineFlexItem) {
-        inlineStyleCoord = flexBasis;
+        inlineStyleCoord = imposedMainSizeStyleCoord.ptr();
       } 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;
+        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;
+          }
         }
       }
     }
   }
 
   // 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
--- a/layout/generic/nsFlexContainerFrame.cpp
+++ b/layout/generic/nsFlexContainerFrame.cpp
@@ -3906,16 +3906,50 @@ public:
     }
   }
 
 private:
   LinkedList<FlexLine>& mLines;
   MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 };
 
+// Class to let us temporarily provide an override value for the the main-size
+// CSS property ('width' or 'height') on a flex item, for use in
+// nsLayoutUtils::ComputeSizeWithIntrinsicDimensions.
+// (We could use this overridden size more broadly, too, but it's probably
+// better to avoid property-table accesses.  So, where possible, we communicate
+// the resolved main-size to the child via modifying its reflow state directly,
+// instead of using this class.)
+class MOZ_RAII AutoFlexItemMainSizeOverride final
+{
+public:
+  explicit AutoFlexItemMainSizeOverride(FlexItem& aItem
+                                        MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
+    : mItemProps(aItem.Frame()->Properties())
+  {
+    MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+
+    MOZ_ASSERT(!mItemProps.Has(nsIFrame::FlexItemMainSizeOverride()),
+               "FlexItemMainSizeOverride prop shouldn't be set already; "
+               "it should only be set temporarily (& not recursively)");
+    NS_ASSERTION(aItem.HasIntrinsicRatio(),
+                 "This should only be needed for items with an aspect ratio");
+
+    mItemProps.Set(nsIFrame::FlexItemMainSizeOverride(), aItem.GetMainSize());
+  }
+
+  ~AutoFlexItemMainSizeOverride() {
+    mItemProps.Remove(nsIFrame::FlexItemMainSizeOverride());
+  }
+
+private:
+  const FrameProperties mItemProps;
+  MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
+};
+
 void
 nsFlexContainerFrame::DoFlexLayout(nsPresContext*           aPresContext,
                                    nsHTMLReflowMetrics&     aDesiredSize,
                                    const nsHTMLReflowState& aReflowState,
                                    nsReflowStatus&          aStatus,
                                    nscoord aContentBoxMainSize,
                                    nscoord aAvailableBSizeForContent,
                                    nsTArray<StrutInfo>& aStruts,
@@ -3944,28 +3978,42 @@ nsFlexContainerFrame::DoFlexLayout(nsPre
   // ===================================================
   // Calculate the hypothetical cross size of each item:
   nscoord sumLineCrossSizes = 0;
   for (FlexLine* line = lines.getFirst(); line; line = line->getNext()) {
     for (FlexItem* item = line->GetFirstItem(); item; item = item->getNext()) {
       // The item may already have the correct cross-size; only recalculate
       // if the item's main size resolution (flexing) could have influenced it:
       if (item->CanMainSizeInfluenceCrossSize(aAxisTracker)) {
+        Maybe<AutoFlexItemMainSizeOverride> sizeOverride;
+        if (item->HasIntrinsicRatio()) {
+          // For flex items with an aspect ratio, we have to impose an override
+          // for the main-size property *before* we even instantiate the reflow
+          // state, in order for aspect ratio calculations to produce the right
+          // cross size in the reflow state. (For other flex items, it's OK
+          // (and cheaper) to impose our main size *after* the reflow state has
+          // been constructed, since the main size shouldn't influence anything
+          // about cross-size measurement until we actually reflow the child.)
+          sizeOverride.emplace(*item);
+        }
+
         WritingMode wm = item->Frame()->GetWritingMode();
         LogicalSize availSize = aReflowState.ComputedSize(wm);
         availSize.BSize(wm) = NS_UNCONSTRAINEDSIZE;
         nsHTMLReflowState childReflowState(aPresContext, aReflowState,
                                            item->Frame(), availSize);
-        // Override computed main-size
-        if (aAxisTracker.IsMainAxisHorizontal()) {
-          childReflowState.SetComputedWidth(item->GetMainSize());
-        } else {
-          childReflowState.SetComputedHeight(item->GetMainSize());
+        if (!sizeOverride) {
+          // Directly override the computed main-size, by tweaking reflow state:
+          if (aAxisTracker.IsMainAxisHorizontal()) {
+            childReflowState.SetComputedWidth(item->GetMainSize());
+          } else {
+            childReflowState.SetComputedHeight(item->GetMainSize());
+          }
         }
-        
+
         SizeItemInCrossAxis(aPresContext, aAxisTracker,
                             childReflowState, *item);
       }
     }
     // Now that we've finished with this line's items, size the line itself:
     line->ComputeCrossSizeAndBaseline(aAxisTracker);
     sumLineCrossSizes += line->GetLineCrossSize();
   }
--- a/layout/generic/nsIFrame.h
+++ b/layout/generic/nsIFrame.h
@@ -927,16 +927,20 @@ public:
 #endif
 
   NS_DECLARE_FRAME_PROPERTY_DELETABLE(UsedMarginProperty, nsMargin)
   NS_DECLARE_FRAME_PROPERTY_DELETABLE(UsedPaddingProperty, nsMargin)
   NS_DECLARE_FRAME_PROPERTY_DELETABLE(UsedBorderProperty, nsMargin)
 
   NS_DECLARE_FRAME_PROPERTY_SMALL_VALUE(LineBaselineOffset, nscoord)
 
+  // Temporary override for a flex item's main-size property (either width
+  // or height), imposed by its flex container.
+  NS_DECLARE_FRAME_PROPERTY_SMALL_VALUE(FlexItemMainSizeOverride, nscoord)
+
   NS_DECLARE_FRAME_PROPERTY_RELEASABLE(CachedBackgroundImage, gfxASurface)
   NS_DECLARE_FRAME_PROPERTY_RELEASABLE(CachedBackgroundImageDT, DrawTarget)
 
   NS_DECLARE_FRAME_PROPERTY_DELETABLE(InvalidationRect, nsRect)
 
   NS_DECLARE_FRAME_PROPERTY_SMALL_VALUE(RefusedAsyncAnimationProperty, bool)
 
   NS_DECLARE_FRAME_PROPERTY_WITH_DTOR(GenConProperty, ContentArray,