Bug 1415485 - Make the pref layout.display-list.retain live again r=mattwoodrow

MozReview-Commit-ID: 6QFc4Sq3WiC

--HG--
extra : rebase_source : 268c2959d38e955d1af9ab39fdb8cce0146788e3
extra : intermediate-source : 7ee9657574a2ee223b6184d1657803a0d382be59
extra : source : 558439637eba09eaae8a653ffc77d5830b16ff42

gfx/thebes/gfxPrefs.h

@@ -644,7 +644,7 @@ private:
   DECL_GFX_PREF(Live, "layout.css.touch_action.enabled",       TouchActionEnabled, bool, false);
 
   DECL_GFX_PREF(Live, "layout.display-list.build-twice",       LayoutDisplayListBuildTwice, bool, false);
-  DECL_GFX_PREF(Once, "layout.display-list.retain",            LayoutRetainDisplayList, bool, true);
+  DECL_GFX_PREF(Live, "layout.display-list.retain",            LayoutRetainDisplayList, bool, true);
   DECL_GFX_PREF(Live, "layout.display-list.rebuild-frame-limit", LayoutRebuildFrameLimit, uint32_t, 500);
   DECL_GFX_PREF(Live, "layout.display-list.dump",              LayoutDumpDisplayList, bool, false);
   DECL_GFX_PREF(Live, "layout.display-list.dump-content",      LayoutDumpDisplayListContent, bool, false);

layout/base/nsLayoutUtils.cpp

@@ -3619,6 +3619,33 @@ nsLayoutUtils::AddExtraBackgroundItems(nsDisplayListBuilder& aBuilder,
   }
 }
 
