Bug 1429932 - Part 3: Refactor RetainedDisplayListBuilder::AttemptPartialUpdate to have an early return instead of a nested scope. r=miko draft
authorMatt Woodrow <mwoodrow@mozilla.com>
Fri, 12 Jan 2018 11:43:41 +1300
changeset 719319 c408d4275560385b88df99f8966e9e02acdb5949
parent 719318 cba40c9b4341f7b6266a1ee49de31c72a3fcb248
child 719320 a1dbe9186f09293a7455033927592b48e44364fc
push id95221
push usermwoodrow@mozilla.com
push dateThu, 11 Jan 2018 22:51:36 +0000
reviewersmiko
bugs1429932
milestone59.0a1
Bug 1429932 - Part 3: Refactor RetainedDisplayListBuilder::AttemptPartialUpdate to have an early return instead of a nested scope. r=miko MozReview-Commit-ID: L91euwUeJ5x
layout/painting/RetainedDisplayListBuilder.cpp
--- a/layout/painting/RetainedDisplayListBuilder.cpp
+++ b/layout/painting/RetainedDisplayListBuilder.cpp
@@ -816,98 +816,115 @@ void
 RetainedDisplayListBuilder::ClearModifiedFrameProps()
 {
   nsTArray<nsIFrame*> modifiedFrames =
     GetModifiedFrames(&mBuilder);
 
   ClearFrameProps(modifiedFrames);
 }
 
