Bug 838642 - Introduce nsFrameList::StartRemoveFrame/ContinueRemoveFrame that can be used in concert to remove a frame in O(1) time from a set of frame lists when its exact frame list is unknown. Use them to make nsContainerFrame::StealFrame O(1). r=bzbarsky

layout/generic/nsBlockFrame.cpp

@@ -5629,13 +5629,18 @@ nsBlockFrame::StealFrame(nsPresContext* aPresContext,
 
   if ((aChild->GetStateBits() & NS_FRAME_OUT_OF_FLOW) &&
       aChild->IsFloating()) {
-    bool removed = mFloats.RemoveFrameIfPresent(aChild);
+    MOZ_ASSERT(mFloats.ContainsFrame(aChild) ||
+               (GetPushedFloats() && GetPushedFloats()->ContainsFrame(aChild)),
+               "aChild is not our child");
+    bool removed = mFloats.StartRemoveFrame(aChild);
     if (!removed) {
       nsFrameList* list = GetPushedFloats();
       if (list) {
-        removed = list->RemoveFrameIfPresent(aChild);
+        removed = list->ContinueRemoveFrame(aChild);
+        // XXXmats delete the property if the list is now empty?
       }
     }
+    MOZ_ASSERT(removed, "StealFrame failed to remove the float");
     return removed ? NS_OK : NS_ERROR_UNEXPECTED;
   }
 
@@ -5716,6 +5721,7 @@ nsBlockFrame::StealFrame(nsPresContext* aPresContext,
       prevSibling = nullptr;
     }
   }
+  MOZ_ASSERT(false, "StealFrame failed to remove the frame");
   return NS_ERROR_UNEXPECTED;
 }
 

layout/generic/nsContainerFrame.cpp

@@ -1215,30 +1215,63 @@ nsContainerFrame::DisplayOverflowContainers(nsDisplayListBuilder*   aBuilder,
   }
 }
 
