Bug 1901515 Part 3 - Add methods to set previous continuation without updating first continuation cache. r=layout-reviewers,dholbert

The correctness assumption for this patch is that we destroy all frame
continuations when removing an element, so any stale first continuation cache
during frame destruction is acceptable.

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

layout/generic/nsIFrame.cpp

@@ -7192,6 +7192,10 @@ void nsIFrame::SetPrevContinuation(nsIFrame*) {
   MOZ_ASSERT_UNREACHABLE("Not splittable!");
 }
 
+void nsIFrame::SetPrevContinuationWithoutUpdatingCache(nsIFrame*) {
+  MOZ_ASSERT_UNREACHABLE("Not splittable!");
+}
+
 nsIFrame* nsIFrame::GetNextContinuation() const { return nullptr; }
 
 void nsIFrame::SetNextContinuation(nsIFrame*) {
@@ -7204,6 +7208,10 @@ void nsIFrame::SetPrevInFlow(nsIFrame*) {
   MOZ_ASSERT_UNREACHABLE("Not splittable!");
 }
 
+void nsIFrame::SetPrevInFlowWithoutUpdatingCache(nsIFrame*) {
+  MOZ_ASSERT_UNREACHABLE("Not splittable!");
+}
+
 nsIFrame* nsIFrame::GetNextInFlow() const { return nullptr; }
 
 void nsIFrame::SetNextInFlow(nsIFrame*) {

layout/generic/nsIFrame.h

@@ -2528,6 +2528,23 @@ class nsIFrame : public nsQueryFrame {
    */
   virtual nsIFrame* LastInFlow() const { return const_cast<nsIFrame*>(this); }
 
+  /**
+   * SetPrevContinuationWithoutUpdatingCache() and
+   * SetPrevInFlowWithoutUpdatingCache() are designed for performance
+   * optimization during frame destruction. Normal operations should use
+   * SetPrevContinuation() and SetPrevInFlow().
+   *
+   * Subclasses that implement a first continuation cache can override these two
+   * methods to set the prev-continuation and prev-in-flow without updating the
+   * cache.
+   *
+   * WARNING: Calling these two methods can result in a stale cache in later
+   * continuations. Therefore, they should only be called during frame
+   * destruction where any stale cache is acceptable.
+   */
+  virtual void SetPrevContinuationWithoutUpdatingCache(nsIFrame*);
+  virtual void SetPrevInFlowWithoutUpdatingCache(nsIFrame*);
+
   /**
    * Mark any stored intrinsic inline size information as dirty (requiring
    * re-calculation).  Note that this should generally not be called

layout/generic/nsSplittableFrame.cpp

@@ -50,13 +50,18 @@ nsIFrame* nsSplittableFrame::GetPrevContinuation() const {
   return mPrevContinuation;
 }
 
-void nsSplittableFrame::SetPrevContinuation(nsIFrame* aFrame) {
+void nsSplittableFrame::SetPrevContinuationWithoutUpdatingCache(
+    nsIFrame* aFrame) {
   NS_ASSERTION(!aFrame || Type() == aFrame->Type(),
                "setting a prev continuation with incorrect type!");
   NS_ASSERTION(!IsInPrevContinuationChain(aFrame, this),
                "creating a loop in continuation chain!");
   mPrevContinuation = aFrame;
   RemoveStateBits(NS_FRAME_IS_FLUID_CONTINUATION);
+}
+
+void nsSplittableFrame::SetPrevContinuation(nsIFrame* aFrame) {
+  SetPrevContinuationWithoutUpdatingCache(aFrame);
   UpdateFirstContinuationAndFirstInFlowCache();
 }
 
@@ -133,13 +138,17 @@ nsIFrame* nsSplittableFrame::GetPrevInFlow() const {
                                                          : nullptr;
 }
 
-void nsSplittableFrame::SetPrevInFlow(nsIFrame* aFrame) {
+void nsSplittableFrame::SetPrevInFlowWithoutUpdatingCache(nsIFrame* aFrame) {
   NS_ASSERTION(!aFrame || Type() == aFrame->Type(),
                "setting a prev in flow with incorrect type!");
   NS_ASSERTION(!IsInPrevContinuationChain(aFrame, this),
                "creating a loop in continuation chain!");
   mPrevContinuation = aFrame;
   AddStateBits(NS_FRAME_IS_FLUID_CONTINUATION);
+}
+
+void nsSplittableFrame::SetPrevInFlow(nsIFrame* aFrame) {
+  SetPrevInFlowWithoutUpdatingCache(aFrame);
   UpdateFirstContinuationAndFirstInFlowCache();
 }
 
@@ -186,7 +195,6 @@ nsIFrame* nsSplittableFrame::LastInFlow() const {
   return lastInFlow;
 }
 
-// Remove this frame from the flow. Connects prev in flow and next in flow
 void nsSplittableFrame::RemoveFromFlow(nsIFrame* aFrame) {
   nsIFrame* prevContinuation = aFrame->GetPrevContinuation();
   nsIFrame* nextContinuation = aFrame->GetNextContinuation();
@@ -198,23 +206,20 @@ void nsSplittableFrame::RemoveFromFlow(nsIFrame* aFrame) {
       prevContinuation->SetNextInFlow(nextContinuation);
     }
     if (nextContinuation) {
-      nextContinuation->SetPrevInFlow(prevContinuation);
+      nextContinuation->SetPrevInFlowWithoutUpdatingCache(prevContinuation);
     }
   } else {
     if (prevContinuation) {
       prevContinuation->SetNextContinuation(nextContinuation);
     }
     if (nextContinuation) {
-      nextContinuation->SetPrevContinuation(prevContinuation);
+      nextContinuation->SetPrevContinuationWithoutUpdatingCache(
+          prevContinuation);
     }
   }
 
-  // **Note: it is important here that we clear the Next link from aFrame
-  // BEFORE clearing its Prev link, because in nsContinuingTextFrame,
-  // SetPrevInFlow() would follow the Next pointers, wiping out the cached
-  // mFirstContinuation field from each following frame in the list.
   aFrame->SetNextInFlow(nullptr);
-  aFrame->SetPrevInFlow(nullptr);
+  aFrame->SetPrevInFlowWithoutUpdatingCache(nullptr);
 }
 
 void nsSplittableFrame::UpdateFirstContinuationAndFirstInFlowCache() {

layout/generic/nsSplittableFrame.h

@@ -83,9 +83,13 @@ class nsSplittableFrame : public nsIFrame {
   nsIFrame* FirstInFlow() const final;
   nsIFrame* LastInFlow() const final;
 
-  // Remove the frame from the flow. Connects the frame's prev-in-flow
-  // and its next-in-flow. This should only be called in frame Destroy()
-  // methods.
+  // See their counterparts in nsIFrame.h for the purpose of these two methods.
+  void SetPrevContinuationWithoutUpdatingCache(nsIFrame*) final;
+  void SetPrevInFlowWithoutUpdatingCache(nsIFrame*) final;
+
+  // Remove the frame from the flow. Connects the frame's prev-in-flow and its
+  // next-in-flow. This should only be called during frame destruction, e.g. in
+  // frame's Destroy() method.
   static void RemoveFromFlow(nsIFrame* aFrame);
 
  protected:

layout/generic/nsTextFrame.cpp

@@ -4115,7 +4115,7 @@ void nsTextFrame::Destroy(DestroyContext& aContext) {
   // type might be changing.  Not clear whether it's worth it.
   ClearTextRuns();
   if (mNextContinuation) {
-    mNextContinuation->SetPrevInFlow(nullptr);
+    mNextContinuation->SetPrevInFlowWithoutUpdatingCache(nullptr);
   }
   // Let the base class destroy the frame
   nsIFrame::Destroy(aContext);
@@ -4164,7 +4164,8 @@ class nsContinuingTextFrame final : public nsTextFrame {
 
   nsTextFrame* GetPrevContinuation() const final { return mPrevContinuation; }
 
-  void SetPrevContinuation(nsIFrame* aPrevContinuation) final {
+  void SetPrevContinuationWithoutUpdatingCache(
+      nsIFrame* aPrevContinuation) final {
     NS_ASSERTION(!aPrevContinuation || Type() == aPrevContinuation->Type(),
                  "setting a prev continuation with incorrect type!");
     NS_ASSERTION(
@@ -4172,6 +4173,10 @@ class nsContinuingTextFrame final : public nsTextFrame {
         "creating a loop in continuation chain!");
     mPrevContinuation = static_cast<nsTextFrame*>(aPrevContinuation);
     RemoveStateBits(NS_FRAME_IS_FLUID_CONTINUATION);
+  }
+
+  void SetPrevContinuation(nsIFrame* aPrevContinuation) final {
+    SetPrevContinuationWithoutUpdatingCache(aPrevContinuation);
     UpdateCachedContinuations();
   }
 
@@ -4180,7 +4185,7 @@ class nsContinuingTextFrame final : public nsTextFrame {
                                                            : nullptr;
   }
 
-  void SetPrevInFlow(nsIFrame* aPrevInFlow) final {
+  void SetPrevInFlowWithoutUpdatingCache(nsIFrame* aPrevInFlow) final {
     NS_ASSERTION(!aPrevInFlow || Type() == aPrevInFlow->Type(),
                  "setting a prev in flow with incorrect type!");
     NS_ASSERTION(
@@ -4188,6 +4193,10 @@ class nsContinuingTextFrame final : public nsTextFrame {
         "creating a loop in continuation chain!");
     mPrevContinuation = static_cast<nsTextFrame*>(aPrevInFlow);
     AddStateBits(NS_FRAME_IS_FLUID_CONTINUATION);
+  }
+
+  void SetPrevInFlow(nsIFrame* aPrevInFlow) final {
+    SetPrevInFlowWithoutUpdatingCache(aPrevInFlow);
     UpdateCachedContinuations();
   }
 
@@ -9188,13 +9197,11 @@ static void RemoveEmptyInFlows(nsTextFrame* aFrame,
   prevContinuation->SetNextInFlow(aFirstToNotRemove);
   aFirstToNotRemove->SetPrevInFlow(prevContinuation);
 
-  // **Note: it is important here that we clear the Next link from lastRemoved
-  // BEFORE clearing the Prev link from aFrame, because SetPrevInFlow() will
-  // follow the Next pointers, wiping out the cached mFirstContinuation field
-  // from each following frame in the list. We need this to stop when it
-  // reaches lastRemoved!
+  // We are going to remove aFrame and all its later continuations up to
+  // lastRemoved. Therefore, it is OK to not update first continuation cache for
+  // them.
   lastRemoved->SetNextInFlow(nullptr);
-  aFrame->SetPrevInFlow(nullptr);
+  aFrame->SetPrevInFlowWithoutUpdatingCache(nullptr);
 
   nsContainerFrame* parent = aFrame->GetParent();
   nsIFrame::DestroyContext context(aFrame->PresShell());