Bug 508473 Part II: Remove DeletingFrameSubtree r=bz sr=roc
authorfantasai <fantasai.cvs@inkedblade.net>
Thu, 24 Dec 2009 00:20:41 -0500
changeset 36646 912c5206ca3e735bb39494edcc0e489d5baea510
parent 36645 5c0c836b0d06157dc1c9410aa534dcb5c675bbec
child 36647 1e37be566afa2f59aae7a09e8571f0350bc14542
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersbz, roc
bugs508473
milestone1.9.3a1pre
Bug 508473 Part II: Remove DeletingFrameSubtree r=bz sr=roc
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsCSSFrameConstructor.h
layout/base/nsFrameManager.cpp
layout/base/nsFrameManager.h
layout/base/nsLayoutUtils.cpp
layout/base/nsLayoutUtils.h
layout/generic/nsBlockFrame.cpp
layout/generic/nsFrame.cpp
layout/generic/nsFrameList.cpp
layout/generic/nsFrameList.h
layout/generic/nsIFrame.h
layout/generic/nsLineBox.cpp
layout/generic/nsPlaceholderFrame.cpp
layout/generic/nsPlaceholderFrame.h
layout/tables/nsTableFrame.cpp
layout/xul/base/src/nsListBoxBodyFrame.cpp
layout/xul/base/src/nsPopupSetFrame.cpp
layout/xul/base/src/nsPopupSetFrame.h
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -361,20 +361,16 @@ NS_NewScrollbarButtonFrame (nsIPresShell
 #ifdef NOISY_FINDFRAME
 static PRInt32 FFWC_totalCount=0;
 static PRInt32 FFWC_doLoop=0;
 static PRInt32 FFWC_doSibling=0;
 static PRInt32 FFWC_recursions=0;
 static PRInt32 FFWC_nextInFlows=0;
 #endif
 
-static nsresult
-DeletingFrameSubtree(nsFrameManager* aFrameManager,
-                     nsIFrame*       aFrame);
-
 static inline nsIFrame*
 GetFieldSetBlockFrame(nsIFrame* aFieldsetFrame)
 {
   // Depends on the fieldset child frame order - see ConstructFieldSetFrame() below.
   nsIFrame* firstChild = aFieldsetFrame->GetFirstChild(nsnull);
   return firstChild && firstChild->GetNextSibling() ? firstChild->GetNextSibling() : firstChild;
 }
 
@@ -616,70 +612,16 @@ IsOutOfFlowList(nsIAtom* aListName)
 {
   return
     aListName == nsGkAtoms::floatList ||
     aListName == nsGkAtoms::absoluteList ||
     aListName == nsGkAtoms::overflowOutOfFlowList ||
     aListName == nsGkAtoms::fixedList;
 }
 
-// Helper function that recursively removes content to frame mappings and
-// undisplayed content mappings.
-// This differs from DeletingFrameSubtree() because the frames have not yet been
-// added to the frame hierarchy.
-// XXXbz it would really help if we merged the two methods somehow... :(
-static void
-DoCleanupFrameReferences(nsFrameManager*  aFrameManager,
-                         nsIFrame*        aFrameIn)
-{
-  nsIContent* content = aFrameIn->GetContent();
-
-  if (aFrameIn->GetType() == nsGkAtoms::placeholderFrame) {
-    nsPlaceholderFrame* placeholder = static_cast<nsPlaceholderFrame*>
-                                                 (aFrameIn);
-    // if the frame is a placeholder use the out of flow frame
-    aFrameIn = nsPlaceholderFrame::GetRealFrameForPlaceholder(placeholder);
-
-    // And don't forget to unregister the placeholder mapping.  Note that this
-    // means it's the caller's responsibility to actually destroy the
-    // out-of-flow pointed to by the placeholder, since after this point the
-    // out-of-flow is not reachable via the placeholder.
-    aFrameManager->UnregisterPlaceholderFrame(placeholder);
-  }
-
-  // Remove the mapping from the content object to its frame
-  aFrameManager->RemoveAsPrimaryFrame(content, aFrameIn);
-  aFrameManager->ClearAllUndisplayedContentIn(content);
-
-  // Recursively walk the child frames.
-  nsIAtom* childListName = nsnull;
-  PRInt32 childListIndex = 0;
-  do {
-    nsIFrame* childFrame = aFrameIn->GetFirstChild(childListName);
-    while (childFrame) {
-      DoCleanupFrameReferences(aFrameManager, childFrame);
-    
-      // Get the next sibling child frame
-      childFrame = childFrame->GetNextSibling();
-    }
-
-    childListName = aFrameIn->GetAdditionalChildListName(childListIndex++);
-  } while (childListName);
-}
-
-// Helper function that walks a frame list and calls DoCleanupFrameReference()
-static void
-CleanupFrameReferences(nsFrameManager*  aFrameManager,
-                       const nsFrameList& aFrameList)
-{
-  for (nsFrameList::Enumerator e(aFrameList); !e.AtEnd(); e.Next()) {
-    DoCleanupFrameReferences(aFrameManager, e.get());
-  }
-}
-
 // -----------------------------------------------------------
 
 // Structure used when constructing formatting object trees.
 struct nsFrameItems : public nsFrameList
 {
   // Appends the frame to the end of the list
   void AddChild(nsIFrame* aChild);
 };
@@ -872,18 +814,17 @@ public:
    *        positioned
    * @param aCanBeFloated pass false if the frame isn't allowed to be
    *        floated
    * @param aIsOutOfFlowPopup pass true if the frame is an out-of-flow popup
    *        (XUL-only)
    * @throws NS_ERROR_OUT_OF_MEMORY if it happens.
    * @note If this method throws, that means that aNewFrame was not inserted
    *       into any frame lists.  Furthermore, this method will handle cleanup
-   *       of aNewFrame (via calling CleanupFrameReferences() and Destroy() on
-   *       it).
+   *       of aNewFrame (via calling Destroy() on it).
    */
   nsresult AddChild(nsIFrame* aNewFrame,
                     nsFrameItems& aFrameItems,
                     nsIContent* aContent,
                     nsStyleContext* aStyleContext,
                     nsIFrame* aParentFrame,
                     PRBool aCanBePositioned = PR_TRUE,
                     PRBool aCanBeFloated = PR_TRUE,
@@ -1222,17 +1163,16 @@ nsFrameConstructorState::AddChild(nsIFra
                                                        nsnull,
                                                        placeholderType,
                                                        &placeholderFrame);
     if (NS_FAILED(rv)) {
       // Note that aNewFrame could be the top frame for a scrollframe setup,
       // hence already set as the primary frame.  So we have to clean up here.
       // But it shouldn't have any out-of-flow kids.
       // XXXbz Maybe add a utility function to assert that?
-      DoCleanupFrameReferences(mFrameManager, aNewFrame);
       aNewFrame->Destroy();
       return rv;
     }
 
     placeholderFrame->AddStateBits(mAdditionalStateBits);
     // Add the placeholder frame to the flow
     aFrameItems.AddChild(placeholderFrame);
   }
@@ -1429,88 +1369,16 @@ MoveChildrenTo(nsPresContext* aPresConte
   if (aNewParent->GetChildList(nsnull).IsEmpty() &&
       (aNewParent->GetStateBits() & NS_FRAME_FIRST_REFLOW)) {
     aNewParent->SetInitialChildList(nsnull, aFrameList);
   } else {
     aNewParent->AppendFrames(nsnull, aFrameList);
   }
 }
 
-// -----------------------------------------------------------
-
-// Helper function that determines the child list name that aChildFrame
-// is contained in
-static nsIAtom*
-GetChildListNameFor(nsIFrame*       aChildFrame)
-{
-  nsIAtom*      listName;
-
-  if (aChildFrame->GetStateBits() & NS_FRAME_IS_OVERFLOW_CONTAINER) {
-    listName = nsGkAtoms::overflowContainersList;
-  }
-  // See if the frame is moved out of the flow
-  else if (aChildFrame->GetStateBits() & NS_FRAME_OUT_OF_FLOW) {
-    // Look at the style information to tell
-    const nsStyleDisplay* disp = aChildFrame->GetStyleDisplay();
-    
-    if (NS_STYLE_POSITION_ABSOLUTE == disp->mPosition) {
-      listName = nsGkAtoms::absoluteList;
-    } else if (NS_STYLE_POSITION_FIXED == disp->mPosition) {
-      if (nsLayoutUtils::IsReallyFixedPos(aChildFrame)) {
-        listName = nsGkAtoms::fixedList;
-      } else {
-        listName = nsGkAtoms::absoluteList;
-      }
-#ifdef MOZ_XUL
-    } else if (NS_STYLE_DISPLAY_POPUP == disp->mDisplay) {
-      // Out-of-flows that are DISPLAY_POPUP must be kids of the root popup set
-#ifdef DEBUG
-      nsIFrame* parent = aChildFrame->GetParent();
-      NS_ASSERTION(parent && parent->GetType() == nsGkAtoms::popupSetFrame,
-                   "Unexpected parent");
-#endif // DEBUG
-
-      // XXX FIXME: Bug 350740
-      // Return here, because the postcondition for this function actually
-      // fails for this case, since the popups are not in a "real" frame list
-      // in the popup set.
-      return nsGkAtoms::popupList;      
-#endif // MOZ_XUL
-    } else {
-      NS_ASSERTION(aChildFrame->GetStyleDisplay()->IsFloating(),
-                   "not a floated frame");
-      listName = nsGkAtoms::floatList;
-    }
-
-  } else {
-    listName = nsnull;
-  }
-
-#ifdef NS_DEBUG
-  // Verify that the frame is actually in that child list or in the
-  // corresponding overflow list.
-  nsIFrame* parent = aChildFrame->GetParent();
-  PRBool found = parent->GetChildList(listName).ContainsFrame(aChildFrame);
-  if (!found) {
-    if (!(aChildFrame->GetStateBits() & NS_FRAME_OUT_OF_FLOW)) {
-      found = parent->GetChildList(nsGkAtoms::overflowList)
-                .ContainsFrame(aChildFrame);
-    }
-    else if (aChildFrame->GetStyleDisplay()->IsFloating()) {
-      found = parent->GetChildList(nsGkAtoms::overflowOutOfFlowList)
-                .ContainsFrame(aChildFrame);
-    }
-    // else it's positioned and should have been on the 'listName' child list.
-    NS_POSTCONDITION(found, "not in child list");
-  }
-#endif
-
-  return listName;
-}
-
 //----------------------------------------------------------------------
 
 nsCSSFrameConstructor::nsCSSFrameConstructor(nsIDocument *aDocument,
                                              nsIPresShell *aPresShell)
   : mDocument(aDocument)
   , mPresShell(aPresShell)
   , mRootElementFrame(nsnull)
   , mRootElementStyleFrame(nsnull)
@@ -3965,17 +3833,16 @@ nsCSSFrameConstructor::ConstructFrameFro
         } else {
           newItems.AddChild(f);
         }
       }
       rv = FlushAccumulatedBlock(aState, content, newFrame, &currentBlock, &newItems);
 
       if (childItems.NotEmpty()) {
         // an error must have occurred, delete unprocessed frames
-        CleanupFrameReferences(aState.mFrameManager, childItems);
         childItems.DestroyFrames();
       }
 
       childItems = newItems;
     }
 #endif
 
     // Set the frame's initial child list
