Bug 1159042 - p4. Make mDirtyRoots manage roots in preferred depth order - r=dbaron

When popping a dirty root, take the shallowest one first (so we reflow from
outer frames first, to avoid potential duplicate reflow of inner frames).

Prevent duplicate roots (to be reworked in a future bug).

Differential Revision: https://phabricator.services.mozilla.com/D9490

layout/base/PresShell.cpp

@@ -591,27 +591,40 @@ private:
 };
 
 void
-nsIPresShell::DirtyRootsList::AppendElement(nsIFrame* aFrame)
+nsIPresShell::DirtyRootsList::Add(nsIFrame* aFrame)
 {
-  mList.AppendElement(aFrame);
+  // Is this root already scheduled for reflow?
+  // FIXME: This could possibly be changed to a uniqueness assertion, with some
+  // work in ResizeReflowIgnoreOverride (and maybe others?)
+  if (mList.Contains(aFrame)) {
+    // We don't expect frame to change depths.
+    MOZ_ASSERT(aFrame->GetDepthInFrameTree() ==
+               mList[mList.IndexOf(aFrame)].mDepth);
+    return;
+  }
+
+  mList.InsertElementSorted(
+    FrameAndDepth{ aFrame, aFrame->GetDepthInFrameTree() },
+    FrameAndDepth::CompareByReverseDepth{});
 }
 
 void
-nsIPresShell::DirtyRootsList::RemoveElement(nsIFrame* aFrame)
+nsIPresShell::DirtyRootsList::Remove(nsIFrame* aFrame)
 {
   mList.RemoveElement(aFrame);
 }
 
-void
+nsIFrame*
-nsIPresShell::DirtyRootsList::RemoveElements(nsIFrame* aFrame)
+nsIPresShell::DirtyRootsList::PopShallowestRoot()
 {
-  mList.RemoveElementsBy([&](nsIFrame* aRoot){ return aRoot == aFrame; });
+  // List is sorted in order of decreasing depth, so there are no deeper
-}
+  // frames than the last one.
-
+  const FrameAndDepth& lastFAD = mList.LastElement();
-void
+  nsIFrame* frame = lastFAD.mFrame;
-nsIPresShell::DirtyRootsList::RemoveElementAt(size_t aIndex)
+  // We don't expect frame to change depths.
-{
+  MOZ_ASSERT(frame->GetDepthInFrameTree() == lastFAD.mDepth);
-  return mList.RemoveElementAt(aIndex);
+  mList.RemoveLastElement();
+  return frame;
 }
 
 void
@@ -632,10 +645,24 @@ nsIPresShell::DirtyRootsList::IsEmpty() const
   return mList.IsEmpty();
 }
 
-size_t
+bool
-nsIPresShell::DirtyRootsList::Length() const
+nsIPresShell::DirtyRootsList::FrameIsAncestorOfDirtyRoot(nsIFrame* aFrame) const
 {
-  return mList.Length();
+  MOZ_ASSERT(aFrame);
+
+  // Look for a path from any dirty roots to aFrame, following GetParent().
+  // This check mirrors what FrameNeedsReflow() would have done if the reflow
+  // root didn't get in the way.
+  for (nsIFrame* dirtyFrame : mList) {
+    do {
+      if (dirtyFrame == aFrame) {
+        return true;
+      }
+      dirtyFrame = dirtyFrame->GetParent();
+    } while (dirtyFrame);
+  }
+
+  return false;
 }
 
 bool PresShell::sDisableNonTestMouseEvents = false;
