From dca75f84eb316858b2bf520d3d0cca43766ffcd2 Mon Sep 17 00:00:00 2001 From: janekptacijarabaci Date: Tue, 24 Apr 2018 20:21:41 +0200 Subject: Bug 1336708: Don't reuse cached flex-item reflow measurements if the item's computed height has changed --- layout/generic/nsFlexContainerFrame.cpp | 60 +++++++++++++++++++++------------ layout/generic/nsFlexContainerFrame.h | 2 +- 2 files changed, 39 insertions(+), 23 deletions(-) (limited to 'layout/generic') 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, -- cgit v1.2.3