Bug 1555819 - Remove invalidated display items during PreProcessDisplayLists, since we might not merge their display list. r=miko

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

--HG--
extra : moz-landing-system : lando

layout/generic/nsFrame.cpp

@@ -987,7 +987,14 @@ void nsIFrame::DiscardOldItems() {
   }
 
   for (nsDisplayItemBase* i : *items) {
-    i->DiscardIfOldItem();
+    // Only discard items that are invalidated by this frame,
+    // as we're only guaranteed to rebuild those items. Table
+    // background items are created by the relevant table part,
+    // but have the cell frame as the primary frame, and we don't
+    // want to remove them if this is the cell.
+    if (i->FrameForInvalidation() == this) {
+      i->DiscardIfOldItem();
+    }
   }
 }
 

layout/painting/RetainedDisplayListBuilder.cpp

@@ -121,6 +121,23 @@ void RetainedDisplayListBuilder::AddSizeOfIncludingThis(
   mList.AddSizeOfExcludingThis(aSizes);
 }
 
+bool AnyContentAncestorModified(nsIFrame* aFrame, nsIFrame* aStopAtFrame) {
+  nsIFrame* f = aFrame;
+  while (f) {
+    if (f->IsFrameModified()) {
+      return true;
+    }
+
+    if (aStopAtFrame && f == aStopAtFrame) {
+      break;
+    }
+
+    f = nsLayoutUtils::GetDisplayListParent(f);
+  }
+
+  return false;
+}
+
 // 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.
@@ -129,8 +146,8 @@ void RetainedDisplayListBuilder::AddSizeOfIncludingThis(
 // jump into those immediately rather than walking the entire thing.
 bool RetainedDisplayListBuilder::PreProcessDisplayList(
     RetainedDisplayList* aList, AnimatedGeometryRoot* aAGR,
-    PartialUpdateResult& aUpdated, uint32_t aCallerKey, uint32_t aNestingDepth,
-    bool aKeepLinked) {
+    PartialUpdateResult& aUpdated, nsIFrame* aOuterFrame, 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
@@ -186,7 +203,8 @@ bool RetainedDisplayListBuilder::PreProcessDisplayList(
       }
     }
 
-    if (!item->CanBeReused() || item->HasDeletedFrame()) {
+    if (!item->CanBeReused() || item->HasDeletedFrame() ||
+        AnyContentAncestorModified(item->FrameForInvalidation(), aOuterFrame)) {
       if (initializeOldItems) {
         aList->mOldItems.AppendElement(OldItemInfo(nullptr));
       } else {
@@ -238,9 +256,10 @@ bool RetainedDisplayListBuilder::PreProcessDisplayList(
         keepLinked = true;
       }
 
-      if (!PreProcessDisplayList(
-              item->GetChildren(), SelectAGRForFrame(f, aAGR), aUpdated,
-              item->GetPerFrameKey(), aNestingDepth + 1, keepLinked)) {
+      if (!PreProcessDisplayList(item->GetChildren(),
+                                 SelectAGRForFrame(f, aAGR), aUpdated,
+                                 item->Frame(), item->GetPerFrameKey(),
+                                 aNestingDepth + 1, keepLinked)) {
         MOZ_RELEASE_ASSERT(
             !aKeepLinked,
             "Can't early return since we need to move the out list back");
@@ -302,23 +321,6 @@ void RetainedDisplayListBuilder::IncrementSubDocPresShellPaintCount(
   mBuilder.IncrementPresShellPaintCount(presShell);
 }
 
-bool AnyContentAncestorModified(nsIFrame* aFrame, nsIFrame* aStopAtFrame) {
-  nsIFrame* f = aFrame;
-  while (f) {
-    if (f->IsFrameModified()) {
-      return true;
-    }
-
-    if (aStopAtFrame && f == aStopAtFrame) {
-      break;
-    }
-
-    f = nsLayoutUtils::GetDisplayListParent(f);
-  }
-
-  return false;
-}
-
 static Maybe<const ActiveScrolledRoot*> SelectContainerASR(
     const DisplayItemClipChain* aClipChain, const ActiveScrolledRoot* aItemASR,
     Maybe<const ActiveScrolledRoot*>& aContainerASR) {
@@ -627,10 +629,12 @@ class MergeState {
     return false;
   }
 
+#ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
   bool HasModifiedFrame(nsDisplayItem* aItem) {
     nsIFrame* stopFrame = mOuterItem ? mOuterItem->Frame() : nullptr;
     return AnyContentAncestorModified(aItem->FrameForInvalidation(), stopFrame);
   }
+#endif
 
   void UpdateContainerASR(nsDisplayItem* aItem) {
     mContainerASR = SelectContainerASR(
@@ -668,7 +672,7 @@ class MergeState {
   void ProcessOldNode(OldListIndex aNode,
                       nsTArray<MergedListIndex>&& aDirectPredecessors) {
     nsDisplayItem* item = mOldItems[aNode.val].mItem;
-    if (mOldItems[aNode.val].IsChanged() || HasModifiedFrame(item)) {
+    if (mOldItems[aNode.val].IsChanged()) {
       if (item && item->IsGlassItem() &&
           item == mBuilder->Builder()->GetGlassDisplayItem()) {
         mBuilder->Builder()->ClearGlassDisplayItem();

layout/painting/RetainedDisplayListBuilder.h

@@ -217,6 +217,7 @@ struct RetainedDisplayListBuilder {
   bool PreProcessDisplayList(RetainedDisplayList* aList,
                              AnimatedGeometryRoot* aAGR,
                              PartialUpdateResult& aUpdated,
+                             nsIFrame* aOuterFrame = nullptr,
                              uint32_t aCallerKey = 0,
                              uint32_t aNestingDepth = 0,
                              bool aKeepLinked = false);

layout/painting/crashtests/1555819-1.html

@@ -0,0 +1,12 @@
+<style>
+body {
+  width: 1px;
+  height: 5vmax;
+  -webkit-filter: brightness(1);
+}
+* {
+  grid-template-areas: '';
+  columns: 1px;
+}
+</style>
+<keygen autofocus="">aaaa

layout/painting/crashtests/crashtests.list

@@ -20,4 +20,5 @@ load 1514544-1.html
 load 1547420-1.html
 load 1549909.html
 asserts(6) load 1551389-1.html # bug 847368
+asserts(0-2) load 1555819-1.html