Bug 512336. Make frame lists doubly-linked. r=roc,fantasai

layout/base/nsCSSFrameConstructor.cpp

@@ -2014,14 +2014,11 @@ static void
 PullOutCaptionFrames(nsFrameItems& aItems, nsFrameItems& aCaptions)
 {
   nsIFrame *child = aItems.FirstChild();
-  nsIFrame* prev = nsnull;
   while (child) {
     nsIFrame *nextSibling = child->GetNextSibling();
     if (nsGkAtoms::tableCaptionFrame == child->GetType()) {
-      aItems.RemoveFrame(child, prev);
+      aItems.RemoveFrame(child);
       aCaptions.AddChild(child);
-    } else {
-      prev = child;
     }
     child = nextSibling;
   }
@@ -3463,17 +3460,15 @@ nsCSSFrameConstructor::ConstructFieldSetFrame(nsFrameConstructorState& aState,
   nsFrameItems fieldsetKids;
   fieldsetKids.AddChild(blockFrame);
 
-  for (nsFrameList::FrameLinkEnumerator link(childItems);
+  for (nsFrameList::Enumerator e(childItems); !e.AtEnd(); e.Next()) {
-       !link.AtEnd();
+    nsLegendFrame* legendFrame = do_QueryFrame(e.get());
-       link.Next()) {
-    nsLegendFrame* legendFrame = do_QueryFrame(link.NextFrame());
     if (legendFrame) {
       // We want the legend to be the first frame in the fieldset child list.
       // That way the EventStateManager will do the right thing when tabbing
       // from a selection point within the legend (bug 236071), which is
       // used for implementing legend access keys (bug 81481).
       // GetAdjustedParentFrame() below depends on this frame order.
-      childItems.RemoveFrame(link.NextFrame(), link.PrevFrame());
+      childItems.RemoveFrame(legendFrame);
       // Make sure to reparent the legend so it has the fieldset as the parent.
       fieldsetKids.InsertFrame(newFrame, nsnull, legendFrame);
       break;
@@ -3970,7 +3965,7 @@ nsCSSFrameConstructor::ConstructFrameFromItemInternal(FrameConstructionItem& aIt
             break;
         }
 
-        childItems.RemoveFrame(f, nsnull);
+        childItems.RemoveFrame(f);
         if (wrapFrame) {
           currentBlock.AddChild(f);
         } else {
@@ -5700,7 +5695,7 @@ FindAppendPrevSibling(nsIFrame* aParentFrame, nsIFrame* aAfterFrame)
 {
   if (aAfterFrame) {
     NS_ASSERTION(aAfterFrame->GetParent() == aParentFrame, "Wrong parent");
-    return aParentFrame->GetChildList(nsnull).GetPrevSiblingFor(aAfterFrame);
+    return aAfterFrame->GetPrevSibling();
   }
 
   return aParentFrame->GetLastChild(nsnull);
@@ -10183,7 +10178,7 @@ nsCSSFrameConstructor::WrapFramesInFirstLetterFrame(
     if (parentFrame == aBlockFrame) {
       // Take textFrame out of the block's frame list and substitute the
       // letter frame(s) instead.
-      aBlockFrames.DestroyFrame(textFrame, prevFrame);
+      aBlockFrames.DestroyFrame(textFrame);
       aBlockFrames.InsertFrames(nsnull, prevFrame, letterFrames);
     }
     else {
@@ -10344,18 +10339,7 @@ nsCSSFrameConstructor::RemoveFloatingFirstLetterFrames(
     frameToDelete = nextFrameToDelete;
   }
 
-  // First find out where (in the content) the placeholder frames
+  nsIFrame* prevSibling = placeholderFrame->GetPrevSibling();
-  // text is and its previous sibling frame, if any.  Note that:
-  // 1)  The placeholder had better be in the principal child list of
-  //     parentFrame.
-  // 2)  It's probably near the beginning (since we're a first-letter frame),
-  //     so just doing a linear search for the prevSibling is ok.
-  // 3)  Trying to use FindPreviousSibling will fail if the first-letter is in
-  //     anonymous content (eg generated content).
-  const nsFrameList& siblingList(parentFrame->GetChildList(nsnull));
-  NS_ASSERTION(siblingList.ContainsFrame(placeholderFrame),
-               "Placeholder not in parent's principal child list?");
-  nsIFrame* prevSibling = siblingList.GetPrevSiblingFor(placeholderFrame);
 
   // Now that everything is set...
 #ifdef NOISY_FIRST_LETTER

layout/base/nsFrameTraversal.cpp

@@ -504,10 +504,7 @@ nsFrameIterator::GetNextSiblingInner(nsIFrame* aFrame) {
 
 nsIFrame*
 nsFrameIterator::GetPrevSiblingInner(nsIFrame* aFrame) {
-  nsIFrame* parent = GetParentFrame(aFrame);
+  return aFrame->GetPrevSibling();
-  if (!parent)
-    return nsnull;
-  return parent->GetChildList(nsnull).GetPrevSiblingFor(aFrame);
 }
 
 

layout/generic/nsBlockFrame.cpp

@@ -3261,7 +3261,7 @@ nsBlockFrame::ReflowBlockFrame(nsBlockReflowState& aState,
               NS_ENSURE_SUCCESS(rv, rv);
             }
             else if (madeContinuation) {
-              mFrames.RemoveFrame(nextFrame, frame);
+              mFrames.RemoveFrame(nextFrame);
             }
 
             // Put it in our overflow list
@@ -5203,30 +5203,22 @@ nsBlockFrame::DoRemoveFrame(nsIFrame* aDeletedFrame, PRUint32 aFlags)
 
   nsIPresShell* presShell = presContext->PresShell();
 
-  // Find the line and the previous sibling that contains
+  // Find the line that contains deletedFrame
-  // deletedFrame; we also find the pointer to the line.
   nsLineList::iterator line_start = mLines.begin(),
                        line_end = mLines.end();
   nsLineList::iterator line = line_start;
   PRBool searchingOverflowList = PR_FALSE;
-  nsIFrame* prevSibling = nsnull;
   // Make sure we look in the overflow lines even if the normal line
   // list is empty
   TryAllLines(&line, &line_start, &line_end, &searchingOverflowList);
   while (line != line_end) {
-    nsIFrame* frame = line->mFirstChild;
+    if (line->Contains(aDeletedFrame)) {
-    PRInt32 n = line->GetChildCount();
+      break;
-    while (--n >= 0) {
-      if (frame == aDeletedFrame) {
-        goto found_frame;
-      }
-      prevSibling = frame;
-      frame = frame->GetNextSibling();
     }
     ++line;
     TryAllLines(&line, &line_start, &line_end, &searchingOverflowList);
   }
-found_frame:;
+
   if (line == line_end) {
     NS_ERROR("can't find deleted frame in lines");
     return NS_ERROR_FAILURE;
@@ -5243,13 +5235,6 @@ found_frame:;
     }
   }
 
-  if (prevSibling && !prevSibling->GetNextSibling()) {
-    // We must have found the first frame in the overflow line list. So
-    // there is no prevSibling
-    prevSibling = nsnull;
-  }
-  NS_ASSERTION(!prevSibling || prevSibling->GetNextSibling() == aDeletedFrame, "bad prevSibling");
-
   while ((line != line_end) && (nsnull != aDeletedFrame)) {
     NS_ASSERTION(this == aDeletedFrame->GetParent(), "messed up delete code");
     NS_ASSERTION(line->Contains(aDeletedFrame), "frame not in line");
@@ -5287,13 +5272,14 @@ found_frame:;
     // prevSibling will only be nsnull when we are deleting the very
     // 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);
       }
     } else {
-      mFrames.RemoveFrame(aDeletedFrame, prevSibling);
+      mFrames.RemoveFrame(aDeletedFrame);
     }
 
     // Update the child count of the line to be accurate
@@ -5385,20 +5371,10 @@ found_frame:;
           line = line_end;
         }
 
-        PRBool wasSearchingOverflowList = searchingOverflowList;
         TryAllLines(&line, &line_start, &line_end, &searchingOverflowList);
-        if (NS_UNLIKELY(searchingOverflowList && !wasSearchingOverflowList &&
-                        prevSibling)) {
-          // We switched to the overflow line list and we have a prev sibling
-          // (in the main list), in this case we don't want to pick up any
-          // sibling list from the deceased frames (bug 344557).
-          NS_ASSERTION(!prevSibling->GetNextSibling(), "Unexpected next sibling");
-          prevSibling = nsnull;
-        }
 #ifdef NOISY_REMOVE_FRAME
-        printf("DoRemoveFrame: now on %s line=%p prevSibling=%p\n",
+        printf("DoRemoveFrame: now on %s line=%p\n",
-               searchingOverflowList?"overflow":"normal", line.get(),
+               searchingOverflowList?"overflow":"normal", line.get());
-               prevSibling);
 #endif
       }
     }
@@ -5467,7 +5443,7 @@ nsBlockFrame::StealFrame(nsPresContext* aPresContext,
             prevSibling->SetNextSibling(frame->GetNextSibling());
           frame->SetNextSibling(nsnull);
         } else {
-          mFrames.RemoveFrame(frame, prevSibling);
+          mFrames.RemoveFrame(frame);
         }
 
         // Register removal with the line boxes

layout/generic/nsFrame.cpp

@@ -4770,11 +4770,10 @@ FindBlockFrameOrBR(nsIFrame* aFrame, nsDirection aDirection)
 
   // Iterate over children and call ourselves recursively
   if (aDirection == eDirPrevious) {
-    const nsFrameList& children(aFrame->GetChildList(nsnull));
+    nsIFrame* child = aFrame->GetChildList(nsnull).LastChild();
-    nsIFrame* child = children.LastChild();
     while(child && !result.mContent) {
       result = FindBlockFrameOrBR(child, aDirection);
-      child = children.GetPrevSiblingFor(child);
+      child = child->GetPrevSibling();
     }
   } else { // eDirNext
     nsIFrame* child = aFrame->GetFirstChild(nsnull);
@@ -4807,11 +4806,10 @@ nsIFrame::PeekOffsetParagraph(nsPeekOffsetStruct *aPos)
         reachedBlockAncestor = PR_TRUE;
         break;
       }
-      const nsFrameList& siblings(parent->GetChildList(nsnull));
+      nsIFrame* sibling = frame->GetPrevSibling();
-      nsIFrame* sibling = siblings.GetPrevSiblingFor(frame);
       while (sibling && !blockFrameOrBR.mContent) {
         blockFrameOrBR = FindBlockFrameOrBR(sibling, eDirPrevious);
-        sibling = siblings.GetPrevSiblingFor(sibling);
+        sibling = sibling->GetPrevSibling();
       }
       if (blockFrameOrBR.mContent) {
         aPos->mResultContent = blockFrameOrBR.mContent;

layout/generic/nsFrameList.cpp

@@ -94,7 +94,7 @@ nsFrameList::SetFrames(nsIFrame* aFrameList)
 }
 
 void
-nsFrameList::RemoveFrame(nsIFrame* aFrame, nsIFrame* aPrevSiblingHint)
+nsFrameList::RemoveFrame(nsIFrame* aFrame)
 {
   NS_PRECONDITION(aFrame, "null ptr");
   NS_PRECONDITION(ContainsFrame(aFrame), "wrong list");
@@ -108,16 +108,13 @@ nsFrameList::RemoveFrame(nsIFrame* aFrame, nsIFrame* aPrevSiblingHint)
     }
   }
   else {
-    nsIFrame* prevSibling = aPrevSiblingHint;
+    nsIFrame* prevSibling = aFrame->GetPrevSibling();
-    if (!prevSibling || prevSibling->GetNextSibling() != aFrame) {
+    NS_ASSERTION(prevSibling && prevSibling->GetNextSibling() == aFrame,
-      prevSibling = GetPrevSiblingFor(aFrame);
+                 "Broken frame linkage");
-    }
+    prevSibling->SetNextSibling(nextFrame);
-    if (prevSibling) {
+    aFrame->SetNextSibling(nsnull);
-      prevSibling->SetNextSibling(nextFrame);
+    if (!nextFrame) {
-      aFrame->SetNextSibling(nsnull);
+      mLastChild = prevSibling;
-      if (!nextFrame) {
-        mLastChild = prevSibling;
-      }
     }
   }
 }
@@ -127,9 +124,9 @@ nsFrameList::RemoveFrameIfPresent(nsIFrame* aFrame)
 {
   NS_PRECONDITION(aFrame, "null ptr");
 
-  for (FrameLinkEnumerator iter(*this); !iter.AtEnd(); iter.Next()) {
+  for (Enumerator e(*this); !e.AtEnd(); e.Next()) {
-    if (iter.NextFrame() == aFrame) {
+    if (e.get() == aFrame) {
-      RemoveFrame(aFrame, iter.PrevFrame());
+      RemoveFrame(aFrame);
       return PR_TRUE;
     }
   }
@@ -161,10 +158,10 @@ nsFrameList::RemoveFirstChild()
 }
 
 void
-nsFrameList::DestroyFrame(nsIFrame* aFrame, nsIFrame* aPrevSiblingHint)
+nsFrameList::DestroyFrame(nsIFrame* aFrame)
 {
   NS_PRECONDITION(aFrame, "null ptr");
-  RemoveFrame(aFrame, aPrevSiblingHint);
+  RemoveFrame(aFrame);
   aFrame->Destroy();
 }
 
@@ -431,28 +428,6 @@ nsFrameList::SortByContentOrder()
   mLastChild = f;
 }
 
-nsIFrame*
-nsFrameList::GetPrevSiblingFor(nsIFrame* aFrame) const
-{
-  NS_PRECONDITION(aFrame, "null ptr");
-  NS_PRECONDITION(mFirstChild, "GetPrevSiblingFor() on empty frame list");
-
-  if (aFrame == mFirstChild) {
-    return nsnull;
-  }
-  nsIFrame* frame = mFirstChild;
-  while (frame) {
-    nsIFrame* next = frame->GetNextSibling();
-    if (next == aFrame) {
-      break;
-    }
-    frame = next;
-  }
-
-  NS_POSTCONDITION(frame, "GetPrevSiblingFor() on wrong frame list");
-  return frame;
-}
-
 void
 nsFrameList::ApplySetParent(nsIFrame* aParent) const
 {
@@ -485,7 +460,7 @@ nsFrameList::GetPrevVisualFor(nsIFrame* aFrame) const
   
   nsIFrame* parent = mFirstChild->GetParent();
   if (!parent)
-    return aFrame ? GetPrevSiblingFor(aFrame) : LastChild();
+    return aFrame ? aFrame->GetPrevSibling() : LastChild();
 
   nsBidiLevel baseLevel = nsBidiPresUtils::GetFrameBaseLevel(mFirstChild);  
   nsBidiPresUtils* bidiUtils = mFirstChild->PresContext()->GetBidiUtils();
@@ -504,7 +479,7 @@ nsFrameList::GetPrevVisualFor(nsIFrame* aFrame) const
       // Just get the next or prev sibling, depending on block and frame direction.
       nsBidiLevel frameEmbeddingLevel = nsBidiPresUtils::GetFrameEmbeddingLevel(mFirstChild);
       if ((frameEmbeddingLevel & 1) == (baseLevel & 1)) {
-        return aFrame ? GetPrevSiblingFor(aFrame) : LastChild();
+        return aFrame ? aFrame->GetPrevSibling() : LastChild();
       } else {
         return aFrame ? aFrame->GetNextSibling() : mFirstChild;
       }    
@@ -560,7 +535,7 @@ nsFrameList::GetNextVisualFor(nsIFrame* aFrame) const
   
   nsIFrame* parent = mFirstChild->GetParent();
   if (!parent)
-    return aFrame ? GetPrevSiblingFor(aFrame) : mFirstChild;
+    return aFrame ? aFrame->GetPrevSibling() : mFirstChild;
 
   nsBidiLevel baseLevel = nsBidiPresUtils::GetFrameBaseLevel(mFirstChild);
   nsBidiPresUtils* bidiUtils = mFirstChild->PresContext()->GetBidiUtils();
@@ -581,7 +556,7 @@ nsFrameList::GetNextVisualFor(nsIFrame* aFrame) const
       if ((frameEmbeddingLevel & 1) == (baseLevel & 1)) {
         return aFrame ? aFrame->GetNextSibling() : mFirstChild;
       } else {
-        return aFrame ? GetPrevSiblingFor(aFrame) : LastChild();
+        return aFrame ? aFrame->GetPrevSibling() : LastChild();
       }
     }
   }

layout/generic/nsFrameList.h

@@ -122,12 +122,11 @@ public:
   }
 
   /**
+   * Take aFrame out of the frame list. This also disconnects aFrame
    * from the sibling list. The frame must be non-null and present on
    * this list.
-   * The second frame is a hint for the prev-sibling of aFrame; if the
-   * hint is correct, then this is O(1) time.
    */
-  void RemoveFrame(nsIFrame* aFrame, nsIFrame* aPrevSiblingHint = nsnull);
+  void RemoveFrame(nsIFrame* aFrame);
 
   /**
    * Take aFrame out of the frame list, if present. This also disconnects
@@ -153,10 +152,8 @@ public:
   /**
    * Take aFrame out of the frame list and then destroy it.
    * The frame must be non-null and present on this list.
-   * The second frame is a hint for the prev-sibling of aFrame; if the
-   * hint is correct, then this is O(1) time.
    */
-  void DestroyFrame(nsIFrame* aFrame, nsIFrame* aPrevSiblingHint = nsnull);
+  void DestroyFrame(nsIFrame* aFrame);
 
   /**
    * If aFrame is present on this list then take it out of the list and
@@ -235,8 +232,6 @@ public:
 
   PRInt32 GetLength() const;
 
-  nsIFrame* GetPrevSiblingFor(nsIFrame* aFrame) const;
-
   /**
    * If this frame list has only one frame, return that frame.
    * Otherwise, return null.

layout/generic/nsIFrame.h

@@ -877,14 +877,22 @@ public:
   }
 
   /**
-   * Child frames are linked together in a singly-linked list
+   * Child frames are linked together in a doubly-linked list
    */
   nsIFrame* GetNextSibling() const { return mNextSibling; }
   void SetNextSibling(nsIFrame* aNextSibling) {
-    NS_ASSERTION(this != aNextSibling, "Creating a circular frame list, this is very bad."); 
+    NS_ASSERTION(this != aNextSibling, "Creating a circular frame list, this is very bad.");
+    if (mNextSibling && mNextSibling->GetPrevSibling() == this) {
+      mNextSibling->mPrevSibling = nsnull;
+    }
     mNextSibling = aNextSibling;
+    if (mNextSibling) {
+      mNextSibling->mPrevSibling = this;
+    }
   }
 
+  nsIFrame* GetPrevSibling() const { return mPrevSibling; }
+
   /**
    * Builds the display lists for the content represented by this frame
    * and its descendants. The background+borders of this element must
@@ -2319,7 +2327,8 @@ protected:
   nsStyleContext*  mStyleContext;
   nsIFrame*        mParent;
 private:
-  nsIFrame*        mNextSibling;  // singly-linked list of frames
+  nsIFrame*        mNextSibling;  // doubly-linked list of frames
+  nsIFrame*        mPrevSibling;  // Do not touch outside SetNextSibling!
 protected:
   nsFrameState     mState;
 

layout/mathml/nsMathMLmoFrame.cpp

@@ -352,11 +352,10 @@ nsMathMLmoFrame::ProcessOperatorData()
       mFlags &= ~NS_MATHML_OPERATOR_EMBELLISH_ANCESTOR;
 
     // find the position of our outermost embellished container w.r.t
-    // its siblings (frames are singly-linked together).
+    // its siblings.
-    const nsFrameList& frameList(parentAncestor->GetChildList(nsnull));
 
     nsIFrame* nextSibling = embellishAncestor->GetNextSibling();
-    nsIFrame* prevSibling = frameList.GetPrevSiblingFor(embellishAncestor);
+    nsIFrame* prevSibling = embellishAncestor->GetPrevSibling();
 
     // flag to distinguish from a real infix
     if (!prevSibling && !nextSibling)

layout/tables/nsTableFrame.cpp

@@ -1953,7 +1953,6 @@ nsTableFrame::PushChildren(const FrameArray& aFrames,
   // extract the frames from the array into a sibling list
   nsFrameList frames;
   PRUint32 childX;
-  nsIFrame* prevSiblingHint = aFrames.SafeElementAt(aPushFrom - 1);
   for (childX = aPushFrom; childX < aFrames.Length(); ++childX) {
     nsIFrame* f = aFrames[childX];
     // Don't push repeatable frames, do push non-rowgroup frames.
@@ -1963,7 +1962,7 @@ nsTableFrame::PushChildren(const FrameArray& aFrames,
     nsTableRowGroupFrame* rgFrame = GetRowGroupFrame(f);
     NS_ASSERTION(rgFrame, "Unexpected non-row-group frame");
     if (!rgFrame || !rgFrame->IsRepeatable()) {
-      mFrames.RemoveFrame(f, prevSiblingHint);
+      mFrames.RemoveFrame(f);
       frames.AppendFrame(nsnull, f);
     }
   }
@@ -1975,7 +1974,7 @@ nsTableFrame::PushChildren(const FrameArray& aFrames,
     nsIFrame* firstBodyFrame = nextInFlow->GetFirstBodyRowGroupFrame();
     nsIFrame* prevSibling = nsnull;
     if (firstBodyFrame) {
-      prevSibling = nextInFlow->mFrames.GetPrevSiblingFor(firstBodyFrame);
+      prevSibling = firstBodyFrame->GetPrevSibling();
     }
     // When pushing and pulling frames we need to check for whether any
     // views need to be reparented.

layout/xul/base/src/nsBoxFrame.cpp

@@ -2093,34 +2093,28 @@ nsBoxFrame::RelayoutChildAtOrdinal(nsBoxLayoutState& aState, nsIBox* aChild)
 
   PRUint32 ord = aChild->GetOrdinal(aState);
   
-  nsIFrame *child = mFrames.FirstChild();
+  nsIFrame* child = mFrames.FirstChild();
-  nsIFrame *curPrevSib = nsnull, *newPrevSib = nsnull;
+  nsIFrame* newPrevSib = nsnull;
-  PRBool foundPrevSib = PR_FALSE, foundNewPrevSib = PR_FALSE;
 
   while (child) {
-    if (child == aChild)
+    if (ord < child->GetOrdinal(aState)) {
-      foundPrevSib = PR_TRUE;
+      break;
-    else if (!foundPrevSib)
+    }
-      curPrevSib = child;
 
-    PRUint32 ordCmp = child->GetOrdinal(aState);
+    if (child != aChild) {
-    if (ord < ordCmp)
-      foundNewPrevSib = PR_TRUE;
-    else if (!foundNewPrevSib && child != aChild)
       newPrevSib = child;
+    }
 
     child = child->GetNextBox();
   }
 
-  NS_ASSERTION(foundPrevSib, "aChild not in frame list?");
+  if (aChild->GetPrevSibling() == newPrevSib) {
-
-  if (curPrevSib == newPrevSib) {
     // This box is not moving.
     return NS_OK;
   }
 
   // Take |aChild| out of its old position in the child list.
-  mFrames.RemoveFrame(aChild, curPrevSib);
+  mFrames.RemoveFrame(aChild);
 
   // Insert it after |newPrevSib| or at the start if it's null.
   mFrames.InsertFrame(nsnull, newPrevSib, aChild);

layout/xul/base/src/nsListBoxBodyFrame.cpp

@@ -1110,7 +1110,7 @@ nsListBoxBodyFrame::ReverseDestroyRows(PRInt32& aRowsToLose)
     --aRowsToLose;
     
     nsIFrame* prevFrame;
-    prevFrame = mFrames.GetPrevSiblingFor(childFrame);
+    prevFrame = childFrame->GetPrevSibling();
     RemoveChildFrame(state, childFrame);
 
     mBottomFrame = childFrame = prevFrame;

layout/xul/base/src/nsSplitterFrame.cpp

@@ -874,8 +874,7 @@ nsSplitterFrameInner::UpdateState()
     // Find the splitter's immediate sibling.
     nsIFrame* splitterSibling;
     if (newState == CollapsedBefore || mState == CollapsedBefore) {
-      splitterSibling =
+      splitterSibling = mOuter->GetPrevSibling();
-        mOuter->GetParent()->GetChildList(nsnull).GetPrevSiblingFor(mOuter);
     } else {
       splitterSibling = mOuter->GetNextSibling();
     }

layout/xul/base/src/nsXULPopupManager.cpp

@@ -1809,7 +1809,7 @@ nsXULPopupManager::GetPreviousMenuItem(nsIFrame* aParent,
 
   nsIFrame* currFrame = nsnull;
   if (aStart)
-    currFrame = frames.GetPrevSiblingFor(aStart);
+    currFrame = aStart->GetPrevSibling();
   else
     currFrame = frames.LastChild();
 
@@ -1819,7 +1819,7 @@ nsXULPopupManager::GetPreviousMenuItem(nsIFrame* aParent,
       return (currFrame->GetType() == nsGkAtoms::menuFrame) ?
              static_cast<nsMenuFrame *>(currFrame) : nsnull;
     }
-    currFrame = frames.GetPrevSiblingFor(currFrame);
+    currFrame = currFrame->GetPrevSibling();
   }
 
   currFrame = frames.LastChild();
@@ -1832,7 +1832,7 @@ nsXULPopupManager::GetPreviousMenuItem(nsIFrame* aParent,
              static_cast<nsMenuFrame *>(currFrame) : nsnull;
     }
 
-    currFrame = frames.GetPrevSiblingFor(currFrame);
+    currFrame = currFrame->GetPrevSibling();
   }
 
   // No luck. Just return our start value.

layout/xul/base/src/tree/src/nsTreeColFrame.cpp

@@ -130,7 +130,7 @@ nsDisplayXULTreeColSplitterTarget::HitTest(nsDisplayListBuilder* aBuilder,
     const nsFrameList& frames(mFrame->GetParent()->GetChildList(nsnull));
     nsIFrame* child;
     if (left)
-      child = frames.GetPrevSiblingFor(mFrame);
+      child = mFrame->GetPrevSibling();
     else
       child = mFrame->GetNextSibling();