Bug 1278080 - Make nsContainerFrame::RemoveFrame deal with overflow containers. r=dholbert

layout/generic/nsBlockFrame.cpp

@@ -5953,8 +5953,7 @@ FindLineFor(nsIFrame*             aChild,
 }
 
 nsresult
-nsBlockFrame::StealFrame(nsIFrame* aChild,
-                         bool      aForceNormal)
+nsBlockFrame::StealFrame(nsIFrame* aChild)
 {
   MOZ_ASSERT(aChild->GetParent() == this);
 
@@ -5964,9 +5963,8 @@ nsBlockFrame::StealFrame(nsIFrame* aChild,
     return NS_OK;
   }
 
-  if ((aChild->GetStateBits() & NS_FRAME_IS_OVERFLOW_CONTAINER)
-      && !aForceNormal) {
-    return nsContainerFrame::StealFrame(aChild);
+  if (MaybeStealOverflowContainerFrame(aChild)) {
+    return NS_OK;
   }
 
   MOZ_ASSERT(!(aChild->GetStateBits() & NS_FRAME_OUT_OF_FLOW));

layout/generic/nsBlockFrame.h

@@ -273,8 +273,7 @@ public:
    */
   virtual bool DrainSelfOverflowList() override;
 
-  virtual nsresult StealFrame(nsIFrame* aChild,
-                              bool      aForceNormal = false) override;
+  virtual nsresult StealFrame(nsIFrame* aChild) override;
 
   virtual void DeleteNextInFlowChild(nsIFrame* aNextInFlow,
                                      bool      aDeletingEmptyFrames) override;

layout/generic/nsCanvasFrame.h

@@ -23,7 +23,9 @@ class nsRenderingContext;
  *
  * The root frame is the parent frame for the document element's frame.
  * It only supports having a single child frame which must be an area
- * frame
+ * frame.
+ * @note nsCanvasFrame keeps overflow container continuations of its child
+ * frame in the main child list.
  */
 class nsCanvasFrame final : public nsContainerFrame,
                             public nsIScrollPositionListener,
@@ -121,19 +123,6 @@ public:
    */
   virtual nsIAtom* GetType() const override;
 
-  virtual nsresult StealFrame(nsIFrame* aChild, bool aForceNormal) override
-  {
-    NS_ASSERTION(!aForceNormal, "No-one should be passing this in here");
-
-    // nsCanvasFrame keeps overflow container continuations of its child
-    // frame in main child list
-    nsresult rv = nsContainerFrame::StealFrame(aChild, true);
-    if (NS_FAILED(rv)) {
-      rv = nsContainerFrame::StealFrame(aChild);
-    }
-    return rv;
-  }
-
 #ifdef DEBUG_FRAME_DUMP
   virtual nsresult GetFrameName(nsAString& aResult) const override;
 #endif

layout/generic/nsColumnSetFrame.h

@@ -12,7 +12,13 @@
 #include "nsContainerFrame.h"
 #include "nsIFrameInlines.h" // for methods used by IS_TRUE_OVERFLOW_CONTAINER
 
-class nsColumnSetFrame final : public nsContainerFrame {
+/**
+ * nsColumnSetFrame implements CSS multi-column layout.
+ * @note nsColumnSetFrame keeps true overflow containers in the normal flow
+ * child lists (i.e. the principal and overflow lists).
+ */
+class nsColumnSetFrame final : public nsContainerFrame
+{
 public:
   NS_DECL_FRAMEARENA_HELPERS
 
@@ -54,14 +60,6 @@ public:
     return frame->GetContentInsertionFrame();
   }
 
-  virtual nsresult StealFrame(nsIFrame* aChild, bool aForceNormal) override
-  {
-    // nsColumnSetFrame keeps true overflow containers in the normal flow
-    // child lists (i.e. the principal and overflow lists).
-    return nsContainerFrame::StealFrame(aChild,
-                                        IS_TRUE_OVERFLOW_CONTAINER(aChild));
-  }
-
   virtual bool IsFrameOfType(uint32_t aFlags) const override
    {
      return nsContainerFrame::IsFrameOfType(aFlags &

layout/generic/nsContainerFrame.cpp

@@ -164,7 +164,7 @@ nsContainerFrame::RemoveFrame(ChildListID aListID,
     // Please note that 'parent' may not actually be where 'aOldFrame' lives.
     // We really MUST use StealFrame() and nothing else here.
     // @see nsInlineFrame::StealFrame for details.
-    parent->StealFrame(aOldFrame, true);
+    parent->StealFrame(aOldFrame);
     aOldFrame->Destroy();
     aOldFrame = oldFrameNextContinuation;
     if (parent != lastParent && generateReflowCommand) {
@@ -1349,9 +1349,27 @@ TryRemoveFrame(nsIFrame* aFrame, FramePropertyTable* aPropTable,
   return false;
 }
 
+bool
+nsContainerFrame::MaybeStealOverflowContainerFrame(nsIFrame* aChild)
+{
+  bool removed = false;
+  if (MOZ_UNLIKELY(aChild->GetStateBits() & NS_FRAME_IS_OVERFLOW_CONTAINER)) {
+    FramePropertyTable* propTable = PresContext()->PropertyTable();
+    // Try removing from the overflow container list.
+    removed = ::TryRemoveFrame(this, propTable, OverflowContainersProperty(),
+                               aChild);
+    if (!removed) {
+      // It might be in the excess overflow container list.
+      removed = ::TryRemoveFrame(this, propTable,
+                                 ExcessOverflowContainersProperty(),
+                                 aChild);
+    }
+  }
+  return removed;
+}
+
 nsresult
-nsContainerFrame::StealFrame(nsIFrame* aChild,
-                             bool      aForceNormal)
+nsContainerFrame::StealFrame(nsIFrame* aChild)
 {
 #ifdef DEBUG
   if (!mFrames.ContainsFrame(aChild)) {
@@ -1368,20 +1386,11 @@ nsContainerFrame::StealFrame(nsIFrame* aChild,
   }
 #endif
 
-  bool removed;
-  if ((aChild->GetStateBits() & NS_FRAME_IS_OVERFLOW_CONTAINER)
-      && !aForceNormal) {
-    FramePropertyTable* propTable = PresContext()->PropertyTable();
-    // Try removing from the overflow container list.
-    removed = ::TryRemoveFrame(this, propTable, OverflowContainersProperty(),
-                               aChild);
-    if (!removed) {
-      // It must be in the excess overflow container list.
-      removed = ::TryRemoveFrame(this, propTable,
-                                 ExcessOverflowContainersProperty(),
-                                 aChild);
-    }
-  } else {
+  bool removed = MaybeStealOverflowContainerFrame(aChild);
+  if (!removed) {
+    // NOTE nsColumnSetFrame and nsCanvasFrame have their overflow containers
+    // on the normal lists so we might get here also if the frame bit
+    // NS_FRAME_IS_OVERFLOW_CONTAINER is set.
     removed = mFrames.StartRemoveFrame(aChild);
     if (!removed) {
       // We didn't find the child in our principal child list.

layout/generic/nsContainerFrame.h

@@ -413,17 +413,13 @@ public:
 
   /**
    * Removes aChild without destroying it and without requesting reflow.
-   * Continuations are not affected. Checks the primary and overflow
-   * or overflow containers and excess overflow containers lists, depending
-   * on whether the NS_FRAME_IS_OVERFLOW_CONTAINER flag is set. Does not
-   * check any other auxiliary lists.
-   * Returns NS_ERROR_UNEXPECTED if we failed to remove aChild.
-   * Returns other error codes if we failed to put back a proptable list.
-   * If aForceNormal is true, only checks the primary and overflow lists
-   * even when the NS_FRAME_IS_OVERFLOW_CONTAINER flag is set.
+   * Continuations are not affected.  Checks the principal and overflow lists,
+   * and also the [excess] overflow containers lists if the frame bit
+   * NS_FRAME_IS_OVERFLOW_CONTAINER is set.  It does not check any other lists.
+   * Returns NS_ERROR_UNEXPECTED if aChild wasn't found on any of the lists
+   * mentioned above.
    */
-  virtual nsresult StealFrame(nsIFrame* aChild,
-                              bool      aForceNormal = false);
+  virtual nsresult StealFrame(nsIFrame* aChild);
 
   /**
    * Removes the next-siblings of aChild without destroying them and without
@@ -494,6 +490,11 @@ protected:
    */
   void DestroyAbsoluteFrames(nsIFrame* aDestructRoot);
 
+  /**
+   * Helper for StealFrame.  Returns true if aChild was removed from its list.
+   */
+  bool MaybeStealOverflowContainerFrame(nsIFrame* aChild);
+
   /**
    * Builds a display list for non-block children that behave like
    * inlines. This puts the background of each child into the

layout/generic/nsInlineFrame.cpp

@@ -208,12 +208,10 @@ nsInlineFrame::DestroyFrom(nsIFrame* aDestructRoot)
 }
 
 nsresult
-nsInlineFrame::StealFrame(nsIFrame* aChild,
-                          bool      aForceNormal)
+nsInlineFrame::StealFrame(nsIFrame* aChild)
 {
-  if (aChild->HasAnyStateBits(NS_FRAME_IS_OVERFLOW_CONTAINER) &&
-      !aForceNormal) {
-    return nsContainerFrame::StealFrame(aChild, aForceNormal);
+  if (MaybeStealOverflowContainerFrame(aChild)) {
+    return NS_OK;
   }
 
   nsInlineFrame* parent = this;

layout/generic/nsInlineFrame.h

@@ -62,7 +62,7 @@ public:
                                      bool aRespectClusters = true) override;
   
   virtual void DestroyFrom(nsIFrame* aDestructRoot) override;
-  virtual nsresult StealFrame(nsIFrame* aChild, bool aForceNormal) override;
+  virtual nsresult StealFrame(nsIFrame* aChild) override;
 
   // nsIHTMLReflow overrides
   virtual void AddInlineMinISize(nsRenderingContext *aRenderingContext,