+/**
+ * Returns a retained display list builder for frame |aFrame|. If there is no
+ * retained display list builder property set for the frame, and if the flag
+ * |aRetainingEnabled| is true, a new retained display list builder is created,
+ * stored as a property for the frame, and returned.
+ */
+static RetainedDisplayListBuilder*
+GetOrCreateRetainedDisplayListBuilder(nsIFrame* aFrame, bool aRetainingEnabled,
+                                      bool aBuildCaret)
+{
+  RetainedDisplayListBuilder* retainedBuilder =
+    aFrame->GetProperty(RetainedDisplayListBuilder::Cached());
+
+  if (retainedBuilder) {
+    return retainedBuilder;
+  }
+
+  if (aRetainingEnabled) {
+    retainedBuilder =
+      new RetainedDisplayListBuilder(aFrame, nsDisplayListBuilderMode::PAINTING,
+                                     aBuildCaret);
+    aFrame->SetProperty(RetainedDisplayListBuilder::Cached(), retainedBuilder);
+  }
+
+  return retainedBuilder;
+}
+
 nsresult
 nsLayoutUtils::PaintFrame(gfxContext* aRenderingContext, nsIFrame* aFrame,
                           const nsRegion& aDirtyRegion, nscolor aBackstop,
@@ -3655,31 +3682,28 @@ nsLayoutUtils::PaintFrame(gfxContext* aRenderingContext, nsIFrame* aFrame,
 
   TimeStamp startBuildDisplayList = TimeStamp::Now();
 
+  const bool buildCaret = !(aFlags & PaintFrameFlags::PAINT_HIDE_CARET);
+  const bool isForPainting = (aFlags & PaintFrameFlags::PAINT_WIDGET_LAYERS) &&
+    aBuilderMode == nsDisplayListBuilderMode::PAINTING;
+
+  // Retained display list builder is currently only used for content processes.
+  const bool retainingEnabled = isForPainting &&
+    gfxPrefs::LayoutRetainDisplayList() && XRE_IsContentProcess();
+
+  RetainedDisplayListBuilder* retainedBuilder =
+    GetOrCreateRetainedDisplayListBuilder(aFrame, retainingEnabled, buildCaret);
+
+  // Only use the retained display list builder if the retaining is currently
+  // enabled. This check is needed because it is possible that the pref has been
+  // disabled after creating the retained display list builder.
+  const bool useRetainedBuilder = retainedBuilder && retainingEnabled;
+
   Maybe<nsDisplayListBuilder> nonRetainedBuilder;
   Maybe<nsDisplayList> nonRetainedList;
   nsDisplayListBuilder* builderPtr = nullptr;
   nsDisplayList* listPtr = nullptr;
-  RetainedDisplayListBuilder* retainedBuilder = nullptr;
 
-  const bool buildCaret = !(aFlags & PaintFrameFlags::PAINT_HIDE_CARET);
-
-  // Enable display list retaining if the pref is set and if we are in a
-  // content process.
-  const bool retainDisplayList =
-    gfxPrefs::LayoutRetainDisplayList() && XRE_IsContentProcess();
-
-  if (retainDisplayList &&
-      aBuilderMode == nsDisplayListBuilderMode::PAINTING &&
-      (aFlags & PaintFrameFlags::PAINT_WIDGET_LAYERS)) {
-    retainedBuilder = aFrame->GetProperty(RetainedDisplayListBuilder::Cached());
-
-    if (!retainedBuilder) {
-      retainedBuilder =
-        new RetainedDisplayListBuilder(aFrame, aBuilderMode, buildCaret);
-      aFrame->SetProperty(RetainedDisplayListBuilder::Cached(), retainedBuilder);
-    }
-
-    MOZ_ASSERT(retainedBuilder);
+  if (useRetainedBuilder) {
     builderPtr = retainedBuilder->Builder();
     listPtr = retainedBuilder->List();
   } else {
@@ -3688,6 +3712,16 @@ nsLayoutUtils::PaintFrame(gfxContext* aRenderingContext, nsIFrame* aFrame,
     nonRetainedList.emplace();
     listPtr = nonRetainedList.ptr();
   }
+
+  // Retained builder exists, but display list retaining is disabled.
+  if (!useRetainedBuilder && retainedBuilder) {
+    // Clear the modified frames lists and frame properties.
+    retainedBuilder->ClearModifiedFrameProps();
+
+    // Clear the retained display list.
+    retainedBuilder->List()->DeleteAll(retainedBuilder->Builder());
+  }
+
   nsDisplayListBuilder& builder = *builderPtr;
   nsDisplayList& list = *listPtr;
 
@@ -3817,14 +3851,12 @@ nsLayoutUtils::PaintFrame(gfxContext* aRenderingContext, nsIFrame* aFrame,
           builder.IsBuildingLayerEventRegions() &&
           nsLayoutUtils::HasDocumentLevelListenersForApzAwareEvents(presShell));
 
-      const bool paintedPreviously =
-        aFrame->HasProperty(nsIFrame::ModifiedFrameList());
-
       // Attempt to do a partial build and merge into the existing list.
       // This calls BuildDisplayListForStacking context on a subset of the
       // viewport.
       bool merged = false;
-      if (retainedBuilder && paintedPreviously) {
+
+      if (useRetainedBuilder) {
         merged = retainedBuilder->AttemptPartialUpdate(aBackstop);
       }
 
@@ -4058,12 +4090,11 @@ nsLayoutUtils::PaintFrame(gfxContext* aRenderingContext, nsIFrame* aFrame,
     AUTO_PROFILER_TRACING("Paint", "DisplayListResources");
 
     // Flush the list so we don't trigger the IsEmpty-on-destruction assertion
-    if (!retainedBuilder) {
+    if (!useRetainedBuilder) {
       list.DeleteAll(&builder);
-      builder.EndFrame();
-    } else {
-      builder.EndFrame();
     }
+
+    builder.EndFrame();
   }
   return NS_OK;
 }

layout/painting/RetainedDisplayListBuilder.cpp

@@ -792,9 +792,21 @@ ClearFrameProps(nsTArray<nsIFrame*>& aFrames)
   }
 }
 
+void
+RetainedDisplayListBuilder::ClearModifiedFrameProps()
+{
+  nsTArray<nsIFrame*> modifiedFrames =
+    GetModifiedFrames(mBuilder.RootReferenceFrame());
+
+  ClearFrameProps(modifiedFrames);
+}
+
 bool
 RetainedDisplayListBuilder::AttemptPartialUpdate(nscolor aBackstop)
 {
+  const bool hasInvalidations =
+    mBuilder.RootReferenceFrame()->HasProperty(nsIFrame::ModifiedFrameList());
+
   mBuilder.RemoveModifiedWindowDraggingRegion();
   if (mBuilder.ShouldSyncDecodeImages()) {
     MarkFramesWithItemsAndImagesModified(&mList);
@@ -808,7 +820,7 @@ RetainedDisplayListBuilder::AttemptPartialUpdate(nscolor aBackstop)
   // Do not allow partial builds if the retained display list is empty, or if
   // ShouldBuildPartial heuristic fails.
   const bool shouldBuildPartial =
-    !mList.IsEmpty() && ShouldBuildPartial(modifiedFrames);
+    hasInvalidations && !mList.IsEmpty() && ShouldBuildPartial(modifiedFrames);
 
   if (mPreviousCaret != mBuilder.GetCaretFrame()) {
     if (mPreviousCaret) {

layout/painting/RetainedDisplayListBuilder.h

@@ -27,6 +27,14 @@ struct RetainedDisplayListBuilder {
 
   bool AttemptPartialUpdate(nscolor aBackstop);
 
+  /**
+   * Iterates through the display list builder reference frame document and
+   * subdocuments, and clears the modified frame lists from the root frames.
+   * Also clears the frame properties set by RetainedDisplayListBuilder for all
+   * the frames in the modified frame lists.
+   */
+  void ClearModifiedFrameProps();
+
   NS_DECLARE_FRAME_PROPERTY_DELETABLE(Cached, RetainedDisplayListBuilder)
 
 private: