Bug 1461812 - Make sure we fully cleanup any partially constructed display lists when returning a failure from AttemptPartialUpdate. r?miko draft
authorMatt Woodrow <mwoodrow@mozilla.com>
Wed, 16 May 2018 12:55:08 +1200
changeset 795556 9482e4d69c01a302fe2cc5ccbe0409a8e718c8f5
parent 795546 380cf87c1ee3966dd94499942b73085754dc4824
child 795563 24dea044d82b6d216c2c079d7c157a7877f626f1
push id110009
push usermwoodrow@mozilla.com
push dateWed, 16 May 2018 00:55:30 +0000
reviewersmiko
bugs1461812
milestone62.0a1
Bug 1461812 - Make sure we fully cleanup any partially constructed display lists when returning a failure from AttemptPartialUpdate. r?miko MozReview-Commit-ID: DJBG6UFcoyR
layout/base/crashtests/1461812.html
layout/base/crashtests/crashtests.list
layout/painting/RetainedDisplayListBuilder.cpp
layout/painting/nsDisplayList.h
new file mode 100644
--- /dev/null
+++ b/layout/base/crashtests/1461812.html
@@ -0,0 +1,19 @@
+<style>
+:not(feConvolveMatrix) {
+  width: -moz-max-content;
+  column-width: 0em;
+  text-indent: 1pt;
+}
+.cl {
+  padding-bottom: 93vw;
+}
+</style>
+<script>
+function go() {
+  b.appendChild(a);
+}
+</script>
+<marquee id="b">4H</marquee>
+<details class="cl">
+<summary id="a" style="mix-blend-mode:color-dodge">A</summary>
+<style onload="go()">
--- a/layout/base/crashtests/crashtests.list
+++ b/layout/base/crashtests/crashtests.list
@@ -526,8 +526,9 @@ load 1442506.html
 load 1437155.html
 load 1443027-1.html
 load 1448841-1.html
 load 1452839.html
 load 1453702.html
 load 1453342.html
 load 1453196.html
 pref(dom.webcomponents.shadowdom.enabled,true) load 1414303.html
+load 1461812.html
--- a/layout/painting/RetainedDisplayListBuilder.cpp
+++ b/layout/painting/RetainedDisplayListBuilder.cpp
@@ -455,21 +455,16 @@ public:
   // Unfortunately we can't use strong typing for the hashtables
   // since they internally encode the type with the mOps pointer,
   // and assert when we try swap the contents
   nsDisplayList mMergedItems;
   DirectedAcyclicGraph<MergedListUnits> mMergedDAG;
   bool mResultIsModified;
 };
 
-void RetainedDisplayList::ClearDAG()
-{
-  mDAG.Clear();
-}
-
 /**
  * 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.
  *
  * Once we've merged a list, we then retain the DAG (as part of the RetainedDisplayList
@@ -1111,17 +1106,17 @@ RetainedDisplayListBuilder::AttemptParti
 
   nsRect modifiedDirty;
   AnimatedGeometryRoot* modifiedAGR = nullptr;
   if (!shouldBuildPartial ||
       !ComputeRebuildRegion(modifiedFrames.Frames(), &modifiedDirty,
                            &modifiedAGR, framesWithProps.Frames()) ||
       !PreProcessDisplayList(&mList, modifiedAGR)) {
     mBuilder.LeavePresShell(mBuilder.RootReferenceFrame(), List());
-    mList.ClearDAG();
+    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()->PresShell()->GetRootScrollFrameAsScrollable();
   if (sf) {
     nsCanvasFrame* canvasFrame = do_QueryFrame(sf->GetScrolledFrame());
@@ -1149,17 +1144,18 @@ RetainedDisplayListBuilder::AttemptParti
                                            mBuilder.RootReferenceFrame()->GetVisualOverflowRectRelativeToSelf(),
                                            aBackstop);
   }
   mBuilder.SetPartialUpdate(false);
 
   if (mBuilder.PartialBuildFailed()) {
     mBuilder.SetPartialBuildFailed(false);
     mBuilder.LeavePresShell(mBuilder.RootReferenceFrame(), List());
-    mList.ClearDAG();
+    mList.DeleteAll(&mBuilder);
+    modifiedDL.DeleteAll(&mBuilder);
     return PartialUpdateResult::Failed;
   }
 
   if (aChecker) {
     aChecker->Set(&modifiedDL, "TM");
   }
 
   //printf_stderr("Painting --- Modified list (dirty %d,%d,%d,%d):\n",
--- a/layout/painting/nsDisplayList.h
+++ b/layout/painting/nsDisplayList.h
@@ -3454,21 +3454,20 @@ public:
   void DeleteAll(nsDisplayListBuilder* aBuilder)
   {
     for (OldItemInfo& i : mOldItems) {
       if (i.mItem) {
         i.mItem->Destroy(aBuilder);
       }
     }
     mOldItems.Clear();
+    mDAG.Clear();
     nsDisplayList::DeleteAll(aBuilder);
   }
 
-  void ClearDAG();
-
   DirectedAcyclicGraph<MergedListUnits> mDAG;
 
   // Temporary state initialized during the preprocess pass
   // of RetainedDisplayListBuilder and then used during merging.
   nsTArray<OldItemInfo> mOldItems;
 };
 
 class nsDisplayImageContainer : public nsDisplayItem {