@@ -6998,225 +6865,16 @@ nsCSSFrameConstructor::ContentInserted(n
     printf("nsCSSFrameConstructor::ContentInserted: resulting frame model:\n");
     parentFrame->List(stdout, 0);
   }
 #endif
 
   return NS_OK;
 }
 
-static void
-DoDeletingFrameSubtree(nsFrameManager*      aFrameManager,
-                       nsTArray<nsIFrame*>& aDestroyQueue,
-                       nsIFrame*            aRemovedFrame,
-                       nsIFrame*            aFrame);
-
-static void
-DoDeletingOverflowContainers(nsFrameManager*      aFrameManager,
-                             nsTArray<nsIFrame*>& aDestroyQueue,
-                             nsIFrame*            aRemovedFrame,
-                             nsIFrame*            aFrame)
-{
-  // The invariant that "continuing frames should be found as part of the
-  // walk over the top-most frame's continuing frames" does not hold for
-  // out-of-flow overflow containers, so we need to walk them too.
-  // Note that DoDeletingFrameSubtree() skips the child lists where
-  // overflow containers live so we won't process them twice.
-  const PRBool orphanSubtree = aRemovedFrame == aFrame;
-  for (nsIFrame* next = aFrame->GetNextContinuation();
-       next && (next->GetStateBits() & NS_FRAME_IS_OVERFLOW_CONTAINER);
-       next = next->GetNextContinuation()) {
-    DoDeletingFrameSubtree(aFrameManager, aDestroyQueue,
-                           orphanSubtree ? next : aRemovedFrame,
-                           next);
-  }
-}
-
-/**
- * Called when a frame subtree is about to be deleted. Two important
- * things happen:
- *
- * 1. For each frame in the subtree, we remove the mapping from the
- *    content object to its frame
- *
- * 2. For child frames that have been moved out of the flow, we enqueue
- *    the out-of-flow frame for deletion *if* the out-of-flow frame's
- *    geometric parent is not in |aRemovedFrame|'s hierarchy (e.g., an
- *    absolutely positioned element that has been promoted to be a direct
- *    descendant of an area frame).
- *
- * Note: this function should only be called by DeletingFrameSubtree()
- *
- * @param   aRemovedFrame this is the frame that was removed from the
- *            content model. As we recurse we need to remember this so we
- *            can check if out-of-flow frames are a descendant of the frame
- *            being removed
- * @param   aFrame the local subtree that is being deleted. This is initially
- *            the same as aRemovedFrame, but as we recurse down the tree
- *            this changes
- */
-static void
-DoDeletingFrameSubtree(nsFrameManager*      aFrameManager,
-                       nsTArray<nsIFrame*>& aDestroyQueue,
-                       nsIFrame*            aRemovedFrame,
-                       nsIFrame*            aFrame)
-{
-#undef RECURSE
-#define RECURSE(top, child)                                                  \
-  DoDeletingFrameSubtree(aFrameManager, aDestroyQueue, (top), (child));      \
-  DoDeletingOverflowContainers(aFrameManager, aDestroyQueue, (top), (child));
-
-  // Remove the mapping from the content object to its frame.
-  nsIContent* content = aFrame->GetContent();
-  if (content) {
-    aFrameManager->RemoveAsPrimaryFrame(content, aFrame);
-    aFrameManager->ClearAllUndisplayedContentIn(content);
-  }
-
-  nsIAtom* childListName = nsnull;
-  PRInt32 childListIndex = 0;
-
-  do {
-    // Walk aFrame's normal flow child frames looking for placeholder frames.
-    nsIFrame* childFrame = aFrame->GetFirstChild(childListName);
-    for (; childFrame; childFrame = childFrame->GetNextSibling()) {
-      NS_ASSERTION(!(childFrame->GetStateBits() & NS_FRAME_OUT_OF_FLOW),
-                   "out-of-flow on wrong child list");
-      if (NS_LIKELY(nsGkAtoms::placeholderFrame != childFrame->GetType())) {
-        RECURSE(aRemovedFrame, childFrame);
-      } else {
-        nsIFrame* outOfFlowFrame =
-          nsPlaceholderFrame::GetRealFrameForPlaceholder(childFrame);
-  
-        // Don't SetOutOfFlowFrame(nsnull) here because the float cache depends
-        // on it when the float is removed later on, see bug 348688 comment 6.
-        
-        // Queue the out-of-flow frame to be destroyed only if aRemovedFrame is _not_
-        // one of its ancestor frames or if it is a popup frame. 
-        // If aRemovedFrame is an ancestor of the out-of-flow frame, then 
-        // the out-of-flow frame will be destroyed by aRemovedFrame.
-        if (outOfFlowFrame->GetStyleDisplay()->mDisplay == NS_STYLE_DISPLAY_POPUP ||
-            !nsLayoutUtils::IsProperAncestorFrame(aRemovedFrame, outOfFlowFrame)) {
-          NS_ASSERTION(!aDestroyQueue.Contains(outOfFlowFrame),
-                       "out-of-flow is already in the destroy queue");
-          aDestroyQueue.AppendElement(outOfFlowFrame);
-          // Recurse into the out-of-flow, it is now the aRemovedFrame.
-          RECURSE(outOfFlowFrame, outOfFlowFrame);
-        }
-        else {
-          // Also recurse into the out-of-flow when it's a descendant of aRemovedFrame
-          // since we don't walk those lists, see |childListName| increment below.
-          RECURSE(aRemovedFrame, outOfFlowFrame);
-        }
-      }
-    }
-
-    // Move to next child list but skip lists with frames we should have
-    // a placeholder for or that contains only next-in-flow overflow containers
-    // (which we walk explicitly above).
-    do {
-      childListName = aFrame->GetAdditionalChildListName(childListIndex++);
-    } while (IsOutOfFlowList(childListName) ||
-             childListName == nsGkAtoms::overflowContainersList ||
-             childListName == nsGkAtoms::excessOverflowContainersList);
-  } while (childListName);
-}
-
-/**
- * Called when a frame is about to be deleted. Calls DoDeletingFrameSubtree()
- * for aFrame and each of its continuing frames
- */
-static nsresult
-DeletingFrameSubtree(nsFrameManager* aFrameManager,
-                     nsIFrame*       aFrame)
-{
-  NS_ENSURE_TRUE(aFrame, NS_OK); // XXXldb Remove this sometime in the future.
-
-  // If there's no frame manager it's probably because the pres shell is
-  // being destroyed.
-  if (NS_UNLIKELY(!aFrameManager)) {
-    return NS_OK;
-  }
-
-  nsAutoTArray<nsIFrame*, 8> destroyQueue;
-
-  // If it's a "special" block-in-inline frame, then we can't really deal.
-  // That really shouldn't be happening.
-  NS_ASSERTION(!IsFrameSpecial(aFrame),
-               "DeletingFrameSubtree on a special frame.  Prepare to crash.");
-
-  do {
-    DoDeletingFrameSubtree(aFrameManager, destroyQueue, aFrame, aFrame);
-
-    // If it's split, then get the continuing frame. Note that we only do
-    // this for the top-most frame being deleted. Don't do it if we're
-    // recursing over a subtree, because those continuing frames should be
-    // found as part of the walk over the top-most frame's continuing frames.
-    // Walking them again will make this an N^2/2 algorithm.
-    // The above is true for normal child next-in-flows but not overflow
-    // containers which we do walk because they *can* escape the subtree
-    // we're deleting.  We skip [excess]overflowContainersList where
-    // they live to avoid processing them more than once.
-    aFrame = aFrame->GetNextContinuation();
-  } while (aFrame);
-
-  // Now destroy any out-of-flow frames that have been enqueued for
-  // destruction.
-  for (PRInt32 i = destroyQueue.Length() - 1; i >= 0; --i) {
-    nsIFrame* outOfFlowFrame = destroyQueue[i];
-
-    // Ask the out-of-flow's parent to delete the out-of-flow
-    // frame from the right list.
-    aFrameManager->RemoveFrame(outOfFlowFrame->GetParent(),
-                               GetChildListNameFor(outOfFlowFrame),
-                               outOfFlowFrame);
-  }
-
-  return NS_OK;
-}
-
-nsresult
-nsCSSFrameConstructor::RemoveMappingsForFrameSubtree(nsIFrame* aRemovedFrame)
-{
-  NS_ASSERTION(!(aRemovedFrame->GetStateBits() & NS_FRAME_OUT_OF_FLOW),
-               "RemoveMappingsForFrameSubtree doesn't handle out-of-flows");
-
-  if (NS_UNLIKELY(mIsDestroyingFrameTree)) {
-    // The frame tree might not be in a consistent state after
-    // WillDestroyFrameTree() has been called. Most likely we're destroying
-    // the pres shell which means the frame manager takes care of clearing all
-    // mappings so there is no need to walk the frame tree here, bug 372576.
-    return NS_OK;
-  }
-
-  nsFrameManager *frameManager = mPresShell->FrameManager();
-  if (nsGkAtoms::placeholderFrame == aRemovedFrame->GetType()) {
-    nsIFrame *placeholderFrame = aRemovedFrame;
-    do {
-      NS_ASSERTION(placeholderFrame->GetType() == nsGkAtoms::placeholderFrame,
-                   "continuation must be of same type");
-      nsIFrame* outOfFlowFrame =
-        nsPlaceholderFrame::GetRealFrameForPlaceholder(placeholderFrame);
-      // Remove the mapping from the out-of-flow frame to its placeholder.
-      frameManager->UnregisterPlaceholderFrame(
-        static_cast<nsPlaceholderFrame*>(placeholderFrame));
-      ::DeletingFrameSubtree(frameManager, outOfFlowFrame);
-      frameManager->RemoveFrame(outOfFlowFrame->GetParent(),
-                                GetChildListNameFor(outOfFlowFrame),
-                                outOfFlowFrame);
-      placeholderFrame = placeholderFrame->GetNextContinuation();
-    } while (placeholderFrame);
-  }
-
-  // Save the frame tree's state before deleting it
-  CaptureStateFor(aRemovedFrame, mTempFrameTreeState);
-
-  return ::DeletingFrameSubtree(frameManager, aRemovedFrame);
-}
-
 nsresult
 nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer,
                                       nsIContent* aChild,
                                       PRInt32     aIndexInContainer,
                                       RemoveFlags aFlags,
                                       PRBool*     aDidReconstruct)
 {
   AUTO_LAYOUT_PHASE_ENTRY_POINT(mPresShell->GetPresContext(), FrameC);
@@ -7388,55 +7046,26 @@ nsCSSFrameConstructor::ContentRemoved(ns
     if (gReallyNoisyContentUpdates) {
       printf("nsCSSFrameConstructor::ContentRemoved: childFrame=");
       nsFrame::ListTag(stdout, childFrame);
       putchar('\n');
       parentFrame->List(stdout, 0);
     }
 #endif
 
-    // Walk the frame subtree deleting any out-of-flow frames, and
-    // remove the mapping from content objects to frames
-    ::DeletingFrameSubtree(frameManager, childFrame);
-
-    // See if the child frame is an out-of-flow
+
+    // Notify the parent frame that it should delete the frame
     if (childFrame->GetStateBits() & NS_FRAME_OUT_OF_FLOW) {
-      nsPlaceholderFrame* placeholderFrame =
-        frameManager->GetPlaceholderFrameFor(childFrame);
-      NS_ASSERTION(placeholderFrame, "No placeholder for out-of-flow?");
-
-      // Now we remove the out-of-flow frame
-      // XXX has to be done first for now: for floats, the block's line list
-      // contains an array of pointers to the placeholder - we have to
-      // remove the float first (which gets rid of the lines
-      // reference to the placeholder and float) and then remove the
-      // placeholder
-      rv = frameManager->RemoveFrame(parentFrame,
-                                     GetChildListNameFor(childFrame),
-                                     childFrame);
-
-      // Remove the placeholder frame first (XXX second for now) (so
-      // that it doesn't retain a dangling pointer to memory)
-      nsIFrame* placeholderParent = placeholderFrame->GetParent();
-      ::DeletingFrameSubtree(frameManager, placeholderFrame);
-      rv |= frameManager->RemoveFrame(placeholderParent,
-                                      nsnull, placeholderFrame);
-    } else {
-      // Notify the parent frame that it should delete the frame
-      // check for a table caption which goes on an additional child list with a different parent
-      nsIFrame* outerTableFrame; 
-      if (GetCaptionAdjustedParent(parentFrame, childFrame, &outerTableFrame)) {
-        rv = frameManager->RemoveFrame(outerTableFrame,
-                                       nsGkAtoms::captionList,
-                                       childFrame);
-      }
-      else {
-        rv = frameManager->RemoveFrame(parentFrame, nsnull, childFrame);
-      }
-    }
+      childFrame = frameManager->GetPlaceholderFrameFor(childFrame);
+      NS_ASSERTION(childFrame, "Missing placeholder frame for out of flow.");
+      parentFrame = childFrame->GetParent();
+    }
+    rv = frameManager->RemoveFrame(nsLayoutUtils::GetChildListNameFor(childFrame),
+                                   childFrame);
+    //XXXfr NS_ENSURE_SUCCESS(rv, rv) ?
 
     if (isRoot) {
       mRootElementFrame = nsnull;
       mRootElementStyleFrame = nsnull;
       mDocElementContainingBlock = nsnull;
       mPageSequenceFrame = nsnull;
       mGfxScrollFrame = nsnull;
       mHasRootAbsPosContainingBlock = PR_FALSE;
@@ -10309,18 +9938,17 @@ nsCSSFrameConstructor::WrapFramesInFirst
     if (parentFrame == aBlockFrame) {
       // Take textFrame out of the block's frame list and substitute the
       // letter frame(s) instead.
       aBlockFrames.DestroyFrame(textFrame);
       aBlockFrames.InsertFrames(nsnull, prevFrame, letterFrames);
     }
     else {
       // Take the old textFrame out of the inline parent's child list
-      ::DeletingFrameSubtree(mPresShell->FrameManager(), textFrame);
-      parentFrame->RemoveFrame(nsnull, textFrame);
+      mPresShell->FrameManager()->RemoveFrame(nsnull, textFrame);
 
       // Insert in the letter frame(s)
       parentFrame->InsertFrames(nsnull, prevFrame, letterFrames);
     }
   }
 
   return rv;
 }
