From 41ef1458338ff58218392fb5fff5a82ba95d4285 Mon Sep 17 00:00:00 2001 From: Daniel Holbert Date: Wed, 25 Mar 2020 05:02:35 +0000 Subject: [PATCH] Bug 1624247 part 3: Refactor CachedBAxisMeasurement to be an optional member in a more general CachedFlexItemData class. r=TYLin This patch should not impact behavior; it is purely a refactoring change. In a later patch, I will add more (non-optional) members to this new class, to cache additional data. One happy outcome of the current patch is that CachedBAxisMeasurement's members can now become (mostly) 'const' again, now that we can reconstruct it in-place by virtue of it being stored in a Maybe<>. Differential Revision: https://phabricator.services.mozilla.com/D67802 --HG-- extra : moz-landing-system : lando --- layout/generic/nsFlexContainerFrame.cpp | 79 ++++++++++++++++++------- layout/generic/nsFlexContainerFrame.h | 1 + 2 files changed, 60 insertions(+), 20 deletions(-) diff --git a/layout/generic/nsFlexContainerFrame.cpp b/layout/generic/nsFlexContainerFrame.cpp index d4efab5e5f11..585d0dd2e37a 100644 --- a/layout/generic/nsFlexContainerFrame.cpp +++ b/layout/generic/nsFlexContainerFrame.cpp @@ -1666,10 +1666,10 @@ void nsFlexContainerFrame::ResolveAutoFlexBasisAndMinSize( */ class nsFlexContainerFrame::CachedBAxisMeasurement { struct Key { - LogicalSize mComputedSize; - nscoord mComputedMinBSize; - nscoord mComputedMaxBSize; - nscoord mAvailableBSize; + const LogicalSize mComputedSize; + const nscoord mComputedMinBSize; + const nscoord mComputedMaxBSize; + const nscoord mAvailableBSize; explicit Key(const ReflowInput& aRI) : mComputedSize(aRI.ComputedSize()), @@ -1685,10 +1685,12 @@ class nsFlexContainerFrame::CachedBAxisMeasurement { } }; - Key mKey; + const Key mKey; + // This could/should be const, but it's non-const for now just because it's + // assigned via a series of steps in the constructor body: nscoord mBSize; - nscoord mAscent; + const nscoord mAscent; public: CachedBAxisMeasurement(const ReflowInput& aReflowInput, @@ -1715,25 +1717,62 @@ class nsFlexContainerFrame::CachedBAxisMeasurement { nscoord BSize() const { return mBSize; } nscoord Ascent() const { return mAscent; } +}; + +/** + * When we instantiate/update a CachedFlexItemData, this enum must be used to + * indicate the sort of reflow whose results we're capturing. This impacts + * what we cache & how we use the cached information. + */ +enum class FlexItemReflowType { + // A reflow to measure the block-axis size of a flex item (as an input to the + // flex layout algorithm). + Measuring, + + // A reflow with the flex item's "final" size at the end of the flex layout + // algorithm. + // XXXdholbert Unused for now, but will be used in a later patch. + Final, +}; + +/** + * This class stores information about the previous reflow for a given flex + * item. This should hopefully help us avoid redundant reflows of that + * flex item. + */ +class nsFlexContainerFrame::CachedFlexItemData { + public: + CachedFlexItemData(const ReflowInput& aReflowInput, + const ReflowOutput& aReflowOutput, + FlexItemReflowType aType) { + if (aType == FlexItemReflowType::Measuring) { + mBAxisMeasurement.emplace(aReflowInput, aReflowOutput); + } + } + + // If the flex container needs a measuring reflow for the flex item, then the + // resulting block-axis measurements can be cached here. If no measurement + // has been needed so far, then this member will be Nothing(). + Maybe mBAxisMeasurement; // Instances of this class are stored under this frame property, on // frames that are flex items: - NS_DECLARE_FRAME_PROPERTY_DELETABLE(Prop, CachedBAxisMeasurement) + NS_DECLARE_FRAME_PROPERTY_DELETABLE(Prop, CachedFlexItemData) }; void nsFlexContainerFrame::MarkCachedFlexMeasurementsDirty( nsIFrame* aItemFrame) { - aItemFrame->RemoveProperty(CachedBAxisMeasurement::Prop()); + aItemFrame->RemoveProperty(CachedFlexItemData::Prop()); } const CachedBAxisMeasurement& nsFlexContainerFrame::MeasureAscentAndBSizeForFlexItem( FlexItem& aItem, ReflowInput& aChildReflowInput) { - auto* cachedResult = - aItem.Frame()->GetProperty(CachedBAxisMeasurement::Prop()); - if (cachedResult) { - if (cachedResult->IsValidFor(aChildReflowInput)) { - return *cachedResult; + auto* cachedData = aItem.Frame()->GetProperty(CachedFlexItemData::Prop()); + + if (cachedData && cachedData->mBAxisMeasurement) { + if (cachedData->mBAxisMeasurement->IsValidFor(aChildReflowInput)) { + return *(cachedData->mBAxisMeasurement); } FLEX_LOG("[perf] MeasureAscentAndBSizeForFlexItem rejected cached value"); } else { @@ -1771,15 +1810,15 @@ nsFlexContainerFrame::MeasureAscentAndBSizeForFlexItem( // Update (or add) our cached measurement, so that we can hopefully skip this // measuring reflow the next time around: - if (cachedResult) { - *cachedResult = - CachedBAxisMeasurement(aChildReflowInput, childReflowOutput); + if (cachedData) { + cachedData->mBAxisMeasurement.reset(); + cachedData->mBAxisMeasurement.emplace(aChildReflowInput, childReflowOutput); } else { - cachedResult = - new CachedBAxisMeasurement(aChildReflowInput, childReflowOutput); - aItem.Frame()->SetProperty(CachedBAxisMeasurement::Prop(), cachedResult); + cachedData = new CachedFlexItemData(aChildReflowInput, childReflowOutput, + FlexItemReflowType::Measuring); + aItem.Frame()->SetProperty(CachedFlexItemData::Prop(), cachedData); } - return *cachedResult; + return *(cachedData->mBAxisMeasurement); } /* virtual */ diff --git a/layout/generic/nsFlexContainerFrame.h b/layout/generic/nsFlexContainerFrame.h index 371f915181dc..bf778a23550f 100644 --- a/layout/generic/nsFlexContainerFrame.h +++ b/layout/generic/nsFlexContainerFrame.h @@ -104,6 +104,7 @@ class nsFlexContainerFrame final : public nsContainerFrame { class FlexboxAxisTracker; struct StrutInfo; class CachedBAxisMeasurement; + class CachedFlexItemData; // nsIFrame overrides void Init(nsIContent* aContent, nsContainerFrame* aParent,