Backed out changeset 32958aa32cb5 (bug 1415485) for frequently failing for failing reftest layout/reftests/async-scrolling/position-fixed-in-scroll-container.html on Linux x64 QuantumRender. r=backout a=backout on a CLOSED TREE
authorSebastian Hengst <archaeopteryx@coole-files.de>
Tue, 14 Nov 2017 14:47:14 +0200
changeset 436226 077422bf7047cbfc8490aaa193bef3f0af72fb6a
parent 436225 3c8d0b156f9b53ab75c08e90e007583c315a9fc1
child 436227 b0a6b4678b2f7dfb499328946b95366775f71edd
child 436259 904b9bdfe37ad1c0432dc45a1b5cdb7de7187589
push id117
push userfmarier@mozilla.com
push dateTue, 28 Nov 2017 20:17:16 +0000
reviewersbackout, backout
bugs1415485
milestone59.0a1
backs out32958aa32cb5495eaefc478eb4d74a2222e06185
Backed out changeset 32958aa32cb5 (bug 1415485) for frequently failing for failing reftest layout/reftests/async-scrolling/position-fixed-in-scroll-container.html on Linux x64 QuantumRender. r=backout a=backout on a CLOSED TREE
gfx/thebes/gfxPrefs.h
layout/base/nsLayoutUtils.cpp
layout/generic/nsFrame.cpp
layout/painting/RetainedDisplayListBuilder.cpp
layout/painting/RetainedDisplayListBuilder.h
--- a/gfx/thebes/gfxPrefs.h
+++ b/gfx/thebes/gfxPrefs.h
@@ -639,17 +639,17 @@ private:
   DECL_GFX_PREF(Live, "layout.css.scroll-behavior.enabled",    ScrollBehaviorEnabled, bool, true);
   DECL_GFX_PREF(Live, "layout.css.scroll-behavior.spring-constant", ScrollBehaviorSpringConstant, float, 250.0f);
   DECL_GFX_PREF(Live, "layout.css.scroll-snap.prediction-max-velocity", ScrollSnapPredictionMaxVelocity, int32_t, 2000);
   DECL_GFX_PREF(Live, "layout.css.scroll-snap.prediction-sensitivity", ScrollSnapPredictionSensitivity, float, 0.750f);
   DECL_GFX_PREF(Live, "layout.css.scroll-snap.proximity-threshold", ScrollSnapProximityThreshold, int32_t, 200);
   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(Live, "layout.display-list.retain",            LayoutRetainDisplayList, bool, true);
+  DECL_GFX_PREF(Once, "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);
   DECL_GFX_PREF(Live, "layout.display-list.dump-parent",       LayoutDumpDisplayListParent, bool, false);
   DECL_GFX_PREF(Live, "layout.display-list.show-rebuild-area", LayoutDisplayListShowArea, bool, false);
 
   DECL_GFX_PREF(Live, "layout.event-regions.enabled",          LayoutEventRegionsEnabledDoNotUseDirectly, bool, false);
   DECL_GFX_PREF(Once, "layout.frame_rate",                     LayoutFrameRate, int32_t, -1);
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -3588,43 +3588,16 @@ nsLayoutUtils::AddExtraBackgroundItems(n
     canvasArea.IntersectRect(aCanvasArea, canvasArea);
     nsDisplayListBuilder::AutoBuildingDisplayList
       buildingDisplayList(&aBuilder, aFrame, canvasArea, canvasArea, false);
     presShell->AddCanvasBackgroundColorItem(
       aBuilder, aList, aFrame, canvasArea, aBackstop);
   }
 }
 
