Bug 1461812 - Make sure we fully cleanup any partially constructed display lists when returning a failure from AttemptPartialUpdate. r=miko, a=RyanVM
authorMatt Woodrow <mwoodrow@mozilla.com>
Wed, 16 May 2018 12:55:08 +1200
changeset 470822 909e56123552
parent 470821 2932a4b5f7e7
child 470823 457462d0bb3d
push id9233
push userryanvm@gmail.com
push date2018-05-17 01:26 +0000
treeherdermozilla-beta@233d36dfd5c9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmiko, RyanVM
bugs1461812
milestone61.0
Bug 1461812 - Make sure we fully cleanup any partially constructed display lists when returning a failure from AttemptPartialUpdate. r=miko, a=RyanVM 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
@@ -525,8 +525,9 @@ pref(dom.webcomponents.shadowdom.enabled
 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
+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
@@ -3451,21 +3451,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 {