Fix ReplaceChild() on positioned inlines to delegate to

nsAbsoluteContainingBlock.h.  Bug 176915, r=dbaron, sr=rbs

layout/base/nsFrameManager.cpp

@@ -783,6 +783,8 @@ FrameManager::RegisterPlaceholderFrame(nsPlaceholderFrame* aPlaceholderFrame)
                               PL_DHASH_ADD));
   if (!entry)
     return NS_ERROR_OUT_OF_MEMORY;
+
+  NS_ASSERTION(!entry->placeholderFrame, "Registering a placeholder for a frame that already has a placeholder!");
   entry->placeholderFrame = aPlaceholderFrame;
   return NS_OK;
 }

layout/base/public/nsFrameList.h

@@ -100,14 +100,11 @@ public:
     aFrameList.mFirstChild = nsnull;
   }
 
-  PRBool ReplaceFrame(nsIFrame* aParent,
+  PRBool ReplaceFrame(nsIPresContext* aPresContext,
+                      nsIFrame* aParent,
                       nsIFrame* aOldFrame,
-                      nsIFrame* aNewFrame);
+                      nsIFrame* aNewFrame,
-
+                      PRBool aDestroy);
-  PRBool ReplaceAndDestroyFrame(nsIPresContext* aPresContext,
-                                nsIFrame* aParent,
-                                nsIFrame* aOldFrame,
-                                nsIFrame* aNewFrame);
 
   PRBool Split(nsIFrame* aAfterFrame, nsIFrame** aNextFrameResult);
 
@@ -155,6 +152,11 @@ public:
   void List(nsIPresContext* aPresContext, FILE* out) const;
 #endif
 
+private:
+  PRBool DoReplaceFrame(nsIFrame* aParent,
+                        nsIFrame* aOldFrame,
+                        nsIFrame* aNewFrame);
+  
 protected:
   nsIFrame* mFirstChild;
 };

layout/base/src/nsFrameList.cpp

@@ -281,44 +281,51 @@ nsFrameList::InsertFrames(nsIFrame* aParent,
 }
 
 PRBool