@@ -10456,41 +10084,31 @@ nsCSSFrameConstructor::RemoveFloatingFir
     return NS_ERROR_OUT_OF_MEMORY;;
   }
   newTextFrame->Init(textContent, parentFrame, nsnull);
 
   // Destroy the old text frame's continuations (the old text frame
   // will be destroyed when its letter frame is destroyed).
   nsIFrame* frameToDelete = textFrame->GetLastContinuation();
   while (frameToDelete != textFrame) {
-    nsIFrame* frameToDeleteParent = frameToDelete->GetParent();
     nsIFrame* nextFrameToDelete = frameToDelete->GetPrevContinuation();
-    if (frameToDeleteParent) {
-      ::DeletingFrameSubtree(aFrameManager, frameToDelete);
-      aFrameManager->RemoveFrame(frameToDeleteParent, nsnull, frameToDelete);
-    }
+    aFrameManager->RemoveFrame(nsnull, frameToDelete);
     frameToDelete = nextFrameToDelete;
   }
 
   nsIFrame* prevSibling = placeholderFrame->GetPrevSibling();
 
   // Now that everything is set...
 #ifdef NOISY_FIRST_LETTER
   printf("RemoveFloatingFirstLetterFrames: textContent=%p oldTextFrame=%p newTextFrame=%p\n",
          textContent.get(), textFrame, newTextFrame);
 #endif
 
