Bug 1526972 - P5: Avoid HasModifiedFrame check for new nsDisplayItems. r=miko
☠☠ backed out by 9f0f38c38ccc ☠ ☠
authorDan Glastonbury <dan.glastonbury@gmail.com>
Mon, 15 Apr 2019 00:23:15 +0000
changeset 469451 2fb940b13971110b8ef299730a03f37f65ae372a
parent 469450 8543b9d465212a64ee52a84f1da4760cfda702fa
child 469452 2ea2f8533078605736d98959a36c596414d793de
push id112792
push userncsoregi@mozilla.com
push dateMon, 15 Apr 2019 09:49:11 +0000
treeherdermozilla-inbound@a57f27d3ccd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmiko
bugs1526972
milestone68.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 1526972 - P5: Avoid HasModifiedFrame check for new nsDisplayItems. r=miko aBuilder->InInvalidSubtree() tracks the modified state. Save the state during construction of nsDisplayItem and use in ProcessItemFromNewList. Depends on D24462 Differential Revision: https://phabricator.services.mozilla.com/D26138
layout/painting/RetainedDisplayListBuilder.cpp
layout/painting/RetainedDisplayListHelpers.h
layout/painting/nsDisplayList.cpp
layout/painting/nsDisplayList.h
--- a/layout/painting/RetainedDisplayListBuilder.cpp
+++ b/layout/painting/RetainedDisplayListBuilder.cpp
@@ -208,18 +208,17 @@ void RetainedDisplayListBuilder::Increme
   MOZ_ASSERT(subDocFrame);
 
   nsIPresShell* presShell = subDocFrame->GetSubdocumentPresShellForPainting(0);
   MOZ_ASSERT(presShell);
 
   mBuilder.IncrementPresShellPaintCount(presShell);
 }
 
