Bug 577309 part 15. Stop using content indices entirely in nsCSSFrameConstructor::ContentRemoved. r=tnikkel
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 15 Jul 2010 00:38:24 -0400
changeset 47669 f12a52c2b5892ec438c7494c2b09eff96b1dd9ca
parent 47668 c879c3795adbe021c97513150bdf6dc4aebabb88
child 47670 adaf0592cdfa73a057f0e3ca6d017af06f0b40f1
push idunknown
push userunknown
push dateunknown
reviewerstnikkel
bugs577309
milestone2.0b2pre
Bug 577309 part 15. Stop using content indices entirely in nsCSSFrameConstructor::ContentRemoved. r=tnikkel
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsCSSFrameConstructor.h
layout/base/nsPresShell.cpp
layout/xul/base/src/nsListBoxBodyFrame.cpp
layout/xul/base/src/nsListBoxBodyFrame.h
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -6160,32 +6160,28 @@ nsCSSFrameConstructor::AddTextItemIfNeed
   NS_ASSERTION(!aPossibleTextContent->GetPrimaryFrame(),
                "Text node has a frame and NS_CREATE_FRAME_IF_NON_WHITESPACE");
   AddFrameConstructionItems(aState, aPossibleTextContent, PR_FALSE,
                             aParentFrame, aItems);
 }
 
 void
 nsCSSFrameConstructor::ReframeTextIfNeeded(nsIContent* aParentContent,
-                                           PRInt32 aContentIndex)
-{
-  NS_ASSERTION(aContentIndex >= 0 &&
-               PRUint32(aContentIndex) < aParentContent->GetChildCount(),
-               "child index out of range");
-  nsIContent* content = aParentContent->GetChildAt(aContentIndex);
-  if (!content->IsNodeOfType(nsINode::eTEXT) ||
-      !content->HasFlag(NS_CREATE_FRAME_IF_NON_WHITESPACE)) {
+                                           nsIContent* aContent)
+{
+  if (!aContent->IsNodeOfType(nsINode::eTEXT) ||
+      !aContent->HasFlag(NS_CREATE_FRAME_IF_NON_WHITESPACE)) {
     // Not text, or not suppressed due to being all-whitespace (if it
     // were being suppressed, it would have the
     // NS_CREATE_FRAME_IF_NON_WHITESPACE flag)
     return;
   }
-  NS_ASSERTION(!content->GetPrimaryFrame(),
+  NS_ASSERTION(!aContent->GetPrimaryFrame(),
                "Text node has a frame and NS_CREATE_FRAME_IF_NON_WHITESPACE");
-  ContentInserted(aParentContent, content, nsnull, PR_FALSE);
+  ContentInserted(aParentContent, aContent, nsnull, PR_FALSE);
 }
 
 // We want to disable lazy frame construction for nodes that are under an
 // editor. We use nsINode::IsEditable, but that includes inputs with type text
 // and password and textareas, which are common and aren't really editable (the
 // native anonymous content under them is what is actually editable) so we want
 // to construct frames for those lazily.
 // The logic for this check is based on
@@ -6752,36 +6748,34 @@ enum content_operation
 };
 
 // Helper function to lookup the listbox body frame and send a notification
 // for insertion or removal of content
 static
 PRBool NotifyListBoxBody(nsPresContext*    aPresContext,
                          nsIContent*        aContainer,
                          nsIContent*        aChild,
-                         PRInt32            aIndexInContainer,
+                         // Only used for the removed notification
+                         nsIContent*        aOldNextSibling,
                          nsIDocument*       aDocument,                         
                          nsIFrame*          aChildFrame,
                          content_operation  aOperation)
 {
   nsListBoxBodyFrame* listBoxBodyFrame =
     MaybeGetListBoxBodyFrame(aContainer, aChild);
   if (listBoxBodyFrame) {
     if (aOperation == CONTENT_REMOVED) {
       // Except if we have an aChildFrame and its parent is not the right
       // thing, then we don't do this.  Pseudo frames are so much fun....
       if (!aChildFrame || aChildFrame->GetParent() == listBoxBodyFrame) {
         listBoxBodyFrame->OnContentRemoved(aPresContext, aContainer,
-                                           aChildFrame, aIndexInContainer);
+                                           aChildFrame, aOldNextSibling);
         return PR_TRUE;
       }
     } else {
-      // If this codepath ever starts using aIndexInContainer, need to
-      // change ContentInserted to pass in something resembling a correct
-      // one in the XBL cases.
       listBoxBodyFrame->OnContentInserted(aPresContext, aChild);
       return PR_TRUE;
     }
   }
 
   return PR_FALSE;
 }
 #endif // MOZ_XUL