-  // Remove the float frame
-  ::DeletingFrameSubtree(aFrameManager, floatFrame);
-  aFrameManager->RemoveFrame(aBlockFrame, nsGkAtoms::floatList,
-                             floatFrame);
-
-  // Remove placeholder frame
-  ::DeletingFrameSubtree(aFrameManager, placeholderFrame);
-  aFrameManager->RemoveFrame(parentFrame, nsnull, placeholderFrame);
+  // Remove placeholder frame and the float
+  aFrameManager->RemoveFrame(nsnull, placeholderFrame);
 
   // Insert text frame in its place
   nsFrameList textList(newTextFrame, newTextFrame);
   aFrameManager->InsertFrames(parentFrame, nsnull, prevSibling, textList);
 
   return NS_OK;
 }
 
@@ -10525,18 +10143,17 @@ nsCSSFrameConstructor::RemoveFirstLetter
       newSC = aPresShell->StyleSet()->ResolveStyleForNonElement(parentSC);
       if (!newSC) {
         break;
       }
       textFrame = NS_NewTextFrame(aPresShell, newSC);
       textFrame->Init(textContent, aFrame, nsnull);
 
       // Next rip out the kid and replace it with the text frame
-      ::DeletingFrameSubtree(aFrameManager, kid);
-      aFrameManager->RemoveFrame(aFrame, nsnull, kid);
+      aFrameManager->RemoveFrame(nsnull, kid);
 
       // Insert text frame in its place
       nsFrameList textList(textFrame, textFrame);
       aFrameManager->InsertFrames(aFrame, nsnull, prevSibling, textList);
 
       *aStopLooking = PR_TRUE;
       aFrame->RemoveStateBits(NS_BLOCK_HAS_FIRST_LETTER_CHILD);
       break;
