Bug 1180107: Factor out logic for determining whether a flex item's main size could influence cross size. r=mats a=lizzard
authorDaniel Holbert <dholbert@cs.stanford.edu>
Mon, 25 Apr 2016 15:58:55 -0700
changeset 332670 26100e0d8a4d95f1b1b0b8aea9b32958e00d329c
parent 332669 990f98f0a8d6e20eeb95bf849a709efb0475363f
child 332671 2f5b903562626c82d700c9cfa7902758588717e2
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmats, lizzard
bugs1180107
milestone48.0a2
Bug 1180107: Factor out logic for determining whether a flex item's main size could influence cross size. r=mats a=lizzard MozReview-Commit-ID: 702zyjj4TPd
layout/generic/nsFlexContainerFrame.cpp
--- a/layout/generic/nsFlexContainerFrame.cpp
+++ b/layout/generic/nsFlexContainerFrame.cpp
@@ -729,16 +729,20 @@ public:
     mMargin.Side(aSide) = aLength;
   }
 
   void ResolveStretchedCrossSize(nscoord aLineCrossSize,
                                  const FlexboxAxisTracker& aAxisTracker);
 
   uint32_t GetNumAutoMarginsInAxis(AxisOrientationType aAxis) const;
 
+  // Once the main size has been resolved, should we bother doing layout to
+  // establish the cross size?
+  bool CanMainSizeInfluenceCrossSize(const FlexboxAxisTracker& aAxisTracker) const;
+
 protected:
   // Helper called by the constructor, to set mNeedsMinSizeAutoResolution:
   void CheckForMinSizeAuto(const nsHTMLReflowState& aFlexItemReflowState,
                            const FlexboxAxisTracker& aAxisTracker);
 
   // Our frame:
   nsIFrame* const mFrame;
 
@@ -1873,16 +1877,48 @@ FlexItem::GetNumAutoMarginsInAxis(AxisOr
   // Mostly for clarity:
   MOZ_ASSERT(numAutoMargins <= 2,
              "We're just looking at one item along one dimension, so we "
              "should only have examined 2 margins");
 
   return numAutoMargins;
 }
 
+bool
+FlexItem::CanMainSizeInfluenceCrossSize(
+  const FlexboxAxisTracker& aAxisTracker) const
+{
+  if (mIsStretched) {
+    // We've already had our cross-size stretched for "align-self:stretch").
+    // The container is imposing its cross size on us.
+    return false;
+  }
+
+  if (mIsStrut) {
+    // Struts (for visibility:collapse items) have a predetermined size;
+    // no need to measure anything.
+    return false;
+  }
+
+  if (aAxisTracker.IsCrossAxisHorizontal()) {
+    // If the cross axis is horizontal, then changes to the item's main size
+    // (height) can't influence its cross size (width), if the item is a block
+    // with a horizontal writing-mode.
+    // XXXdholbert This doesn't account for vertical writing-modes, items with
+    // aspect ratios, items that are multicol elements, & items that are
+    // multi-line vertical flex containers. In all of those cases, a change to
+    // the height could influence the width.
+    return false;
+  }
+
+  // Default assumption, if we haven't proven otherwise: the resolved main size
+  // *can* change the cross size.
+  return true;
+}
+
 // Keeps track of our position along a particular axis (where a '0' position
 // corresponds to the 'start' edge of that axis).
 // This class shouldn't be instantiated directly -- rather, it should only be
 // instantiated via its subclasses defined below.
 class MOZ_STACK_CLASS PositionTracker {
 public:
   // Accessor for the current value of the position that we're tracking.
   inline nscoord GetPosition() const { return mPosition; }
@@ -3896,35 +3932,19 @@ nsFlexContainerFrame::DoFlexLayout(nsPre
   }
 
   // Cross Size Determination - Flexbox spec section 9.4
   // ===================================================
   // 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()) {
-      // Note that we may already have the correct cross size. (We guess at it
-      // in GenerateFlexItemForChild(), and we also may resolve it early for
-      // stretched flex items.)
-      //
-      // We can skip measuring an item's cross size here in a few scenarios:
-      // (A) If the flex item has already been stretched, then we're imposing
-      //     the container's cross size on it; no need to measure.
-      // (B) If the flex item is a "strut", then it's just a placeholder with a
-      //     predetermined cross size; no need to measure.
-      // (C) If the item's main-size can't affect its cross-size, then the
-      //     item's tentative cross size (which we got from the reflow state in
-      //     GenerateFlexItemForChild()) is correct. So, no need to re-measure.
-      //     (For now, this is equivalent to checking if the cross-axis is
-      //     horizontal, because until we enable vertical writing-modes, an
-      //     element's computed width can't be influenced by its computed
-      //     height.)
-      if (!item->IsStretched() && // !A
-          !item->IsStrut() &&     // !B
-          !aAxisTracker.IsCrossAxisHorizontal()) { // !C
+      // 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)) {
         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());