@@ -2069,7 +2096,7 @@ PresShell::ResizeReflowIgnoreOverride(nscoord aWidth, nscoord aHeight,
         AUTO_LAYOUT_PHASE_ENTRY_POINT(GetPresContext(), Reflow);
         nsViewManager::AutoDisableRefresh refreshBlocker(viewManager);
 
-        mDirtyRoots.RemoveElement(rootFrame);
+        mDirtyRoots.Remove(rootFrame);
         DoReflow(rootFrame, true, nullptr);
 
         if (shrinkToFit) {
@@ -2205,7 +2232,7 @@ PresShell::NotifyDestroyingFrame(nsIFrame* aFrame)
 
     mFrameConstructor->NotifyDestroyingFrame(aFrame);
 
-    mDirtyRoots.RemoveElements(aFrame);
+    mDirtyRoots.Remove(aFrame);
 
     // Remove frame properties
     aFrame->DeleteAllProperties();
@@ -2863,7 +2890,7 @@ PresShell::FrameNeedsReflow(nsIFrame *aFrame, IntrinsicDirty aIntrinsicDirty,
       if (FRAME_IS_REFLOW_ROOT(f) || !f->GetParent()) {
         // we've hit a reflow root or the root frame
         if (!wasDirty) {
-          mDirtyRoots.AppendElement(f);
+          mDirtyRoots.Add(f);
           SetNeedLayoutFlush();
         }
 #ifdef DEBUG
@@ -4671,22 +4698,7 @@ PresShell::NotifyCounterStylesAreDirty()
 bool
 nsIPresShell::FrameIsAncestorOfDirtyRoot(nsIFrame* aFrame) const
 {
-  MOZ_ASSERT(aFrame);
+  return mDirtyRoots.FrameIsAncestorOfDirtyRoot(aFrame);
-
-  // Look for a path from any dirty roots to aFrame, following GetParent().
-  // This check mirrors what FrameNeedsReflow() would have done if the reflow
-  // root didn't get in the way.
-  for (nsIFrame* dirtyFrame : mDirtyRoots) {
-    while (dirtyFrame) {
-      if (dirtyFrame == aFrame) {
-        return true;
-      }
-
-      dirtyFrame = dirtyFrame->GetParent();
-    }
-  }
-
-  return false;
 }
 
 void
@@ -9140,7 +9152,7 @@ PresShell::DoReflow(nsIFrame* target, bool aInterruptible,
     }
 
     NS_ASSERTION(NS_SUBTREE_DIRTY(target), "Why is the target not dirty?");
-    mDirtyRoots.AppendElement(target);
+    mDirtyRoots.Add(target);
     SetNeedLayoutFlush();
 
     // Clear mFramesToDirty after we've done the NS_SUBTREE_DIRTY(target)
@@ -9237,9 +9249,7 @@ PresShell::ProcessReflowCommands(bool aInterruptible)
 
       do {
         // Send an incremental reflow notification to the target frame.
-        int32_t idx = mDirtyRoots.Length() - 1;
+        nsIFrame *target = mDirtyRoots.PopShallowestRoot();
-        nsIFrame *target = mDirtyRoots[idx];
-        mDirtyRoots.RemoveElementAt(idx);
 
         if (!NS_SUBTREE_DIRTY(target)) {
           // It's not dirty anymore, which probably means the notification

layout/base/nsIPresShell.h

@@ -1787,22 +1787,48 @@ protected:
   class DirtyRootsList
   {
   public:
-    void AppendElement(nsIFrame* aFrame);
+    // Add a dirty root.
-    void RemoveElement(nsIFrame* aFrame);
+    void Add(nsIFrame* aFrame);
-    void RemoveElements(nsIFrame* aFrame);
+    // Remove this frame if present.
-    void RemoveElementAt(size_t aIndex);
+    void Remove(nsIFrame* aFrame);
+    // Remove and return one of the shallowest dirty roots from the list.
+    // (If two roots are at the same depth, order is indeterminate.)
+    nsIFrame* PopShallowestRoot();
+    // Remove all dirty roots.
     void Clear();
+    // Is this frame one of the dirty roots?
     bool Contains(nsIFrame* aFrame) const;
+    // Are there no dirty roots?
     bool IsEmpty() const;
-    size_t Length() const;
+    // Is the given frame an ancestor of any dirty root?
-    auto begin() const { return mList.begin(); }
+    bool FrameIsAncestorOfDirtyRoot(nsIFrame* aFrame) const;
-    auto begin() { return mList.begin(); }
-    auto end() const { return mList.end(); }
-    auto end() { return mList.end(); }
-    auto& operator[](size_t i) { return mList[i]; }
 
   private:
-    nsTArray<nsIFrame*> mList;
+    struct FrameAndDepth
+    {
+      nsIFrame* mFrame;
+      const uint32_t mDepth;
+
+      // Easy conversion to nsIFrame*, as it's the most likely need.
+      operator nsIFrame*() const { return mFrame; }
+
+      // Used to sort by reverse depths, i.e., deeper < shallower.
+      class CompareByReverseDepth
+      {
+      public:
+        bool Equals(const FrameAndDepth& aA, const FrameAndDepth& aB) const
+        {
+          return aA.mDepth == aB.mDepth;
+        }
+        bool LessThan(const FrameAndDepth& aA, const FrameAndDepth& aB) const
+        {
+          // Reverse depth! So '>' instead of '<'.
+          return aA.mDepth > aB.mDepth;
+        }
+      };
+    };
+    // List of all known dirty roots, sorted by decreasing depths.
+    nsTArray<FrameAndDepth> mList;
   };
 
   // Reflow roots that need to be reflowed.