@@ -6857,18 +6851,18 @@ nsCSSFrameConstructor::ContentRangeInser
   NS_ASSERTION(isSingleInsert || aEndChild,
                "range should not include all nodes after aStartChild");
 
 #ifdef MOZ_XUL
   if (aContainer && IsXULListBox(aContainer)) {
     if (isSingleInsert) {
       if (NotifyListBoxBody(mPresShell->GetPresContext(), aContainer,
                             // The insert case in NotifyListBoxBody
-                            // doesn't use the index.
-                            aStartChild, -1, 
+                            // doesn't use "old next sibling".
+                            aStartChild, nsnull, 
                             mDocument, nsnull, CONTENT_INSERTED)) {
         return NS_OK;
       }
     } else {
       // We don't handle a range insert to a listbox parent, issue single
       // ContertInserted calls for each node inserted.
       LAYOUT_PHASE_TEMP_EXIT();
       IssueSingleInsertNofications(aContainer, aStartChild, aEndChild,
@@ -7313,35 +7307,36 @@ nsCSSFrameConstructor::ContentRangeInser
 #endif
 
   return NS_OK;
 }
 
 nsresult
 nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer,
                                       nsIContent* aChild,
-                                      PRInt32     aIndexInContainer,
+                                      nsIContent* aOldNextSibling,
                                       RemoveFlags aFlags,
                                       PRBool*     aDidReconstruct)
 {
   AUTO_LAYOUT_PHASE_ENTRY_POINT(mPresShell->GetPresContext(), FrameC);
   NS_PRECONDITION(mUpdateCount != 0,
                   "Should be in an update while destroying frames");
 
   *aDidReconstruct = PR_FALSE;
   
   // XXXldb Do we need to re-resolve style to handle the CSS2 + combinator and
   // the :empty pseudo-class?
 
 #ifdef DEBUG
   if (gNoisyContentUpdates) {
-    printf("nsCSSFrameConstructor::ContentRemoved container=%p child=%p index=%d\n",
+    printf("nsCSSFrameConstructor::ContentRemoved container=%p child=%p "
+           "old-next-sibling=%p\n",
            static_cast<void*>(aContainer),
            static_cast<void*>(aChild),
-           aIndexInContainer);
+           static_cast<void*>(aOldNextSibling));
     if (gReallyNoisyContentUpdates) {
       aContainer->List(stdout, 0);
     }
   }
 #endif
 
   nsFrameManager *frameManager = mPresShell->FrameManager();
   nsPresContext *presContext = mPresShell->GetPresContext();
@@ -7352,17 +7347,17 @@ nsCSSFrameConstructor::ContentRemoved(ns
 
   if (!childFrame || childFrame->GetContent() != aChild) {
     // XXXbz the GetContent() != aChild check is needed due to bug 135040.
     // Remove it once that's fixed.
     frameManager->ClearUndisplayedContentIn(aChild, aContainer);
   }
 
 #ifdef MOZ_XUL
-  if (NotifyListBoxBody(presContext, aContainer, aChild, aIndexInContainer, 
+  if (NotifyListBoxBody(presContext, aContainer, aChild, aOldNextSibling, 
                         mDocument, childFrame, CONTENT_REMOVED))
     return NS_OK;
 
 #endif // MOZ_XUL
 
   // If we're removing the root, then make sure to remove things starting at
   // the viewport's child instead of the primary frame (which might even be
   // null if the root had an XBL binding or display:none, even though the
@@ -7527,39 +7522,43 @@ nsCSSFrameConstructor::ContentRemoved(ns
     }
 
     // If we're just reconstructing frames for the element, then the
     // following ContentInserted notification on the element will
     // take care of fixing up any adjacent text nodes.  We don't need
     // to do this if the table parent type of our parent type is not
     // eTypeBlock, though, because in that case the whitespace isn't
     // being suppressed due to us anyway.
-    if (aContainer && aIndexInContainer >= 0 &&
+    if (aContainer && !aChild->IsRootOfAnonymousSubtree() &&
         aFlags != REMOVE_FOR_RECONSTRUCTION &&
         GetParentType(parentType) == eTypeBlock) {
       // Adjacent whitespace-only text nodes might have been suppressed if
       // this node does not have inline ends. Create frames for them now
       // if necessary.
-      PRInt32 childCount = aContainer->GetChildCount();
       // Reframe any text node just before the node being removed, if there is
       // one, and if it's not the last child or the first child. If a whitespace
       // textframe was being suppressed and it's now the last child or first
       // child then it can stay suppressed since the parent must be a block
       // and hence it's adjacent to a block end.
-      PRInt32 prevSiblingIndex = aIndexInContainer - 1;
-      if (prevSiblingIndex > 0 && prevSiblingIndex < childCount - 1) {
-        LAYOUT_PHASE_TEMP_EXIT();
-        ReframeTextIfNeeded(aContainer, prevSiblingIndex);
-        LAYOUT_PHASE_TEMP_REENTER();
+      // If aOldNextSibling is null, then the text node before the node being
+      // removed is the last node, and we don't need to worry about it.
+      if (aOldNextSibling) {
+        nsIContent* prevSibling = aOldNextSibling->GetPreviousSibling();
+        if (prevSibling && prevSibling->GetPreviousSibling()) {
+          LAYOUT_PHASE_TEMP_EXIT();
+          ReframeTextIfNeeded(aContainer, prevSibling);
+          LAYOUT_PHASE_TEMP_REENTER();
+        }
       }
       // Reframe any text node just after the node being removed, if there is
       // one, and if it's not the last child or the first child.
-      if (aIndexInContainer > 0 && aIndexInContainer < childCount - 1) {
+      if (aOldNextSibling && aOldNextSibling->GetNextSibling() &&
+          aOldNextSibling->GetPreviousSibling()) {
         LAYOUT_PHASE_TEMP_EXIT();
-        ReframeTextIfNeeded(aContainer, aIndexInContainer);
+        ReframeTextIfNeeded(aContainer, aOldNextSibling);
         LAYOUT_PHASE_TEMP_REENTER();
       }
     }
 
 #ifdef DEBUG
     if (gReallyNoisyContentUpdates && parentFrame) {
       printf("nsCSSFrameConstructor::ContentRemoved: resulting frame model:\n");
       parentFrame->List(stdout, 0);
@@ -9043,33 +9042,32 @@ nsCSSFrameConstructor::RecreateFramesFor
 
   nsresult rv = NS_OK;
 
   if (frame && MaybeRecreateContainerForFrameRemoval(frame, &rv)) {
     return rv;
   }
 
   nsINode* containerNode = aContent->GetNodeParent();
+  // XXXbz how can containerNode be null here?
   if (containerNode) {
-    // XXXbz what if this is anonymous content?
-    // XXXroc should we recreate frames for the container here instead?
-    PRInt32 indexInContainer = containerNode->IndexOf(aContent);
-
     // Before removing the frames associated with the content object,
     // ask them to save their state onto a temporary state object.
     CaptureStateForFramesOf(aContent, mTempFrameTreeState);
 
-    // Need both parents, since we need to pass null to ContentInserted and
-    // ContentRemoved but want to use our real parent node for IndexOf.
+    // Need the nsIContent parent, which might be null here, since we need to
+    // pass it to ContentInserted and ContentRemoved.
     nsCOMPtr<nsIContent> container = aContent->GetParent();
 
-    // Remove the frames associated with the content object on which
-    // the attribute change occurred.
+    // Remove the frames associated with the content object.
     PRBool didReconstruct;
-    rv = ContentRemoved(container, aContent, indexInContainer,
+    rv = ContentRemoved(container, aContent,
+                        aContent->IsRootOfAnonymousSubtree() ?
+                          nsnull :
+                          aContent->GetNextSibling(),
                         REMOVE_FOR_RECONSTRUCTION, &didReconstruct);
 
     if (NS_SUCCEEDED(rv) && !didReconstruct) {
       // Now, recreate the frames associated with this content object. If
       // ContentRemoved triggered reconstruction, then we don't need to do this
       // because the frames will already have been built.
       if (aAsyncInsert) {
         PostRestyleEvent(aContent->AsElement(), nsRestyleHint(0),
--- a/layout/base/nsCSSFrameConstructor.h
+++ b/layout/base/nsCSSFrameConstructor.h
@@ -233,17 +233,17 @@ public:
                                 nsIContent*            aStartChild,
                                 nsIContent*            aEndChild,
                                 nsILayoutHistoryState* aFrameState,
                                 PRBool                 aAllowLazyConstruction);
 
   enum RemoveFlags { REMOVE_CONTENT, REMOVE_FOR_RECONSTRUCTION };
   nsresult ContentRemoved(nsIContent* aContainer,
                           nsIContent* aChild,
-                          PRInt32     aIndexInContainer,
+                          nsIContent* aOldNextSibling,
                           RemoveFlags aFlags,
                           PRBool*     aDidReconstruct);
 
   nsresult CharacterDataChanged(nsIContent* aContent,
                                 CharacterDataChangeInfo* aInfo);
 
   nsresult ContentStatesChanged(nsIContent*     aContent1,
                                 nsIContent*     aContent2,
@@ -1199,20 +1199,20 @@ private:
 
   // If aPossibleTextContent is a text node and doesn't have a frame, append a
   // frame construction item for it to aItems.
   void AddTextItemIfNeeded(nsFrameConstructorState& aState,
                            nsIFrame* aParentFrame,
                            nsIContent* aPossibleTextContent,
                            FrameConstructionItemList& aItems);
 
-  // If aParentContent's child at aContentIndex is a text node and
+  // If aParentContent's child aContent is a text node and
   // doesn't have a frame, try to create a frame for it.
   void ReframeTextIfNeeded(nsIContent* aParentContent,
-                           PRInt32 aContentIndex);
+                           nsIContent* aContent);
 
   void AddPageBreakItem(nsIContent* aContent,
                         nsStyleContext* aMainStyleContext,
                         FrameConstructionItemList& aItems);
 
   // Function to find FrameConstructionData for aContent.  Will return
   // null if aContent is not HTML.
   // aParentFrame might be null.  If it is, that means it was an
--- a/layout/base/nsPresShell.cpp
+++ b/layout/base/nsPresShell.cpp
@@ -5020,22 +5020,29 @@ PresShell::ContentRemoved(nsIDocument *a
   // it can clean up any state related to the content.
   mPresContext->EventStateManager()->ContentRemoved(aDocument, aChild);
 
   nsAutoCauseReflowNotifier crNotifier(this);
 
   // Call this here so it only happens for real content mutations and
   // not cases when the frame constructor calls its own methods to force
   // frame reconstruction.
+  nsIContent* oldNextSibling;
+  if (aContainer) {
+    oldNextSibling = aContainer->GetChildAt(aIndexInContainer);
+  } else {
+    oldNextSibling = nsnull;
+  }
+  
   if (aContainer)
     mFrameConstructor->RestyleForRemove(aContainer->AsElement(), aChild,
-                                        aContainer->GetChildAt(aIndexInContainer));
+                                        oldNextSibling);
 
   PRBool didReconstruct;
-  mFrameConstructor->ContentRemoved(aContainer, aChild, aIndexInContainer,
+  mFrameConstructor->ContentRemoved(aContainer, aChild, oldNextSibling,
                                     nsCSSFrameConstructor::REMOVE_CONTENT,
                                     &didReconstruct);
 
   VERIFY_STYLE_TREE;
 }
 
 nsresult
 PresShell::ReconstructFrames(void)
--- a/layout/xul/base/src/nsListBoxBodyFrame.cpp
+++ b/layout/xul/base/src/nsListBoxBodyFrame.cpp
@@ -1351,34 +1351,34 @@ nsListBoxBodyFrame::OnContentInserted(ns
 
 // 
 // Called by nsCSSFrameConstructor when listitem content is removed.
 //
 void
 nsListBoxBodyFrame::OnContentRemoved(nsPresContext* aPresContext,
                                      nsIContent* aContainer,
                                      nsIFrame* aChildFrame,
-                                     PRInt32 aIndex)
+                                     nsIContent* aOldNextSibling)
 {
   NS_ASSERTION(!aChildFrame || aChildFrame->GetParent() == this,
                "Removing frame that's not our child... Not good");
   
   if (mRowCount >= 0)
     --mRowCount;
 
   if (aContainer) {
     if (!aChildFrame) {
       // The row we are removing is out of view, so we need to try to
       // determine the index of its next sibling.
-      nsIContent *oldNextSiblingContent = aContainer->GetChildAt(aIndex);
-  
       PRInt32 siblingIndex = -1;
-      if (oldNextSiblingContent) {
+      if (aOldNextSibling) {
         nsCOMPtr<nsIContent> nextSiblingContent;
-        GetListItemNextSibling(oldNextSiblingContent, getter_AddRefs(nextSiblingContent), siblingIndex);
+        GetListItemNextSibling(aOldNextSibling,
+                               getter_AddRefs(nextSiblingContent),
+                               siblingIndex);
       }
     
       // if the row being removed is off-screen and above the top frame, we need to
       // adjust our top index and tell the scrollbar to shift up one row.
       if (siblingIndex >= 0 && siblingIndex-1 < mCurrentIndex) {
         NS_PRECONDITION(mCurrentIndex > 0, "mCurrentIndex > 0");
         --mCurrentIndex;
         mYPosition = mCurrentIndex*mRowHeight;
--- a/layout/xul/base/src/nsListBoxBodyFrame.h
+++ b/layout/xul/base/src/nsListBoxBodyFrame.h
@@ -135,17 +135,17 @@ public:
   void ReverseDestroyRows(PRInt32& aRowsToLose);
   nsIBox* GetFirstItemBox(PRInt32 aOffset, PRBool* aCreated);
   nsIBox* GetNextItemBox(nsIBox* aBox, PRInt32 aOffset, PRBool* aCreated);
   PRBool ContinueReflow(nscoord height);
   NS_IMETHOD ListBoxAppendFrames(nsFrameList& aFrameList);
   NS_IMETHOD ListBoxInsertFrames(nsIFrame* aPrevFrame, nsFrameList& aFrameList);
   void OnContentInserted(nsPresContext* aPresContext, nsIContent* aContent);
   void OnContentRemoved(nsPresContext* aPresContext,  nsIContent* aContainer,
-                        nsIFrame* aChildFrame, PRInt32 aIndex);
+                        nsIFrame* aChildFrame, nsIContent* aOldNextSibling);
 
   void GetListItemContentAt(PRInt32 aIndex, nsIContent** aContent);
   void GetListItemNextSibling(nsIContent* aListItem, nsIContent** aContent, PRInt32& aSiblingIndex);
 
   void PostReflowCallback();
 
   PRBool SetBoxObject(nsPIBoxObject* aBoxObject)
   {