Bug 1529831 - Don't create wrap lists for positioned frames that are leaves. r=miko
authorMatt Woodrow <mwoodrow@mozilla.com>
Tue, 05 Mar 2019 20:45:40 +0000
changeset 520389 ec0a441cb8f91ba7dd0ccb91abb295c2515d20dd
parent 520388 904012057423b23bf9a382da27a40869c6c00301
child 520390 c57bc49cdb13a66e16ca34aec71629d3d316be37
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmiko
bugs1529831
milestone67.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 1529831 - Don't create wrap lists for positioned frames that are leaves. r=miko Differential Revision: https://phabricator.services.mozilla.com/D20773
layout/generic/nsFrame.cpp
layout/generic/nsIFrame.h
layout/painting/nsDisplayList.h
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -900,16 +900,30 @@ bool nsIFrame::HasDisplayItems() {
 bool nsIFrame::HasDisplayItem(nsDisplayItem* aItem) {
   DisplayItemArray* items = GetProperty(DisplayItems());
   if (!items) {
     return false;
   }
   return items->Contains(aItem);
 }
 
+bool nsIFrame::HasDisplayItem(uint32_t aKey) {
+  DisplayItemArray* items = GetProperty(DisplayItems());
+  if (!items) {
+    return false;
+  }
+
+  for (nsDisplayItem* i : *items) {
+    if (i->GetPerFrameKey() == aKey) {
+      return true;
+    }
+  }
+  return false;
+}
+
 void nsIFrame::RemoveDisplayItemDataForDeletion() {
   // Destroying a WebRenderUserDataTable can cause destruction of other objects
   // which can remove frame properties in their destructor. If we delete a frame
   // property it runs the destructor of the stored object in the middle of
   // updating the frame property table, so if the destruction of that object
   // causes another update to the frame property table it would leave the frame
   // property table in an inconsistent state. So we remove it from the table and
   // then destroy it. (bug 1530657)
@@ -3399,28 +3413,57 @@ void nsIFrame::BuildDisplayListForStacki
   }
 
   aList->AppendToTop(&resultList);
 }
 
 static nsDisplayItem* WrapInWrapList(nsDisplayListBuilder* aBuilder,
                                      nsIFrame* aFrame, nsDisplayList* aList,
                                      const ActiveScrolledRoot* aContainerASR,
-                                     bool aCanSkipWrapList = false) {
+                                     bool aBuiltContainerItem = false) {
   nsDisplayItem* item = aList->GetBottom();
   if (!item) {
     return nullptr;
   }
 
-  if (aCanSkipWrapList) {
-    MOZ_ASSERT(!item->GetAbove());
+  // We need a wrap list if there are multiple items, or if the single
+  // item has a different frame.
+  bool needsWrapList = item->GetAbove() || item->Frame() != aFrame;
+
+  // If we don't need a wrap list, and we're doing a full build, or if
+  // we have an explicit container item that guarantees to wrap all other
+  // items then we can skip.
+  if (!needsWrapList && (!aBuilder->IsPartialUpdate() || aBuiltContainerItem)) {
     aList->RemoveBottom();
     return item;
   }
 
+  // If we're doing a partial build and we didn't need a wrap list
+  // previously then we can try to work from there.
+  if (aBuilder->IsPartialUpdate() &&
+      !aFrame->HasDisplayItem(uint32_t(DisplayItemType::TYPE_WRAP_LIST))) {
+    // If we now need a wrap list, mark this frame as modified so that merging
+    // removes the unwrapped item. We don't need to add to the dirty rect since
+    // not needing a wrap list means that we must have only had a display item
+    // from this frame, which we're already building items for. All new items
+    // will have already been included in the dirty area.
+    if (needsWrapList) {
+      aBuilder->MarkFrameModifiedDuringBuilding(aFrame);
+    } else {
+      aList->RemoveBottom();
+      return item;
+    }
+  }
+
+  // The last case is when we previously had a wrap list, but no longer
+  // need it. Unfortunately we can't differentiate this case from a partial
+  // build where other children exist but we just didn't build them.
+  // TODO:RetainedDisplayListBuilder's merge phase has the full list and
+  // could strip them out.
+
   // Clear clip rect for the construction of the items below. Since we're
   // clipping all their contents, they themselves don't need to be clipped.
   return MakeDisplayItem<nsDisplayWrapList>(aBuilder, aFrame, aList,
                                             aContainerASR, true);
 }
 
 /**
  * Check if a frame should be visited for building display list.
@@ -3724,27 +3767,27 @@ void nsIFrame::BuildDisplayListForChild(
       parent == this ? ourDisp : parent->StyleDisplay();
   if (ApplyOverflowClipping(aBuilder, parent, parentDisp, clipState)) {
     awayFromCommonPath = true;
   }
 
   nsDisplayList list;
   nsDisplayList extraPositionedDescendants;
   const ActiveScrolledRoot* wrapListASR;
-  bool canSkipWrapList = false;
+  bool builtContainerItem = false;
   if (isStackingContext) {
     // True stacking context.
     // For stacking contexts, BuildDisplayListForStackingContext handles
     // clipping and MarkAbsoluteFramesForDisplayList.
     nsDisplayListBuilder::AutoContainerASRTracker contASRTracker(aBuilder);
     child->BuildDisplayListForStackingContext(aBuilder, &list,
-                                              &canSkipWrapList);
+                                              &builtContainerItem);
     wrapListASR = contASRTracker.GetContainerASR();
     if (aBuilder->GetCaretFrame() == child) {
-      canSkipWrapList = false;
+      builtContainerItem = false;
     }
   } else {
     Maybe<nsRect> clipPropClip =
         child->GetClipPropClipRect(disp, effects, child->GetSize());
     if (clipPropClip) {
       aBuilder->IntersectVisibleRect(*clipPropClip);
       aBuilder->IntersectDirtyRect(*clipPropClip);
       clipState.ClipContentDescendants(*clipPropClip +
@@ -3790,17 +3833,17 @@ void nsIFrame::BuildDisplayListForChild(
         child, pseudoStack.BorderBackground(), differentAGR || isPositioned);
 
     aBuilder->AdjustWindowDraggingRegion(child);
     nsDisplayListBuilder::AutoContainerASRTracker contASRTracker(aBuilder);
     aBuilder->Check();
     child->BuildDisplayList(aBuilder, pseudoStack);
     aBuilder->Check();
     if (aBuilder->DisplayCaret(child, pseudoStack.Content())) {
-      canSkipWrapList = false;
+      builtContainerItem = false;
     }
     wrapListASR = contASRTracker.GetContainerASR();
 
     list.AppendToTop(pseudoStack.BorderBackground());
     list.AppendToTop(pseudoStack.BlockBorderBackgrounds());
     list.AppendToTop(pseudoStack.Floats());
     list.AppendToTop(pseudoStack.Content());
     list.AppendToTop(pseudoStack.Outlines());
@@ -3812,18 +3855,18 @@ void nsIFrame::BuildDisplayListForChild(
 
   buildingForChild.RestoreBuildingInvisibleItemsValue();
 
   if (isPositioned || isStackingContext ||
       (aFlags & DISPLAY_CHILD_FORCE_STACKING_CONTEXT)) {
     // Genuine stacking contexts, and positioned pseudo-stacking-contexts,
     // go in this level.
     if (!list.IsEmpty()) {
-      nsDisplayItem* item =
-          WrapInWrapList(aBuilder, child, &list, wrapListASR, canSkipWrapList);
+      nsDisplayItem* item = WrapInWrapList(aBuilder, child, &list, wrapListASR,
+                                           builtContainerItem);
       if (isSVG) {
         aLists.Content()->AppendToTop(item);
       } else {
         aLists.PositionedDescendants()->AppendToTop(item);
       }
     }
   } else if (!isSVG && disp->IsFloating(child)) {
     if (!list.IsEmpty()) {
--- a/layout/generic/nsIFrame.h
+++ b/layout/generic/nsIFrame.h
@@ -4132,16 +4132,17 @@ class nsIFrame : public nsQueryFrame {
     return mDisplayItemData;
   }
 
   void AddDisplayItem(nsDisplayItem* aItem);
   bool RemoveDisplayItem(nsDisplayItem* aItem);
   void RemoveDisplayItemDataForDeletion();
   bool HasDisplayItems();
   bool HasDisplayItem(nsDisplayItem* aItem);
+  bool HasDisplayItem(uint32_t aKey);
 
   bool ForceDescendIntoIfVisible() const { return mForceDescendIntoIfVisible; }
   void SetForceDescendIntoIfVisible(bool aForce) {
     mForceDescendIntoIfVisible = aForce;
   }
 
   bool BuiltDisplayList() { return mBuiltDisplayList; }
   void SetBuiltDisplayList(bool aBuilt) { mBuiltDisplayList = aBuilt; }
--- a/layout/painting/nsDisplayList.h
+++ b/layout/painting/nsDisplayList.h
@@ -2984,19 +2984,17 @@ class nsDisplayList {
    */
   nsDisplayList()
       : mLength(0), mIsOpaque(false), mForceTransparentSurface(false) {
     mTop = &mSentinel;
     mSentinel.mAbove = nullptr;
   }
 
   virtual ~nsDisplayList() {
-    if (mSentinel.mAbove) {
-      NS_WARNING("Nonempty list left over?");
-    }
+    MOZ_RELEASE_ASSERT(!mSentinel.mAbove, "Nonempty list left over?");
   }
 
   nsDisplayList(nsDisplayList&& aOther) {
     mIsOpaque = aOther.mIsOpaque;
     mForceTransparentSurface = aOther.mForceTransparentSurface;
 
     if (aOther.mSentinel.mAbove) {
       AppendToTop(&aOther);