Bug 1461812 - Make sure we fully cleanup any partially constructed display lists when returning a failure from AttemptPartialUpdate. r=miko
authorMatt Woodrow <mwoodrow@mozilla.com>
Wed, 16 May 2018 12:55:08 +1200
changeset 418484 5543294befe9494593370f33c40ba50c8239e0c6
parent 418483 235b702765bbb35a206777cbe94f9de84e1dee30
child 418485 acaaa40ebdf142fda38d5661f7631f029a2406c6
push id34003
push usershindli@mozilla.com
push dateWed, 16 May 2018 18:31:07 +0000
treeherdermozilla-central@a35fc68a2471 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmiko
bugs1461812
milestone62.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 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
@@ -458,21 +458,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
@@ -1114,17 +1109,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());
@@ -1152,17 +1147,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
@@ -3469,21 +3469,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 {