@@ -10607,18 +10224,17 @@ nsCSSFrameConstructor::RecoverLetterFram
     if (stopLooking) {
       break;
     }
     aBlockFrame = aBlockFrame->GetNextContinuation();
   } while (aBlockFrame);
 
   if (parentFrame) {
     // Take the old textFrame out of the parents child list
-    ::DeletingFrameSubtree(mPresShell->FrameManager(), textFrame);
-    parentFrame->RemoveFrame(nsnull, textFrame);
+    mPresShell->FrameManager()->RemoveFrame(nsnull, textFrame);
 
     // Insert in the letter frame(s)
     parentFrame->InsertFrames(nsnull, prevFrame, letterFrames);
   }
   return rv;
 }
 
 //----------------------------------------------------------------------
--- a/layout/base/nsCSSFrameConstructor.h
+++ b/layout/base/nsCSSFrameConstructor.h
@@ -98,16 +98,17 @@ public:
   // Maintain global objects - gXBLService
   static nsIXBLService * GetXBLService();
   static void ReleaseGlobals() { NS_IF_RELEASE(gXBLService); }
 
   // get the alternate text for a content node
   static void GetAlternateTextFor(nsIContent*    aContent,
                                   nsIAtom*       aTag,  // content object's tag
                                   nsXPIDLString& aAltText);
+
 private: 
   // These are not supported and are not implemented! 
   nsCSSFrameConstructor(const nsCSSFrameConstructor& aCopy); 
   nsCSSFrameConstructor& operator=(const nsCSSFrameConstructor& aCopy); 
 
 public:
   // XXXbz this method needs to actually return errors!
   nsresult ConstructRootFrame(nsIFrame** aNewFrame);
@@ -303,18 +304,16 @@ public:
                                 nsIFrame*       aParentFrame,
                                 nsIFrame*       aPrevFrame,
                                 nsIContent*     aChild,
                                 nsIFrame**      aResult,
                                 PRBool          aIsAppend,
                                 PRBool          aIsScrollbar,
                                 nsILayoutHistoryState* aFrameState);
 
-  nsresult RemoveMappingsForFrameSubtree(nsIFrame* aRemovedFrame);
-
   // GetInitialContainingBlock() is deprecated in favor of GetRootElementFrame();
   // nsIFrame* GetInitialContainingBlock() { return mRootElementFrame; }
   // This returns the outermost frame for the root element
   nsIFrame* GetRootElementFrame() { return mRootElementFrame; }
   // This returns the frame for the root element that does not
   // have a psuedo-element style
   nsIFrame* GetRootElementStyleFrame() { return mRootElementStyleFrame; }
   nsIFrame* GetPageSequenceFrame() { return mPageSequenceFrame; }
--- a/layout/base/nsFrameManager.cpp
+++ b/layout/base/nsFrameManager.cpp
@@ -711,54 +711,56 @@ nsFrameManager::InsertFrames(nsIFrame*  
                   || IS_TRUE_OVERFLOW_CONTAINER(aPrevFrame->GetNextContinuation()))
                   && !IS_TRUE_OVERFLOW_CONTAINER(aPrevFrame),
                   "aPrevFrame must be the last continuation in its chain!");
 
   return aParentFrame->InsertFrames(aListName, aPrevFrame, aFrameList);
 }
 
 nsresult
