Bug 1336708: Don't reuse cached flex-item reflow measurements if the item's computed height has changed

2 files changed, 39 insertions, 23 deletions

diff --git a/layout/generic/nsFlexContainerFrame.cpp b/layout/generic/nsFlexContainerFrame.cpp
index 4ecc7d877..5bc852c52 100644
--- a/layout/generic/nsFlexContainerFrame.cpp
+++ b/layout/generic/nsFlexContainerFrame.cpp
@@ -1761,31 +1761,50 @@ nsFlexContainerFrame::
 /**
  * A cached result for a measuring reflow.
  *
- * Right now we only need to cache the available size, the height and the
- * ascent. This can be extended later if needed.
+ * Right now we only need to cache the available size and the computed height
+ * for checking that the reflow input is valid, and the height and the ascent
+ * to be used. This can be extended later if needed.
  *
  * The assumption here is that a given flex item measurement won't change until
- * either the available size changes, or the flex container intrinsic size is
- * marked as dirty (due to a style or DOM change).
+ * either the available size or computed height changes, or the flex container
+ * intrinsic size is marked as dirty (due to a style or DOM change).
+ *
+ * In particular the computed height may change between measuring reflows due to
+ * how the mIsFlexContainerMeasuringReflow flag affects size computation (see
+ * bug 1336708).
  *
  * Caching it prevents us from doing exponential reflows in cases of deeply
  * nested flex and scroll frames.
  *
  * We store them in the frame property table for simplicity.
  */
-struct nsFlexContainerFrame::CachedMeasuringReflowResult
+class nsFlexContainerFrame::CachedMeasuringReflowResult
 {
-  LogicalSize mAvailableSize;
+  // Members that are part of the cache key:
+  const LogicalSize mAvailableSize;
+  const nscoord mComputedHeight;
+
+  // Members that are part of the cache value:
   const nscoord mHeight;
   const nscoord mAscent;
 
-  CachedMeasuringReflowResult(const LogicalSize& aAvailableSize,
-                              nscoord aHeight,
-                              nscoord aAscent)
-    : mAvailableSize(aAvailableSize)
-    , mHeight(aHeight)
-    , mAscent(aAscent)
+public:
+  CachedMeasuringReflowResult(const ReflowInput& aReflowInput,
+                              const ReflowOutput& aDesiredSize)
+    : mAvailableSize(aReflowInput.AvailableSize())
+    , mComputedHeight(aReflowInput.ComputedHeight())
+    , mHeight(aDesiredSize.Height())
+    , mAscent(aDesiredSize.BlockStartAscent())
   {}
+
+  bool IsValidFor(const ReflowInput& aReflowInput) const {
+    return mAvailableSize == aReflowInput.AvailableSize() &&
+      mComputedHeight == aReflowInput.ComputedHeight();
+  }
+
+  nscoord Height() const { return mHeight; }
+
+  nscoord Ascent() const { return mAscent; }
 };
 
 NS_DECLARE_FRAME_PROPERTY_DELETABLE(CachedFlexMeasuringReflow,
@@ -1797,10 +1816,9 @@ nsFlexContainerFrame::MeasureAscentAndHeightForFlexItem(
   nsPresContext* aPresContext,
   ReflowInput& aChildReflowInput)
 {
-  const auto availableSize = aChildReflowInput.AvailableSize();
   const FrameProperties props = aItem.Frame()->Properties();
   if (const auto* cachedResult = props.Get(CachedFlexMeasuringReflow())) {
-    if (cachedResult->mAvailableSize == availableSize) {
+    if (cachedResult->IsValidFor(aChildReflowInput)) {
       return *cachedResult;
     }
   }
@@ -1827,9 +1845,7 @@ nsFlexContainerFrame::MeasureAscentAndHeightForFlexItem(
                     childDesiredSize, &aChildReflowInput, 0, 0, flags);
 
   auto result =
-    new CachedMeasuringReflowResult(availableSize,
-                                    childDesiredSize.Height(),
-                                    childDesiredSize.BlockStartAscent());
+    new CachedMeasuringReflowResult(aChildReflowInput, childDesiredSize);
 
   props.Set(CachedFlexMeasuringReflow(), result);
   return *result;
@@ -1874,11 +1890,11 @@ nsFlexContainerFrame::
     MeasureAscentAndHeightForFlexItem(aFlexItem, aPresContext,
                                       childRIForMeasuringHeight);
 
-  aFlexItem.SetAscent(reflowResult.mAscent);
+  aFlexItem.SetAscent(reflowResult.Ascent());
 
   // Subtract border/padding in vertical axis, to get _just_
   // the effective computed value of the "height" property.
-  nscoord childDesiredHeight = reflowResult.mHeight -
+  nscoord childDesiredHeight = reflowResult.Height() -
     childRIForMeasuringHeight.ComputedPhysicalBorderPadding().TopBottom();
 
   return std::max(0, childDesiredHeight);
@@ -4049,7 +4065,7 @@ nsFlexContainerFrame::SizeItemInCrossAxis(
   // so we don't bother with making aAxisTracker pick the cross-axis component
   // for us.)
   nscoord crossAxisBorderPadding = aItem.GetBorderPadding().TopBottom();
-  if (reflowResult.mHeight < crossAxisBorderPadding) {
+  if (reflowResult.Height() < crossAxisBorderPadding) {
     // Child's requested size isn't large enough for its border/padding!
     // This is OK for the trivial nsFrame::Reflow() impl, but other frame
     // classes should know better. So, if we get here, the child had better be
@@ -4062,10 +4078,10 @@ nsFlexContainerFrame::SizeItemInCrossAxis(
     aItem.SetCrossSize(0);
   } else {
     // (normal case)
-    aItem.SetCrossSize(reflowResult.mHeight - crossAxisBorderPadding);
+    aItem.SetCrossSize(reflowResult.Height() - crossAxisBorderPadding);
   }
 
-  aItem.SetAscent(reflowResult.mAscent);
+  aItem.SetAscent(reflowResult.Ascent());
 }
 
 void
diff --git a/layout/generic/nsFlexContainerFrame.h b/layout/generic/nsFlexContainerFrame.h
index d9cac8930..459ae8e20 100644
--- a/layout/generic/nsFlexContainerFrame.h
+++ b/layout/generic/nsFlexContainerFrame.h
@@ -56,7 +56,7 @@ public:
   class FlexLine;
   class FlexboxAxisTracker;
   struct StrutInfo;
-  struct CachedMeasuringReflowResult;
+  class CachedMeasuringReflowResult;
 
   // nsIFrame overrides
   void Init(nsIContent*       aContent,