-/**
- * 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,
                           nsDisplayListBuilderMode aBuilderMode,
                           PaintFrameFlags aFlags)
 {
   AUTO_PROFILER_LABEL("nsLayoutUtils::PaintFrame", GRAPHICS);
 
@@ -3651,56 +3624,49 @@ nsLayoutUtils::PaintFrame(gfxContext* aR
   nsIPresShell* presShell = presContext->PresShell();
   nsRootPresContext* rootPresContext = presContext->GetRootPresContext();
   if (!rootPresContext) {
     return NS_OK;
   }
 
   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;
-
-  if (useRetainedBuilder) {
+  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);
     builderPtr = retainedBuilder->Builder();
     listPtr = retainedBuilder->List();
   } else {
     nonRetainedBuilder.emplace(aFrame, aBuilderMode, buildCaret);
     builderPtr = nonRetainedBuilder.ptr();
     nonRetainedList.emplace(builderPtr);
     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;
 
   builder.BeginFrame();
 
   if (aFlags & PaintFrameFlags::PAINT_IN_TRANSFORM) {
     builder.SetInTransform(true);
   }
@@ -3820,22 +3786,24 @@ nsLayoutUtils::PaintFrame(gfxContext* aR
       nsDisplayListBuilder::AutoCurrentScrollParentIdSetter idSetter(&builder, id);
 
       builder.SetVisibleRect(visibleRect);
       builder.SetIsBuilding(true);
       builder.SetAncestorHasApzAwareEventHandler(
           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 (useRetainedBuilder) {
+      if (retainedBuilder && paintedPreviously) {
         merged = retainedBuilder->AttemptPartialUpdate(aBackstop);
       }
 
       if (merged && gfxPrefs::LayoutDisplayListBuildTwice()) {
         merged = false;
         if (gfxPrefs::LayersDrawFPS()) {
           if (RefPtr<LayerManager> lm = builder.GetWidgetLayerManager()) {
             if (PaintTiming* pt = ClientLayerManager::MaybeGetPaintTiming(lm)) {
@@ -4053,21 +4021,22 @@ nsLayoutUtils::PaintFrame(gfxContext* aR
       layerManager->ScheduleComposite();
     }
   }
 
   {
     AUTO_PROFILER_TRACING("Paint", "DisplayListResources");
 
     // Flush the list so we don't trigger the IsEmpty-on-destruction assertion
-    if (!useRetainedBuilder) {
+    if (!retainedBuilder) {
       list.DeleteAll(&builder);
-    }
-
-    builder.EndFrame();
+      builder.EndFrame();
+    } else {
+      builder.EndFrame();
+    }
   }
   return NS_OK;
 }
 
 /**
  * Uses a binary search for find where the cursor falls in the line of text
  * It also keeps track of the part of the string that has already been measured
  * so it doesn't have to keep measuring the same text over and over
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -1035,16 +1035,24 @@ nsIFrame::MarkNeedsDisplayItemRebuild()
     // If the list starts getting too big, then just mark the root frame
     // as needing a rebuild.
     rootFrame->MarkNeedsDisplayItemRebuild();
     return;
   }
 
   modifiedFrames->AppendElement(this);
 
+  // TODO: this is a bit of a hack. We are using ModifiedFrameList property to
+  // decide whether we are trying to reuse the display list.
+  if (displayRoot != rootFrame &&
+      !displayRoot->HasProperty(nsIFrame::ModifiedFrameList())) {
+    displayRoot->SetProperty(nsIFrame::ModifiedFrameList(),
+                             new nsTArray<nsIFrame*>());
+  }
+
   MOZ_ASSERT(PresContext()->LayoutPhaseCount(eLayoutPhase_DisplayListBuilding) == 0);
   SetFrameIsModified(true);
 
   // Hopefully this is cheap, but we could use a frame state bit to note
   // the presence of dependencies to speed it up.
   DisplayItemArray* items = GetProperty(DisplayItems());
   if (items) {
     for (nsDisplayItem* i : *items) {
--- a/layout/painting/RetainedDisplayListBuilder.cpp
+++ b/layout/painting/RetainedDisplayListBuilder.cpp
@@ -772,25 +772,16 @@ ClearFrameProps(nsTArray<nsIFrame*>& aFr
       f->DeleteProperty(nsDisplayListBuilder::DisplayListBuildingRect());
       f->DeleteProperty(nsDisplayListBuilder::DisplayListBuildingDisplayPortRect());
     }
 
     f->SetFrameIsModified(false);
   }
 }
 
-void
-RetainedDisplayListBuilder::ClearModifiedFrameProps()
-{
-  nsTArray<nsIFrame*> modifiedFrames =
-    GetModifiedFrames(mBuilder.RootReferenceFrame());
-
-  ClearFrameProps(modifiedFrames);
-}
-
 bool
 RetainedDisplayListBuilder::AttemptPartialUpdate(nscolor aBackstop)
 {
   mBuilder.RemoveModifiedWindowDraggingRegion();
   if (mBuilder.ShouldSyncDecodeImages()) {
     MarkFramesWithItemsAndImagesModified(&mList);
   }
 
--- a/layout/painting/RetainedDisplayListBuilder.h
+++ b/layout/painting/RetainedDisplayListBuilder.h
@@ -23,24 +23,16 @@ struct RetainedDisplayListBuilder {
   }
 
   nsDisplayListBuilder* Builder() { return &mBuilder; }
 
   nsDisplayList* List() { return &mList; }
 
   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:
   void PreProcessDisplayList(nsDisplayList* aList, AnimatedGeometryRoot* aAGR);
 
   void MergeDisplayLists(nsDisplayList* aNewList,
                          nsDisplayList* aOldList,
                          nsDisplayList* aOutList,