+class AutoClearFramePropsArray
+{
+public:
+  AutoClearFramePropsArray()
+  {}
+
+  explicit AutoClearFramePropsArray(nsTArray<nsIFrame*>&& aArray)
+    : mFrames(Move(aArray))
+  {}
+
+  ~AutoClearFramePropsArray()
+  {
+    ClearFrameProps(mFrames);
+  }
+
+  operator nsTArray<nsIFrame*>&() { return mFrames; }
+
+  bool IsEmpty() const { return mFrames.IsEmpty(); }
+
+  nsTArray<nsIFrame*> mFrames;
+};
+
 bool
 RetainedDisplayListBuilder::AttemptPartialUpdate(nscolor aBackstop)
 {
   mBuilder.RemoveModifiedWindowDraggingRegion();
   if (mBuilder.ShouldSyncDecodeImages()) {
     MarkFramesWithItemsAndImagesModified(&mList);
   }
 
   mBuilder.EnterPresShell(mBuilder.RootReferenceFrame());
 
-  nsTArray<nsIFrame*> modifiedFrames =
-    GetModifiedFrames(&mBuilder);
+  // We set the override dirty regions during ComputeRebuildRegion or in
+  // nsLayoutUtils::InvalidateForDisplayPortChange. The display port change also
+  // marks the frame modified, so those regions are cleared here as well.
+  AutoClearFramePropsArray modifiedFrames(Move(GetModifiedFrames(&mBuilder)));
 
   // Do not allow partial builds if the retained display list is empty, or if
   // ShouldBuildPartial heuristic fails.
   const bool shouldBuildPartial = !mList.IsEmpty() && ShouldBuildPartial(modifiedFrames);
 
   if (mPreviousCaret != mBuilder.GetCaretFrame()) {
     if (mPreviousCaret) {
       if (mBuilder.MarkFrameModifiedDuringBuilding(mPreviousCaret)) {
-        modifiedFrames.AppendElement(mPreviousCaret);
+        modifiedFrames.mFrames.AppendElement(mPreviousCaret);
       }
     }
 
     if (mBuilder.GetCaretFrame()) {
       if (mBuilder.MarkFrameModifiedDuringBuilding(mBuilder.GetCaretFrame())) {
-        modifiedFrames.AppendElement(mBuilder.GetCaretFrame());
+        modifiedFrames.mFrames.AppendElement(mBuilder.GetCaretFrame());
       }
     }
 
     mPreviousCaret = mBuilder.GetCaretFrame();
   }
 
   nsRect modifiedDirty;
   AnimatedGeometryRoot* modifiedAGR = nullptr;
-  nsTArray<nsIFrame*> framesWithProps;
-  bool merged = false;
-  if (shouldBuildPartial &&
-      ComputeRebuildRegion(modifiedFrames, &modifiedDirty,
-                           &modifiedAGR, &framesWithProps)) {
-    modifiedDirty.IntersectRect(modifiedDirty, mBuilder.RootReferenceFrame()->GetVisualOverflowRectRelativeToSelf());
-
-    PreProcessDisplayList(&mList, modifiedAGR);
-
-    nsDisplayList modifiedDL;
-    if (!modifiedDirty.IsEmpty() || !framesWithProps.IsEmpty()) {
-      mBuilder.SetDirtyRect(modifiedDirty);
-      mBuilder.SetPartialUpdate(true);
-      mBuilder.RootReferenceFrame()->BuildDisplayListForStackingContext(&mBuilder, &modifiedDL);
-      nsLayoutUtils::AddExtraBackgroundItems(mBuilder, modifiedDL, mBuilder.RootReferenceFrame(),
-                                             nsRect(nsPoint(0, 0), mBuilder.RootReferenceFrame()->GetSize()),
-                                             mBuilder.RootReferenceFrame()->GetVisualOverflowRectRelativeToSelf(),
-                                             aBackstop);
-      mBuilder.SetPartialUpdate(false);
-
-      //printf_stderr("Painting --- Modified list (dirty %d,%d,%d,%d):\n",
-      //      modifiedDirty.x, modifiedDirty.y, modifiedDirty.width, modifiedDirty.height);
-      //nsFrame::PrintDisplayList(&mBuilder, modifiedDL);
-
-    } else {
-      // TODO: We can also skip layer building and painting if
-      // PreProcessDisplayList didn't end up changing anything
-      // Invariant: display items should have their original state here.
-      // printf_stderr("Skipping display list building since nothing needed to be done\n");
-    }
-
-    // |modifiedDL| can sometimes be empty here. We still perform the
-    // display list merging to prune unused items (for example, items that
-    // are not visible anymore) from the old list.
-    // TODO: Optimization opportunity. In this case, MergeDisplayLists()
-    // unnecessarily creates a hashtable of the old items.
-    Maybe<const ActiveScrolledRoot*> dummy;
-    MergeDisplayLists(&modifiedDL, &mList, &mList, dummy);
-
-    //printf_stderr("Painting --- Merged list:\n");
-    //nsFrame::PrintDisplayList(&mBuilder, mList);
-
-    merged = true;
+  AutoClearFramePropsArray framesWithProps;
+  if (!shouldBuildPartial ||
+      !ComputeRebuildRegion(modifiedFrames, &modifiedDirty,
+                           &modifiedAGR, &framesWithProps.mFrames)) {
+    mBuilder.LeavePresShell(mBuilder.RootReferenceFrame(), &mList);
+    return false;
   }
 
-  mBuilder.LeavePresShell(mBuilder.RootReferenceFrame(), &mList);
+  modifiedDirty.IntersectRect(modifiedDirty, mBuilder.RootReferenceFrame()->GetVisualOverflowRectRelativeToSelf());
+
+  PreProcessDisplayList(&mList, modifiedAGR);
+
+  nsDisplayList modifiedDL;
+  if (!modifiedDirty.IsEmpty() || !framesWithProps.IsEmpty()) {
+    mBuilder.SetDirtyRect(modifiedDirty);
+    mBuilder.SetPartialUpdate(true);
+    mBuilder.RootReferenceFrame()->BuildDisplayListForStackingContext(&mBuilder, &modifiedDL);
+    nsLayoutUtils::AddExtraBackgroundItems(mBuilder, modifiedDL, mBuilder.RootReferenceFrame(),
+                                           nsRect(nsPoint(0, 0), mBuilder.RootReferenceFrame()->GetSize()),
+                                           mBuilder.RootReferenceFrame()->GetVisualOverflowRectRelativeToSelf(),
+                                           aBackstop);
+    mBuilder.SetPartialUpdate(false);
+
+    //printf_stderr("Painting --- Modified list (dirty %d,%d,%d,%d):\n",
+    //      modifiedDirty.x, modifiedDirty.y, modifiedDirty.width, modifiedDirty.height);
+    //nsFrame::PrintDisplayList(&mBuilder, modifiedDL);
 
-  // We set the override dirty regions during ComputeRebuildRegion or in
-  // nsLayoutUtils::InvalidateForDisplayPortChange. The display port change also
-  // marks the frame modified, so those regions are cleared here as well.
-  ClearFrameProps(modifiedFrames);
-  ClearFrameProps(framesWithProps);
+  } else {
+    // TODO: We can also skip layer building and painting if
+    // PreProcessDisplayList didn't end up changing anything
+    // Invariant: display items should have their original state here.
+    // printf_stderr("Skipping display list building since nothing needed to be done\n");
+  }
 
-  return merged;
+  // |modifiedDL| can sometimes be empty here. We still perform the
+  // display list merging to prune unused items (for example, items that
+  // are not visible anymore) from the old list.
+  // TODO: Optimization opportunity. In this case, MergeDisplayLists()
+  // unnecessarily creates a hashtable of the old items.
+  Maybe<const ActiveScrolledRoot*> dummy;
+  MergeDisplayLists(&modifiedDL, &mList, &mList, dummy);
+
+  //printf_stderr("Painting --- Merged list:\n");
+  //nsFrame::PrintDisplayList(&mBuilder, mList);
+
+  mBuilder.LeavePresShell(mBuilder.RootReferenceFrame(), &mList);
+  return true;
 }