Bug 1462497 - Add assertions to try diagnose which wrapper item went away. r=mstange, a=RyanVM
☠☠ backed out by 0f95c023462e ☠ ☠
authorMatt Woodrow <mwoodrow@mozilla.com>
Fri, 18 May 2018 20:50:04 +1200
changeset 470895 b14f835411dbb2fab3821075b8b0a850636f12fa
parent 470894 54ba8ee1c9ceefb0607f2a65ea6aedd984717113
child 470896 21f67c8e36fb517a2eb617334322f96ecd365813
push id9257
push userryanvm@gmail.com
push dateThu, 24 May 2018 15:49:59 +0000
treeherdermozilla-beta@5277d1553fd2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmstange, RyanVM
bugs1462497
milestone61.0
Bug 1462497 - Add assertions to try diagnose which wrapper item went away. r=mstange, a=RyanVM MozReview-Commit-ID: BkU5tTlgFum
layout/painting/RetainedDisplayListBuilder.cpp
layout/painting/RetainedDisplayListBuilder.h
layout/painting/nsDisplayList.h
--- a/layout/painting/RetainedDisplayListBuilder.cpp
+++ b/layout/painting/RetainedDisplayListBuilder.cpp
@@ -93,17 +93,19 @@ SelectAGRForFrame(nsIFrame* aFrame, Anim
 // Removes any display items that belonged to a frame that was deleted,
 // and mark frames that belong to a different AGR so that get their
 // items built again.
 // TODO: We currently descend into all children even if we don't have an AGR
 // to mark, as child stacking contexts might. It would be nice if we could
 // jump into those immediately rather than walking the entire thing.
 bool
 RetainedDisplayListBuilder::PreProcessDisplayList(RetainedDisplayList* aList,
-                                                  AnimatedGeometryRoot* aAGR)
+                                                  AnimatedGeometryRoot* aAGR,
+                                                  uint32_t aCallerKey,
+                                                  uint32_t aNestingDepth)
 {
   // The DAG merging algorithm does not have strong mechanisms in place to keep the
   // complexity of the resulting DAG under control. In some cases we can build up
   // edges very quickly. Detect those cases and force a full display list build if
   // we hit them.
   static const uint32_t kMaxEdgeRatio = 5;
   bool initializeDAG = !aList->mDAG.Length();
   if (!initializeDAG &&
@@ -135,30 +137,30 @@ RetainedDisplayListBuilder::PreProcessDi
           aList->mDAG.AddNode(Span<const MergedListIndex>(&previous, 1));
         }
       }
       continue;
     }
 
     size_t i = aList->mOldItems.Length();
     aList->mOldItems.AppendElement(OldItemInfo(item));