+static bool
+TryRemoveFrame(nsIFrame* aFrame, FramePropertyTable* aPropTable,
+               const FramePropertyDescriptor* aProp, nsIFrame* aChildToRemove,
+               bool (nsFrameList::*aRemoveMethod)(nsIFrame* aFrame))
+{
+  nsFrameList* list = static_cast<nsFrameList*>(aPropTable->Get(aFrame, aProp));
+  if (list && (list->*aRemoveMethod)(aChildToRemove)) {
+    // aChildToRemove *may* have been removed from this list.
+    if (list->IsEmpty()) {
+      aPropTable->Remove(aFrame, aProp);
+      delete list;
+    }
+    return true;
+  }
+  return false;
+}
+
 nsresult
 nsContainerFrame::StealFrame(nsPresContext* aPresContext,
                              nsIFrame*      aChild,
                              bool           aForceNormal)
 {
-  bool removed = true;
-  if ((aChild->GetStateBits() & NS_FRAME_IS_OVERFLOW_CONTAINER)
-      && !aForceNormal) {
-    // Try removing from the overflow container list
-    if (!RemovePropTableFrame(aPresContext, aChild,
-                              OverflowContainersProperty())) {
-      // It must be in the excess overflow container list
-      removed = RemovePropTableFrame(aPresContext, aChild,
-                                     ExcessOverflowContainersProperty());
+#ifdef DEBUG
+  if (!mFrames.ContainsFrame(aChild)) {
+    FramePropertyTable* propTable = aPresContext->PropertyTable();
+    nsFrameList* list = static_cast<nsFrameList*>(
+                          propTable->Get(this, OverflowContainersProperty()));
+    if (!list || !list->ContainsFrame(aChild)) {
+      list = static_cast<nsFrameList*>(
+               propTable->Get(this, ExcessOverflowContainersProperty()));
+      MOZ_ASSERT(list && list->ContainsFrame(aChild), "aChild is not our child "
+                 "or on a frame list not supported by StealFrame");
     }
   }
-  else {
-    if (!mFrames.RemoveFrameIfPresent(aChild)) {
-      removed = false;
-      // We didn't find the child in the parent's principal child list.
+#endif
+
+  bool removed;
+  if ((aChild->GetStateBits() & NS_FRAME_IS_OVERFLOW_CONTAINER)
+      && !aForceNormal) {
+    FramePropertyTable* propTable = aPresContext->PropertyTable();
+    // Try removing from the overflow container list.
+    removed = ::TryRemoveFrame(this, propTable, OverflowContainersProperty(),
+                               aChild, &nsFrameList::StartRemoveFrame);
+    if (!removed) {
+      // It must be in the excess overflow container list.
+      removed = ::TryRemoveFrame(this, propTable,
+                                 ExcessOverflowContainersProperty(),
+                                 aChild, &nsFrameList::ContinueRemoveFrame);
+    }
+  } else {
+    removed = mFrames.StartRemoveFrame(aChild);
+    if (!removed) {
+      // We didn't find the child in our principal child list.
       // Maybe it's on the overflow list?
       nsFrameList* frameList = GetOverflowFrames();
       if (frameList) {
-        removed = frameList->RemoveFrameIfPresent(aChild);
+        removed = frameList->ContinueRemoveFrame(aChild);
         if (frameList->IsEmpty()) {
           DestroyOverflowList(aPresContext, nullptr);
         }

layout/generic/nsFrameList.cpp

@@ -351,6 +351,17 @@ nsFrameList::ApplySetParent(nsIFrame* aParent) const
   }
 }
 
+/* static */ void
+nsFrameList::UnhookFrameFromSiblings(nsIFrame* aFrame)
+{
+  MOZ_ASSERT(aFrame->GetPrevSibling() && aFrame->GetNextSibling());
+  nsIFrame* const nextSibling = aFrame->GetNextSibling();
+  nsIFrame* const prevSibling = aFrame->GetPrevSibling();
+  aFrame->SetNextSibling(nullptr);
+  prevSibling->SetNextSibling(nextSibling);
+  MOZ_ASSERT(!aFrame->GetPrevSibling() && !aFrame->GetNextSibling());
+}
+
 #ifdef DEBUG
 void
 nsFrameList::List(FILE* out) const

layout/generic/nsFrameList.h

@@ -159,6 +159,36 @@ public:
    */
   nsIFrame* RemoveFirstChild();
 
+  /**
+   * The following two functions are intended to be used in concert for
+   * removing a frame from its frame list when the set of possible frame
+   * lists is known in advance, but the exact frame list is unknown.
+   * aFrame must be non-null.
+   * Example use:
+   *   bool removed = frameList1.StartRemoveFrame(aFrame) ||
+   *                  frameList2.ContinueRemoveFrame(aFrame) ||
+   *                  frameList3.ContinueRemoveFrame(aFrame);
+   *   MOZ_ASSERT(removed);
+   *
+   * @note One of the frame lists MUST contain aFrame, if it's on some other
+   *       frame list then the example above will likely lead to crashes.
+   * This function is O(1).
+   * @return true iff aFrame was removed from /some/ list, not necessarily
+   *         this one.  If it was removed from a different list then it is
+   *         guaranteed that that list is still non-empty.
+   * (this method is implemented in nsIFrame.h to be able to inline)
+   */
+  inline bool StartRemoveFrame(nsIFrame* aFrame);
+
+  /**
+   * Precondition: StartRemoveFrame MUST be called before this.
+   * This function is O(1).
+   * @see StartRemoveFrame
+   * @return true iff aFrame was removed from this list
+   * (this method is implemented in nsIFrame.h to be able to inline)
+   */
+  inline bool ContinueRemoveFrame(nsIFrame* aFrame);
+
   /**
    * Take aFrame out of the frame list and then destroy it.
    * The frame must be non-null and present on this list.
@@ -452,6 +482,14 @@ private:
   static const nsFrameList* sEmptyList;
 
 protected:
+  /**
+   * Disconnect aFrame from its siblings.  This must only be called if aFrame
+   * is NOT the first or last sibling, because otherwise its nsFrameList will
+   * have a stale mFirst/LastChild pointer.  This precondition is asserted.
+   * This function is O(1).
+   */
+  static void UnhookFrameFromSiblings(nsIFrame* aFrame);
+
   nsIFrame* mFirstChild;
   nsIFrame* mLastChild;
 };

layout/generic/nsIFrame.h

@@ -3247,6 +3247,44 @@ private:
   nsIFrame*     mFrame;
 };
 
+inline bool
+nsFrameList::ContinueRemoveFrame(nsIFrame* aFrame)
+{
+  MOZ_ASSERT(!aFrame->GetPrevSibling() || !aFrame->GetNextSibling(),
+             "Forgot to call StartRemoveFrame?");
+  if (aFrame == mLastChild) {
+    MOZ_ASSERT(!aFrame->GetNextSibling(), "broken frame list");
+    nsIFrame* prevSibling = aFrame->GetPrevSibling();
+    if (!prevSibling) {
+      MOZ_ASSERT(aFrame == mFirstChild, "broken frame list");
+      mFirstChild = mLastChild = nullptr;
+      return true;
+    }
+    MOZ_ASSERT(prevSibling->GetNextSibling() == aFrame, "Broken frame linkage");
+    prevSibling->SetNextSibling(nullptr);
+    mLastChild = prevSibling;
+    return true;
+  }
+  if (aFrame == mFirstChild) {
+    MOZ_ASSERT(!aFrame->GetPrevSibling(), "broken frame list");
+    mFirstChild = aFrame->GetNextSibling();
+    aFrame->SetNextSibling(nullptr);
+    MOZ_ASSERT(mFirstChild, "broken frame list");
+    return true;
+  }
+  return false;
+}
+
+inline bool
+nsFrameList::StartRemoveFrame(nsIFrame* aFrame)
+{
+  if (aFrame->GetPrevSibling() && aFrame->GetNextSibling()) {
+    UnhookFrameFromSiblings(aFrame);
+    return true;
+  }
+  return ContinueRemoveFrame(aFrame);
+}
+
 inline void
 nsFrameList::Enumerator::Next()
 {