Bug 1544948 - Skip merging display lists that we're sure can't have changed. r=miko
authorMatt Woodrow <mwoodrow@mozilla.com>
Mon, 29 Apr 2019 03:14:49 +0000
changeset 530526 900ceaf4d39c8968eb4ba8c7769b47064f34ad18
parent 530525 90c4bb8c0d5cb33135e4229c982f746bc1aa9ee7
child 530527 3bc847f39bd7ba6359082cddeed7c0fe2bc625cc
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmiko
bugs1544948
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1544948 - Skip merging display lists that we're sure can't have changed. r=miko ComputeRebuildRegion sets ForceDescendIntoIfVisible on all modified frames and their ancestors, so we can use this to detect if a display list might have modified children by looking for this flag on the container item. We still need to run PreProcessDisplayList on the list, so that we can remove items that belong to a deleted frame, and build the old items array (including placeholders for the deleted items) so that it matches our DAG. If we wanted to skip serialization to the old items array, then we'd need to remove the deleted item entries from the DAG too, including connecting predecessors of the deleted entry to entries that have that deleted entry as their predecessor. That's hard to do in-place, so we leave the empty entries in the old items list to act as placeholders until we next merge this list properly. Differential Revision: https://phabricator.services.mozilla.com/D27822
layout/painting/RetainedDisplayListBuilder.cpp
layout/painting/RetainedDisplayListBuilder.h
layout/painting/RetainedDisplayListHelpers.h
layout/painting/nsDisplayList.h
layout/reftests/display-list/1544948-1-ref.html
layout/reftests/display-list/1544948-1.html
layout/reftests/display-list/reftest.list
--- a/layout/painting/RetainedDisplayListBuilder.cpp
+++ b/layout/painting/RetainedDisplayListBuilder.cpp
@@ -116,91 +116,163 @@ static AnimatedGeometryRoot* SelectAGRFo
 
 // Removes any display items that belonged to a frame that was deleted,
 // and mark frames that belong to a different AGR so that get their
 // items built again.
 // TODO: We currently descend into all children even if we don't have an AGR
 // to mark, as child stacking contexts might. It would be nice if we could
 // jump into those immediately rather than walking the entire thing.
 bool RetainedDisplayListBuilder::PreProcessDisplayList(
-    RetainedDisplayList* aList, AnimatedGeometryRoot* aAGR, uint32_t aCallerKey,
-    uint32_t aNestingDepth) {
+    RetainedDisplayList* aList, AnimatedGeometryRoot* aAGR,
+    PartialUpdateResult& aUpdated, uint32_t aCallerKey, uint32_t aNestingDepth,
+    bool aKeepLinked) {
   // The DAG merging algorithm does not have strong mechanisms in place to keep
   // the complexity of the resulting DAG under control. In some cases we can
   // build up edges very quickly. Detect those cases and force a full display
   // list build if we hit them.
   static const uint32_t kMaxEdgeRatio = 5;
-  bool initializeDAG = !aList->mDAG.Length();
+  const bool initializeDAG = !aList->mDAG.Length();
   if (!initializeDAG && aList->mDAG.mDirectPredecessorList.Length() >
                             (aList->mDAG.mNodesInfo.Length() * kMaxEdgeRatio)) {
     return false;
   }
 
-  MOZ_RELEASE_ASSERT(initializeDAG || aList->mDAG.Length() == aList->Count());
+  // If we had aKeepLinked=true for this list on the previous paint, then
+  // mOldItems will already be initialized as it won't have been consumed during
+  // a merge.
+  const bool initializeOldItems = aList->mOldItems.IsEmpty();
+  if (initializeOldItems) {
+    aList->mOldItems.SetCapacity(aList->Count());
+  } else {
+    MOZ_RELEASE_ASSERT(!initializeDAG);
+  }
 
-  nsDisplayList saved;
-  aList->mOldItems.SetCapacity(aList->Count());
-  MOZ_RELEASE_ASSERT(aList->mOldItems.IsEmpty());
+  MOZ_RELEASE_ASSERT(
+      initializeDAG ||
+      aList->mDAG.Length() ==
+          (initializeOldItems ? aList->Count() : aList->mOldItems.Length()));
+
+  nsDisplayList out;
+
+  size_t i = 0;
   while (nsDisplayItem* item = aList->RemoveBottom()) {
 #ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
     item->SetMergedPreProcessed(false, true);
 #endif
 
-    if (!item->CanBeReused() || item->HasDeletedFrame()) {
-      size_t i = aList->mOldItems.Length();
-      aList->mOldItems.AppendElement(OldItemInfo(nullptr));
-      item->Destroy(&mBuilder);
-
-      if (initializeDAG) {
-        if (i == 0) {
-          aList->mDAG.AddNode(Span<const MergedListIndex>());
-        } else {
-          MergedListIndex previous(i - 1);
-          aList->mDAG.AddNode(Span<const MergedListIndex>(&previous, 1));
-        }
+    // If we have a previously initialized old items list, then it can differ
+    // from the current list due to items removed for having a deleted frame.
+    // We can't easily remove these, since the DAG has entries for those indices
+    // and it's hard to rewrite in-place.
+    // Skip over entries with no current item to keep the iterations in sync.
+    if (!initializeOldItems) {
+      while (!aList->mOldItems[i].mItem) {
+        i++;
       }
-      continue;
     }
 
-    size_t i = aList->mOldItems.Length();
-    aList->mOldItems.AppendElement(OldItemInfo(item));
-    item->SetOldListIndex(aList, OldListIndex(i), aCallerKey, aNestingDepth);
     if (initializeDAG) {
       if (i == 0) {
         aList->mDAG.AddNode(Span<const MergedListIndex>());
       } else {
         MergedListIndex previous(i - 1);
         aList->mDAG.AddNode(Span<const MergedListIndex>(&previous, 1));
       }
     }
 
+    if (!item->CanBeReused() || item->HasDeletedFrame()) {
+      if (initializeOldItems) {
+        aList->mOldItems.AppendElement(OldItemInfo(nullptr));
+      } else {
+        MOZ_RELEASE_ASSERT(aList->mOldItems[i].mItem == item);
+        aList->mOldItems[i].mItem = nullptr;
+      }
+      item->Destroy(&mBuilder);
+
+      i++;
+      aUpdated = PartialUpdateResult::Updated;
+      continue;
+    }
+
+    if (initializeOldItems) {
+      aList->mOldItems.AppendElement(OldItemInfo(item));
+    }
+
+    // If we're not going to keep the list linked, then this old item entry
+    // is the only pointer to the item. Let it know that it now strongly
+    // owns the item, so it can destroy it if it goes away.
+    aList->mOldItems[i].mOwnsItem = !aKeepLinked;
+
+    item->SetOldListIndex(aList, OldListIndex(i), aCallerKey, aNestingDepth);
+
     nsIFrame* f = item->Frame();
 
     if (item->GetChildren()) {
-      if (!PreProcessDisplayList(item->GetChildren(),
-                                 SelectAGRForFrame(f, aAGR),
-                                 item->GetPerFrameKey(), aNestingDepth + 1)) {
+      // If children inside this list were invalid, then we'd have walked the
+      // ancestors and set ForceDescendIntoVisible on the current frame. If an
+      // ancestor is modified, then we'll throw this away entirely. Either way,
+      // we won't need to run merging on this sublist, and we can keep the items
+      // linked into their display list.
+      // The caret can move without invalidating, but we always set the force
+      // descend into frame state bit on that frame, so check for that too.
+      // TODO: AGR marking below can call MarkFrameForDisplayIfVisible and make
+      // us think future siblings need to be merged, even though we don't really
+      // need to.
+      bool keepLinked = aKeepLinked;
+      nsIFrame* invalid = item->FrameForInvalidation();
+      if (!invalid->ForceDescendIntoIfVisible() &&
+          !(invalid->GetStateBits() &
+            NS_FRAME_FORCE_DISPLAY_LIST_DESCEND_INTO)) {
+        keepLinked = true;
+      }
+
+      if (!PreProcessDisplayList(
+              item->GetChildren(), SelectAGRForFrame(f, aAGR), aUpdated,
+              item->GetPerFrameKey(), aNestingDepth + 1, keepLinked)) {
         return false;
       }
     }
 
     // TODO: We should be able to check the clipped bounds relative
     // to the common AGR (of both the existing item and the invalidated
     // frame) and determine if they can ever intersect.
+    // TODO: We only really need to build the ancestor container item that is a
+    // sibling of the changed thing to get correct ordering. The changed content
+    // is a frame though, and it's hard to map that to container items in this
+    // list.
     if (aAGR && item->GetAnimatedGeometryRoot()->GetAsyncAGR() != aAGR) {
       mBuilder.MarkFrameForDisplayIfVisible(f, mBuilder.RootReferenceFrame());
     }
 
     // TODO: This is here because we sometimes reuse the previous display list
     // completely. For optimization, we could only restore the state for reused
     // display items.
     item->RestoreState();
+
+    // If we're going to keep this linked list and not merge it, then mark the
+    // item as used and put it back into the list.
+    if (aKeepLinked) {
+      item->SetReused(true);
+      if (item->GetChildren()) {
+        item->UpdateBounds(Builder());
+      }
+      if (item->GetType() == DisplayItemType::TYPE_SUBDOCUMENT) {
+        IncrementSubDocPresShellPaintCount(item);
+      }
+      out.AppendToTop(item);
+    }
+    i++;
   }
+
   MOZ_RELEASE_ASSERT(aList->mOldItems.Length() == aList->mDAG.Length());
   aList->RestoreState();
+
+  if (aKeepLinked) {
+    aList->AppendToTop(&out);
+  }
   return true;
 }
 
 void RetainedDisplayListBuilder::IncrementSubDocPresShellPaintCount(
     nsDisplayItem* aItem) {
   MOZ_ASSERT(aItem->GetType() == DisplayItemType::TYPE_SUBDOCUMENT);
 
   nsSubDocumentFrame* subDocFrame =
@@ -274,18 +346,37 @@ static void UpdateASR(nsDisplayItem* aIt
   }
 
   wrapList->SetActiveScrolledRoot(ActiveScrolledRoot::PickAncestor(
       wrapList->GetFrameActiveScrolledRoot(), *asr));
 
   wrapList->UpdateHitTestInfoActiveScrolledRoot(*asr);
 }
 
+static void CopyASR(nsDisplayItem* aOld, nsDisplayItem* aNew) {
+  const ActiveScrolledRoot* hitTest = nullptr;
+  if (aOld->HasHitTestInfo()) {
+    MOZ_ASSERT(aNew->HasHitTestInfo());
+    const HitTestInfo& info =
+        static_cast<nsDisplayHitTestInfoItem*>(aOld)->GetHitTestInfo();
+    hitTest = info.mASR;
+  }
+
+  aNew->SetActiveScrolledRoot(aOld->GetActiveScrolledRoot());
+
+  // SetActiveScrolledRoot for most items will also set the hit-test info item's
+  // asr, so we need to manually set that again to what we saved earlier.
+  if (aOld->HasHitTestInfo()) {
+    static_cast<nsDisplayHitTestInfoItem*>(aNew)
+        ->UpdateHitTestInfoActiveScrolledRoot(hitTest);
+  }
+}
+
 OldItemInfo::OldItemInfo(nsDisplayItem* aItem)
-    : mItem(aItem), mUsed(false), mDiscarded(false) {
+    : mItem(aItem), mUsed(false), mDiscarded(false), mOwnsItem(false) {
   if (mItem) {
     // Clear cached modified frame state when adding an item to the old list.
     mItem->SetModifiedFrame(false);
   }
 }
 
 void OldItemInfo::AddedMatchToMergedList(RetainedDisplayListBuilder* aBuilder,
                                          MergedListIndex aIndex) {
@@ -293,16 +384,17 @@ void OldItemInfo::AddedMatchToMergedList
 }
 
 void OldItemInfo::Discard(RetainedDisplayListBuilder* aBuilder,
                           nsTArray<MergedListIndex>&& aDirectPredecessors) {
   MOZ_ASSERT(!IsUsed());
   mUsed = mDiscarded = true;
   mDirectPredecessors = std::move(aDirectPredecessors);
   if (mItem) {
+    MOZ_ASSERT(mOwnsItem);
     mItem->Destroy(aBuilder->Builder());
   }
   mItem = nullptr;
 }
 
 bool OldItemInfo::IsChanged() {
   return !mItem || !mItem->CanBeReused() || mItem->HasDeletedFrame();
 }
@@ -352,27 +444,17 @@ class MergeState {
           destItem = oldItem;
           // The building rect can depend on the overflow rect (when the parent
           // frame is position:fixed), which can change without invalidating
           // the frame/items. If we're using the old item, copy the building
           // rect across from the new item.
           oldItem->SetBuildingRect(aNewItem->GetBuildingRect());
         }
 
-        if (aNewItem->GetChildren()) {
-          Maybe<const ActiveScrolledRoot*> containerASRForChildren;
-          if (mBuilder->MergeDisplayLists(
-                  aNewItem->GetChildren(), oldItem->GetChildren(),
-                  destItem->GetChildren(), containerASRForChildren, aNewItem)) {
-            destItem->InvalidateCachedChildInfo(mBuilder->Builder());
-            mResultIsModified = true;
-          }
-          UpdateASR(destItem, containerASRForChildren);
-          destItem->UpdateBounds(mBuilder->Builder());
-        }
+        MergeChildLists(aNewItem, oldItem, destItem);
 
         AutoTArray<MergedListIndex, 2> directPredecessors =
             ProcessPredecessorsOfOldNode(oldIndex);
         MergedListIndex newIndex = AddNewNode(
             destItem, Some(oldIndex), directPredecessors, aPreviousItem);
         mOldItems[oldIndex.val].AddedMatchToMergedList(mBuilder, newIndex);
         if (destItem == aNewItem) {
           oldItem->Destroy(mBuilder->Builder());
@@ -382,16 +464,43 @@ class MergeState {
         return Some(newIndex);
       }
     }
     mResultIsModified = true;
     return Some(AddNewNode(aNewItem, Nothing(), Span<MergedListIndex>(),
                            aPreviousItem));
   }
 
+  void MergeChildLists(nsDisplayItem* aNewItem, nsDisplayItem* aOldItem,
+                       nsDisplayItem* aOutItem) {
+    if (!aOutItem->GetChildren()) {
+      return;
+    }
+
+    Maybe<const ActiveScrolledRoot*> containerASRForChildren;
+    nsDisplayList empty;
+    const bool modified = mBuilder->MergeDisplayLists(
+        aNewItem ? aNewItem->GetChildren() : &empty, aOldItem->GetChildren(),
+        aOutItem->GetChildren(), containerASRForChildren, aOutItem);
+    if (modified) {
+      aOutItem->InvalidateCachedChildInfo(mBuilder->Builder());
+      UpdateASR(aOutItem, containerASRForChildren);
+      mResultIsModified = true;
+    } else if (aOutItem == aNewItem) {
+      // If nothing changed, but we copied the contents across to
+      // the new item, then also copy the ASR data.
+      CopyASR(aOldItem, aNewItem);
+    }
+    // Ideally we'd only UpdateBounds if something changed, but
+    // nsDisplayWrapList also uses this to update the clip chain for the
+    // current ASR, which gets reset during RestoreState(), so we always need
+    // to run it again.
+    aOutItem->UpdateBounds(mBuilder->Builder());
+  }
+
   bool ShouldUseNewItem(nsDisplayItem* aNewItem) {
     // Generally we want to use the old item when the frame isn't marked as
     // modified so that any cached information on the item (or referencing the
     // item) gets retained. Quite a few FrameLayerBuilder performance
     // improvements benefit by this. Sometimes, however, we can end up where the
     // new item paints something different from the old item, even though we
     // haven't modified the frame, and it's hard to fix. In these cases we just
     // always use the new item to be safe.
@@ -527,28 +636,18 @@ class MergeState {
 
   void ProcessOldNode(OldListIndex aNode,
                       nsTArray<MergedListIndex>&& aDirectPredecessors) {
     nsDisplayItem* item = mOldItems[aNode.val].mItem;
     if (mOldItems[aNode.val].IsChanged() || HasModifiedFrame(item)) {
       mOldItems[aNode.val].Discard(mBuilder, std::move(aDirectPredecessors));
       mResultIsModified = true;
     } else {
-      if (item->GetChildren()) {
-        Maybe<const ActiveScrolledRoot*> containerASRForChildren;
-        nsDisplayList empty;
-        if (mBuilder->MergeDisplayLists(&empty, item->GetChildren(),
-                                        item->GetChildren(),
-                                        containerASRForChildren, item)) {
-          item->InvalidateCachedChildInfo(mBuilder->Builder());
-          mResultIsModified = true;
-        }
-        UpdateASR(item, containerASRForChildren);
-        item->UpdateBounds(mBuilder->Builder());
-      }
+      MergeChildLists(nullptr, item, item);
+
       if (item->GetType() == DisplayItemType::TYPE_SUBDOCUMENT) {
         mBuilder->IncrementSubDocPresShellPaintCount(item);
       }
       item->SetReused(true);
       mOldItems[aNode.val].AddedToMergedList(
           AddNewNode(item, Some(aNode), aDirectPredecessors, Nothing()));
     }
   }
@@ -633,16 +732,29 @@ class MergeState {
   // since they internally encode the type with the mOps pointer,
   // and assert when we try swap the contents
   nsDisplayList mMergedItems;
   DirectedAcyclicGraph<MergedListUnits> mMergedDAG;
   nsDisplayItem* mOuterItem;
   bool mResultIsModified;
 };
 
+#ifdef DEBUG
+void VerifyNotModified(nsDisplayList* aList) {
+  for (nsDisplayItem* item = aList->GetBottom(); item;
+       item = item->GetAbove()) {
+    MOZ_ASSERT(!AnyContentAncestorModified(item->FrameForInvalidation()));
+
+    if (item->GetChildren()) {
+      VerifyNotModified(item->GetChildren());
+    }
+  }
+}
+#endif
+
 /**
  * Takes two display lists and merges them into an output list.
  *
  * Display lists wthout an explicit DAG are interpreted as linear DAGs (with a
  * maximum of one direct predecessor and one direct successor per node). We add
  * the two DAGs together, and then output the topological sorted ordering as the
  * final display list.
  *
@@ -651,16 +763,33 @@ class MergeState {
  */
 bool RetainedDisplayListBuilder::MergeDisplayLists(
     nsDisplayList* aNewList, RetainedDisplayList* aOldList,
     RetainedDisplayList* aOutList,
     mozilla::Maybe<const mozilla::ActiveScrolledRoot*>& aOutContainerASR,
     nsDisplayItem* aOuterItem) {
   AUTO_PROFILER_LABEL_CATEGORY_PAIR(GRAPHICS_DisplayListMerging);
 
+  if (!aOldList->IsEmpty()) {
+    // If we still have items in the actual list, then it is because
+    // PreProcessDisplayList decided that it was sure it can't be modified. We
+    // can just use it directly, and throw any new items away.
+
+    aNewList->DeleteAll(&mBuilder);
+#ifdef DEBUG
+    VerifyNotModified(aOldList);
+#endif
+
+    if (aOldList != aOutList) {
+      *aOutList = std::move(*aOldList);
+    }
+
+    return false;
+  }
+
   MergeState merge(this, *aOldList, aOuterItem);
 
   Maybe<MergedListIndex> previousItemIndex;
   while (nsDisplayItem* item = aNewList->RemoveBottom()) {
     previousItemIndex = merge.ProcessItemFromNewList(item, previousItemIndex);
   }
 
   *aOutList = merge.Finalize();
@@ -1259,20 +1388,21 @@ auto RetainedDisplayListBuilder::Attempt
       }
     }
 
     mPreviousCaret = mBuilder.GetCaretFrame();
   }
 
   nsRect modifiedDirty;
   AnimatedGeometryRoot* modifiedAGR = nullptr;
+  PartialUpdateResult result = PartialUpdateResult::NoChange;
   if (!shouldBuildPartial ||
       !ComputeRebuildRegion(modifiedFrames.Frames(), &modifiedDirty,
                             &modifiedAGR, framesWithProps.Frames()) ||
-      !PreProcessDisplayList(&mList, modifiedAGR)) {
+      !PreProcessDisplayList(&mList, modifiedAGR, result)) {
     mBuilder.LeavePresShell(mBuilder.RootReferenceFrame(), nullptr);
     mList.DeleteAll(&mBuilder);
     return PartialUpdateResult::Failed;
   }
 
   // This is normally handled by EnterPresShell, but we skipped it so that we
   // didn't call MarkFrameForDisplayIfVisible before ComputeRebuildRegion.
   nsIScrollableFrame* sf = mBuilder.RootReferenceFrame()
@@ -1326,17 +1456,16 @@ auto RetainedDisplayListBuilder::Attempt
   // display list merging to prune unused items (for example, items that
   // are not visible anymore) from the old list.
   // TODO: Optimization opportunity. In this case, MergeDisplayLists()
   // unnecessarily creates a hashtable of the old items.
   // TODO: Ideally we could skip this if result is NoChange, but currently when
   // we call RestoreState on nsDisplayWrapList it resets the clip to the base
   // clip, and we need the UpdateBounds call (within MergeDisplayLists) to
   // move it to the correct inner clip.
-  PartialUpdateResult result = PartialUpdateResult::NoChange;
   Maybe<const ActiveScrolledRoot*> dummy;
   if (MergeDisplayLists(&modifiedDL, &mList, &mList, dummy)) {
     result = PartialUpdateResult::Updated;
   }
 
   // printf_stderr("Painting --- Merged list:\n");
   // nsFrame::PrintDisplayList(&mBuilder, mList);
 
--- a/layout/painting/RetainedDisplayListBuilder.h
+++ b/layout/painting/RetainedDisplayListBuilder.h
@@ -113,28 +113,43 @@ struct RetainedDisplayListBuilder {
    * Also clears the frame properties set by RetainedDisplayListBuilder for all
    * the frames in the modified frame lists.
    */
   void ClearFramesWithProps();
 
   NS_DECLARE_FRAME_PROPERTY_DELETABLE(Cached, RetainedDisplayListBuilder)
 
  private:
+  /**
+   * Recursively pre-processes the old display list tree before building the
+   * new partial display lists, and serializes the old list into an array,
+   * recording indices on items for fast lookup during merging. Builds an
+   * initial linear DAG for the list if we don't have an existing one. Finds
+   * items that have a different AGR from the specified one, and marks them to
+   * also be built so that we get relative ordering correct. Passes
+   * aKeepLinked=true internally for sub-lists that can't be changed to keep the
+   * original list structure linked for fast re-use.
+   */
   bool PreProcessDisplayList(RetainedDisplayList* aList,
                              AnimatedGeometryRoot* aAGR,
+                             PartialUpdateResult& aUpdated,
                              uint32_t aCallerKey = 0,
-                             uint32_t aNestingDepth = 0);
+                             uint32_t aNestingDepth = 0,
+                             bool aKeepLinked = false);
 
   /**
    * Merges items from aNewList into non-invalidated items from aOldList and
    * stores the result in aOutList.
    *
    * aOuterItem is a pointer to an item that owns one of the lists, if
    * available. If both lists are populated, then both outer items must not be
    * invalidated, and identical, so either can be passed here.
+   *
+   * Returns true if changes were made, and the resulting display list (in
+   * aOutList) is different from aOldList.
    */
   bool MergeDisplayLists(
       nsDisplayList* aNewList, RetainedDisplayList* aOldList,
       RetainedDisplayList* aOutList,
       mozilla::Maybe<const mozilla::ActiveScrolledRoot*>& aOutContainerASR,
       nsDisplayItem* aOuterItem = nullptr);
 
   bool ComputeRebuildRegion(nsTArray<nsIFrame*>& aModifiedFrames,
--- a/layout/painting/RetainedDisplayListHelpers.h
+++ b/layout/painting/RetainedDisplayListHelpers.h
@@ -164,14 +164,15 @@ struct OldItemInfo {
 
   bool IsChanged();
 
   nsDisplayItem* mItem;
   nsTArray<MergedListIndex> mDirectPredecessors;
   MergedListIndex mIndex;
   bool mUsed;
   bool mDiscarded;
+  bool mOwnsItem;
 };
 
 bool AnyContentAncestorModified(nsIFrame* aFrame,
                                 nsIFrame* aStopAtFrame = nullptr);
 
 #endif  // RETAINEDDISPLAYLISTHELPERS_H_
--- a/layout/painting/nsDisplayList.h
+++ b/layout/painting/nsDisplayList.h
@@ -2080,16 +2080,17 @@ MOZ_ALWAYS_INLINE T* MakeDisplayItem(nsD
  * highest in z-order.
  */
 class nsDisplayItemLink {
   // This is never instantiated directly, so no need to count constructors and
   // destructors.
  protected:
   nsDisplayItemLink() : mAbove(nullptr) {}
   nsDisplayItemLink(const nsDisplayItemLink&) : mAbove(nullptr) {}
+  ~nsDisplayItemLink() { MOZ_RELEASE_ASSERT(!mAbove); }
   nsDisplayItem* mAbove;
 
   friend class nsDisplayList;
 };
 
 /*
  * nsDisplayItemBase is a base-class for all display items. It is mainly
  * responsible for handling the frame-display item 1:n relationship, as well as
@@ -3675,23 +3676,27 @@ class RetainedDisplayList : public nsDis
     MOZ_ASSERT(mOldItems.IsEmpty(), "Must empty list before destroying");
   }
 
   RetainedDisplayList& operator=(RetainedDisplayList&& aOther) {
     MOZ_ASSERT(!Count(), "Can only move into an empty list!");
     MOZ_ASSERT(mOldItems.IsEmpty(), "Can only move into an empty list!");
     AppendToTop(&aOther);
     mDAG = std::move(aOther.mDAG);
+    mOldItems = std::move(aOther.mOldItems);
     return *this;
   }
 
   void DeleteAll(nsDisplayListBuilder* aBuilder) override {
     for (OldItemInfo& i : mOldItems) {
-      if (i.mItem) {
+      if (i.mItem && i.mOwnsItem) {
         i.mItem->Destroy(aBuilder);
+        MOZ_ASSERT(!GetBottom(),
+                   "mOldItems should not be owning items if we also have items "
+                   "in the normal list");
       }
     }
     mOldItems.Clear();
     mDAG.Clear();
     nsDisplayList::DeleteAll(aBuilder);
   }
 
   DirectedAcyclicGraph<MergedListUnits> mDAG;
new file mode 100644
--- /dev/null
+++ b/layout/reftests/display-list/1544948-1-ref.html
@@ -0,0 +1,32 @@
+<html>
+<head>
+<style>
+  div {
+    width:10px;
+    height:10px;
+    background-color:green;
+    display: inline-block;
+    position: relative;
+  }
+  #wrapper {
+    opacity: 0.5;
+    width: 200px;
+    height: 200px;
+    background-color: transparent;
+  }
+
+  #first {
+    z-index: 1;
+  }
+
+  #second {
+    background-color: blue;
+  }
+</style>
+</head>
+<body id="body">
+  <div id="wrapper">
+    <div id="first" class="reftest-no-display-list"></div><div id="second" class="reftest-no-display-list"></div>
+  </div>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/display-list/1544948-1.html
@@ -0,0 +1,43 @@
+<html class="reftest-wait">
+<head>
+<style>
+  div {
+    width:10px;
+    height:10px;
+    background-color:green;
+    display: inline-block;
+    position: relative;
+  }
+  #wrapper {
+    opacity: 0.5;
+    width: 200px;
+    height: 200px;
+    background-color: transparent;
+  }
+
+  #first {
+    z-index: 1;
+  }
+</style>
+<script>
+function doTest2() {
+  var elem = document.getElementById("second");
+  elem.style.backgroundColor = "blue";
+}
+
+function doTest() {
+  var elem = document.getElementById("third");
+  elem.parentNode.removeChild(elem);
+  document.documentElement.removeAttribute("class");
+  requestAnimationFrame(doTest2);
+}
+
+window.addEventListener("MozReftestInvalidate", doTest);
+</script>
+</head>
+<body id="body">
+  <div id="wrapper">
+    <div id="first" class="reftest-no-display-list"></div><div id="second"></div><div id="third"></div>
+  </div>
+</body>
+</html>
--- a/layout/reftests/display-list/reftest.list
+++ b/layout/reftests/display-list/reftest.list
@@ -35,8 +35,9 @@ skip-if(!asyncPan) == 1437374-1.html 143
 == 1453541-1.html 1453541-ref.html
 == 1453541-2.html 1453541-ref.html
 == 1452805-1.html 1452805-ref.html
 == 1461231-1.html about:blank
 fuzzy(0-2,0-40000) skip-if(!asyncPan) == 1464288-1.html 1464288-ref.html
 == 1482403-1.html 1482403-1-ref.html
 == 1504233-1.html 1504233-1-ref.html
 == 1533317-1.html 1533317-1-ref.html
+== 1544948-1.html 1544948-1-ref.html