Back out bug 390425 to fix performance regression.
authorbzbarsky@mit.edu
Fri, 10 Aug 2007 15:55:21 -0700
changeset 4501 0bf7c9a3cbd987bc1dc703256b1b44859e768081
parent 4500 ab976c32105f71c18fb759c6ceab9c4ca1fdfc1c
child 4502 e80c8f021fd88b95ac36e6bc62afbfc9f86d29dd
push idunknown
push userunknown
push dateunknown
bugs390425
milestone1.9a8pre
Back out bug 390425 to fix performance regression.
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsCSSFrameConstructor.h
layout/base/nsPresShell.cpp
layout/reftests/reftest.list
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -446,32 +446,33 @@ GetFieldSetAreaFrame(nsIFrame* aFieldset
 // more easily.
 
 static inline PRBool
 IsFrameSpecial(nsIFrame* aFrame)
 {
   return (aFrame->GetStateBits() & NS_FRAME_IS_SPECIAL) != 0;
 }
 
-static nsIFrame* GetSpecialSibling(nsIFrame* aFrame)
+static void
+GetSpecialSibling(nsFrameManager* aFrameManager, nsIFrame* aFrame, nsIFrame** aResult)
 {
   // We only store the "special sibling" annotation with the first
-  // frame in the continuation chain. Walk back to find that frame now.
-  aFrame = aFrame->GetFirstContinuation();
+  // frame in the flow. Walk back to find that frame now.
+  aFrame = aFrame->GetFirstInFlow();
 
   void* value = aFrame->GetProperty(nsGkAtoms::IBSplitSpecialSibling);
 
-  return static_cast<nsIFrame*>(value);
+  *aResult = static_cast<nsIFrame*>(value);
 }
 
 static nsIFrame*
-GetLastSpecialSibling(nsIFrame* aFrame)
+GetLastSpecialSibling(nsFrameManager* aFrameManager, nsIFrame* aFrame)
 {
   for (nsIFrame *frame = aFrame, *next; ; frame = next) {
-    next = GetSpecialSibling(frame);
+    GetSpecialSibling(aFrameManager, frame, &next);
     if (!next)
       return frame;
   }
   NS_NOTREACHED("unreachable code");
   return nsnull;
 }
 
 static void
@@ -531,28 +532,20 @@ GetIBContainingBlockFor(nsIFrame* aFrame
 //----------------------------------------------------------------------
 
 static PRBool
 IsInlineOutside(nsIFrame* aFrame)
 {
   return aFrame->GetStyleDisplay()->IsInlineOutside();
 }
 
-/**
- * True if aFrame is an actual inline frame in the sense of non-replaced
- * display:inline CSS boxes.  In other words, it can be affected by {ib}
- * splitting and can contain first-letter frames.  Basically, this is either an
- * inline frame (positioned or otherwise) or an line frame (this last because
- * it can contain first-letter and because inserting blocks in the middle of it
- * needs to terminate it).
- */
 static PRBool
-IsInlineFrame(const nsIFrame* aFrame)
-{
-  return aFrame->IsFrameOfType(nsIFrame::eLineParticipant);
+IsBlockOutside(nsIFrame* aFrame)
+{
+  return aFrame->GetStyleDisplay()->IsBlockOutside();
 }
 
 //----------------------------------------------------------------------
 
 // Block/inline frame construction logic. We maintain a few invariants here:
 //
 // 1. Block frames contain block and inline frames.
 //
@@ -1418,17 +1411,17 @@ nsFrameConstructorState::AddChild(nsIFra
     frameItems->InsertChildAfter(aNewFrame, aInsertAfterFrame);
   } else {
     frameItems->AddChild(aNewFrame);
   }
 
   // Now add the special siblings too.
   nsIFrame* specialSibling = aNewFrame;
   while (specialSibling && IsFrameSpecial(specialSibling)) {
-    specialSibling = GetSpecialSibling(specialSibling);
+    GetSpecialSibling(mFrameManager, specialSibling, &specialSibling);
     if (specialSibling) {
       NS_ASSERTION(frameItems == &aFrameItems,
                    "IB split ending up in an out-of-flow childlist?");
       frameItems->AddChild(specialSibling);
     }
   }
   
   return NS_OK;
@@ -7960,17 +7953,18 @@ FindPreviousAnonymousSibling(nsIPresShel
 
     // Get its frame. If it doesn't have one, continue on to the
     // anonymous element that preceded it.
     nsIFrame* prevSibling = aPresShell->GetPrimaryFrameFor(child);
     if (prevSibling) {
       // The frame may be a special frame (a split inline frame that
       // contains a block).  Get the last part of that split.
       if (IsFrameSpecial(prevSibling)) {
-        prevSibling = GetLastSpecialSibling(prevSibling);
+        prevSibling = GetLastSpecialSibling(aPresShell->FrameManager(),
+                                            prevSibling);
       }
 
       // The frame may have a continuation. If so, we want the
       // last continuation as our previous sibling.
       prevSibling = prevSibling->GetLastContinuation();
 
       // If the frame is out-of-flow, GPFF() will have returned the
       // out-of-flow frame; we want the placeholder.
@@ -8145,17 +8139,18 @@ nsCSSFrameConstructor::FindPreviousSibli
   // `display: hidden') so keep looking until we find a previous frame
   while (iter-- != first) {
     nsIFrame* prevSibling = mPresShell->GetPrimaryFrameFor(nsCOMPtr<nsIContent>(*iter));
 
     if (prevSibling) {
       // The frame may be a special frame (a split inline frame that
       // contains a block).  Get the last part of that split.
       if (IsFrameSpecial(prevSibling)) {
-        prevSibling = GetLastSpecialSibling(prevSibling);
+        prevSibling = GetLastSpecialSibling(mPresShell->FrameManager(),
+                                            prevSibling);
       }
 
       // The frame may have a continuation. Get the last continuation
       prevSibling = prevSibling->GetLastContinuation();
 
       // XXXbz should the IsValidSibling check be after we get the
       // placeholder for out-of-flows?
       const nsStyleDisplay* display = prevSibling->GetStyleDisplay();
@@ -8418,17 +8413,17 @@ nsCSSFrameConstructor::ContentAppended(n
         for (ChildIterator::Init(insertionContent, &iter, &last);
          iter != last;
          ++iter) {
           LAYOUT_PHASE_TEMP_EXIT();
           nsIContent* item = nsCOMPtr<nsIContent>(*iter);
           if (item == child)
             // Call ContentInserted with this index.
             ContentInserted(aContainer, child,
-                            iter.index(), mTempFrameTreeState);
+                            iter.index(), mTempFrameTreeState, PR_FALSE);
           LAYOUT_PHASE_TEMP_REENTER();
         }
       }
 
       return NS_OK;
     }
   }
 
@@ -8445,31 +8440,72 @@ nsCSSFrameConstructor::ContentAppended(n
     }
   }
   
   if (parentFrame->IsLeaf()) {
     // Nothing to do here; we shouldn't be constructing kids of leaves
     return NS_OK;
   }
   
-  // If the frame we are manipulating is a ``special'' frame (that is, one
-  // that's been created as a result of a block-in-inline situation) then we
-  // need to append to the last special sibling, not to the frame itself.
+  // If the frame we are manipulating is a ``special'' frame (that
+  // is, one that's been created as a result of a block-in-inline
+  // situation) then do something different instead of just
+  // appending newly created frames. Note that only the
+  // first-in-flow is marked so we check before getting to the
+  // last-in-flow.
+  //
+  // We run into this situation occasionally while loading web
+  // pages, typically when some content generation tool has
+  // sprinkled invalid markup into the document. More often than
+  // not, we'll be able to just use the normal fast-path frame
+  // construction code, because the frames will be appended to the
+  // ``anonymous'' block that got created to parent the block
+  // children of the inline.
   if (IsFrameSpecial(parentFrame)) {
 #ifdef DEBUG
     if (gNoisyContentUpdates) {
       printf("nsCSSFrameConstructor::ContentAppended: parentFrame=");
       nsFrame::ListTag(stdout, parentFrame);
       printf(" is special\n");
     }
 #endif
 
     // Since we're appending, we'll walk to the last anonymous frame
     // that was created for the broken inline frame.
-    parentFrame = GetLastSpecialSibling(parentFrame);
+    nsFrameManager *frameManager = mPresShell->FrameManager();
+
+    while (1) {
+      nsIFrame* sibling;
+      GetSpecialSibling(frameManager, parentFrame, &sibling);
+      if (! sibling)
+        break;
+
+      parentFrame = sibling;
+    }
+
+    // If this frame is the anonymous block frame, then all's well:
+    // just append frames as usual.
+    const nsStyleDisplay* display = parentFrame->GetStyleDisplay();
+
+    if (NS_STYLE_DISPLAY_BLOCK != display->mDisplay) {
+      // Nope, it's an inline, so just reframe the entire stinkin' mess if the
+      // content is a block. We _could_ do better here with a little more work...
+      // find out if the child is a block or inline, an inline means we don't have to reframe
+      nsIContent *child = aContainer->GetChildAt(aNewIndexInContainer);
+      PRBool needReframe = !child;
+      if (child && child->IsNodeOfType(nsINode::eELEMENT)) {
+        nsRefPtr<nsStyleContext> styleContext;
+        styleContext = ResolveStyleContext(parentFrame, child);
+        // XXX since the block child goes in the last inline of the sacred triad, frames would 
+        // need to be moved into the 2nd triad (block) but that is more work, for now bail.
+        needReframe = styleContext->GetStyleDisplay()->IsBlockOutside();
+      }
+      if (needReframe)
+        return ReframeContainingBlock(parentFrame);
+    }
   }
 
   // Get the parent frame's last continuation
   parentFrame = parentFrame->GetLastContinuation();
 
   nsIAtom* frameType = parentFrame->GetType();
   // Deal with fieldsets
   parentFrame = ::GetAdjustedParentFrame(parentFrame, frameType,
@@ -8555,17 +8591,18 @@ nsCSSFrameConstructor::ContentAppended(n
     firstAppendedFrame = captionItems.childList;
   }
 
   // Notify the parent frame passing it the list of new frames
   if (NS_SUCCEEDED(result) && firstAppendedFrame) {
     // Perform special check for diddling around with the frames in
     // a special inline frame.
 
-    if (WipeContainingBlock(state, containingBlock, parentFrame, frameItems)) {
+    if (WipeContainingBlock(state, containingBlock, parentFrame,
+                            frameItems.childList)) {
       return NS_OK;
     }
 
     // Append the flowed frames to the principal child list, tables need special treatment
     if (nsGkAtoms::tableFrame == frameType) {
       if (captionItems.childList) { // append the caption to the outer table
         nsIFrame* outerTable = parentFrame->GetParent();
         if (outerTable) { 
@@ -8599,16 +8636,131 @@ nsCSSFrameConstructor::ContentAppended(n
       fdbg->List(stdout, 0);
     }
   }
 #endif
 
   return NS_OK;
 }
 
+// Return TRUE if the insertion of aChild into aParent1,2 should force a reframe. aParent1 is 
+// the special inline container which contains a block. aParentFrame is approximately aParent1's
+// primary frame and will be set to the correct parent of aChild if a reframe is not necessary. 
+// aParent2 is aParentFrame's content. aPrevSibling will be set to the correct prev sibling of
+// aChild if a reframe is not necessary.
+PRBool
+nsCSSFrameConstructor::NeedSpecialFrameReframe(nsIContent*     aParent1,     
+                                               nsIContent*     aParent2,     
+                                               nsIFrame*&      aParentFrame, 
+                                               nsIContent*     aChild,
+                                               PRInt32         aIndexInContainer,
+                                               nsIFrame*&      aPrevSibling,
+                                               nsIFrame*       aNextSibling)
+{
+  // XXXbz aNextSibling is utterly unused.  Why?
+  
+  if (IsBlockOutside(aParentFrame)) 
+    return PR_FALSE;
+
+  // find out if aChild is a block or inline
+  PRBool childIsBlock = PR_FALSE;
+  if (aChild->IsNodeOfType(nsINode::eELEMENT)) {
+    nsRefPtr<nsStyleContext> styleContext;
+    styleContext = ResolveStyleContext(aParentFrame, aChild);
+    const nsStyleDisplay* display = styleContext->GetStyleDisplay();
+    childIsBlock = display->IsBlockOutside();
+  }
+  nsIFrame* prevParent; // parent of prev sibling
+  nsIFrame* nextParent; // parent of next sibling
+
+  if (childIsBlock) { 
+    if (aPrevSibling) {
+      prevParent = aPrevSibling->GetParent(); 
+      NS_ASSERTION(prevParent, "program error - null parent frame");
+      if (!IsBlockOutside(prevParent)) { // prevParent is an inline
+        // XXX we need to find out if prevParent is the 1st inline or the last. If it
+        // is the 1st, then aChild and the frames after aPrevSibling within the 1st inline
+        // need to be moved to the block(inline). If it is the last, then aChild and the
+        // frames before aPrevSibling within the last need to be moved to the block(inline). 
+        return PR_TRUE; // For now, bail.
+      }        
+      aParentFrame = prevParent; // prevParent is a block, put aChild there
+    }
+    else {
+      // XXXbz see comments in ContentInserted about this being wrong in many
+      // cases!  Why doesn't this just use aNextSibling anyway?  Why are we
+      // looking sometimes in aParent1 and sometimes in aParent2?
+      nsIFrame* nextSibling = (aIndexInContainer >= 0)
+                              ? FindNextSibling(aParent2, aParentFrame,
+                                                aIndexInContainer)
+                              : FindNextAnonymousSibling(mPresShell, mDocument,
+                                                         aParent1, aChild);
+      if (nextSibling) {
+        nextParent = nextSibling->GetParent(); 
+        NS_ASSERTION(nextParent, "program error - null parent frame");
+        if (!IsBlockOutside(nextParent)) {
+          // XXX we need to move aChild, aNextSibling and all the frames after aNextSibling within
+          // the 1st inline to the block(inline).
+          return PR_TRUE; // for now, bail
+        }
+        // put aChild in nextParent which is the block(inline) and leave aPrevSibling null
+        aParentFrame = nextParent;
+      }
+    }           
+  }
+  else { // aChild is an inline
+    if (aPrevSibling) {
+      prevParent = aPrevSibling->GetParent(); 
+      NS_ASSERTION(prevParent, "program error - null parent frame");
+      if (!IsBlockOutside(prevParent)) { // prevParent is an inline
+        // aChild goes into the same inline frame as aPrevSibling
+        aParentFrame = aPrevSibling->GetParent();
+        NS_ASSERTION(aParentFrame, "program error - null parent frame");
+      }
+      else { // prevParent is a block
+        // XXXbz see comments in ContentInserted about this being wrong in many
+        // cases!  Why doesn't this just use aNextSibling anyway?  Why are we
+        // looking sometimes in aParent1 and sometimes in aParent2?
+        nsIFrame* nextSibling = (aIndexInContainer >= 0)
+                                ? FindNextSibling(aParent2, aParentFrame,
+                                                  aIndexInContainer)
+                                : FindNextAnonymousSibling(mPresShell,
+                                                           mDocument, aParent1,
+                                                           aChild);
+        if (nextSibling) {
+          nextParent = nextSibling->GetParent();
+          NS_ASSERTION(nextParent, "program error - null parent frame");
+          if (!IsBlockOutside(nextParent)) {
+            // nextParent is the ending inline frame. Put aChild there and
+            // set aPrevSibling to null so aChild is its first element.
+            aParentFrame = nextSibling->GetParent(); 
+            NS_ASSERTION(aParentFrame, "program error - null parent frame");
+            aPrevSibling = nsnull; 
+          }
+          else { // nextParent is a block
+            // prevParent and nextParent should be the same, and aChild goes there
+            NS_ASSERTION(prevParent == nextParent, "special frame error");
+            aParentFrame = prevParent;
+          }
+        }
+        else {
+          // The child has no next sibling, so we can't find the ending inline
+          // frame (which might not exist in this case anyway!), but aChild
+          // should go in there.  Force a reframe.
+          // XXXbz wouldn't getting prevParent's special sibling work, with
+          // reframing only needed if that's null?
+          return PR_TRUE;
+        }
+      }
+    }
+    // else aChild goes into the 1st inline frame which is aParentFrame
+  }
+  return PR_FALSE;
+}
+
 #ifdef MOZ_XUL
 
 enum content_operation
 {
     CONTENT_INSERTED,
     CONTENT_REMOVED
 };
 
@@ -8671,17 +8823,18 @@ PRBool NotifyListBoxBody(nsPresContext* 
   return PR_FALSE;
 }
 #endif // MOZ_XUL
 
 nsresult
 nsCSSFrameConstructor::ContentInserted(nsIContent*            aContainer,
                                        nsIContent*            aChild,
                                        PRInt32                aIndexInContainer,
-                                       nsILayoutHistoryState* aFrameState)
+                                       nsILayoutHistoryState* aFrameState,
+                                       PRBool                 aInReinsertContent)
 {
   AUTO_LAYOUT_PHASE_ENTRY_POINT(mPresShell->GetPresContext(), FrameC);
   // XXXldb Do we need to re-resolve style to handle the CSS2 + combinator and
   // the :empty pseudo-class?
 #ifdef DEBUG
   if (gNoisyContentUpdates) {
     printf("nsCSSFrameConstructor::ContentInserted container=%p child=%p index=%d\n",
            static_cast<void*>(aContainer),
@@ -8797,24 +8950,28 @@ nsCSSFrameConstructor::ContentInserted(n
     
   // If there is no previous sibling, then find the frame that follows
   if (! prevSibling) {
     nextSibling = (aIndexInContainer >= 0)
       ? FindNextSibling(container, parentFrame, aIndexInContainer, aChild)
       : FindNextAnonymousSibling(mPresShell, mDocument, aContainer, aChild);
   }
 
+  PRBool handleSpecialFrame = IsFrameSpecial(parentFrame) && !aInReinsertContent;
+
   // Now, find the geometric parent so that we can handle
   // continuations properly. Use the prev sibling if we have it;
   // otherwise use the next sibling.
   if (prevSibling) {
-    parentFrame = prevSibling->GetParent()->GetContentInsertionFrame();
+    if (!handleSpecialFrame)
+      parentFrame = prevSibling->GetParent()->GetContentInsertionFrame();
   }
   else if (nextSibling) {
-    parentFrame = nextSibling->GetParent()->GetContentInsertionFrame();
+    if (!handleSpecialFrame)
+      parentFrame = nextSibling->GetParent()->GetContentInsertionFrame();
   }
   else {
     // No previous or next sibling, so treat this like an appended frame.
     isAppend = PR_TRUE;
     // Deal with fieldsets
     parentFrame = ::GetAdjustedParentFrame(parentFrame, parentFrame->GetType(),
                                            aContainer, aIndexInContainer);
     parentFrame =
@@ -8829,28 +8986,50 @@ nsCSSFrameConstructor::ContentInserted(n
     return RecreateFramesForContent(parentFrame->GetContent());
   }
   
   // Don't construct kids of leaves
   if (parentFrame->IsLeaf()) {
     return NS_OK;
   }
   
+  // If the frame we are manipulating is a special frame then see if we need to reframe 
+  // NOTE: if we are in ReinsertContent, then don't reframe as we are already doing just that!
+  if (handleSpecialFrame) {
+    // a special inline frame has propagated some of its children upward to be children 
+    // of the block and those frames may need to move around. Sometimes we may need to reframe
+#ifdef DEBUG
+    if (gNoisyContentUpdates) {
+      printf("nsCSSFrameConstructor::ContentInserted: parentFrame=");
+      nsFrame::ListTag(stdout, parentFrame);
+      printf(" is special\n");
+    }
+#endif
+    // if we don't need to reframe then set parentFrame and prevSibling to the correct values
+    if (NeedSpecialFrameReframe(aContainer, container, parentFrame, 
+                                aChild, aIndexInContainer, prevSibling,
+                                nextSibling)) {
+      return ReframeContainingBlock(parentFrame);
+    }
+  }
+
   nsFrameConstructorState state(mPresShell, mFixedContainingBlock,
                                 GetAbsoluteContainingBlock(parentFrame),
                                 GetFloatContainingBlock(parentFrame),
                                 aFrameState);
 
 
   // Recover state for the containing block - we need to know if
   // it has :first-letter or :first-line style applied to it. The
   // reason we care is that the internal structure in these cases
   // is not the normal structure and requires custom updating
   // logic.
   nsIFrame* containingBlock = state.mFloatedItems.containingBlock;
+  nsStyleContext* blockSC;
+  nsIContent* blockContent = nsnull;
   PRBool haveFirstLetterStyle = PR_FALSE;
   PRBool haveFirstLineStyle = PR_FALSE;
 
   // In order to shave off some cycles, we only dig up the
   // containing block haveFirst* flags if the parent frame where
   // the insertion/append is occurring is an inline or block
   // container. For other types of containers this isn't relevant.
   const nsStyleDisplay* parentDisplay = parentFrame->GetStyleDisplay();
@@ -8893,16 +9072,43 @@ nsCSSFrameConstructor::ContentInserted(n
                                        aChild);
 
       // If there is no previous sibling, then find the frame that follows
       if (! prevSibling) {
         nextSibling = (aIndexInContainer >= 0)
           ? FindNextSibling(container, parentFrame, aIndexInContainer, aChild)
           : FindNextAnonymousSibling(mPresShell, mDocument, aContainer, aChild);
       }
+
+      handleSpecialFrame = IsFrameSpecial(parentFrame) && !aInReinsertContent;
+      if (handleSpecialFrame &&
+          NeedSpecialFrameReframe(aContainer, container, parentFrame,
+                                  aChild, aIndexInContainer, prevSibling,
+                                  nextSibling)) {
+#ifdef DEBUG
+        nsIContent* parentContainer = blockContent->GetParent();
+        if (gNoisyContentUpdates) {
+          printf("nsCSSFrameConstructor::ContentInserted: parentFrame=");
+          nsFrame::ListTag(stdout, parentFrame);
+          printf(" is special inline\n");
+          printf("  ==> blockContent=%p, parentContainer=%p\n",
+                 static_cast<void*>(blockContent),
+                 static_cast<void*>(parentContainer));
+        }
+#endif
+
+        NS_ASSERTION(GetFloatContainingBlock(parentFrame) == containingBlock,
+                     "Unexpected block ancestor for parentFrame");
+
+        // Note that in this case we're guaranteed that the closest block
+        // containing parentFrame is |containingBlock|.  So
+        // ReframeContainingBlock(parentFrame) will make sure to rebuild the
+        // first-letter stuff we just blew away.
+        return ReframeContainingBlock(parentFrame);
+      }
     }
   }
 
   // if the container is a table and a caption will be appended, it needs to be
   // put in the outer table frame's additional child list.
   
   nsFrameItems frameItems, captionItems;
 
@@ -8934,17 +9140,18 @@ nsCSSFrameConstructor::ContentInserted(n
       ::AdjustAppendParentForAfterContent(mPresShell->GetPresContext(),
                                           aContainer,
                                           frameItems.childList->GetParent(),
                                           &appendAfterFrame);
   }
 
   // Perform special check for diddling around with the frames in
   // a special inline frame.
-  if (WipeContainingBlock(state, containingBlock, parentFrame, frameItems))
+  if (WipeContainingBlock(state, containingBlock, parentFrame,
+                          frameItems.childList))
     return NS_OK;
 
   if (haveFirstLineStyle && parentFrame == containingBlock) {
     // It's possible that the new frame goes into a first-line
     // frame. Look at it and see...
     if (isAppend) {
       // Use append logic when appending
       AppendFirstLineFrames(state, containingBlock->GetContent(),
@@ -9052,17 +9259,17 @@ nsCSSFrameConstructor::ReinsertContent(n
   PRInt32 ix = aContainer->IndexOf(aChild);
   // XXX For now, do a brute force remove and insert.
   // XXXbz this probably doesn't work so well with anonymous content
   // XXXbz doesn't this need to do the state-saving stuff that
   // RecreateFramesForContent does?
   nsresult res = ContentRemoved(aContainer, aChild, ix, PR_TRUE);
 
   if (NS_SUCCEEDED(res)) {
-    res = ContentInserted(aContainer, aChild, ix, nsnull);
+    res = ContentInserted(aContainer, aChild, ix, nsnull, PR_TRUE);
   }
 
   return res;
 }
 
 /**
  * Called when a frame subtree is about to be deleted. Two important
  * things happen:
@@ -9275,19 +9482,31 @@ nsCSSFrameConstructor::ContentRemoved(ns
   if (childFrame) {
     InvalidateCanvasIfNeeded(childFrame);
     
     // If the frame we are manipulating is a special frame then do
     // something different instead of just inserting newly created
     // frames.
     // NOTE: if we are in ReinsertContent, 
     //       then do not reframe as we are already doing just that!
-    if (!aInReinsertContent &&
-        MaybeRecreateContainerForIBSplitterFrame(childFrame, &rv)) {
-      return rv;
+    if (IsFrameSpecial(childFrame) && !aInReinsertContent) {
+      // We are pretty harsh here (and definitely not optimal) -- we
+      // wipe out the entire containing block and recreate it from
+      // scratch. The reason is that because we know that a special
+      // inline frame has propagated some of its children upward to be
+      // children of the block and that those frames may need to move
+      // around. This logic guarantees a correct answer.
+#ifdef DEBUG
+      if (gNoisyContentUpdates) {
+        printf("nsCSSFrameConstructor::ContentRemoved: childFrame=");
+        nsFrame::ListTag(stdout, childFrame);
+        printf(" is special\n");
+      }
+#endif
+      return ReframeContainingBlock(childFrame);
     }
 
     // Get the childFrame's parent frame
     nsIFrame* parentFrame = childFrame->GetParent();
 
     if (parentFrame->GetType() == nsGkAtoms::frameSetFrame &&
         IsSpecialFramesetChild(aChild)) {
       // Just reframe the parent, since framesets are weird like that.
@@ -10686,17 +10905,19 @@ nsCSSFrameConstructor::FindPrimaryFrameF
         aFrameManager->SetPrimaryFrameFor(aContent, *aFrame);
         break;
       }
       else if (IsFrameSpecial(parentFrame)) {
         // If it's a "special" frame (that is, part of an inline
         // that's been split because it contained a block), we need to
         // follow the out-of-flow "special sibling" link, and search
         // *that* subtree as well.
-        parentFrame = GetSpecialSibling(parentFrame);
+        nsIFrame* specialSibling = nsnull;
+        GetSpecialSibling(aFrameManager, parentFrame, &specialSibling);
+        parentFrame = specialSibling;
       }
       else {
         break;
       }
     }
   }
 
   if (aHint && !*aFrame)
@@ -10830,59 +11051,29 @@ nsCSSFrameConstructor::MaybeRecreateFram
     if (newContext->GetStyleDisplay()->mDisplay != NS_STYLE_DISPLAY_NONE) {
       result = RecreateFramesForContent(aContent);
     }
   }
   return result;
 }
 
 PRBool
-nsCSSFrameConstructor::MaybeRecreateContainerForIBSplitterFrame(nsIFrame* aFrame,
-                                                                nsresult* aResult)
-{
-  NS_PRECONDITION(aFrame, "Must have a frame");
-  NS_PRECONDITION(aFrame->GetParent(), "Frame shouldn't be root");
-  NS_PRECONDITION(aResult, "Null out param?");
-  NS_PRECONDITION(aFrame == aFrame->GetFirstContinuation(),
-                  "aFrame not the result of GetPrimaryFrameFor()?");
-
-  if (IsFrameSpecial(aFrame)) {
-    // The removal functions can't handle removal of an {ib} split directly; we
-    // need to rebuild the containing block.
-#ifdef DEBUG
-    if (gNoisyContentUpdates) {
-      printf("nsCSSFrameConstructor::MaybeRecreateContainerForIBSplitterFrame: "
-             "frame=");
-      nsFrame::ListTag(stdout, aFrame);
-      printf(" is special\n");
-    }
-#endif
-
-    *aResult = ReframeContainingBlock(aFrame);
-    return PR_TRUE;
-  }
-
-  // We might still need to reconstruct things if the parent of aFrame is
-  // special, since in that case the removal of aFrame might affect the
-  // splitting of its parent.
-  nsIFrame* parent = aFrame->GetParent();
-  if (!IsFrameSpecial(parent)) {
+nsCSSFrameConstructor::MaybeRecreateContainerForIBSplitterFrame(nsIFrame* aFrame, nsresult* aResult)
+{
+  if (!aFrame || !IsFrameSpecial(aFrame))
     return PR_FALSE;
-  }
 
 #ifdef DEBUG
-  if (gNoisyContentUpdates || 1) {
-    printf("nsCSSFrameConstructor::MaybeRecreateContainerForIBSplitterFrame: "
-           "frame=");
-    nsFrame::ListTag(stdout, parent);
+  if (gNoisyContentUpdates) {
+    printf("nsCSSFrameConstructor::RecreateFramesForContent: frame=");
+    nsFrame::ListTag(stdout, aFrame);
     printf(" is special\n");
   }
 #endif
-
-  *aResult = ReframeContainingBlock(parent);
+  *aResult = ReframeContainingBlock(aFrame);
   return PR_TRUE;
 }
  
 nsresult
 nsCSSFrameConstructor::RecreateFramesForContent(nsIContent* aContent)
 {
   // If there is no document, we don't want to recreate frames for it.  (You
   // shouldn't generally be giving this method content without a document
@@ -10890,25 +11081,33 @@ nsCSSFrameConstructor::RecreateFramesFor
   // Rebuilding the frame tree can have bad effects, especially if it's the
   // frame tree for chrome (see bug 157322).
   NS_ENSURE_TRUE(aContent->GetDocument(), NS_ERROR_FAILURE);
 
   // Is the frame `special'? If so, we need to reframe the containing
   // block *here*, rather than trying to remove and re-insert the
   // content (which would otherwise result in *two* nested reframe
   // containing block from ContentRemoved() and ContentInserted(),
-  // below!).  We'd really like to optimize away one of those
-  // containing block reframes, hence the code here.
+  // below!)
 
   nsIFrame* frame = mPresShell->GetPrimaryFrameFor(aContent);
 
   nsresult rv = NS_OK;
 
-  if (frame && MaybeRecreateContainerForIBSplitterFrame(frame, &rv)) {
-    return rv;
+  if (frame) {
+    // If the frame is an anonymous frame created as part of inline-in-block
+    // splitting --- or if its parent is such an anonymous frame (i.e., this
+    // frame might have been the cause of such splitting), then recreate the
+    // containing block.  Note that if |frame| is an inline, then it can't
+    // possibly have caused the splitting, and if the inline is changing to a
+    // block, any reframing that's needed will happen in ContentInserted.
+    if (MaybeRecreateContainerForIBSplitterFrame(frame, &rv) ||
+        (!IsInlineOutside(frame) &&
+         MaybeRecreateContainerForIBSplitterFrame(frame->GetParent(), &rv)))
+      return rv;
   }
 
   nsCOMPtr<nsIContent> container = aContent->GetParent();
   if (container) {
     // XXXbz what if this is anonymous content?
     PRInt32 indexInContainer = container->IndexOf(aContent);
     // Before removing the frames associated with the content object,
     // ask them to save their state onto a temporary state object.
@@ -10917,17 +11116,17 @@ nsCSSFrameConstructor::RecreateFramesFor
     // Remove the frames associated with the content object on which
     // the attribute change occurred.
     rv = ContentRemoved(container, aContent, indexInContainer,
                         PR_FALSE);
 
     if (NS_SUCCEEDED(rv)) {
       // Now, recreate the frames associated with this content object.
       rv = ContentInserted(container, aContent,
-                           indexInContainer, mTempFrameTreeState);
+                           indexInContainer, mTempFrameTreeState, PR_FALSE);
     }
   } else {
     // The content is the root node, so just rebuild the world.
     ReconstructDocElementHierarchy();
   }
 
 #ifdef ACCESSIBILITY
   if (mPresShell->IsAccessibilityActive()) {
@@ -11747,17 +11946,18 @@ nsCSSFrameConstructor::WrapFramesInFirst
   nsresult rv = NS_OK;
 
   nsIFrame* prevFrame = nsnull;
   nsIFrame* frame = aParentFrameList;
 
   while (frame) {
     nsIFrame* nextFrame = frame->GetNextSibling();
 
-    if (nsGkAtoms::textFrame == frame->GetType()) {
+    nsIAtom* frameType = frame->GetType();
+    if (nsGkAtoms::textFrame == frameType) {
       // Wrap up first-letter content in a letter frame
       nsIContent* textContent = frame->GetContent();
       if (IsFirstLetterContent(textContent)) {
         // Create letter frame to wrap up the text
         rv = CreateLetterFrame(aState, aBlockFrame, textContent,
                                aParentFrame, aLetterFrames);
         if (NS_FAILED(rv)) {
           return rv;
@@ -11766,17 +11966,19 @@ nsCSSFrameConstructor::WrapFramesInFirst
         // Provide adjustment information for parent
         *aModifiedParent = aParentFrame;
         *aTextFrame = frame;
         *aPrevFrame = prevFrame;
         *aStopLooking = PR_TRUE;
         return NS_OK;
       }
     }
-    else if (IsInlineFrame(frame)) {
+    else if ((nsGkAtoms::inlineFrame == frameType) ||
+             (nsGkAtoms::lineFrame == frameType) ||
+             (nsGkAtoms::positionedInlineFrame == frameType)) {
       nsIFrame* kids = frame->GetFirstChild(nsnull);
       WrapFramesInFirstLetterFrame(aState, aBlockFrame, frame, kids,
                                    aModifiedParent, aTextFrame,
                                    aPrevFrame, aLetterFrames, aStopLooking);
       if (*aStopLooking) {
         return NS_OK;
       }
     }
@@ -11919,17 +12121,18 @@ nsCSSFrameConstructor::RemoveFirstLetter
                                                nsFrameManager* aFrameManager,
                                                nsIFrame* aFrame,
                                                PRBool* aStopLooking)
 {
   nsIFrame* prevSibling = nsnull;
   nsIFrame* kid = aFrame->GetFirstChild(nsnull);
 
   while (kid) {
-    if (nsGkAtoms::letterFrame == kid->GetType()) {
+    nsIAtom* frameType = kid->GetType();
+    if (nsGkAtoms::letterFrame == frameType) {
       // Bingo. Found it. First steal away the text frame.
       nsIFrame* textFrame = kid->GetFirstChild(nsnull);
       if (!textFrame) {
         break;
       }
 
       // Create a new textframe
       nsStyleContext* parentSC = aFrame->GetStyleContext();
@@ -11953,17 +12156,19 @@ nsCSSFrameConstructor::RemoveFirstLetter
       aFrameManager->RemoveFrame(aFrame, nsnull, kid);
 
       // Insert text frame in its place
       aFrameManager->InsertFrames(aFrame, nsnull, prevSibling, textFrame);
 
       *aStopLooking = PR_TRUE;
       break;
     }
-    else if (IsInlineFrame(kid)) {
+    else if ((nsGkAtoms::inlineFrame == frameType) ||
+             (nsGkAtoms::lineFrame == frameType) ||
+             (nsGkAtoms::positionedInlineFrame == frameType)) {
       // Look inside child inline frame for the letter frame
       RemoveFirstLetterFrames(aPresContext, aPresShell, aFrameManager, kid,
                               aStopLooking);
       if (*aStopLooking) {
         break;
       }
     }
     prevSibling = kid;
@@ -12311,19 +12516,16 @@ nsCSSFrameConstructor::ConstructInline(n
   // block we create...  AdjustFloatParentPtrs() deals with this by moving the
   // float from the outer state |aState| to the inner |state|.
   MoveChildrenTo(state.mFrameManager, blockSC, blockFrame, list2, &state, &aState);
 
   // list3's frames belong to another inline frame
   nsIFrame* inlineFrame = nsnull;
 
   if (list3) {
-    // If we ever start constructing a second inline in the split even when
-    // list3 is null, the logic in MaybeRecreateContainerForIBSplitterFrame
-    // needs to be adjusted.
     if (aIsPositioned) {
       inlineFrame = NS_NewPositionedInlineFrame(mPresShell, aStyleContext);
     }
     else {
       inlineFrame = NS_NewInlineFrame(mPresShell, aStyleContext);
     }
 
     InitAndRestoreFrame(aState, aContent, aParentFrame, nsnull, inlineFrame, PR_FALSE);
@@ -12468,68 +12670,58 @@ nsCSSFrameConstructor::ProcessInlineChil
 
   return rv;
 }
 
 PRBool
 nsCSSFrameConstructor::WipeContainingBlock(nsFrameConstructorState& aState,
                                            nsIFrame* aContainingBlock,
                                            nsIFrame* aFrame,
-                                           const nsFrameItems& aFrameList)
-{
-  if (!aFrameList.childList) {
-    return PR_FALSE;
-  }
-  
+                                           nsIFrame* aFrameList)
+{
   // Before we go and append the frames, check for a special
   // situation: an inline frame that will now contain block
   // frames. This is a no-no and the frame construction logic knows
-  // how to fix this.  See defition of IsInlineFrame() for what "an
-  // inline" is.  Whether we have "a block" is tested for by
-  // AreAllKidsInline.
-
-  // We also need to check for an append of
-  // content ending in an inline to the block in an {ib} split.  If
-  // that happens, we also need to reframe, since that content
-  // needs to go into the following inline in the split.
-
-  if (IsInlineFrame(aFrame)) {
-    // Nothing to do if all kids are inline
-    if (AreAllKidsInline(aFrameList.childList)) {
-      return PR_FALSE;
-    }
-  } else if (!IsFrameSpecial(aFrame) ||
-             !aFrameList.lastChild->GetStyleDisplay()->IsInlineOutside()) {
-    // Nothing to do if aFrame is not special or if the last kid is not inline
+  // how to fix this.
+
+  // If we don't have a block within an inline, just return false.  Here
+  // "an inline" is an actual inline frame (positioned or not) or a lineframe
+  // (corresponding to :first-line), since the latter should stop at the first
+  // block it runs into and we might be inserting one in the middle of it.
+  // Whether we have "a block" is tested for by AreAllKidsInline.
+  nsIAtom* frameType = aFrame->GetType();
+  if ((frameType != nsGkAtoms::inlineFrame &&
+       frameType != nsGkAtoms::positionedInlineFrame &&
+       frameType != nsGkAtoms::lineFrame) ||
+      AreAllKidsInline(aFrameList))
     return PR_FALSE;
-  }
 
   // Ok, reverse tracks: wipe out the frames we just created
   nsFrameManager *frameManager = aState.mFrameManager;
 
   // Destroy the frames. As we do make sure any content to frame mappings
   // or entries in the undisplayed content map are removed
   frameManager->ClearAllUndisplayedContentIn(aFrame->GetContent());
 
-  CleanupFrameReferences(frameManager, aFrameList.childList);
+  CleanupFrameReferences(frameManager, aFrameList);
   if (aState.mAbsoluteItems.childList) {
     CleanupFrameReferences(frameManager, aState.mAbsoluteItems.childList);
   }
   if (aState.mFixedItems.childList) {
     CleanupFrameReferences(frameManager, aState.mFixedItems.childList);
   }
   if (aState.mFloatedItems.childList) {
     CleanupFrameReferences(frameManager, aState.mFloatedItems.childList);
   }
 #ifdef MOZ_XUL
   if (aState.mPopupItems.childList) {
     CleanupFrameReferences(frameManager, aState.mPopupItems.childList);
   }
 #endif
-  nsFrameList tmp(aFrameList.childList);
+  nsFrameList tmp(aFrameList);
   tmp.DestroyFrames();
 
   tmp.SetFrames(aState.mAbsoluteItems.childList);
   tmp.DestroyFrames();
   aState.mAbsoluteItems.childList = nsnull;
 
   tmp.SetFrames(aState.mFixedItems.childList);
   tmp.DestroyFrames();
--- a/layout/base/nsCSSFrameConstructor.h
+++ b/layout/base/nsCSSFrameConstructor.h
@@ -106,17 +106,18 @@ public:
   nsresult ReconstructDocElementHierarchy();
 
   nsresult ContentAppended(nsIContent*     aContainer,
                            PRInt32         aNewIndexInContainer);
 
   nsresult ContentInserted(nsIContent*            aContainer,
                            nsIContent*            aChild,
                            PRInt32                aIndexInContainer,
-                           nsILayoutHistoryState* aFrameState);
+                           nsILayoutHistoryState* aFrameState,
+                           PRBool                 aInReinsertContent);
 
   nsresult ContentRemoved(nsIContent*     aContainer,
                           nsIContent*     aChild,
                           PRInt32         aIndexInContainer,
                           PRBool          aInReinsertContent);
 
   nsresult CharacterDataChanged(nsIContent*     aContent,
                                 PRBool          aAppend);
@@ -748,26 +749,17 @@ private:
                         nsStyleContext*          aStyleContext,
                         PRBool                   aBuildCombobox,
                         nsFrameItems&            aFrameItems);
 
   nsresult MaybeRecreateFramesForContent(nsIContent*      aContent);
 
   nsresult RecreateFramesForContent(nsIContent*      aContent);
 
-  // If removal of aFrame from the frame tree requires reconstruction of some
-  // containing block (either of aFrame or of its parent) due to {ib} splits,
-  // recreate the relevant containing block.  The return value indicates
-  // whether this happened.  If this method returns true, *aResult is the
-  // return value of ReframeContainingBlock.  If this method returns false, the
-  // value of *aResult is no affected.  aFrame and aResult must not be null.
-  // aFrame must be the result of a GetPrimaryFrameFor() call (which means its
-  // parent is also not null).
-  PRBool MaybeRecreateContainerForIBSplitterFrame(nsIFrame* aFrame,
-                                                  nsresult* aResult);
+  PRBool MaybeRecreateContainerForIBSplitterFrame(nsIFrame* aFrame, nsresult* aResult);
 
   nsresult CreateContinuingOuterTableFrame(nsIPresShell*    aPresShell, 
                                            nsPresContext*  aPresContext,
                                            nsIFrame*        aFrame,
                                            nsIFrame*        aParentFrame,
                                            nsIContent*      aContent,
                                            nsStyleContext*  aStyleContext,
                                            nsIFrame**       aContinuingFrame);
@@ -841,19 +833,27 @@ private:
                                  nsIFrame*                aFrame,
                                  PRBool                   aCanHaveGeneratedContent,
                                  nsFrameItems&            aFrameItems,
                                  PRBool*                  aKidsAllInline);
 
   PRBool AreAllKidsInline(nsIFrame* aFrameList);
 
   PRBool WipeContainingBlock(nsFrameConstructorState& aState,
-                             nsIFrame*                aContainingBlock,
+                             nsIFrame*                blockContent,
                              nsIFrame*                aFrame,
-                             const nsFrameItems&      aFrameList);
+                             nsIFrame*                aFrameList);
+
+  PRBool NeedSpecialFrameReframe(nsIContent*      aParent1,
+                                 nsIContent*      aParent2,
+                                 nsIFrame*&       aParentFrame,
+                                 nsIContent*      aChild,
+                                 PRInt32          aIndexInContainer,
+                                 nsIFrame*&       aPrevSibling,
+                                 nsIFrame*        aNextSibling);
 
   nsresult ReframeContainingBlock(nsIFrame* aFrame);
 
   nsresult StyleChangeReflow(nsIFrame* aFrame);
 
   /** Helper function that searches the immediate child frames 
     * (and their children if the frames are "special")
     * for a frame that maps the specified content object
--- a/layout/base/nsPresShell.cpp
+++ b/layout/base/nsPresShell.cpp
@@ -2361,17 +2361,18 @@ PresShell::InitialReflow(nscoord aWidth,
       // Have style sheet processor construct a frame for the
       // precursors to the root content object's frame
       mFrameConstructor->ConstructRootFrame(root, &rootFrame);
       FrameManager()->SetRootFrame(rootFrame);
     }
 
     // Have the style sheet processor construct frame for the root
     // content object down
-    mFrameConstructor->ContentInserted(nsnull, root, 0, nsnull);
+    mFrameConstructor->ContentInserted(nsnull, root, 0,
+                                       nsnull, PR_FALSE);
     VERIFY_STYLE_TREE;
     MOZ_TIMER_DEBUGLOG(("Stop: Frame Creation: PresShell::InitialReflow(), this=%p\n",
                         (void*)this));
     MOZ_TIMER_STOP(mFrameCreationWatch);
 
     // Something in mFrameConstructor->ContentInserted may have caused
     // Destroy() to get called, bug 337586.
     NS_ENSURE_STATE(!mHaveShutDown);
@@ -4533,17 +4534,17 @@ PresShell::ContentInserted(nsIDocument* 
   NS_PRECONDITION(aDocument == mDocument, "Unexpected aDocument");
 
   if (!mDidInitialReflow) {
     return;
   }
   
   WillCauseReflow();
   mFrameConstructor->ContentInserted(aContainer, aChild,
-                                     aIndexInContainer, nsnull);
+                                     aIndexInContainer, nsnull, PR_FALSE);
   VERIFY_STYLE_TREE;
   DidCauseReflow();
 }
 
 void
 PresShell::ContentRemoved(nsIDocument *aDocument,
                           nsIContent* aContainer,
                           nsIContent* aChild,
--- a/layout/reftests/reftest.list
+++ b/layout/reftests/reftest.list
@@ -58,9 +58,9 @@ include z-index/reftest.list
  
 # columns/
 include columns/reftest.list
 
 # image-region/
 include image-region/reftest.list
 
 # block-inside-inline splits
-include ib-split/reftest.list
+#include ib-split/reftest.list