-static bool AnyContentAncestorModified(nsIFrame* aFrame,
-                                       nsIFrame* aStopAtFrame = nullptr) {
+bool AnyContentAncestorModified(nsIFrame* aFrame, nsIFrame* aStopAtFrame) {
   for (nsIFrame* f = aFrame; f;
        f = nsLayoutUtils::GetParentOrPlaceholderForCrossDoc(f)) {
     if (f->IsFrameModified()) {
       return true;
     }
 
     if (aStopAtFrame && f == aStopAtFrame) {
       break;
@@ -269,16 +268,24 @@ static void UpdateASR(nsDisplayItem* aIt
   }
 
   wrapList->SetActiveScrolledRoot(ActiveScrolledRoot::PickAncestor(
       wrapList->GetFrameActiveScrolledRoot(), *asr));
 
   wrapList->UpdateHitTestInfoActiveScrolledRoot(*asr);
 }
 
+OldItemInfo::OldItemInfo(nsDisplayItem* aItem)
+    : mItem(aItem), mUsed(false), mDiscarded(false) {
+  if (mItem) {
+    // Clear cached modified frame state when adding an item to the old list.
+    mItem->SetModifiedFrame(false);
+  }
+}
+
 void OldItemInfo::AddedMatchToMergedList(RetainedDisplayListBuilder* aBuilder,
                                          MergedListIndex aIndex) {
   AddedToMergedList(aIndex);
 }
 
 void OldItemInfo::Discard(RetainedDisplayListBuilder* aBuilder,
                           nsTArray<MergedListIndex>&& aDirectPredecessors) {
   MOZ_ASSERT(!IsUsed());
@@ -317,17 +324,19 @@ class MergeState {
         mResultIsModified(false) {
     mMergedDAG.EnsureCapacityFor(mOldDAG);
     MOZ_RELEASE_ASSERT(mOldItems.Length() == mOldDAG.Length());
   }
 
   Maybe<MergedListIndex> ProcessItemFromNewList(
       nsDisplayItem* aNewItem, const Maybe<MergedListIndex>& aPreviousItem) {
     OldListIndex oldIndex;
-    if (!HasModifiedFrame(aNewItem) &&
+    MOZ_DIAGNOSTIC_ASSERT(aNewItem->HasModifiedFrame() ==
+                          HasModifiedFrame(aNewItem));
+    if (!aNewItem->HasModifiedFrame() &&
         HasMatchingItemInOldList(aNewItem, &oldIndex)) {
       nsDisplayItem* oldItem = mOldItems[oldIndex.val].mItem;
       MOZ_DIAGNOSTIC_ASSERT(oldItem->GetPerFrameKey() ==
                                 aNewItem->GetPerFrameKey() &&
                             oldItem->Frame() == aNewItem->Frame());
       if (!mOldItems[oldIndex.val].IsChanged()) {
         MOZ_DIAGNOSTIC_ASSERT(!mOldItems[oldIndex.val].IsUsed());
         nsDisplayItem* destItem;
--- a/layout/painting/RetainedDisplayListHelpers.h
+++ b/layout/painting/RetainedDisplayListHelpers.h
@@ -134,18 +134,17 @@ class DirectedAcyclicGraph {
   nsTArray<NodeInfo> mNodesInfo;
   nsTArray<Index<T>> mDirectPredecessorList;
 };
 
 struct RetainedDisplayListBuilder;
 class nsDisplayItem;
 
 struct OldItemInfo {
-  explicit OldItemInfo(nsDisplayItem* aItem)
-      : mItem(aItem), mUsed(false), mDiscarded(false) {}
+  explicit OldItemInfo(nsDisplayItem* aItem);
 
   void AddedToMergedList(MergedListIndex aIndex) {
     MOZ_ASSERT(!IsUsed());
     mUsed = true;
     mIndex = aIndex;
     mItem = nullptr;
   }
 
@@ -164,9 +163,12 @@ struct OldItemInfo {
 
   nsDisplayItem* mItem;
   bool mUsed;
   bool mDiscarded;
   MergedListIndex mIndex;
   nsTArray<MergedListIndex> mDirectPredecessors;
 };
 
+bool AnyContentAncestorModified(nsIFrame* aFrame,
+                                nsIFrame* aStopAtFrame = nullptr);
+
 #endif  // RETAINEDDISPLAYLISTHELPERS_H_
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -3216,16 +3216,28 @@ bool nsDisplayItem::ForceActiveLayers() 
   if (!sForceCached) {
     Preferences::AddBoolVarCache(&sForce, "layers.force-active", false);
     sForceCached = true;
   }
 
   return sForce;
 }
 
+bool nsDisplayItem::HasModifiedFrame() const {
+  return mItemFlags.contains(ItemFlag::ModifiedFrame);
+}
+
+void nsDisplayItem::SetModifiedFrame(bool aModified) {
+  if (aModified) {
+    mItemFlags += ItemFlag::ModifiedFrame;
+  } else {
+    mItemFlags -= ItemFlag::ModifiedFrame;
+  }
+}
+
 bool nsDisplayItem::HasDeletedFrame() const {
   return mItemFlags.contains(ItemFlag::DeletedFrame) ||
          (GetType() == DisplayItemType::TYPE_REMOTE &&
           !static_cast<const nsDisplayRemote*>(this)->GetFrameLoader());
 }
 
 int32_t nsDisplayItem::ZIndex() const { return mFrame->ZIndex(); }
 
--- a/layout/painting/nsDisplayList.h
+++ b/layout/painting/nsDisplayList.h
@@ -2106,21 +2106,32 @@ MOZ_ALWAYS_INLINE T* MakeDisplayItem(nsD
         if (!did->HasMergedFrames()) {
           item->SetDisplayItemData(did, did->GetLayer()->Manager());
         }
         break;
       }
     }
   }
 
+  if (aBuilder->InInvalidSubtree() ||
+      item->FrameForInvalidation()->IsFrameModified()) {
+    item->SetModifiedFrame(true);
+  }
+
 #ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
   if (aBuilder->IsRetainingDisplayList() && !aBuilder->IsInPageSequence() &&
       aBuilder->IsBuilding()) {
     AssertUniqueItem(item);
   }
+
+  // Verify that InInvalidSubtree matches invalidation frame's modified state.
+  if (aBuilder->InInvalidSubtree()) {
+    MOZ_DIAGNOSTIC_ASSERT(
+        AnyContentAncestorModified(item->FrameForInvalidation()));
+  }
 #endif
 
   return item;
 }
 
 /**
  * This is the unit of rendering and event testing. Each instance of this
  * class represents an entity that can be drawn on the screen, e.g., a
@@ -2297,16 +2308,19 @@ class nsDisplayItem : public nsDisplayIt
   }
 
   /**
    * @return the nsIFrame that provides the style data, and should
    * be checked when deciding if this display item can be reused.
    */
   virtual nsIFrame* FrameForInvalidation() const { return mFrame; }
 
+  bool HasModifiedFrame() const;
+  void SetModifiedFrame(bool aModified);
+
   bool HasDeletedFrame() const;
 
   virtual nsIFrame* StyleFrame() const { return mFrame; }
 
   /**
    * Compute the used z-index of our frame; returns zero for elements to which
    * z-index does not apply, and for z-index:auto.
    * @note This can be overridden, @see nsDisplayWrapList::SetOverrideZIndex.
@@ -2992,16 +3006,17 @@ class nsDisplayItem : public nsDisplayIt
  protected:
   void SetDeletedFrame();
 
   typedef bool (*PrefFunc)(void);
   bool ShouldUseAdvancedLayer(LayerManager* aManager, PrefFunc aFunc) const;
   bool CanUseAdvancedLayer(LayerManager* aManager) const;
 
   enum class ItemFlag {
+    ModifiedFrame,
     DeletedFrame,
     ForceNotVisible,
     DisableSubpixelAA,
     CantBeReused,
     ReusedItem,
     BackfaceHidden,
     Combines3DTransformWithAncestors,
     PaintRectValid,