-    item->SetOldListIndex(aList, OldListIndex(i));
+    item->SetOldListIndex(aList, OldListIndex(i), aCallerKey, aNestingDepth);
     if (initializeDAG) {
       if (i == 0) {
         aList->mDAG.AddNode(Span<const MergedListIndex>());
       } else {
         MergedListIndex previous(i - 1);
         aList->mDAG.AddNode(Span<const MergedListIndex>(&previous, 1));
       }
     }
 
     nsIFrame* f = item->Frame();
 
     if (item->GetChildren()) {
-      if (!PreProcessDisplayList(item->GetChildren(), SelectAGRForFrame(f, aAGR))) {
+      if (!PreProcessDisplayList(item->GetChildren(), SelectAGRForFrame(f, aAGR), item->GetPerFrameKey(), aNestingDepth + 1)) {
         return false;
       }
     }
 
     // TODO: We should be able to check the clipped bounds relative
     // to the common AGR (of both the existing item and the invalidated
     // frame) and determine if they can ever intersect.
     if (aAGR && item->GetAnimatedGeometryRoot()->GetAsyncAGR() != aAGR) {
@@ -260,21 +262,22 @@ OldItemInfo::IsChanged()
  *
  * MergeState handles combining a new list of display items into an existing
  * DAG and computes the new DAG in a single pass.
  * Each time we add a new item, we resolve all dependencies for it, so that the resulting
  * list and DAG are built in topological ordering.
  */
 class MergeState {
 public:
-  MergeState(RetainedDisplayListBuilder* aBuilder, RetainedDisplayList& aOldList)
+  MergeState(RetainedDisplayListBuilder* aBuilder, RetainedDisplayList& aOldList, uint32_t aOuterKey)
     : mBuilder(aBuilder)
     , mOldList(&aOldList)
     , mOldItems(Move(aOldList.mOldItems))
     , mOldDAG(Move(*reinterpret_cast<DirectedAcyclicGraph<OldListUnits>*>(&aOldList.mDAG)))
+    , mOuterKey(aOuterKey)
     , mResultIsModified(false)
   {
     mMergedDAG.EnsureCapacityFor(mOldDAG);
   }
 
   MergedListIndex ProcessItemFromNewList(nsDisplayItem* aNewItem, const Maybe<MergedListIndex>& aPreviousItem) {
     OldListIndex oldIndex;
     if (!HasModifiedFrame(aNewItem) &&
@@ -284,17 +287,18 @@ public:
                             oldItem->Frame() == aNewItem->Frame());
       if (!mOldItems[oldIndex.val].IsChanged()) {
         MOZ_DIAGNOSTIC_ASSERT(!mOldItems[oldIndex.val].IsUsed());
         if (aNewItem->GetChildren()) {
           Maybe<const ActiveScrolledRoot*> containerASRForChildren;
           if (mBuilder->MergeDisplayLists(aNewItem->GetChildren(),
                                           oldItem->GetChildren(),
                                           aNewItem->GetChildren(),
-                                          containerASRForChildren)) {
+                                          containerASRForChildren,
+                                          aNewItem->GetPerFrameKey())) {
             mResultIsModified = true;
 
           }
           UpdateASR(aNewItem, containerASRForChildren);
           aNewItem->UpdateBounds(mBuilder->Builder());
         }
 
         AutoTArray<MergedListIndex, 2> directPredecessors = ProcessPredecessorsOfOldNode(oldIndex);
@@ -326,17 +330,17 @@ public:
 
   bool HasMatchingItemInOldList(nsDisplayItem* aItem, OldListIndex* aOutIndex)
   {
     nsIFrame::DisplayItemArray* items = aItem->Frame()->GetProperty(nsIFrame::DisplayItems());
     // Look for an item that matches aItem's frame and per-frame-key, but isn't the same item.
     for (nsDisplayItem* i : *items) {
       if (i != aItem && i->Frame() == aItem->Frame() &&
           i->GetPerFrameKey() == aItem->GetPerFrameKey()) {
-        *aOutIndex = i->GetOldListIndex(mOldList);
+        *aOutIndex = i->GetOldListIndex(mOldList, mOuterKey);
         return true;
       }
     }
     return false;
   }
 
   bool HasModifiedFrame(nsDisplayItem* aItem) {
     return AnyContentAncestorModified(aItem->FrameForInvalidation());
@@ -386,17 +390,17 @@ public:
     if (mOldItems[aNode.val].IsChanged() || HasModifiedFrame(item)) {
       mOldItems[aNode.val].Discard(mBuilder, Move(aDirectPredecessors));
       mResultIsModified = true;
     } else {
       if (item->GetChildren()) {
         Maybe<const ActiveScrolledRoot*> containerASRForChildren;
         nsDisplayList empty;
         if (mBuilder->MergeDisplayLists(&empty, item->GetChildren(), item->GetChildren(),
-                                        containerASRForChildren)) {
+                                        containerASRForChildren, item->GetPerFrameKey())) {
           mResultIsModified = true;
         }
         UpdateASR(item, containerASRForChildren);
         item->UpdateBounds(mBuilder->Builder());
       }
       if (item->GetType() == DisplayItemType::TYPE_SUBDOCUMENT) {
         mBuilder->IncrementSubDocPresShellPaintCount(item);
       }
@@ -475,16 +479,17 @@ public:
   Maybe<const ActiveScrolledRoot*> mContainerASR;
   nsTArray<OldItemInfo> mOldItems;
   DirectedAcyclicGraph<OldListUnits> mOldDAG;
   // Unfortunately we can't use strong typing for the hashtables
   // since they internally encode the type with the mOps pointer,
   // and assert when we try swap the contents
   nsDisplayList mMergedItems;
   DirectedAcyclicGraph<MergedListUnits> mMergedDAG;
+  uint32_t mOuterKey;
   bool mResultIsModified;
 };
 
 /**
  * Takes two display lists and merges them into an output list.
  *
  * Display lists wthout an explicit DAG are interpreted as linear DAGs (with a maximum
  * of one direct predecessor and one direct successor per node). We add the two DAGs
@@ -492,19 +497,20 @@ public:
  *
  * Once we've merged a list, we then retain the DAG (as part of the RetainedDisplayList
  * object) to use for future merges.
  */
 bool
 RetainedDisplayListBuilder::MergeDisplayLists(nsDisplayList* aNewList,
                                               RetainedDisplayList* aOldList,
                                               RetainedDisplayList* aOutList,
-                                              mozilla::Maybe<const mozilla::ActiveScrolledRoot*>& aOutContainerASR)
+                                              mozilla::Maybe<const mozilla::ActiveScrolledRoot*>& aOutContainerASR,
+                                              uint32_t aOuterKey)
 {
-  MergeState merge(this, *aOldList);
+  MergeState merge(this, *aOldList, aOuterKey);
 
   Maybe<MergedListIndex> previousItemIndex;
   while (nsDisplayItem* item = aNewList->RemoveBottom()) {
     previousItemIndex = Some(merge.ProcessItemFromNewList(item, previousItemIndex));
   }
 
   *aOutList = Move(merge.Finalize());
   aOutContainerASR = merge.mContainerASR;
--- a/layout/painting/RetainedDisplayListBuilder.h
+++ b/layout/painting/RetainedDisplayListBuilder.h
@@ -44,21 +44,23 @@ struct RetainedDisplayListBuilder {
    * Also clears the frame properties set by RetainedDisplayListBuilder for all
    * the frames in the modified frame lists.
    */
   void ClearFramesWithProps();
 
   NS_DECLARE_FRAME_PROPERTY_DELETABLE(Cached, RetainedDisplayListBuilder)
 
 private:
-  bool PreProcessDisplayList(RetainedDisplayList* aList, AnimatedGeometryRoot* aAGR);
+  bool PreProcessDisplayList(RetainedDisplayList* aList, AnimatedGeometryRoot* aAGR,
+                             uint32_t aCallerKey = 0, uint32_t aNestingDepth = 0);
   bool MergeDisplayLists(nsDisplayList* aNewList,
                          RetainedDisplayList* aOldList,
                          RetainedDisplayList* aOutList,
-                         mozilla::Maybe<const mozilla::ActiveScrolledRoot*>& aOutContainerASR);
+                         mozilla::Maybe<const mozilla::ActiveScrolledRoot*>& aOutContainerASR,
+                         uint32_t aOuterKey = 0);
 
   bool ComputeRebuildRegion(nsTArray<nsIFrame*>& aModifiedFrames,
                             nsRect* aOutDirty,
                             AnimatedGeometryRoot** aOutModifiedAGR,
                             nsTArray<nsIFrame*>& aOutFramesWithProps);
   bool ProcessFrame(nsIFrame* aFrame, nsDisplayListBuilder& aBuilder,
                     nsIFrame* aStopAtFrame, nsTArray<nsIFrame*>& aOutFramesWithProps,
                     const bool aStopAtStackingContext,
--- a/layout/painting/nsDisplayList.h
+++ b/layout/painting/nsDisplayList.h
@@ -2840,28 +2840,30 @@ public:
     mDisplayItemData = aDID;
   }
 
   mozilla::DisplayItemData* GetDisplayItemData() { return mDisplayItemData; }
 
   // Set the nsDisplayList that this item belongs to, and what
   // index it is within that list. Temporary state for merging
   // used by RetainedDisplayListBuilder.
-  void SetOldListIndex(nsDisplayList* aList, OldListIndex aIndex)
+  void SetOldListIndex(nsDisplayList* aList, OldListIndex aIndex, uint32_t aListKey, uint32_t aNestingDepth)
   {
 #ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
     mOldList = reinterpret_cast<uintptr_t>(aList);
+    mOldListKey = aListKey;
+    mOldNestingDepth = aNestingDepth;
 #endif
     mOldListIndex = aIndex;
   }
-  OldListIndex GetOldListIndex(nsDisplayList* aList)
+  OldListIndex GetOldListIndex(nsDisplayList* aList, uint32_t aListKey)
   {
 #ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
     if (mOldList != reinterpret_cast<uintptr_t>(aList)) {
-      MOZ_CRASH_UNSAFE_PRINTF("Item found was in the wrong list! type %d", GetPerFrameKey());
+      MOZ_CRASH_UNSAFE_PRINTF("Item found was in the wrong list! type %d (outer type was %d at depth %d, now is %d)", GetPerFrameKey(), mOldListKey, mOldNestingDepth, aListKey);
     }
 #endif
     return mOldListIndex;
   }
 
 protected:
   nsDisplayItem() = delete;
 
@@ -2885,16 +2887,18 @@ protected:
   // of the item by intersecting the current visible region with the bounds
   // of the item. Paint implementations can use this to limit their drawing.
   // Guaranteed to be contained in GetBounds().
   nsRect    mVisibleRect;
 
 #ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
 public:
   uintptr_t mOldList = 0;
+  uint32_t mOldListKey = 0;
+  uint32_t mOldNestingDepth = 0;
   bool mMergedItem = false;
   bool mPreProcessedItem = false;
 protected:
 #endif
   OldListIndex mOldListIndex;
 
   bool      mForceNotVisible;
   bool      mDisableSubpixelAA;