Bug 1926015 - Percentage-basis aware intrinsic cache. r=TYLin,dholbert

This avoids returning wrong intrinsic values with different calls into
intrinsic size computation, which is wrong by definition.

We shouldn't be wallpapering over it by clearing intrinsic sizes
mid-layout as the original patch was doing.

Differential Revision: https://phabricator.services.mozilla.com/D229621

layout/generic/ColumnSetWrapperFrame.cpp

@@ -151,17 +151,10 @@ void ColumnSetWrapperFrame::MarkIntrinsicISizesDirty() {
 
 nscoord ColumnSetWrapperFrame::IntrinsicISize(const IntrinsicSizeInput& aInput,
                                               IntrinsicISizeType aType) {
-  if (aType == IntrinsicISizeType::MinISize) {
+  return mCachedIntrinsics.GetOrSet(*this, aType, aInput, [&] {
-    if (mCachedMinISize == NS_INTRINSIC_ISIZE_UNKNOWN) {
+    return aType == IntrinsicISizeType::MinISize ? MinISize(aInput)
-      mCachedMinISize = MinISize(aInput);
+                                                 : PrefISize(aInput);
-    }
+  });
-    return mCachedMinISize;
-  }
-
-  if (mCachedPrefISize == NS_INTRINSIC_ISIZE_UNKNOWN) {
-    mCachedPrefISize = PrefISize(aInput);
-  }
-  return mCachedPrefISize;
 }
 
 nscoord ColumnSetWrapperFrame::MinISize(const IntrinsicSizeInput& aInput) {

layout/generic/IntrinsicISizesCache.h

@@ -0,0 +1,160 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef mozilla_IntrinsicISizesCache_h
+#define mozilla_IntrinsicISizesCache_h
+
+#include "nsIFrame.h"
+
+namespace mozilla {
+
+// Some frame classes keep a cache of intrinsic inline sizes. This class
+// encapsulates the logic for caching them depending on the IntrinsicSizeInput.
+//
+// The cache is intended to take as little space as possible
+// (max(sizeof(nscoord) * 2, sizeof(void*))), when there are no percentage-size
+// dependencies.
+struct IntrinsicISizesCache final {
+  IntrinsicISizesCache() {
+    new (&mInline) InlineCache();
+    MOZ_ASSERT(IsInline());
+  }
+
+  ~IntrinsicISizesCache() { delete GetOutOfLine(); }
+
+  template <typename Compute>
+  nscoord GetOrSet(nsIFrame& aFrame, IntrinsicISizeType aType,
+                   const IntrinsicSizeInput& aInput, Compute aCompute) {
+    bool dependentOnPercentBSize = aFrame.HasAnyStateBits(
+        NS_FRAME_DESCENDANT_INTRINSIC_ISIZE_DEPENDS_ON_BSIZE);
+    nscoord value = Get(dependentOnPercentBSize, aType, aInput);
+    if (value != kNotFound) {
+      return value;
+    }
+    value = aCompute();
+    // Inside of aCompute(), we might have newly discovered that we do have a
+    // descendant whose intrinsic isize depends on our bsize; so we check that
+    // state bit again before updating the cache.
+    dependentOnPercentBSize = aFrame.HasAnyStateBits(
+        NS_FRAME_DESCENDANT_INTRINSIC_ISIZE_DEPENDS_ON_BSIZE);
+    Set(dependentOnPercentBSize, aType, aInput, value);
+    return value;
+  }
+
+  void Clear() {
+    if (auto* ool = GetOutOfLine()) {
+      ool->mCacheWithPercentageBasis.Clear();
+      ool->mCacheWithoutPercentageBasis.Clear();
+      ool->mLastPercentageBasis.reset();
+    } else {
+      mInline.Clear();
+    }
+  }
+
+ private:
+  // We use nscoord_MAX rather than NS_INTRINSIC_ISIZE_UNKNOWN as our sentinel
+  // value so that our high bit is always free.
+  static constexpr nscoord kNotFound = nscoord_MAX;
+
+  nscoord Get(bool aDependentOnPercentBSize, IntrinsicISizeType aType,
+              const IntrinsicSizeInput& aInput) const {
+    if (!aDependentOnPercentBSize ||
+        !aInput.HasSomePercentageBasisForChildren()) {
+      if (auto* ool = GetOutOfLine()) {
+        return ool->mCacheWithoutPercentageBasis.Get(aType);
+      }
+      return mInline.Get(aType);
+    }
+    if (auto* ool = GetOutOfLine()) {
+      if (ool->mLastPercentageBasis == aInput.mPercentageBasisForChildren) {
+        return ool->mCacheWithPercentageBasis.Get(aType);
+      }
+    }
+    return kNotFound;
+  }
+
+  void Set(bool aDependentOnPercentBSize, IntrinsicISizeType aType,
+           const IntrinsicSizeInput& aInput, nscoord aValue) {
+    // Intrinsic sizes should be nonnegative, so this std::max clamping should
+    // rarely be necessary except in cases of integer overflow.  We have to be
+    // strict about it, though, because of how we (ab)use the high bit
+    // (see kHighBit)
+    aValue = std::max(aValue, 0);
+    const bool usePercentAwareCache =
+        aDependentOnPercentBSize && aInput.HasSomePercentageBasisForChildren();
+    if (usePercentAwareCache) {
+      auto* ool = EnsureOutOfLine();
+      ool->mLastPercentageBasis = aInput.mPercentageBasisForChildren;
+      ool->mCacheWithPercentageBasis.Set(aType, aValue);
+    } else if (auto* ool = GetOutOfLine()) {
+      ool->mCacheWithoutPercentageBasis.Set(aType, aValue);
+    } else {
+      mInline.Set(aType, aValue);
+      // No inline value should be able to cause us to misinterpret our
+      // representation as out-of-line, because intrinsic isizes should always
+      // be non-negative.
+      MOZ_DIAGNOSTIC_ASSERT(IsInline());
+    }
+  }
+
+  struct InlineCache {
+    nscoord mCachedMinISize = kNotFound;
+    nscoord mCachedPrefISize = kNotFound;
+
+    nscoord Get(IntrinsicISizeType aType) const {
+      return aType == IntrinsicISizeType::MinISize ? mCachedMinISize
+                                                   : mCachedPrefISize;
+    }
+    void Set(IntrinsicISizeType aType, nscoord aValue) {
+      MOZ_ASSERT(aValue >= 0);
+      if (aType == IntrinsicISizeType::MinISize) {
+        mCachedMinISize = aValue;
+      } else {
+        mCachedPrefISize = aValue;
+      }
+    }
+
+    void Clear() { *this = {}; }
+  };
+
+  struct OutOfLineCache {
+    InlineCache mCacheWithoutPercentageBasis;
+    InlineCache mCacheWithPercentageBasis;
+    Maybe<LogicalSize> mLastPercentageBasis;
+  };
+
+  // If the high bit of mOutOfLine is 1, then it points to an OutOfLineCache.
+  union {
+    InlineCache mInline;
+    uintptr_t mOutOfLine = 0;
+  };
+
+  static constexpr uintptr_t kHighBit = uintptr_t(1)
+                                        << (sizeof(void*) * CHAR_BIT - 1);
+
+  bool IsOutOfLine() const { return mOutOfLine & kHighBit; }
+  bool IsInline() const { return !IsOutOfLine(); }
+  OutOfLineCache* EnsureOutOfLine() {
+    if (auto* ool = GetOutOfLine()) {
+      return ool;
+    }
+    auto inlineCache = mInline;
+    auto* ool = new OutOfLineCache();
+    ool->mCacheWithoutPercentageBasis = inlineCache;
+    MOZ_ASSERT((reinterpret_cast<uintptr_t>(ool) & kHighBit) == 0);
+    mOutOfLine = reinterpret_cast<uintptr_t>(ool) | kHighBit;
+    return ool;
+  }
+
+  OutOfLineCache* GetOutOfLine() const {
+    return IsOutOfLine()
+               ? reinterpret_cast<OutOfLineCache*>(mOutOfLine & ~kHighBit)
+               : nullptr;
+  }
+};
+}  // namespace mozilla
+
+#endif

layout/generic/MiddleCroppingBlockFrame.cpp

@@ -58,26 +58,28 @@ void MiddleCroppingBlockFrame::UpdateDisplayedValueToUncroppedValue(
 
 nscoord MiddleCroppingBlockFrame::IntrinsicISize(
     const IntrinsicSizeInput& aInput, IntrinsicISizeType aType) {
-  nsAutoString prevValue;
+  auto* first = FirstContinuation();
-  bool restoreOldValue = false;
+  if (this != first) {
-
+    return first->IntrinsicISize(aInput, aType);
-  // Make sure we measure with the uncropped value.
-  if (mCropped && mCachedPrefISize == NS_INTRINSIC_ISIZE_UNKNOWN) {
-    mTextNode->GetNodeValue(prevValue);
-    restoreOldValue = true;
-    UpdateDisplayedValueToUncroppedValue(false);
   }
-
+  return mCachedIntrinsics.GetOrSet(*this, aType, aInput, [&] {
-  // Our min inline size is the same as our pref inline size, so we always
+    nsAutoString prevValue;
-  // delegate to nsBlockFrame's pref inline size.
+    bool restoreOldValue = false;
-  nscoord result =
+    if (mCropped) {
-      nsBlockFrame::IntrinsicISize(aInput, IntrinsicISizeType::PrefISize);
+      // Make sure we measure with the uncropped value, if we're currently
-
+      // cropped.
-  if (restoreOldValue) {
+      mTextNode->GetNodeValue(prevValue);
-    UpdateDisplayedValue(prevValue, /* aIsCropped = */ true, false);
+      UpdateDisplayedValueToUncroppedValue(false);
-  }
+      restoreOldValue = true;
-
+    }
-  return result;
+    // Our min inline size is the same as our pref inline size, so we always
+    // delegate to nsBlockFrame's pref inline size.
+    const nscoord result = nsBlockFrame::PrefISize(aInput);
+    if (restoreOldValue) {
+      UpdateDisplayedValue(prevValue, /* aIsCropped = */ true, false);
+    }
+    return result;
+  });
 }
 
 bool MiddleCroppingBlockFrame::CropTextToWidth(gfxContext& aRenderingContext,
@@ -177,8 +179,9 @@ void MiddleCroppingBlockFrame::Reflow(nsPresContext* aPresContext,
         aStatus.Reset();
         MarkSubtreeDirty();
         AddStateBits(NS_BLOCK_NEEDS_BIDI_RESOLUTION);
-        mCachedMinISize = NS_INTRINSIC_ISIZE_UNKNOWN;
+        // FIXME(emilio): Why do we need to clear cached intrinsics, if they are
-        mCachedPrefISize = NS_INTRINSIC_ISIZE_UNKNOWN;
+        // always based off our uncropped value?
+        mCachedIntrinsics.Clear();
         cropped = true;
         continue;
       }

layout/generic/WritingModes.h

@@ -969,7 +969,10 @@ class LogicalSize {
   /**
    * Test if a size is (0, 0).
    */
-  bool IsAllZero() const { return ISize() == 0 && BSize() == 0; }
+  bool IsAllZero() const { return IsAllValues(0); }
+  bool IsAllValues(nscoord aValue) const {
+    return ISize() == aValue && BSize() == aValue;
+  }
 
   /**
    * Various binary operators on LogicalSize. These are valid ONLY for operands

layout/generic/moz.build

@@ -142,6 +142,7 @@ EXPORTS.mozilla += [
     "ColumnUtils.h",
     "CSSAlignUtils.h",
     "CSSOrderAwareFrameIterator.h",
+    "IntrinsicISizesCache.h",
     "LayoutMessageUtils.h",
     "nsVideoFrame.h",
     "PrintedSheetFrame.h",

layout/generic/nsBlockFrame.cpp

@@ -765,8 +765,7 @@ static bool RemoveFirstLine(nsLineList& aFromLines, nsFrameList& aFromFrames,
 /* virtual */
 void nsBlockFrame::MarkIntrinsicISizesDirty() {
   nsBlockFrame* dirtyBlock = static_cast<nsBlockFrame*>(FirstContinuation());
-  dirtyBlock->mCachedMinISize = NS_INTRINSIC_ISIZE_UNKNOWN;
+  dirtyBlock->mCachedIntrinsics.Clear();
-  dirtyBlock->mCachedPrefISize = NS_INTRINSIC_ISIZE_UNKNOWN;
   if (!HasAnyStateBits(NS_BLOCK_NEEDS_BIDI_RESOLUTION)) {
     for (nsIFrame* frame = dirtyBlock; frame;
          frame = frame->GetNextContinuation()) {
@@ -784,8 +783,7 @@ void nsBlockFrame::CheckIntrinsicCacheAgainstShrinkWrapState() {
   }
   bool inflationEnabled = !presContext->mInflationDisabledForShrinkWrap;
   if (inflationEnabled != HasAnyStateBits(NS_BLOCK_INTRINSICS_INFLATED)) {
-    mCachedMinISize = NS_INTRINSIC_ISIZE_UNKNOWN;
+    mCachedIntrinsics.Clear();
-    mCachedPrefISize = NS_INTRINSIC_ISIZE_UNKNOWN;
     AddOrRemoveStateBits(NS_BLOCK_INTRINSICS_INFLATED, inflationEnabled);
   }
 }
@@ -823,17 +821,10 @@ nscoord nsBlockFrame::IntrinsicISize(const IntrinsicSizeInput& aInput,
 
   CheckIntrinsicCacheAgainstShrinkWrapState();
 
-  if (aType == IntrinsicISizeType::MinISize) {
+  return mCachedIntrinsics.GetOrSet(*this, aType, aInput, [&] {
-    if (mCachedMinISize == NS_INTRINSIC_ISIZE_UNKNOWN) {
+    return aType == IntrinsicISizeType::MinISize ? MinISize(aInput)
-      mCachedMinISize = MinISize(aInput);
+                                                 : PrefISize(aInput);
-    }
+  });
-    return mCachedMinISize;
-  }
-
-  if (mCachedPrefISize == NS_INTRINSIC_ISIZE_UNKNOWN) {
-    mCachedPrefISize = PrefISize(aInput);
-  }
-  return mCachedPrefISize;
 }
 
 /* virtual */

layout/generic/nsBlockFrame.h

@@ -17,6 +17,7 @@
 #include "nsLineBox.h"
 #include "nsCSSPseudoElements.h"
 #include "nsFloatManager.h"
+#include "mozilla/IntrinsicISizesCache.h"
 
 enum class LineReflowStatus {
   // The line was completely reflowed and fit in available width, and we should
@@ -273,6 +274,7 @@ class nsBlockFrame : public nsContainerFrame {
                                     BaselineSharingGroup aBaselineGroup,
                                     BaselineExportContext aExportContext) const;
 
+ protected:
   // MinISize() and PrefISize() are helpers to implement IntrinsicISize().
   nscoord MinISize(const mozilla::IntrinsicSizeInput& aInput);
   nscoord PrefISize(const mozilla::IntrinsicSizeInput& aInput);
@@ -1034,8 +1036,7 @@ class nsBlockFrame : public nsContainerFrame {
   int32_t GetDepth() const;
 #endif
 
-  nscoord mCachedMinISize = NS_INTRINSIC_ISIZE_UNKNOWN;
+  mozilla::IntrinsicISizesCache mCachedIntrinsics;
-  nscoord mCachedPrefISize = NS_INTRINSIC_ISIZE_UNKNOWN;
 
   nsLineList mLines;
 

layout/generic/nsFlexContainerFrame.cpp

@@ -2062,9 +2062,7 @@ const CachedBAxisMeasurement& nsFlexContainerFrame::MeasureBSizeForFlexItem(
 
 /* virtual */
 void nsFlexContainerFrame::MarkIntrinsicISizesDirty() {
-  mCachedMinISize = NS_INTRINSIC_ISIZE_UNKNOWN;
+  mCachedIntrinsicSizes.Clear();
-  mCachedPrefISize = NS_INTRINSIC_ISIZE_UNKNOWN;
-
   nsContainerFrame::MarkIntrinsicISizesDirty();
 }
 
@@ -6551,13 +6549,9 @@ nscoord nsFlexContainerFrame::ComputeIntrinsicISize(
 
 nscoord nsFlexContainerFrame::IntrinsicISize(const IntrinsicSizeInput& aInput,
                                              IntrinsicISizeType aType) {
-  nscoord& cachedISize = aType == IntrinsicISizeType::MinISize
+  return mCachedIntrinsicSizes.GetOrSet(*this, aType, aInput, [&] {
-                             ? mCachedMinISize
+    return ComputeIntrinsicISize(aInput, aType);
-                             : mCachedPrefISize;
+  });
-  if (cachedISize == NS_INTRINSIC_ISIZE_UNKNOWN) {
-    cachedISize = ComputeIntrinsicISize(aInput, aType);
-  }
-  return cachedISize;
 }
 
 int32_t nsFlexContainerFrame::GetNumLines() const {

layout/generic/nsFlexContainerFrame.h

@@ -12,6 +12,7 @@
 #include <tuple>
 
 #include "mozilla/dom/FlexBinding.h"
+#include "mozilla/IntrinsicISizesCache.h"
 #include "nsContainerFrame.h"
 #include "nsILineIterator.h"
 
@@ -656,8 +657,7 @@ class nsFlexContainerFrame final : public nsContainerFrame,
   /**
    * Cached values to optimize IntrinsicISize().
    */
-  nscoord mCachedMinISize = NS_INTRINSIC_ISIZE_UNKNOWN;
+  mozilla::IntrinsicISizesCache mCachedIntrinsicSizes;
-  nscoord mCachedPrefISize = NS_INTRINSIC_ISIZE_UNKNOWN;
 
   /**
    * Cached baselines computed in our last reflow to optimize

layout/generic/nsGridContainerFrame.cpp

@@ -9697,19 +9697,13 @@ nscoord nsGridContainerFrame::IntrinsicISize(const IntrinsicSizeInput& aInput,
   if (firstCont != this) {
     return firstCont->IntrinsicISize(aInput, aType);
   }
-
+  return mCachedIntrinsicSizes.GetOrSet(*this, aType, aInput, [&] {
-  nscoord& cachedISize = aType == IntrinsicISizeType::MinISize
+    return ComputeIntrinsicISize(aInput, aType);
-                             ? mCachedMinISize
+  });
-                             : mCachedPrefISize;
-  if (cachedISize == NS_INTRINSIC_ISIZE_UNKNOWN) {
-    cachedISize = ComputeIntrinsicISize(aInput, aType);
-  }
-  return cachedISize;
 }
 
 void nsGridContainerFrame::MarkIntrinsicISizesDirty() {
-  mCachedMinISize = NS_INTRINSIC_ISIZE_UNKNOWN;
+  mCachedIntrinsicSizes.Clear();
-  mCachedPrefISize = NS_INTRINSIC_ISIZE_UNKNOWN;
   for (auto& perAxisBaseline : mBaseline) {
     for (auto& baseline : perAxisBaseline) {
       baseline = NS_INTRINSIC_ISIZE_UNKNOWN;

layout/generic/nsGridContainerFrame.h

@@ -10,6 +10,7 @@
 #define nsGridContainerFrame_h___
 
 #include "mozilla/CSSOrderAwareFrameIterator.h"
+#include "mozilla/IntrinsicISizesCache.h"
 #include "mozilla/MathAlgorithms.h"
 #include "mozilla/Maybe.h"
 #include "mozilla/HashTable.h"
@@ -333,9 +334,7 @@ class nsGridContainerFrame final : public nsContainerFrame,
       mozilla::PresShell* aPresShell, ComputedStyle* aStyle);
   explicit nsGridContainerFrame(ComputedStyle* aStyle,
                                 nsPresContext* aPresContext)
-      : nsContainerFrame(aStyle, aPresContext, kClassID),
+      : nsContainerFrame(aStyle, aPresContext, kClassID) {
-        mCachedMinISize(NS_INTRINSIC_ISIZE_UNKNOWN),
-        mCachedPrefISize(NS_INTRINSIC_ISIZE_UNKNOWN) {
     for (auto& perAxisBaseline : mBaseline) {
       for (auto& baseline : perAxisBaseline) {
         baseline = NS_INTRINSIC_ISIZE_UNKNOWN;
@@ -532,11 +531,7 @@ class nsGridContainerFrame final : public nsContainerFrame,
   void AddImplicitNamedAreasInternal(LineNameList& aNameList,
                                      ImplicitNamedAreas*& aAreas);
 
-  /**
+  mozilla::IntrinsicISizesCache mCachedIntrinsicSizes;
-   * Cached values to optimize IntrinsicISize().
-   */
-  nscoord mCachedMinISize;
-  nscoord mCachedPrefISize;
 
   // Our baselines, one per BaselineSharingGroup per axis.
   PerLogicalAxis<PerBaseline<nscoord>> mBaseline;

layout/generic/nsIFrame.h

@@ -449,6 +449,11 @@ struct MOZ_STACK_CLASS IntrinsicSizeInput final {
   // to NS_UNCONSTRAINEDSIZE.
   Maybe<LogicalSize> mPercentageBasisForChildren;
 
+  bool HasSomePercentageBasisForChildren() const {
+    return mPercentageBasisForChildren &&
+           !mPercentageBasisForChildren->IsAllValues(NS_UNCONSTRAINEDSIZE);
+  }
+
   IntrinsicSizeInput(gfxContext* aContext,
                      const Maybe<LogicalSize>& aContainingBlockSize,
                      const Maybe<LogicalSize>& aPercentageBasisForChildren)