-nsFrameList::ReplaceFrame(nsIFrame* aParent,
+nsFrameList::DoReplaceFrame(nsIFrame* aParent,
-                          nsIFrame* aOldFrame,
+                            nsIFrame* aOldFrame,
-                          nsIFrame* aNewFrame)
+                            nsIFrame* aNewFrame)
 {
-  NS_PRECONDITION(nsnull != aOldFrame, "null ptr");
+  NS_PRECONDITION(aOldFrame, "null ptr");
-  NS_PRECONDITION(nsnull != aNewFrame, "null ptr");
+  NS_PRECONDITION(aNewFrame, "null ptr");
-  if ((nsnull != aOldFrame) && (nsnull != aNewFrame)) {
+  if (!aOldFrame || !aNewFrame) {
-    nsIFrame* nextFrame;
+    return PR_FALSE;
-    aOldFrame->GetNextSibling(&nextFrame);
-    if (aOldFrame == mFirstChild) {
-      mFirstChild = aNewFrame;
-      aNewFrame->SetNextSibling(nextFrame);
-    }
-    else {
-      nsIFrame* prevSibling = GetPrevSiblingFor(aOldFrame);
-      if (nsnull != prevSibling) {
-        prevSibling->SetNextSibling(aNewFrame);
-        aNewFrame->SetNextSibling(nextFrame);
-      }
-    }
-    if (nsnull != aParent) {
-      aNewFrame->SetParent(aParent);
-    }
-    return PR_TRUE;
   }
-  return PR_FALSE;
+  
+  nsIFrame* nextFrame;
+  aOldFrame->GetNextSibling(&nextFrame);
+  if (aOldFrame == mFirstChild) {
+    mFirstChild = aNewFrame;
+  }
+  else {
+    nsIFrame* prevSibling = GetPrevSiblingFor(aOldFrame);
+    if (!prevSibling) {
+      NS_WARNING("nsFrameList::ReplaceFrame: aOldFrame not found in list");
+      return PR_FALSE;
+    }
+    prevSibling->SetNextSibling(aNewFrame);
+  }
+
+  aNewFrame->SetNextSibling(nextFrame);
+  
+  if (aParent) {
+    aNewFrame->SetParent(aParent);
+  }
+  return PR_TRUE;
 }
 
 PRBool
-nsFrameList::ReplaceAndDestroyFrame(nsIPresContext* aPresContext,
+nsFrameList::ReplaceFrame(nsIPresContext* aPresContext,
-                                    nsIFrame* aParent,
+                          nsIFrame* aParent,
-                                    nsIFrame* aOldFrame,
+                          nsIFrame* aOldFrame,
-                                    nsIFrame* aNewFrame)
+                          nsIFrame* aNewFrame,
+                          PRBool aDestroy)
 {
-  NS_PRECONDITION(nsnull != aOldFrame, "null ptr");
+  NS_PRECONDITION(aOldFrame, "null ptr");
-  NS_PRECONDITION(nsnull != aNewFrame, "null ptr");
+  NS_PRECONDITION(aNewFrame, "null ptr");
-  if (ReplaceFrame(aParent, aOldFrame, aNewFrame)) {
+  if (DoReplaceFrame(aParent, aOldFrame, aNewFrame)) {
-    aNewFrame->Destroy(aPresContext);
+    if (aDestroy) {
+      aOldFrame->Destroy(aPresContext);
+    }
     return PR_TRUE;
   }
   return PR_FALSE;

layout/generic/nsAbsoluteContainingBlock.cpp

@@ -138,6 +138,21 @@ nsAbsoluteContainingBlock::RemoveFrame(nsIFrame*       aDelegatingFrame,
   return result ? NS_OK : NS_ERROR_FAILURE;
 }
 
+nsresult
+nsAbsoluteContainingBlock::ReplaceFrame(nsIFrame*       aDelegatingFrame,
+                                        nsIPresContext* aPresContext,
+                                        nsIPresShell&   aPresShell,
+                                        nsIAtom*        aListName,
+                                        nsIFrame*       aOldFrame,
+                                        nsIFrame*       aNewFrame)
+{
+  PRBool result = mAbsoluteFrames.ReplaceFrame(aPresContext, aDelegatingFrame,
+                                               aOldFrame, aNewFrame, PR_TRUE);
+  NS_ASSERTION(result, "Problems replacing a frame");
+  return result ? NS_OK : NS_ERROR_FAILURE;
+}
+
+
 // Destructor function for the collapse offset frame property
 static void
 DestroyRectFunc(nsIPresContext* aPresContext,

layout/generic/nsAbsoluteContainingBlock.h

@@ -82,7 +82,13 @@ public:
                        nsIPresShell&   aPresShell,
                        nsIAtom*        aListName,
                        nsIFrame*       aOldFrame);
-  
+  nsresult ReplaceFrame(nsIFrame*       aDelegatingFrame,
+                        nsIPresContext* aPresContext,
+                        nsIPresShell&   aPresShell,
+                        nsIAtom*        aListName,
+                        nsIFrame*       aOldFrame,
+                        nsIFrame*       aNewFrame);
+
   // Called by the delegating frame after it has done its reflow first. This
   // function will reflow any absolutely positioned child frames that need to
   // be reflowed, e.g., because the absolutely positioned child frame has

layout/generic/nsFrameList.cpp

@@ -281,44 +281,51 @@ nsFrameList::InsertFrames(nsIFrame* aParent,
 }
 
 PRBool
-nsFrameList::ReplaceFrame(nsIFrame* aParent,
+nsFrameList::DoReplaceFrame(nsIFrame* aParent,
-                          nsIFrame* aOldFrame,
+                            nsIFrame* aOldFrame,
-                          nsIFrame* aNewFrame)
+                            nsIFrame* aNewFrame)
 {
-  NS_PRECONDITION(nsnull != aOldFrame, "null ptr");
+  NS_PRECONDITION(aOldFrame, "null ptr");
-  NS_PRECONDITION(nsnull != aNewFrame, "null ptr");
+  NS_PRECONDITION(aNewFrame, "null ptr");
-  if ((nsnull != aOldFrame) && (nsnull != aNewFrame)) {
+  if (!aOldFrame || !aNewFrame) {
-    nsIFrame* nextFrame;
+    return PR_FALSE;
-    aOldFrame->GetNextSibling(&nextFrame);
-    if (aOldFrame == mFirstChild) {
-      mFirstChild = aNewFrame;
-      aNewFrame->SetNextSibling(nextFrame);
-    }
-    else {
-      nsIFrame* prevSibling = GetPrevSiblingFor(aOldFrame);
-      if (nsnull != prevSibling) {
-        prevSibling->SetNextSibling(aNewFrame);
-        aNewFrame->SetNextSibling(nextFrame);
-      }
-    }
-    if (nsnull != aParent) {
-      aNewFrame->SetParent(aParent);
-    }
-    return PR_TRUE;
   }
-  return PR_FALSE;
+  
+  nsIFrame* nextFrame;
+  aOldFrame->GetNextSibling(&nextFrame);
+  if (aOldFrame == mFirstChild) {
+    mFirstChild = aNewFrame;
+  }
+  else {
+    nsIFrame* prevSibling = GetPrevSiblingFor(aOldFrame);
+    if (!prevSibling) {
+      NS_WARNING("nsFrameList::ReplaceFrame: aOldFrame not found in list");
+      return PR_FALSE;
+    }
+    prevSibling->SetNextSibling(aNewFrame);
+  }
+
+  aNewFrame->SetNextSibling(nextFrame);
+  
+  if (aParent) {
+    aNewFrame->SetParent(aParent);
+  }
+  return PR_TRUE;
 }
 
 PRBool
-nsFrameList::ReplaceAndDestroyFrame(nsIPresContext* aPresContext,
+nsFrameList::ReplaceFrame(nsIPresContext* aPresContext,
-                                    nsIFrame* aParent,
+                          nsIFrame* aParent,
-                                    nsIFrame* aOldFrame,
+                          nsIFrame* aOldFrame,
-                                    nsIFrame* aNewFrame)
+                          nsIFrame* aNewFrame,
+                          PRBool aDestroy)
 {
-  NS_PRECONDITION(nsnull != aOldFrame, "null ptr");
+  NS_PRECONDITION(aOldFrame, "null ptr");
-  NS_PRECONDITION(nsnull != aNewFrame, "null ptr");
+  NS_PRECONDITION(aNewFrame, "null ptr");
-  if (ReplaceFrame(aParent, aOldFrame, aNewFrame)) {
+  if (DoReplaceFrame(aParent, aOldFrame, aNewFrame)) {
-    aNewFrame->Destroy(aPresContext);
+    if (aDestroy) {
+      aOldFrame->Destroy(aPresContext);
+    }
     return PR_TRUE;
   }
   return PR_FALSE;

layout/generic/nsFrameList.h

@@ -100,14 +100,11 @@ public:
     aFrameList.mFirstChild = nsnull;
   }
 
-  PRBool ReplaceFrame(nsIFrame* aParent,
+  PRBool ReplaceFrame(nsIPresContext* aPresContext,
+                      nsIFrame* aParent,
                       nsIFrame* aOldFrame,
-                      nsIFrame* aNewFrame);
+                      nsIFrame* aNewFrame,
-
+                      PRBool aDestroy);
-  PRBool ReplaceAndDestroyFrame(nsIPresContext* aPresContext,
-                                nsIFrame* aParent,
-                                nsIFrame* aOldFrame,
-                                nsIFrame* aNewFrame);
 
   PRBool Split(nsIFrame* aAfterFrame, nsIFrame** aNextFrameResult);
 
@@ -155,6 +152,11 @@ public:
   void List(nsIPresContext* aPresContext, FILE* out) const;
 #endif
 
+private:
+  PRBool DoReplaceFrame(nsIFrame* aParent,
+                        nsIFrame* aOldFrame,
+                        nsIFrame* aNewFrame);
+  
 protected:
   nsIFrame* mFirstChild;
 };

layout/generic/nsInlineFrame.cpp

@@ -311,21 +311,22 @@ nsInlineFrame::ReplaceFrame(nsIPresContext* aPresContext,
                             nsIFrame* aOldFrame,
                             nsIFrame* aNewFrame)
 {
-  if (nsnull != aListName) {
+  if (aListName) {
+    NS_ERROR("Don't have any special lists on inline frames!");
     return NS_ERROR_INVALID_ARG;
   }
   if (!aOldFrame || !aNewFrame) {
+    NS_ERROR("Missing aOldFrame or aNewFrame");
     return NS_ERROR_INVALID_ARG;
   }
 
-  // Replace the old frame with the new frame in the list, then remove the old frame
+  PRBool retval =
-  mFrames.ReplaceFrame(this, aOldFrame, aNewFrame);
+    mFrames.ReplaceFrame(aPresContext, this, aOldFrame, aNewFrame, PR_TRUE);
-  aOldFrame->Destroy(aPresContext);
+  
-
   // Ask the parent frame to reflow me.
   ReflowDirtyChild(&aPresShell, nsnull);
 
-  return NS_OK;
+  return retval ? NS_OK : NS_ERROR_FAILURE;
 }
 
 
@@ -1195,6 +1196,22 @@ nsPositionedInlineFrame::RemoveFrame(nsIPresContext* aPresContext,
   return rv;
 }
 
+NS_IMETHODIMP
+nsPositionedInlineFrame::ReplaceFrame(nsIPresContext* aPresContext,
+                                      nsIPresShell&   aPresShell,
+                                      nsIAtom*        aListName,
+                                      nsIFrame*       aOldFrame,
+                                      nsIFrame*       aNewFrame)
+{
+  if (nsLayoutAtoms::absoluteList == aListName) {
+    return mAbsoluteContainer.ReplaceFrame(this, aPresContext, aPresShell,
+                                           aListName, aOldFrame, aNewFrame);
+  } else {
+    return nsInlineFrame::ReplaceFrame(aPresContext, aPresShell, aListName,
+                                       aOldFrame, aNewFrame);
+  }
+}
+
 NS_IMETHODIMP
 nsPositionedInlineFrame::GetAdditionalChildListName(PRInt32   aIndex,
                                                     nsIAtom** aListName) const

layout/generic/nsInlineFrame.h

@@ -224,6 +224,11 @@ public:
                          nsIPresShell& aPresShell,
                          nsIAtom* aListName,
                          nsIFrame* aOldFrame);
+  NS_IMETHOD ReplaceFrame(nsIPresContext* aPresContext,
+                          nsIPresShell& aPresShell,
+                          nsIAtom* aListName,
+                          nsIFrame* aOldFrame,
+                          nsIFrame* aNewFrame);
 
   NS_IMETHOD GetAdditionalChildListName(PRInt32   aIndex,
                                         nsIAtom** aListName) const;

layout/html/base/src/nsAbsoluteContainingBlock.cpp

@@ -138,6 +138,21 @@ nsAbsoluteContainingBlock::RemoveFrame(nsIFrame*       aDelegatingFrame,
   return result ? NS_OK : NS_ERROR_FAILURE;
 }
 
+nsresult
+nsAbsoluteContainingBlock::ReplaceFrame(nsIFrame*       aDelegatingFrame,
+                                        nsIPresContext* aPresContext,
+                                        nsIPresShell&   aPresShell,
+                                        nsIAtom*        aListName,
+                                        nsIFrame*       aOldFrame,
+                                        nsIFrame*       aNewFrame)
+{
+  PRBool result = mAbsoluteFrames.ReplaceFrame(aPresContext, aDelegatingFrame,
+                                               aOldFrame, aNewFrame, PR_TRUE);
+  NS_ASSERTION(result, "Problems replacing a frame");
+  return result ? NS_OK : NS_ERROR_FAILURE;
+}
+
+
 // Destructor function for the collapse offset frame property
 static void
 DestroyRectFunc(nsIPresContext* aPresContext,

layout/html/base/src/nsAbsoluteContainingBlock.h

@@ -82,7 +82,13 @@ public:
                        nsIPresShell&   aPresShell,
                        nsIAtom*        aListName,
                        nsIFrame*       aOldFrame);
-  
+  nsresult ReplaceFrame(nsIFrame*       aDelegatingFrame,
+                        nsIPresContext* aPresContext,
+                        nsIPresShell&   aPresShell,
+                        nsIAtom*        aListName,
+                        nsIFrame*       aOldFrame,
+                        nsIFrame*       aNewFrame);
+
   // Called by the delegating frame after it has done its reflow first. This
   // function will reflow any absolutely positioned child frames that need to
   // be reflowed, e.g., because the absolutely positioned child frame has

layout/html/base/src/nsFrameManager.cpp

@@ -783,6 +783,8 @@ FrameManager::RegisterPlaceholderFrame(nsPlaceholderFrame* aPlaceholderFrame)
                               PL_DHASH_ADD));
   if (!entry)
     return NS_ERROR_OUT_OF_MEMORY;
+
+  NS_ASSERTION(!entry->placeholderFrame, "Registering a placeholder for a frame that already has a placeholder!");
   entry->placeholderFrame = aPlaceholderFrame;
   return NS_OK;
 }

layout/html/base/src/nsInlineFrame.cpp

@@ -311,21 +311,22 @@ nsInlineFrame::ReplaceFrame(nsIPresContext* aPresContext,
                             nsIFrame* aOldFrame,
                             nsIFrame* aNewFrame)
 {
-  if (nsnull != aListName) {
+  if (aListName) {
+    NS_ERROR("Don't have any special lists on inline frames!");
     return NS_ERROR_INVALID_ARG;
   }
   if (!aOldFrame || !aNewFrame) {
+    NS_ERROR("Missing aOldFrame or aNewFrame");
     return NS_ERROR_INVALID_ARG;
   }
 
-  // Replace the old frame with the new frame in the list, then remove the old frame
+  PRBool retval =
-  mFrames.ReplaceFrame(this, aOldFrame, aNewFrame);
+    mFrames.ReplaceFrame(aPresContext, this, aOldFrame, aNewFrame, PR_TRUE);
-  aOldFrame->Destroy(aPresContext);
+  
-
   // Ask the parent frame to reflow me.
   ReflowDirtyChild(&aPresShell, nsnull);
 
-  return NS_OK;
+  return retval ? NS_OK : NS_ERROR_FAILURE;
 }
 
 
@@ -1195,6 +1196,22 @@ nsPositionedInlineFrame::RemoveFrame(nsIPresContext* aPresContext,
   return rv;
 }
 
+NS_IMETHODIMP
+nsPositionedInlineFrame::ReplaceFrame(nsIPresContext* aPresContext,
+                                      nsIPresShell&   aPresShell,
+                                      nsIAtom*        aListName,
+                                      nsIFrame*       aOldFrame,
+                                      nsIFrame*       aNewFrame)
+{
+  if (nsLayoutAtoms::absoluteList == aListName) {
+    return mAbsoluteContainer.ReplaceFrame(this, aPresContext, aPresShell,
+                                           aListName, aOldFrame, aNewFrame);
+  } else {
+    return nsInlineFrame::ReplaceFrame(aPresContext, aPresShell, aListName,
+                                       aOldFrame, aNewFrame);
+  }
+}
+
 NS_IMETHODIMP
 nsPositionedInlineFrame::GetAdditionalChildListName(PRInt32   aIndex,
                                                     nsIAtom** aListName) const

layout/html/base/src/nsInlineFrame.h

@@ -224,6 +224,11 @@ public:
                          nsIPresShell& aPresShell,
                          nsIAtom* aListName,
                          nsIFrame* aOldFrame);
+  NS_IMETHOD ReplaceFrame(nsIPresContext* aPresContext,
+                          nsIPresShell& aPresShell,
+                          nsIAtom* aListName,
+                          nsIFrame* aOldFrame,
+                          nsIFrame* aNewFrame);
 
   NS_IMETHOD GetAdditionalChildListName(PRInt32   aIndex,
                                         nsIAtom** aListName) const;

layout/mathml/base/src/nsMathMLContainerFrame.cpp

@@ -734,7 +734,7 @@ nsMathMLContainerFrame::WrapForeignFrames(nsIPresContext* aPresContext)
         wrapper->Destroy(aPresContext);
         return rv;
       }
-      mFrames.ReplaceFrame(this, child, wrapper);
+      mFrames.ReplaceFrame(aPresContext, this, child, wrapper, PR_FALSE);
       child->SetParent(wrapper);
       child->SetNextSibling(nsnull);
       aPresContext->ReParentStyleContext(child, newStyleContext);
@@ -1013,13 +1013,7 @@ nsMathMLContainerFrame::ReplaceFrame(nsIPresContext* aPresContext,
     return NS_ERROR_INVALID_ARG;
   }
   // Replace the old frame with the new frame in the list
-  mFrames.ReplaceFrame(this, aOldFrame, aNewFrame);
+  mFrames.ReplaceFrame(aPresContext, this, aOldFrame, aNewFrame, PR_TRUE);
-
-  // XXX now destroy the old frame, really? the usage of ReplaceFrame() vs.
-  // XXX ReplaceFrameAndDestroy() is ambiguous - see bug 122748
-  // XXX The style system doesn't call ReplaceFrame() and that's why
-  // XXX nobody seems to have been biten by the ambiguity yet
-  aOldFrame->Destroy(aPresContext);
 
   return ChildListChanged(aPresContext, nsIDOMMutationEvent::MODIFICATION);
 }