Bug 1751965 - Remove destroyed display items from reused display items list r=mstange

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

layout/base/nsLayoutUtils.cpp

@@ -3336,16 +3336,16 @@ void nsLayoutUtils::PaintFrame(gfxContext* aRenderingContext, nsIFrame* aFrame,
       }
 
       if (doFullRebuild) {
-        list->DeleteAll(builder);
-        list->RestoreState();
-
         if (useRetainedBuilder) {
           retainedBuilder->ClearFramesWithProps();
+          retainedBuilder->ClearReuseableDisplayItems();
           mozilla::RDLUtils::AssertFrameSubtreeUnmodified(
               builder->RootReferenceFrame());
-          MOZ_ASSERT(retainedBuilder->List()->IsEmpty());
         }
 
+        list->DeleteAll(builder);
+        list->RestoreState();
+
         builder->ClearRetainedWindowRegions();
         builder->ClearWillChangeBudgets();
 

layout/painting/RetainedDisplayListBuilder.cpp

@@ -1507,7 +1507,8 @@ MOZ_NEVER_INLINE_DEBUG void ReuseStackingContextItem(
     aItem->UpdateBounds(aBuilder);
   }
 
-  DL_LOGD("Retaining display item %p", aItem);
+  aBuilder->AddReusableDisplayItem(aItem);
+  DL_LOGD("Reusing display item %p", aItem);
 }
 
 bool IsSupportedFrameType(const nsIFrame* aFrame) {
@@ -1558,10 +1559,8 @@ bool IsReuseableStackingContextItem(nsDisplayItem* aItem) {
  * linked and collected in given |aOutItems| array.
  */
 void CollectStackingContextItems(nsDisplayListBuilder* aBuilder,
-                                 nsDisplayList* aList,
+                                 nsDisplayList* aList, nsIFrame* aOuterFrame,
-                                 nsTArray<nsDisplayItem*>& aOutItems,
+                                 int aDepth = 0, bool aParentReused = false) {
-                                 nsIFrame* aOuterFrame, int aDepth = 0,
-                                 bool aParentReused = false) {
   nsDisplayList out;
 
   while (nsDisplayItem* item = aList->RemoveBottom()) {
@@ -1593,8 +1592,8 @@ void CollectStackingContextItems(nsDisplayListBuilder* aBuilder,
     const bool isStackingContextItem = IsReuseableStackingContextItem(item);
 
     if (item->GetChildren()) {
-      CollectStackingContextItems(aBuilder, item->GetChildren(), aOutItems,
+      CollectStackingContextItems(aBuilder, item->GetChildren(), item->Frame(),
-                                  item->Frame(), aDepth + 1,
+                                  aDepth + 1,
                                   aParentReused || isStackingContextItem);
     }
 
@@ -1604,7 +1603,6 @@ void CollectStackingContextItems(nsDisplayListBuilder* aBuilder,
       out.AppendToTop(item);
     } else if (isStackingContextItem) {
       // |item| is a stacking context item that can be reused.
-      aOutItems.AppendElement(item);
       ReuseStackingContextItem(aBuilder, item);
     } else {
       // |item| is inside a container item that will be destroyed later.
@@ -1622,27 +1620,6 @@ void CollectStackingContextItems(nsDisplayListBuilder* aBuilder,
   aList->RestoreState();
 }
 
-/**
- * Destroys the retained stacking context items that have not been reused and
- * clears the array.
- */
-void ClearPreviousItems(nsDisplayListBuilder* aBuilder,
-                        nsTArray<nsDisplayItem*>& aItems) {
-  const size_t total = aItems.Length();
-  size_t reused = 0;
-  for (auto* item : aItems) {
-    if (item->IsReusedItem()) {
-      reused++;
-      item->SetReusable();
-    } else {
-      item->Destroy(aBuilder);
-    }
-  }
-
-  DL_LOGI("RDL - Reused %zu of %zu SC display items", reused, total);
-  aItems.Clear();
-}
-
 }  // namespace RDL
 
 bool RetainedDisplayListBuilder::TrySimpleUpdate(
@@ -1652,10 +1629,8 @@ bool RetainedDisplayListBuilder::TrySimpleUpdate(
     return false;
   }
 
-  MOZ_ASSERT(mPreviousItems.IsEmpty());
   RDL::MarkAllAncestorFrames(aModifiedFrames, aOutFramesWithProps);
-  RDL::CollectStackingContextItems(&mBuilder, &mList, mPreviousItems,
+  RDL::CollectStackingContextItems(&mBuilder, &mList, RootReferenceFrame());
-                                   RootReferenceFrame());
 
   return true;
 }
@@ -1746,7 +1721,7 @@ PartialUpdateResult RetainedDisplayListBuilder::AttemptPartialUpdate(
   if (mBuilder.PartialBuildFailed()) {
     DL_LOGI("RDL - Partial update failed!");
     mBuilder.LeavePresShell(RootReferenceFrame(), nullptr);
-    RDL::ClearPreviousItems(&mBuilder, mPreviousItems);
+    mBuilder.ClearReuseableDisplayItems();
     mList.DeleteAll(&mBuilder);
     modifiedDL.DeleteAll(&mBuilder);
     Metrics()->mPartialUpdateFailReason = PartialUpdateFailReason::Content;
@@ -1775,7 +1750,7 @@ PartialUpdateResult RetainedDisplayListBuilder::AttemptPartialUpdate(
   } else {
     MOZ_ASSERT(mList.IsEmpty());
     mList = std::move(modifiedDL);
-    RDL::ClearPreviousItems(&mBuilder, mPreviousItems);
+    mBuilder.ClearReuseableDisplayItems();
     result = PartialUpdateResult::Updated;
   }
 

layout/painting/RetainedDisplayListBuilder.h

@@ -195,6 +195,9 @@ struct RetainedDisplayListBuilder {
    * the frames in the modified frame lists.
    */
   void ClearFramesWithProps();
+
+  void ClearReuseableDisplayItems() { mBuilder.ClearReuseableDisplayItems(); }
+
   void AddSizeOfIncludingThis(nsWindowSizes&) const;
 
   NS_DECLARE_FRAME_PROPERTY_DELETABLE(Cached, RetainedDisplayListBuilder)
@@ -272,9 +275,6 @@ struct RetainedDisplayListBuilder {
   RetainedDisplayList mList;
   WeakFrame mPreviousCaret;
   RetainedDisplayListMetrics mMetrics;
-
-  // Stores reusable items collected during display list preprocessing.
-  nsTArray<nsDisplayItem*> mPreviousItems;
 };
 
 namespace RDLUtils {

layout/painting/nsDisplayList.cpp

@@ -2008,6 +2008,32 @@ void nsDisplayListBuilder::BuildCompositorHitTestInfoIfNeeded(
   }
 }
 
+void nsDisplayListBuilder::AddReusableDisplayItem(nsDisplayItem* aItem) {
+  mReuseableItems.Insert(aItem);
+}
+
+void nsDisplayListBuilder::RemoveReusedDisplayItem(nsDisplayItem* aItem) {
+  MOZ_ASSERT(aItem->IsReusedItem());
+  mReuseableItems.Remove(aItem);
+}
+
+void nsDisplayListBuilder::ClearReuseableDisplayItems() {
+  const size_t total = mReuseableItems.Count();
+
+  size_t reused = 0;
+  for (auto* item : mReuseableItems) {
+    if (item->IsReusedItem()) {
+      reused++;
+      item->SetReusable();
+    } else {
+      item->Destroy(this);
+    }
+  }
+
+  DL_LOGI("RDL - Reused %zu of %zu SC display items", reused, total);
+  mReuseableItems.Clear();
+}
+
 void nsDisplayListBuilder::ReuseDisplayItem(nsDisplayItem* aItem) {
   const auto* previous = mCurrentContainerASR;
   const auto* asr = aItem->GetActiveScrolledRoot();

layout/painting/nsDisplayList.h

@@ -1699,6 +1699,24 @@ class nsDisplayListBuilder {
     return mIsReusingStackingContextItems;
   }
 
+  /**
+   * Adds display item |aItem| to the reuseable display items set.
+   */
+  void AddReusableDisplayItem(nsDisplayItem* aItem);
+
+  /**
+   * Removes display item |aItem| from the reuseable display items set.
+   * This is needed because display items are sometimes deleted during
+   * display list building.
+   * Called by |nsDisplayItem::Destroy()| when the item has been reused.
+   */
+  void RemoveReusedDisplayItem(nsDisplayItem* aItem);
+
+  /**
+   * Clears the reuseable display items set.
+   */
+  void ClearReuseableDisplayItems();
+
   /**
    * Marks the given display item |aItem| as reused, and updates the necessary
    * display list builder state.
@@ -1900,6 +1918,9 @@ class nsDisplayListBuilder {
 
   bool mIsForContent;
   bool mIsReusingStackingContextItems;
+
+  // Stores reusable items collected during display list preprocessing.
+  nsTHashSet<nsDisplayItem*> mReuseableItems;
 };
 
 class nsDisplayItem;
@@ -2094,6 +2115,11 @@ class nsDisplayItem : public nsDisplayItemLink {
     if (aBuilder->IsForPainting() && aBuilder->IsForContent()) {
       DL_LOGV("Destroying display item %p (%s)", this, Name());
     }
+
+    if (IsReusedItem()) {
+      aBuilder->RemoveReusedDisplayItem(this);
+    }
+
     this->~nsDisplayItem();
     aBuilder->Destroy(type, this);
   }