Backed out changeset ff731fad7630 (bug 1420737) for causing bug 1431064. a=RyanVM

layout/painting/RetainedDisplayListBuilder.cpp

@@ -306,13 +306,13 @@ void UpdateASR(nsDisplayItem* aItem,
  *
  * The basic algorithm is:
  *
- * For-each item i in the new list:
+ * For-each item in the new list:
  *     If the item has a matching item in the old list:
- *         Remove items from the start of the old list up until we reach an item that also exists in the new list (leaving the matched item in place):
+ *         Remove items from the bottom of the old list until we reach the matching item:
  *             Add valid items to the merged list, destroy invalid items.
- *     Add i into the merged list.
- *     If the start of the old list matches i, remove and destroy it, otherwise mark the old version of i as used.
- * Add all remaining valid items from the old list into the merged list, skipping over (and destroying) any that are marked as used.
+ *         Destroy the matching item from the old list.
+ *     Add the item from the new list into the merged list.
+ * Add all remaining valid items from the old list into the merged list.
  *
  * If any item has a child display list, then we recurse into the merge
  * algorithm once we match up the new/old versions (if present).
@@ -320,7 +320,7 @@ void UpdateASR(nsDisplayItem* aItem,
  * Example 1:
  *
  * Old List: A,B,C,D
- * Modified List: A,D
+ * New List: A,D
  * Invalidations: C,D
  *
  * We first match the A items, and add the new one to the merged list.
@@ -330,23 +330,10 @@ void UpdateASR(nsDisplayItem* aItem,
  *
  * Merged List: A,B,D
  *
- * Example 2 (layout/reftests/retained-dl-zindex-1.html):
+ * Example 2:
  *
  * Old List: A, B
- * Modified List: B, A
- * Invalidations: A
- *
- * In this example A has been explicitly moved to the back.
- *
- * We match the B items, but don't copy A since it's invalid, and then add the
- * new B into the merged list. We then add A, and we're done.
- *
- * Merged List: B, A
- *
- * Example 3:
- *
- * Old List: A, B
- * Modified List: B, A
+ * New List, B, A
  * Invalidations: -
  *
  * This can happen because a prior merge might have changed the ordering
@@ -356,28 +343,6 @@ void UpdateASR(nsDisplayItem* aItem,
  * and then add the new B into the merged list. We then add A, and we're done.
  *
  * Merged List: B, A
- *
- * Example 4 (layout/reftests/retained-dl-zindex-2.html):
- *
- * Element A has two elements covering it (B and C), that don't intersect each
- * other. We then move C to the back.
- *
- * The correct initial ordering has B and C after A, in any order.
- *
- * Old List: A, B, C
- * Modified List: C, A
- * Invalidations: C
- *
- * We match the C items, but don't add anything from the old list because A is present
- * in both lists. We add C to the merged list, and mark the old version of C as reused.
- *
- * We then match A, add the new version the merged list and delete the old version.
- *
- * We then process the remainder of the old list, B is added (since it is valid,
- * and hasn't been mark as reused), C is destroyed since it's marked as reused and
- * is already present in the merged list.
- *
- * Merged List: C, A, B
  */
 void
 RetainedDisplayListBuilder::MergeDisplayLists(nsDisplayList* aNewList,
@@ -442,49 +407,39 @@ RetainedDisplayListBuilder::MergeDisplayLists(nsDisplayList* aNewList,
       // The new item has a matching counterpart in the old list that we haven't yet reached,
       // so copy all valid items from the old list into the merged list until we get to the
       // matched item.
-      nsDisplayItem* old = nullptr;
-      while ((old = aOldList->GetBottom()) && old != oldItem) {
-        if (IsAnyAncestorModified(old->FrameForInvalidation())) {
-          // The old item is invalid, discard it.
-          oldListLookup.Remove({ old->Frame(), old->GetPerFrameKey() });
-          aOldList->RemoveBottom();
-          old->Destroy(&mBuilder);
-        } else if (newListLookup.Get({ old->Frame(), old->GetPerFrameKey() })) {
-          // This old item is also in the new list, but we haven't got to it yet.
-          // Stop now, and we'll deal with it when we get to the new entry.
-          break;
-        } else {
-          // Recurse into the child list (without a matching new list) to
-          // ensure that we find and remove any invalidated items.
-          if (old->GetChildren()) {
-            nsDisplayList empty;
-            Maybe<const ActiveScrolledRoot*> containerASRForChildren;
-            MergeDisplayLists(&empty, old->GetChildren(),
-                              old->GetChildren(), containerASRForChildren);
-            UpdateASR(old, containerASRForChildren);
-            old->UpdateBounds(&mBuilder);
+      if (!oldItem->IsReused()) {
+        nsDisplayItem* old = nullptr;
+        while ((old = aOldList->RemoveBottom()) && !IsSameItem(newItem, old)) {
+          if (IsAnyAncestorModified(old->FrameForInvalidation())) {
+            // The old item is invalid, discard it.
+            oldListLookup.Remove({ old->Frame(), old->GetPerFrameKey() });
+            old->Destroy(&mBuilder);
+          } else if (newListLookup.Get({ old->Frame(), old->GetPerFrameKey() })) {
+            // The old item is also in the new list, but we haven't got to it yet.
+            // Mark that we've found it, and we'll deal with it when we get to the new
+            // entry.
+            old->SetReused(true);
+          } else {
+            // Recurse into the child list (without a matching new list) to
+            // ensure that we find and remove any invalidated items.
+            if (old->GetChildren()) {
+              nsDisplayList empty;
+              Maybe<const ActiveScrolledRoot*> containerASRForChildren;
+              MergeDisplayLists(&empty, old->GetChildren(),
+                                old->GetChildren(), containerASRForChildren);
+              UpdateASR(old, containerASRForChildren);
+              old->UpdateBounds(&mBuilder);
+            }
+            ReuseItem(old);
           }
-          aOldList->RemoveBottom();
-          ReuseItem(old);
         }
-      }
-      bool destroy = false;
-      if (old == oldItem) {
-        // If we advanced the old list until the matching item then we can pop
-        // the matching item off the old list and make sure we clean it up.
-        aOldList->RemoveBottom();
-        destroy = true;
-      } else {
-        // If we didn't get to the matching item, then mark the old item
-        // as being reused (since we're adding the new version to the new
-        // list now) so that we don't add it twice at the end.
-        oldItem->SetReused(true);
+        MOZ_ASSERT(old && IsSameItem(newItem, old));
+        MOZ_ASSERT(old == oldItem);
       }
 
       // Recursively merge any child lists, destroy the old item and add
       // the new one to the list.
-      if (!destroy &&
-          oldItem->GetType() == DisplayItemType::TYPE_LAYER_EVENT_REGIONS &&
+      if (oldItem->GetType() == DisplayItemType::TYPE_LAYER_EVENT_REGIONS &&
           !IsAnyAncestorModified(oldItem->FrameForInvalidation())) {
         // Event regions items don't have anything interesting other than
         // the lists of regions and frames, so we have no need to use the
@@ -504,9 +459,7 @@ RetainedDisplayListBuilder::MergeDisplayLists(nsDisplayList* aNewList,
           newItem->UpdateBounds(&mBuilder);
         }
 
-        if (destroy) {
-          oldItem->Destroy(&mBuilder);
-        }
+        oldItem->Destroy(&mBuilder);
         UseItem(newItem);
       }
     } else {
@@ -518,8 +471,7 @@ RetainedDisplayListBuilder::MergeDisplayLists(nsDisplayList* aNewList,
 
   // Reuse the remaining valid items from the old display list.
   while (nsDisplayItem* old = aOldList->RemoveBottom()) {
-    if (!IsAnyAncestorModified(old->FrameForInvalidation()) &&
-        !old->IsReused()) {
+    if (!IsAnyAncestorModified(old->FrameForInvalidation())) {
       if (old->GetChildren()) {
         // We are calling MergeDisplayLists() to ensure that the display items
         // with modified or deleted children will be correctly handled.

layout/reftests/display-list/reftest.list

@@ -8,8 +8,6 @@ skip-if(!retainedDisplayList) == retained-dl-scroll-out-of-view-1.html retained-
 skip-if(!retainedDisplayList) == retained-dl-displayport-1.html retained-dl-displayport-1-ref.html
 skip-if(!retainedDisplayList) == retained-dl-prerender-transform-1.html retained-dl-prerender-transform-1-ref.html
 == retained-dl-wrap-list.html retained-dl-wrap-list-ref.html
-== retained-dl-zindex-1.html retained-dl-zindex-1-ref.html
-== retained-dl-zindex-2.html retained-dl-zindex-2-ref.html
 fuzzy(1,235200) == 1413073.html 1413073-ref.html
 == 1416291.html 1416291-ref.html
 == 1417601-1.html 1417601-1-ref.html

layout/reftests/display-list/retained-dl-zindex-1-ref.html

@@ -1,27 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-<style>
-div {
-  width: 200px;
-  height: 200px;
-  position:relative;
-  will-change: transform;
-}
-#one {
-  top:120px;
-  background-color:blue;
-}
-#two {
-  top: -200px;
-  left: 100px;
-  background-color:green;
-  z-index: -1;
-}
-</style>
-</head>
-<body>
-  <div id="one"></div>
-  <div id="two"></div>
-</body>
-</html>

layout/reftests/display-list/retained-dl-zindex-1.html

@@ -1,35 +0,0 @@
-<!DOCTYPE html>
-<html class="reftest-wait">
-<head>
-<style>
-div {
-  width: 200px;
-  height: 200px;
-  position:relative;
-  will-change: transform;
-}
-#one {
-  top:120px;
-  background-color:blue;
-}
-#two {
-  top: -200px;
-  left: 100px;
-  background-color:green;
-  z-index: 1;
-}
-</style>
-</head>
-<body>
-  <div id="one"></div>
-  <div id="two"></div>
-</body>
-<script>
-function doTest() {
-  document.getElementById("two").style.zIndex = -1;
-  document.documentElement.removeAttribute("class");
-}
-
-window.addEventListener("MozReftestInvalidate", doTest);
-</script>
-</html>

layout/reftests/display-list/retained-dl-zindex-2-ref.html

@@ -1,33 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-<style>
-div {
-  width: 200px;
-  height: 200px;
-  position:relative;
-  will-change:transform;
-}
-#one {
-  top:120px;
-  background-color:blue;
-}
-#two {
-  top: -200px;
-  left: 100px;
-  background-color:green;
-}
-#three {
-  top: -180px;
-  left: 100px;
-  background-color:red;
-  z-index: -1;
-}
-</style>
-</head>
-<body>
-  <div id="one"></div>
-  <div id="two"></div>
-  <div id="three"></div>
-</body>
-</html>

layout/reftests/display-list/retained-dl-zindex-2.html

@@ -1,41 +0,0 @@
-<!DOCTYPE html>
-<html class="reftest-wait">
-<head>
-<style>
-div {
-  width: 200px;
-  height: 200px;
-  position:relative;
-  will-change:transform;
-}
-#one {
-  top:120px;
-  background-color:blue;
-}
-#two {
-  top: -200px;
-  left: 100px;
-  background-color:green;
-}
-#three {
-  top: -180px;
-  left: 100px;
-  background-color:red;
-  z-index: 1;
-}
-</style>
-</head>
-<body>
-  <div id="one"></div>
-  <div id="two"></div>
-  <div id="three"></div>
-</body>
-<script>
-function doTest() {
-  document.getElementById("three").style.zIndex = -1;
-  document.documentElement.removeAttribute("class");
-}
-
-window.addEventListener("MozReftestInvalidate", doTest);
-</script>
-</html>