-nsFrameManager::RemoveFrame(nsIFrame*       aParentFrame,
-                            nsIAtom*        aListName,
+nsFrameManager::RemoveFrame(nsIAtom*        aListName,
                             nsIFrame*       aOldFrame)
 {
   PRBool wasDestroyingFrames = mIsDestroyingFrames;
   mIsDestroyingFrames = PR_TRUE;
 
   // In case the reflow doesn't invalidate anything since it just leaves
   // a gap where the old frame was, we invalidate it here.  (This is
   // reasonably likely to happen when removing a last child in a way
   // that doesn't change the size of the parent.)
   // This has to sure to invalidate the entire overflow rect; this
   // is important in the presence of absolute positioning
   aOldFrame->Invalidate(aOldFrame->GetOverflowRect());
 
-  nsresult rv = aParentFrame->RemoveFrame(aListName, aOldFrame);
+  NS_ASSERTION(!aOldFrame->GetPrevContinuation() ||
+               // exception for nsCSSFrameConstructor::RemoveFloatingFirstLetterFrames
+               aOldFrame->GetType() == nsGkAtoms::textFrame,
+               "Must remove first continuation.");
+  NS_ASSERTION(!(aOldFrame->GetStateBits() & NS_FRAME_OUT_OF_FLOW &&
+                 GetPlaceholderFrameFor(aOldFrame)),
+               "Must call RemoveFrame on placeholder for out-of-flows.");
+  nsresult rv = aOldFrame->GetParent()->RemoveFrame(aListName, aOldFrame);
 
   mIsDestroyingFrames = wasDestroyingFrames;
 
   return rv;
 }
 
 //----------------------------------------------------------------------
 
 void
 nsFrameManager::NotifyDestroyingFrame(nsIFrame* aFrame)
 {
-  // We've already removed from the primary frame map once, but we're
-  // going to try to do it again here to fix callers of GetPrimaryFrameFor
-  // during frame destruction, since this problem keeps coming back to
-  // bite us.  We may want to remove the previous caller.
-  if (mPrimaryFrameMap.ops) {
-    PrimaryFrameMapEntry *entry = static_cast<PrimaryFrameMapEntry*>
-                                             (PL_DHashTableOperate(&mPrimaryFrameMap, aFrame->GetContent(), PL_DHASH_LOOKUP));
-    if (PL_DHASH_ENTRY_IS_BUSY(entry) && entry->frame == aFrame) {
-      NS_NOTREACHED("frame was not removed from primary frame map before "
-                    "destruction or was readded to map after being removed");
-      PL_DHashTableRawRemove(&mPrimaryFrameMap, entry);
+  //XXXfr Because we destroy most continuation chains starting from the FIF
+  // this does excess work by triggering on every continuation in the chain
+  nsIContent* content = aFrame->GetContent();
+  if (!aFrame->GetPrevContinuation() && content) {
+    RemoveAsPrimaryFrame(content, aFrame);
+    if (content != aFrame->GetParent()->GetContent()) { // first-letter
+      ClearAllUndisplayedContentIn(content);
     }
   }
 }
 
 #ifdef NS_DEBUG
 static void
 DumpContext(nsIFrame* aFrame, nsStyleContext* aContext)
 {
--- a/layout/base/nsFrameManager.h
+++ b/layout/base/nsFrameManager.h
@@ -154,18 +154,17 @@ public:
     return aParentFrame->AppendFrames(aListName, aFrameList);
   }
 
   NS_HIDDEN_(nsresult) InsertFrames(nsIFrame*       aParentFrame,
                                     nsIAtom*        aListName,
                                     nsIFrame*       aPrevFrame,
                                     nsFrameList&    aFrameList);
 
-  NS_HIDDEN_(nsresult) RemoveFrame(nsIFrame*       aParentFrame,
-                                   nsIAtom*        aListName,
+  NS_HIDDEN_(nsresult) RemoveFrame(nsIAtom*        aListName,
                                    nsIFrame*       aOldFrame);
 
   /*
    * Notification that a frame is about to be destroyed. This allows any
    * outstanding references to the frame to be cleaned up.
    */
   NS_HIDDEN_(void)     NotifyDestroyingFrame(nsIFrame* aFrame);
 
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -175,16 +175,108 @@ GetLastChildFrame(nsIFrame*       aFrame
     }
 
     return lastChildFrame;
   }
 
   return nsnull;
 }
 
+//static
+nsIAtom*
+nsLayoutUtils::GetChildListNameFor(nsIFrame* aChildFrame)
+{
+  nsIAtom*      listName;
+
+  if (aChildFrame->GetStateBits() & NS_FRAME_IS_OVERFLOW_CONTAINER) {
+    nsIFrame* pif = aChildFrame->GetPrevInFlow();
+    if (pif->GetParent() == aChildFrame->GetParent()) {
+      listName = nsGkAtoms::excessOverflowContainersList;
+    }
+    else {
+      listName = nsGkAtoms::overflowContainersList;
+    }
+  }
+  // See if the frame is moved out of the flow
+  else if (aChildFrame->GetStateBits() & NS_FRAME_OUT_OF_FLOW) {
+    // Look at the style information to tell
+    const nsStyleDisplay* disp = aChildFrame->GetStyleDisplay();
+
+    if (NS_STYLE_POSITION_ABSOLUTE == disp->mPosition) {
+      listName = nsGkAtoms::absoluteList;
+    } else if (NS_STYLE_POSITION_FIXED == disp->mPosition) {
+      if (nsLayoutUtils::IsReallyFixedPos(aChildFrame)) {
+        listName = nsGkAtoms::fixedList;
+      } else {
+        listName = nsGkAtoms::absoluteList;
+      }
+#ifdef MOZ_XUL
+    } else if (NS_STYLE_DISPLAY_POPUP == disp->mDisplay) {
+      // Out-of-flows that are DISPLAY_POPUP must be kids of the root popup set
+#ifdef DEBUG
+      nsIFrame* parent = aChildFrame->GetParent();
+      NS_ASSERTION(parent && parent->GetType() == nsGkAtoms::popupSetFrame,
+                   "Unexpected parent");
+#endif // DEBUG
+
+      // XXX FIXME: Bug 350740
+      // Return here, because the postcondition for this function actually
+      // fails for this case, since the popups are not in a "real" frame list
+      // in the popup set.
+      return nsGkAtoms::popupList;
+#endif // MOZ_XUL
+    } else {
+      NS_ASSERTION(aChildFrame->GetStyleDisplay()->IsFloating(),
+                   "not a floated frame");
+      listName = nsGkAtoms::floatList;
+    }
+
+  } else {
+    nsIAtom* childType = aChildFrame->GetType();
+    if (nsGkAtoms::menuPopupFrame == childType) {
+      nsIFrame* parent = aChildFrame->GetParent();
+      nsIFrame* firstPopup = (parent)
+                             ? parent->GetFirstChild(nsGkAtoms::popupList)
+                             : nsnull;
+      NS_ASSERTION(!firstPopup || !firstPopup->GetNextSibling(),
+                   "We assume popupList only has one child, but it has more.");
+      listName = (!firstPopup || firstPopup == aChildFrame)
+                 ? nsGkAtoms::popupList
+                 : nsnull;
+    } else if (nsGkAtoms::tableColGroupFrame == childType) {
+      listName = nsGkAtoms::colGroupList;
+    } else if (nsGkAtoms::tableCaptionFrame == aChildFrame->GetType()) {
+      listName = nsGkAtoms::captionList;
+    } else {
+      listName = nsnull;
+    }
+  }
+
+#ifdef NS_DEBUG
+  // Verify that the frame is actually in that child list or in the
+  // corresponding overflow list.
+  nsIFrame* parent = aChildFrame->GetParent();
+  PRBool found = parent->GetChildList(listName).ContainsFrame(aChildFrame);
+  if (!found) {
+    if (!(aChildFrame->GetStateBits() & NS_FRAME_OUT_OF_FLOW)) {
+      found = parent->GetChildList(nsGkAtoms::overflowList)
+                .ContainsFrame(aChildFrame);
+    }
+    else if (aChildFrame->GetStyleDisplay()->IsFloating()) {
+      found = parent->GetChildList(nsGkAtoms::overflowOutOfFlowList)
+                .ContainsFrame(aChildFrame);
+    }
+    // else it's positioned and should have been on the 'listName' child list.
+    NS_POSTCONDITION(found, "not in child list");
+  }
+#endif
+
+  return listName;
+}
+
 // static
 nsIFrame*
 nsLayoutUtils::GetBeforeFrame(nsIFrame* aFrame)
 {
   NS_PRECONDITION(aFrame, "NULL frame pointer");
   NS_ASSERTION(!aFrame->GetPrevContinuation(),
                "aFrame must be first continuation");
   
--- a/layout/base/nsLayoutUtils.h
+++ b/layout/base/nsLayoutUtils.h
@@ -71,16 +71,23 @@ class nsBlockFrame;
 /**
  * nsLayoutUtils is a namespace class used for various helper
  * functions that are useful in multiple places in layout.  The goal
  * is not to define multiple copies of the same static helper.
  */
 class nsLayoutUtils
 {
 public:
+
+  /**
+   * Uses heuristics to figure out the appropriate child list name
+   * for aChildFrame.
+   */
+  static nsIAtom* GetChildListNameFor(nsIFrame* aChildFrame);
+
   /**
    * GetBeforeFrame returns the outermost :before frame of the given frame, if
    * one exists.  This is typically O(1).  The frame passed in must be
    * the first-in-flow.   
    *
    * @param aFrame the frame whose :before is wanted
    * @return the :before frame or nsnull if there isn't one
    */
--- a/layout/generic/nsBlockFrame.cpp
+++ b/layout/generic/nsBlockFrame.cpp
@@ -5291,16 +5291,17 @@ nsBlockFrame::DoRemoveFrame(nsIFrame* aD
     // first frame in the main or overflow list.
     if (searchingOverflowList) {
       nsIFrame* prevSibling = aDeletedFrame->GetPrevSibling();
       if (prevSibling) {
         // XXXbz If we switch overflow lines to nsFrameList, we should
         // change this SetNextSibling call.
         prevSibling->SetNextSibling(nextFrame);
       }
+      aDeletedFrame->SetNextSibling(nsnull);
     } else {
       mFrames.RemoveFrame(aDeletedFrame);
     }
 
     // Update the child count of the line to be accurate
     PRInt32 lineChildCount = line->GetChildCount();
     lineChildCount--;
     line->SetChildCount(lineChildCount);
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -413,16 +413,18 @@ nsFrame::RemoveFrame(nsIAtom*        aLi
   return NS_ERROR_UNEXPECTED;
 }
 
 void
 nsFrame::Destroy()
 {
   NS_ASSERTION(!nsContentUtils::IsSafeToRunScript(),
     "destroy called on frame while scripts not blocked");
+  NS_ASSERTION(!GetNextSibling() && !GetPrevSibling(),
+               "Frames should be removed before destruction.");
 
 #ifdef MOZ_SVG
   nsSVGEffects::InvalidateDirectRenderingObservers(this);
 #endif
 
   // Get the view pointer now before the frame properties disappear
   // when we call NotifyDestroyingFrame()
   nsIView* view = GetView();
--- a/layout/generic/nsFrameList.cpp
+++ b/layout/generic/nsFrameList.cpp
@@ -69,21 +69,18 @@ nsFrameList::Destroy()
 
   DestroyFrames();
   delete this;
 }
 
 void
 nsFrameList::DestroyFrames()
 {
-  nsIFrame* next;
-  for (nsIFrame* frame = mFirstChild; frame; frame = next) {
-    next = frame->GetNextSibling();
+  while (nsIFrame* frame = RemoveFirstChild()) {
     frame->Destroy();
-    mFirstChild = next;
   }
 
   mLastChild = nsnull;
 }
 
 void
 nsFrameList::SetFrames(nsIFrame* aFrameList)
 {
@@ -147,24 +144,25 @@ nsFrameList::RemoveFramesAfter(nsIFrame*
   nsIFrame* tail = aAfterFrame->GetNextSibling();
   // if (!tail) return EmptyList();  -- worth optimizing this case?
   nsIFrame* oldLastChild = mLastChild;
   mLastChild = aAfterFrame;
   aAfterFrame->SetNextSibling(nsnull);
   return nsFrameList(tail, tail ? oldLastChild : nsnull);
 }
 
-PRBool
+nsIFrame*
 nsFrameList::RemoveFirstChild()
 {
   if (mFirstChild) {
-    RemoveFrame(mFirstChild);
-    return PR_TRUE;
+    nsIFrame* firstChild = mFirstChild;
+    RemoveFrame(firstChild);
+    return firstChild;
   }
-  return PR_FALSE;
+  return nsnull;
 }
 
 void
 nsFrameList::DestroyFrame(nsIFrame* aFrame)
 {
   NS_PRECONDITION(aFrame, "null ptr");
   RemoveFrame(aFrame);
   aFrame->Destroy();
--- a/layout/generic/nsFrameList.h
+++ b/layout/generic/nsFrameList.h
@@ -141,19 +141,19 @@ public:
    * Take the frames after aAfterFrame out of the frame list.
    * @param aAfterFrame a frame in this list
    * @return the removed frames, if any
    */
   nsFrameList RemoveFramesAfter(nsIFrame* aAfterFrame);
 
   /**
    * Take the first frame (if any) out of the frame list.
-   * @return PR_TRUE if a frame was removed
+   * @return the first child, or nsnull if the list is empty
    */
-  PRBool RemoveFirstChild();
+  nsIFrame* RemoveFirstChild();
 
   /**
    * Take aFrame out of the frame list and then destroy it.
    * The frame must be non-null and present on this list.
    */
   void DestroyFrame(nsIFrame* aFrame);
 
   /**
--- a/layout/generic/nsIFrame.h
+++ b/layout/generic/nsIFrame.h
@@ -516,17 +516,21 @@ public:
    * @param   aPrevInFlow the prev-in-flow frame
    */
   NS_IMETHOD  Init(nsIContent*      aContent,
                    nsIFrame*        aParent,
                    nsIFrame*        aPrevInFlow) = 0;
 
   /**
    * Destroys this frame and each of its child frames (recursively calls
-   * Destroy() for each child)
+   * Destroy() for each child). If this frame is a first-continuation, this
+   * also removes the frame from the primary frame man and clears undisplayed
+   * content for its content node.
+   * If the frame is a placeholder, it also ensures the out-of-flow frame's
+   * removal and destruction.
    */
   virtual void Destroy() = 0;
 
   /**
    * Called to set the initial list of frames. This happens after the frame
    * has been initialized.
    *
    * This is only called once for a given child list, and won't be called
--- a/layout/generic/nsLineBox.cpp
+++ b/layout/generic/nsLineBox.cpp
@@ -326,16 +326,17 @@ nsLineBox::DeleteLineList(nsPresContext*
     // Delete our child frames before doing anything else. In particular
     // we do all of this before our base class releases it's hold on the
     // view.
 #ifdef DEBUG
     PRInt32 numFrames = 0;
 #endif
     for (nsIFrame* child = aLines.front()->mFirstChild; child; ) {
       nsIFrame* nextChild = child->GetNextSibling();
+      child->SetNextSibling(nsnull);
       child->Destroy();
       child = nextChild;
 #ifdef DEBUG
       numFrames++;
 #endif
     }
 
     nsIPresShell *shell = aPresContext->PresShell();
--- a/layout/generic/nsPlaceholderFrame.cpp
+++ b/layout/generic/nsPlaceholderFrame.cpp
@@ -36,16 +36,17 @@
  *
  * ***** END LICENSE BLOCK ***** */
 
 /*
  * rendering object for the point that anchors out-of-flow rendering
  * objects such as floats and absolutely positioned elements
  */
 
+#include "nsLayoutUtils.h"
 #include "nsPlaceholderFrame.h"
 #include "nsLineLayout.h"
 #include "nsIContent.h"
 #include "nsPresContext.h"
 #include "nsIRenderingContext.h"
 #include "nsGkAtoms.h"
 #include "nsFrameManager.h"
 #include "nsDisplayList.h"
@@ -124,24 +125,23 @@ nsPlaceholderFrame::Reflow(nsPresContext
   NS_FRAME_SET_TRUNCATION(aStatus, aReflowState, aDesiredSize);
   return NS_OK;
 }
 
 void
 nsPlaceholderFrame::Destroy()
 {
   nsIPresShell* shell = PresContext()->GetPresShell();
-  if (shell && mOutOfFlowFrame) {
-    if (shell->FrameManager()->GetPlaceholderFrameFor(mOutOfFlowFrame)) {
-      NS_ERROR("Placeholder relationship should have been torn down; see "
-               "comments in nsPlaceholderFrame.h.  Unregistering ourselves, "
-               "but this might cause our out-of-flow to be unable to destroy "
-               "itself properly.  Not that it could anyway, with us dead.");
-      shell->FrameManager()->UnregisterPlaceholderFrame(this);
-    }
+  nsIFrame* oof = mOutOfFlowFrame;
+  if (oof) {
+    NS_ASSERTION(shell && oof->GetParent(), "Null presShell or parent!?");
+    // Unregister and destroy out-of-flow frame
+    shell->FrameManager()->UnregisterPlaceholderFrame(this);
+    nsIAtom* listName = nsLayoutUtils::GetChildListNameFor(oof);
+    shell->FrameManager()->RemoveFrame(listName, oof);
   }
 
   nsFrame::Destroy();
 }
 
 nsIAtom*
 nsPlaceholderFrame::GetType() const
 {
--- a/layout/generic/nsPlaceholderFrame.h
+++ b/layout/generic/nsPlaceholderFrame.h
@@ -48,32 +48,24 @@
  *   removed from the frame manager before the placeholder is destroyed.
  * - The mapping from the out-of-flow to the placeholder must be
  *   removed from the frame manager before the out-of-flow is destroyed.
  * - The placeholder must be removed from the frame tree, or have the
  *   mapping from it to its out-of-flow cleared, before the out-of-flow
  *   is destroyed (so that the placeholder will not point to a destroyed
  *   frame while it's in the frame tree).
  *
- * Therefore the safe order of teardown is to:
- *
- * 1)  Unregister the placeholder from the frame manager.
- * 2)  Destroy the placeholder
- * 3)  Destroy the out of flow
- *
- * In certain cases it may be possible to replace step (2) with:
+ * Furthermore, some code assumes that placeholders point to something
+ * useful, so placeholders without an associated out-of-flow should not
+ * remain in the tree.
  *
- * 2') Null out the mOutOfFlowFrame pointer in the placeholder
- *
- * and add
- *
- * 4) Destroy the placeholder
- *
- * but this is somewhat dangerous, since lots of code assumes that
- * placeholders point to something useful.
+ * The placeholder's Destroy() implementation handles the destruction of
+ * the placeholder and its out-of-flow. To avoid crashes, frame removal
+ * and destruction code that works with placeholders must not assume
+ * that the placeholder points to its out-of-flow.
  */
 
 #ifndef nsPlaceholderFrame_h___
 #define nsPlaceholderFrame_h___
 
 #include "nsFrame.h"
 #include "nsGkAtoms.h"
 
--- a/layout/tables/nsTableFrame.cpp
+++ b/layout/tables/nsTableFrame.cpp
@@ -2318,24 +2318,21 @@ nsTableFrame::InsertFrames(nsIAtom*     
 #endif
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsTableFrame::RemoveFrame(nsIAtom*        aListName,
                           nsIFrame*       aOldFrame)
 {
-  // See what kind of frame we have
-  const nsStyleDisplay* display = aOldFrame->GetStyleDisplay();
-
-  // XXX The frame construction code should be separating out child frames
-  // based on the type, bug 343048.
-  if (NS_STYLE_DISPLAY_TABLE_COLUMN_GROUP == display->mDisplay) {
-    NS_ASSERTION(!aListName || aListName == nsGkAtoms::colGroupList,
-                 "unexpected child list");
+  NS_ASSERTION(aListName == nsGkAtoms::colGroupList ||
+               NS_STYLE_DISPLAY_TABLE_COLUMN_GROUP !=
+                 aOldFrame->GetStyleDisplay()->mDisplay,
+               "Wrong list name; use nsGkAtoms::colGroupList iff colgroup");
+  if (aListName == nsGkAtoms::colGroupList) {
     nsIFrame* nextColGroupFrame = aOldFrame->GetNextSibling();
     nsTableColGroupFrame* colGroup = (nsTableColGroupFrame*)aOldFrame;
     PRInt32 firstColIndex = colGroup->GetStartColumnIndex();
     PRInt32 lastColIndex  = firstColIndex + colGroup->GetColCount() - 1;
     mColGroups.DestroyFrame(aOldFrame);
     nsTableColGroupFrame::ResetColIndices(nextColGroupFrame, firstColIndex);
     // remove the cols from the table
     PRInt32 colX;
--- a/layout/xul/base/src/nsListBoxBodyFrame.cpp
+++ b/layout/xul/base/src/nsListBoxBodyFrame.cpp
@@ -1512,20 +1512,16 @@ nsListBoxBodyFrame::RemoveChildFrame(nsB
     NS_ERROR("tried to remove a child frame which isn't our child");
     return;
   }
 
   if (aFrame == GetContentInsertionFrame()) {
     // Don't touch that one
     return;
   }
-  
-  nsPresContext* presContext = PresContext();
-  nsCSSFrameConstructor* fc = presContext->PresShell()->FrameConstructor();
-  fc->RemoveMappingsForFrameSubtree(aFrame);
 
   mFrames.RemoveFrame(aFrame);
   if (mLayoutManager)
     mLayoutManager->ChildrenRemoved(this, aState, aFrame);
   aFrame->Destroy();
 }
 
 // Creation Routines ///////////////////////////////////////////////////////////////////////
--- a/layout/xul/base/src/nsPopupSetFrame.cpp
+++ b/layout/xul/base/src/nsPopupSetFrame.cpp
@@ -51,16 +51,27 @@
 
 nsPopupFrameList::nsPopupFrameList(nsIContent* aPopupContent, nsPopupFrameList* aNext)
 :mNextPopup(aNext), 
  mPopupFrame(nsnull),
  mPopupContent(aPopupContent)
 {
 }
 
+nsPopupFrameList::~nsPopupFrameList()
+{
+  if (mPopupFrame) {
+    nsIFrame* prevSib = mPopupFrame->GetPrevSibling();
+    if (prevSib)
+      prevSib->SetNextSibling(mPopupFrame->GetNextSibling());
+    mPopupFrame->SetNextSibling(nsnull);
+    mPopupFrame->Destroy();
+  }
+}
+
 //
 // NS_NewPopupSetFrame
 //
 // Wrapper for creating a new menu popup container
 //
 nsIFrame*
 NS_NewPopupSetFrame(nsIPresShell* aPresShell, nsStyleContext* aContext)
 {
@@ -133,22 +144,19 @@ nsPopupSetFrame::SetInitialChildList(nsI
   return nsBoxFrame::SetInitialChildList(aListName, aChildList);
 }
 
 void
 nsPopupSetFrame::Destroy()
 {
   // remove each popup from the list as we go.
   while (mPopupList) {
-    if (mPopupList->mPopupFrame) {
-      mPopupList->mPopupFrame->Destroy();
-    }
     nsPopupFrameList* temp = mPopupList;
     mPopupList = mPopupList->mNextPopup;
-    delete temp;
+    delete temp; // destroys frame
   }
 
   // Normally the root box is our grandparent, but in case of wrapping
   // it can be our great-grandparent.
   nsIRootBox *rootBox = nsIRootBox::GetRootBox(PresContext()->GetPresShell());
   if (rootBox) {
     rootBox->SetPopupSetFrame(nsnull);
   }
@@ -237,22 +245,19 @@ nsPopupSetFrame::RemovePopupFrame(nsIFra
       if (temp)
         temp->mNextPopup = currEntry->mNextPopup;
       else
         mPopupList = currEntry->mNextPopup;
       
       NS_ASSERTION((aPopup->GetStateBits() & NS_FRAME_OUT_OF_FLOW) &&
                    aPopup->GetType() == nsGkAtoms::menuPopupFrame,
                    "found wrong type of frame in popupset's ::popupList");
-      // Destroy the frame.
-      currEntry->mPopupFrame->Destroy();
-
       // Delete the entry.
       currEntry->mNextPopup = nsnull;
-      delete currEntry;
+      delete currEntry; // destroys the frame
 #ifdef DEBUG
       found = PR_TRUE;
 #endif
 
       // Break out of the loop.
       break;
     }
 
--- a/layout/xul/base/src/nsPopupSetFrame.h
+++ b/layout/xul/base/src/nsPopupSetFrame.h
@@ -54,16 +54,17 @@ nsIFrame* NS_NewPopupSetFrame(nsIPresShe
 
 struct nsPopupFrameList {
   nsPopupFrameList* mNextPopup;  // The next popup in the list.
   nsMenuPopupFrame* mPopupFrame; // Our popup.
   nsIContent* mPopupContent;     // The content element for the <popup> itself.
 
 public:
   nsPopupFrameList(nsIContent* aPopupContent, nsPopupFrameList* aNext);
+  ~nsPopupFrameList();
 };
 
 class nsPopupSetFrame : public nsBoxFrame
 {
 public:
   NS_DECL_FRAMEARENA_HELPERS
 
   nsPopupSetFrame(nsIPresShell* aShell, nsStyleContext* aContext):