Bug 461410 - nsILineIterator is never used outside of layout, and doesn't need to be refcounted: the callers can just destroy it when they're done with it. We can't do this with a virtual destructor, however, because nsTableRowGroupFrame implements the interface directly, while nsLineIterator is a separately-allocated class. So clients are expected to call DisposeLineIterator when they're done with it.
☠☠ backed out by 66c23339bd12 ☠ ☠
authorBenjamin Smedberg <benjamin@smedbergs.us>
Tue, 28 Oct 2008 00:47:19 -0400
changeset 20989 d4c9a0776667d05b8a1f62ec693c995ef4e327b3
parent 20988 773851e3b10b448adc19921dd851a37b3c6b2c7a
child 20990 3f4cdf853b293402558439ffd50292dfbe0ddb0b
child 20995 66c23339bd12aca0833a922b63d10477ea5b224b
push id1
push userroot
push dateTue, 26 Apr 2011 22:38:44 +0000
treeherdermozilla-beta@bfdb6e623a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs461410
milestone1.9.1b2pre
Bug 461410 - nsILineIterator is never used outside of layout, and doesn't need to be refcounted: the callers can just destroy it when they're done with it. We can't do this with a virtual destructor, however, because nsTableRowGroupFrame implements the interface directly, while nsLineIterator is a separately-allocated class. So clients are expected to call DisposeLineIterator when they're done with it. Instead of accessing nsILineIterator using QueryInterface, we add a nsIFrame::GetLineIterator API, which is cleaner and more efficient all at the same time! r+sr=roc
accessible/src/html/nsHyperTextAccessible.cpp
layout/base/nsPresShell.cpp
layout/generic/nsBlockFrame.cpp
layout/generic/nsBlockFrame.h
layout/generic/nsFrame.cpp
layout/generic/nsFrame.h
layout/generic/nsFrameList.cpp
layout/generic/nsIFrame.h
layout/generic/nsILineIterator.h
layout/generic/nsLineBox.cpp
layout/generic/nsLineBox.h
layout/tables/nsTableRowGroupFrame.cpp
layout/tables/nsTableRowGroupFrame.h
--- a/accessible/src/html/nsHyperTextAccessible.cpp
+++ b/accessible/src/html/nsHyperTextAccessible.cpp
@@ -1680,46 +1680,44 @@ PRInt32 nsHyperTextAccessible::GetCaretL
   PRInt32 caretOffset, returnOffsetUnused;
   domSel->GetFocusOffset(&caretOffset);
   nsFrameSelection::HINT hint = frameSelection->GetHint();
   nsIFrame *caretFrame = frameSelection->GetFrameForNodeOffset(caretContent, caretOffset,
                                                                hint, &returnOffsetUnused);
   NS_ENSURE_TRUE(caretFrame, -1);
 
   PRInt32 lineNumber = 1;
-  nsCOMPtr<nsILineIterator> lineIterForCaret;
+  nsAutoLineIterator lineIterForCaret;
   nsCOMPtr<nsIContent> hyperTextContent = do_QueryInterface(mDOMNode);
   while (caretFrame) {
     if (hyperTextContent == caretFrame->GetContent()) {
       return lineNumber; // Must be in a single line hyper text, there is no line iterator
     }
     nsIFrame *parentFrame = caretFrame->GetParent();
     if (!parentFrame)
       break;
 
     // Add lines for the sibling frames before the caret
     nsIFrame *sibling = parentFrame->GetFirstChild(nsnull);
     while (sibling && sibling != caretFrame) {
-      nsCOMPtr<nsILineIterator> lineIterForSibling = do_QueryInterface(sibling);
+      nsAutoLineIterator lineIterForSibling = sibling->GetLineIterator();
       if (lineIterForSibling) {
-        PRInt32 addLines;
         // For the frames before that grab all the lines
-        lineIterForSibling->GetNumLines(&addLines);
+        PRInt32 addLines = lineIterForSibling->GetNumLines();
         lineNumber += addLines;
       }
       sibling = sibling->GetNextSibling();
     }
 
     // Get the line number relative to the container with lines
     if (!lineIterForCaret) {   // Add the caret line just once
-      lineIterForCaret = do_QueryInterface(parentFrame);
+      lineIterForCaret = parentFrame->GetLineIterator();
       if (lineIterForCaret) {
         // Ancestor of caret
-        PRInt32 addLines;
-        lineIterForCaret->FindLineContaining(caretFrame, &addLines);
+        PRInt32 addLines = lineIterForCaret->FindLineContaining(caretFrame);
         lineNumber += addLines;
       }
     }
 
     caretFrame = parentFrame;
   }
 
   NS_NOTREACHED("DOM ancestry had this hypertext but frame ancestry didn't");
--- a/layout/base/nsPresShell.cpp
+++ b/layout/base/nsPresShell.cpp
@@ -3796,21 +3796,19 @@ UnionRectForClosestScrolledView(nsIFrame
       prevFrame = f;
       f = prevFrame->GetParent();
     }
 
     if (f != aFrame &&
         f &&
         frameType == nsGkAtoms::blockFrame) {
       // find the line containing aFrame and increase the top of |offset|.
-      nsCOMPtr<nsILineIterator> lines(do_QueryInterface(f));
-
+      nsAutoLineIterator lines = f->GetLineIterator();
       if (lines) {
-        PRInt32 index = -1;
-        lines->FindLineContaining(prevFrame, &index);
+        PRInt32 index = lines->FindLineContaining(prevFrame);
         if (index >= 0) {
           nsIFrame *trash1;
           PRInt32 trash2;
           nsRect lineBounds;
           PRUint32 trash3;
 
           if (NS_SUCCEEDED(lines->GetLine(index, &trash1, &trash2,
                                           lineBounds, &trash3))) {
--- a/layout/generic/nsBlockFrame.cpp
+++ b/layout/generic/nsBlockFrame.cpp
@@ -310,45 +310,41 @@ nsBlockFrame::Destroy()
     nsAutoOOFFrameList oofs(this);
     oofs.mList.DestroyFrames();
     // oofs is now empty and will remove the frame list property
   }
 
   nsBlockFrameSuper::Destroy();
 }
 
+/* virtual */ nsILineIterator*
+nsBlockFrame::GetLineIterator()
+{
+  nsLineIterator* it = new nsLineIterator;
+  if (!it)
+    return nsnull;
+
+  const nsStyleVisibility* visibility = GetStyleVisibility();
+  nsresult rv = it->Init(mLines, visibility->mDirection == NS_STYLE_DIRECTION_RTL);
+  if (NS_FAILED(rv)) {
+    delete it;
+    return nsnull;
+  }
+  return it;
+}
+
 NS_IMETHODIMP
 nsBlockFrame::QueryInterface(const nsIID& aIID, void** aInstancePtr)
 {
   NS_PRECONDITION(aInstancePtr, "null out param");
 
   if (aIID.Equals(kBlockFrameCID)) {
     *aInstancePtr = static_cast<void*>(static_cast<nsBlockFrame*>(this));
     return NS_OK;
   }
-  if (aIID.Equals(NS_GET_IID(nsILineIterator)) ||
-      aIID.Equals(NS_GET_IID(nsILineIteratorNavigator))) {
-    nsLineIterator* it = new nsLineIterator;
-    if (!it) {
-      *aInstancePtr = nsnull;
-      return NS_ERROR_OUT_OF_MEMORY;
-    }
-    NS_ADDREF(it); // reference passed to caller
-    const nsStyleVisibility* visibility = GetStyleVisibility();
-    nsresult rv = it->Init(mLines,
-                           visibility->mDirection == NS_STYLE_DIRECTION_RTL);
-    if (NS_FAILED(rv)) {
-      *aInstancePtr = nsnull;
-      NS_RELEASE(it);
-      return rv;
-    }
-    *aInstancePtr = static_cast<nsILineIteratorNavigator*>(it);
-    return NS_OK;
-  }
-
   return nsBlockFrameSuper::QueryInterface(aIID, aInstancePtr);
 }
 
 nsSplittableType
 nsBlockFrame::GetSplittableType() const
 {
   return NS_FRAME_SPLITTABLE_NON_RECTANGULAR;
 }
--- a/layout/generic/nsBlockFrame.h
+++ b/layout/generic/nsBlockFrame.h
@@ -69,17 +69,16 @@ enum LineReflowStatus {
   LINE_REFLOW_TRUNCATED
 };
 
 class nsBlockReflowState;
 class nsBlockInFlowLineIterator;
 class nsBulletFrame;
 class nsLineBox;
 class nsFirstLineFrame;
-class nsILineIterator;
 class nsIntervalSet;
 /**
  * Child list name indices
  * @see #GetAdditionalChildListName()
  */
 #define NS_BLOCK_LIST_COUNT  (NS_CONTAINER_LIST_COUNT_INCL_OC + 4)
 
 /**
@@ -593,16 +592,18 @@ protected:
   static PRBool FrameStartsCounterScope(nsIFrame* aFrame);
 
   void ReflowBullet(nsBlockReflowState& aState,
                     nsHTMLReflowMetrics& aMetrics,
                     nscoord aLineTop);
 
   //----------------------------------------
 
+  virtual nsILineIterator* GetLineIterator();
+
 public:
   nsLineList* GetOverflowLines() const;
 protected:
   nsLineList* RemoveOverflowLines();
   nsresult SetOverflowLines(nsLineList* aOverflowLines);
 
   nsFrameList* GetOverflowPlaceholders() const;
 
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -4553,32 +4553,31 @@ nsFrame::GetChildFrameContainingOffset(P
 nsresult
 nsFrame::GetNextPrevLineFromeBlockFrame(nsPresContext* aPresContext,
                                         nsPeekOffsetStruct *aPos,
                                         nsIFrame *aBlockFrame, 
                                         PRInt32 aLineStart, 
                                         PRInt8 aOutSideLimit
                                         )
 {
+  nsresult result;
+
   //magic numbers aLineStart will be -1 for end of block 0 will be start of block
   if (!aBlockFrame || !aPos)
     return NS_ERROR_NULL_POINTER;
 
   aPos->mResultFrame = nsnull;
   aPos->mResultContent = nsnull;
   aPos->mAttachForward = (aPos->mDirection == eDirNext);
 
-   nsresult result;
-  nsCOMPtr<nsILineIteratorNavigator> it; 
-  result = aBlockFrame->QueryInterface(NS_GET_IID(nsILineIteratorNavigator),getter_AddRefs(it));
-  if (NS_FAILED(result) || !it)
-    return result;
+  nsAutoLineIterator it = aBlockFrame->GetLineIterator();
+  if (!it)
+    return NS_ERROR_FAILURE;
   PRInt32 searchingLine = aLineStart;
-  PRInt32 countLines;
-  result = it->GetNumLines(&countLines);
+  PRInt32 countLines = it->GetNumLines();
   if (aOutSideLimit > 0) //start at end
     searchingLine = countLines;
   else if (aOutSideLimit <0)//start at beginning
     searchingLine = -1;//"next" will be 0  
   else 
     if ((aPos->mDirection == eDirPrevious && searchingLine == 0) || 
        (aPos->mDirection == eDirNext && searchingLine >= (countLines -1) )){
       //we need to jump to new block frame.
@@ -4636,20 +4635,19 @@ nsFrame::GetNextPrevLineFromeBlockFrame(
       nscoord newDesiredX  = aPos->mDesiredX - offset.x;//get desired x into blockframe coordinates!
       result = it->FindFrameAt(searchingLine, newDesiredX, &resultFrame, &isBeforeFirstFrame, &isAfterLastFrame);
       if(NS_FAILED(result))
         continue;
     }
 
     if (NS_SUCCEEDED(result) && resultFrame)
     {
-      nsCOMPtr<nsILineIteratorNavigator> newIt; 
       //check to see if this is ANOTHER blockframe inside the other one if so then call into its lines
-      result = resultFrame->QueryInterface(NS_GET_IID(nsILineIteratorNavigator),getter_AddRefs(newIt));
-      if (NS_SUCCEEDED(result) && newIt)
+      nsAutoLineIterator newIt = resultFrame->GetLineIterator();
+      if (newIt)
       {
         aPos->mResultFrame = resultFrame;
         return NS_OK;
       }
       //resultFrame is not a block frame
 
       nsCOMPtr<nsIFrameEnumerator> frameTraversal;
       result = NS_NewFrameTraversal(getter_AddRefs(frameTraversal),
@@ -5082,25 +5080,26 @@ nsIFrame::PeekOffset(nsPeekOffsetStruct*
       aPos->mResultFrame = current;
       aPos->mResultContent = range.content;
       // Output offset is relative to content, not frame
       aPos->mContentOffset = offset < 0 ? range.end : range.start + offset;
       break;
     }
     case eSelectLine :
     {
-      nsCOMPtr<nsILineIteratorNavigator> iter; 
+      nsAutoLineIterator iter;
       nsIFrame *blockFrame = this;
 
       while (NS_FAILED(result)){
         PRInt32 thisLine = nsFrame::GetLineNumber(blockFrame, aPos->mScrollViewStop, &blockFrame);
         if (thisLine < 0) 
           return  NS_ERROR_FAILURE;
-        result = blockFrame->QueryInterface(NS_GET_IID(nsILineIteratorNavigator),getter_AddRefs(iter));
-        NS_ASSERTION(NS_SUCCEEDED(result) && iter, "GetLineNumber() succeeded but no block frame?");
+        iter = blockFrame->GetLineIterator();
+        NS_ASSERTION(iter, "GetLineNumber() succeeded but no block frame?");
+        result = NS_OK;
 
         int edgeCase = 0;//no edge case. this should look at thisLine
         
         PRBool doneLooping = PR_FALSE;//tells us when no more block frames hit.
         //this part will find a frame or a block frame. if it's a block frame
         //it will "drill down" to find a viable frame or it will return an error.
         nsIFrame *lastFrame = this;
         do {
@@ -5134,30 +5133,33 @@ nsIFrame::PeekOffset(nsPeekOffsetStruct*
             PRBool searchTableBool = PR_FALSE;
             if (aPos->mResultFrame->GetType() == nsGkAtoms::tableOuterFrame ||
                 aPos->mResultFrame->GetType() == nsGkAtoms::tableCellFrame)
             {
               nsIFrame *frame = aPos->mResultFrame->GetFirstChild(nsnull);
               //got the table frame now
               while(frame) //ok time to drill down to find iterator
               {
-                result = frame->QueryInterface(NS_GET_IID(nsILineIteratorNavigator),
-                                                          getter_AddRefs(iter));
-                if (NS_SUCCEEDED(result))
+                iter = frame->GetLineIterator();
+                if (iter)
                 {
                   aPos->mResultFrame = frame;
                   searchTableBool = PR_TRUE;
+                  result = NS_OK;
                   break; //while(frame)
                 }
+                result = NS_ERROR_FAILURE;
                 frame = frame->GetFirstChild(nsnull);
               }
             }
-            if (!searchTableBool)
-              result = aPos->mResultFrame->QueryInterface(NS_GET_IID(nsILineIteratorNavigator),
-                                                        getter_AddRefs(iter));
+
+            if (!searchTableBool) {
+              iter = aPos->mResultFrame->GetLineIterator();
+              result = iter ? NS_OK : NS_ERROR_FAILURE;
+            }
             if (NS_SUCCEEDED(result) && iter)//we've struck another block element!
             {
               doneLooping = PR_FALSE;
               if (aPos->mDirection == eDirPrevious)
                 edgeCase = 1;//far edge, search from end backwards
               else
                 edgeCase = -1;//near edge search from beginning onwards
               thisLine=0;//this line means nothing now.
@@ -5177,36 +5179,34 @@ nsIFrame::PeekOffset(nsPeekOffsetStruct*
     }
 
     case eSelectParagraph:
       return PeekOffsetParagraph(aPos);
 
     case eSelectBeginLine:
     case eSelectEndLine:
     {
-      nsCOMPtr<nsILineIteratorNavigator> it;
       // Adjusted so that the caret can't get confused when content changes
       nsIFrame* blockFrame = AdjustFrameForSelectionStyles(this);
       PRInt32 thisLine = nsFrame::GetLineNumber(blockFrame, aPos->mScrollViewStop, &blockFrame);
       if (thisLine < 0)
         return NS_ERROR_FAILURE;
-      result = blockFrame->QueryInterface(NS_GET_IID(nsILineIteratorNavigator),getter_AddRefs(it));
-      NS_ASSERTION(NS_SUCCEEDED(result) && it, "GetLineNumber() succeeded but no block frame?");
+      nsAutoLineIterator it = blockFrame->GetLineIterator();
+      NS_ASSERTION(it, "GetLineNumber() succeeded but no block frame?");
 
       PRInt32 lineFrameCount;
       nsIFrame *firstFrame;
       nsRect usedRect;
       PRUint32 lineFlags;
       nsIFrame* baseFrame = nsnull;
       PRBool endOfLine = (eSelectEndLine == aPos->mAmount);
       
 #ifdef IBMBIDI
       if (aPos->mVisual && PresContext()->BidiEnabled()) {
-        PRBool lineIsRTL;
-        it->GetDirection(&lineIsRTL);
+        PRBool lineIsRTL = it->GetDirection();
         PRBool isReordered;
         nsIFrame *lastFrame;
         result = it->CheckLineOrder(thisLine, &isReordered, &firstFrame, &lastFrame);
         baseFrame = endOfLine ? lastFrame : firstFrame;
         if (baseFrame) {
           nsBidiLevel embeddingLevel = nsBidiPresUtils::GetFrameEmbeddingLevel(baseFrame);
           // If the direction of the frame on the edge is opposite to that of the line,
           // we'll need to drill down to its opposite end, so reverse endOfLine.
@@ -5352,17 +5352,17 @@ nsFrame::CheckVisibility(nsPresContext* 
 PRInt32
 nsFrame::GetLineNumber(nsIFrame *aFrame, PRBool aLockScroll, nsIFrame** aContainingBlock)
 {
   NS_ASSERTION(aFrame, "null aFrame");
   nsFrameManager* frameManager = aFrame->PresContext()->FrameManager();
   nsIFrame *blockFrame = aFrame;
   nsIFrame *thisBlock;
   PRInt32   thisLine;
-  nsCOMPtr<nsILineIteratorNavigator> it; 
+  nsAutoLineIterator it;
   nsresult result = NS_ERROR_FAILURE;
   while (NS_FAILED(result) && blockFrame)
   {
     thisBlock = blockFrame;
     if (thisBlock->GetStateBits() & NS_FRAME_OUT_OF_FLOW) {
       //if we are searching for a frame that is not in flow we will not find it. 
       //we must instead look for its placeholder
       if (thisBlock->GetStateBits() & NS_FRAME_IS_OVERFLOW_CONTAINER) {
@@ -5373,63 +5373,61 @@ nsFrame::GetLineNumber(nsIFrame *aFrame,
       if (!thisBlock)
         return -1;
     }  
     blockFrame = thisBlock->GetParent();
     result = NS_OK;
     if (blockFrame) {
       if (aLockScroll && blockFrame->GetType() == nsGkAtoms::scrollFrame)
         return -1;
-      result = blockFrame->QueryInterface(NS_GET_IID(nsILineIteratorNavigator),getter_AddRefs(it));
+      it = blockFrame->GetLineIterator();
     }
   }
   if (!blockFrame || !it)
     return -1;
 
   if (aContainingBlock)
     *aContainingBlock = blockFrame;
-  result = it->FindLineContaining(thisBlock, &thisLine);
-  if (NS_FAILED(result))
-    return -1;
-  return thisLine;
+  return it->FindLineContaining(thisBlock);
 }
 
 nsresult
 nsIFrame::GetFrameFromDirection(nsDirection aDirection, PRBool aVisual,
                                 PRBool aJumpLines, PRBool aScrollViewStop, 
                                 nsIFrame** aOutFrame, PRInt32* aOutOffset, PRBool* aOutJumpedLine)
-{  
+{
+  nsresult result;
+
   if (!aOutFrame || !aOutOffset || !aOutJumpedLine)
     return NS_ERROR_NULL_POINTER;
   
   nsPresContext* presContext = PresContext();
   *aOutFrame = nsnull;
   *aOutOffset = 0;
   *aOutJumpedLine = PR_FALSE;
 
   // Find the prev/next selectable frame
   PRBool selectable = PR_FALSE;
   nsIFrame *traversedFrame = this;
   while (!selectable) {
     nsIFrame *blockFrame;
-    nsCOMPtr<nsILineIteratorNavigator> it; 
     
     PRInt32 thisLine = nsFrame::GetLineNumber(traversedFrame, aScrollViewStop, &blockFrame);
     if (thisLine < 0)
       return NS_ERROR_FAILURE;
-    nsresult result = blockFrame->QueryInterface(NS_GET_IID(nsILineIteratorNavigator),getter_AddRefs(it));
-    NS_ASSERTION(NS_SUCCEEDED(result) && it, "GetLineNumber() succeeded but no block frame?");
+
+    nsAutoLineIterator it = blockFrame->GetLineIterator();
+    NS_ASSERTION(it, "GetLineNumber() succeeded but no block frame?");
 
     PRBool atLineEdge;
     nsIFrame *firstFrame;
     nsIFrame *lastFrame;
 #ifdef IBMBIDI
     if (aVisual && presContext->BidiEnabled()) {
-      PRBool lineIsRTL;                                                             
-      it->GetDirection(&lineIsRTL);
+      PRBool lineIsRTL = it->GetDirection();
       PRBool isReordered;
       result = it->CheckLineOrder(thisLine, &isReordered, &firstFrame, &lastFrame);
       nsIFrame** framePtr = aDirection == eDirPrevious ? &firstFrame : &lastFrame;
       if (*framePtr) {
         nsBidiLevel embeddingLevel = nsBidiPresUtils::GetFrameEmbeddingLevel(*framePtr);
         if ((((embeddingLevel & 1) && lineIsRTL) || (!(embeddingLevel & 1) && !lineIsRTL)) ==
             (aDirection == eDirPrevious)) {
           nsFrame::GetFirstLeaf(presContext, framePtr);
@@ -6207,17 +6205,17 @@ nsFrame::RefreshSizeCache(nsBoxLayoutSta
       newRect.x = 0;
       newRect.y = 0;
       Redraw(aState, &newRect);
     }
 
     metrics->mBlockMinSize.height = 0;
     // ok we need the max ascent of the items on the line. So to do this
     // ask the block for its line iterator. Get the max ascent.
-    nsCOMPtr<nsILineIterator> lines = do_QueryInterface(static_cast<nsIFrame*>(this));
+    nsAutoLineIterator lines = GetLineIterator();
     if (lines) 
     {
       metrics->mBlockMinSize.height = 0;
       int count = 0;
       nsIFrame* firstFrame = nsnull;
       PRInt32 framesOnLine;
       nsRect lineBounds;
       PRUint32 lineFlags;
@@ -6250,16 +6248,22 @@ nsFrame::RefreshSizeCache(nsBoxLayoutSta
                                                      metrics->mBlockPrefSize.height,
                                                      metrics->mBlockAscent);
 #endif
   }
 
   return rv;
 }
 
+/* virtual */ nsILineIterator*
+nsFrame::GetLineIterator()
+{
+  return nsnull;
+}
+
 nsSize
 nsFrame::GetPrefSize(nsBoxLayoutState& aState)
 {
   nsSize size(0,0);
   DISPLAY_PREF_SIZE(this, size);
   // If the size is cached, and there are no HTML constraints that we might
   // be depending on, then we just return the cached size.
   nsBoxLayoutMetrics *metrics = BoxMetrics();
--- a/layout/generic/nsFrame.h
+++ b/layout/generic/nsFrame.h
@@ -616,16 +616,18 @@ private:
                      nscoord aX,
                      nscoord aY,
                      nscoord aWidth,
                      nscoord aHeight,
                      PRBool aMoveFrame = PR_TRUE);
 
   NS_IMETHODIMP RefreshSizeCache(nsBoxLayoutState& aState);
 
+  virtual nsILineIterator* GetLineIterator();
+
 protected:
   NS_IMETHOD_(nsrefcnt) AddRef(void);
   NS_IMETHOD_(nsrefcnt) Release(void);
 };
 
 // Start Display Reflow Debugging
 #ifdef DEBUG
 
--- a/layout/generic/nsFrameList.cpp
+++ b/layout/generic/nsFrameList.cpp
@@ -422,30 +422,28 @@ nsFrameList::List(FILE* out) const
   fputs(">\n", out);
 }
 #endif
 
 #ifdef IBMBIDI
 nsIFrame*
 nsFrameList::GetPrevVisualFor(nsIFrame* aFrame) const
 {
-  nsCOMPtr<nsILineIterator> iter;
-
   if (!mFirstChild)
     return nsnull;
   
   nsIFrame* parent = mFirstChild->GetParent();
   if (!parent)
     return aFrame ? GetPrevSiblingFor(aFrame) : LastChild();
 
   nsBidiLevel baseLevel = nsBidiPresUtils::GetFrameBaseLevel(mFirstChild);  
   nsBidiPresUtils* bidiUtils = mFirstChild->PresContext()->GetBidiUtils();
 
-  nsresult result = parent->QueryInterface(NS_GET_IID(nsILineIterator), getter_AddRefs(iter));
-  if (NS_FAILED(result) || !iter) { 
+  nsAutoLineIterator iter = parent->GetLineIterator();
+  if (!iter) { 
     // Parent is not a block Frame
     if (parent->GetType() == nsGkAtoms::lineFrame) {
       // Line frames are not bidi-splittable, so need to consider bidi reordering
       if (baseLevel == NSBIDI_LTR) {
         return bidiUtils->GetFrameToLeftOf(aFrame, mFirstChild, -1);
       } else { // RTL
         return bidiUtils->GetFrameToRightOf(aFrame, mFirstChild, -1);
       }
@@ -460,21 +458,21 @@ nsFrameList::GetPrevVisualFor(nsIFrame* 
     }
   }
 
   // Parent is a block frame, so use the LineIterator to find the previous visual 
   // sibling on this line, or the last one on the previous line.
 
   PRInt32 thisLine;
   if (aFrame) {
-    result = iter->FindLineContaining(aFrame, &thisLine);
-    if (NS_FAILED(result) || thisLine < 0)
+    thisLine = iter->FindLineContaining(aFrame);
+    if (thisLine < 0)
       return nsnull;
   } else {
-    iter->GetNumLines(&thisLine);
+    thisLine = iter->GetNumLines();
   }
 
   nsIFrame* frame = nsnull;
   nsIFrame* firstFrameOnLine;
   PRInt32 numFramesOnLine;
   nsRect lineBounds;
   PRUint32 lineFlags;
 
@@ -499,30 +497,28 @@ nsFrameList::GetPrevVisualFor(nsIFrame* 
     }
   }
   return frame;
 }
 
 nsIFrame*
 nsFrameList::GetNextVisualFor(nsIFrame* aFrame) const
 {
-  nsCOMPtr<nsILineIterator> iter;
-
   if (!mFirstChild)
     return nsnull;
   
   nsIFrame* parent = mFirstChild->GetParent();
   if (!parent)
     return aFrame ? GetPrevSiblingFor(aFrame) : mFirstChild;
 
   nsBidiLevel baseLevel = nsBidiPresUtils::GetFrameBaseLevel(mFirstChild);
   nsBidiPresUtils* bidiUtils = mFirstChild->PresContext()->GetBidiUtils();
   
-  nsresult result = parent->QueryInterface(NS_GET_IID(nsILineIterator), getter_AddRefs(iter));
-  if (NS_FAILED(result) || !iter) { 
+  nsAutoLineIterator iter = parent->GetLineIterator();
+  if (!iter) { 
     // Parent is not a block Frame
     if (parent->GetType() == nsGkAtoms::lineFrame) {
       // Line frames are not bidi-splittable, so need to consider bidi reordering
       if (baseLevel == NSBIDI_LTR) {
         return bidiUtils->GetFrameToRightOf(aFrame, mFirstChild, -1);
       } else { // RTL
         return bidiUtils->GetFrameToLeftOf(aFrame, mFirstChild, -1);
       }
@@ -537,18 +533,18 @@ nsFrameList::GetNextVisualFor(nsIFrame* 
     }
   }
 
   // Parent is a block frame, so use the LineIterator to find the next visual 
   // sibling on this line, or the first one on the next line.
   
   PRInt32 thisLine;
   if (aFrame) {
-    result = iter->FindLineContaining(aFrame, &thisLine);
-    if (NS_FAILED(result) || thisLine < 0)
+    thisLine = iter->FindLineContaining(aFrame);
+    if (thisLine < 0)
       return nsnull;
   } else {
     thisLine = -1;
   }
 
   nsIFrame* frame = nsnull;
   nsIFrame* firstFrameOnLine;
   PRInt32 numFramesOnLine;
@@ -560,18 +556,17 @@ nsFrameList::GetNextVisualFor(nsIFrame* 
     
     if (baseLevel == NSBIDI_LTR) {
       frame = bidiUtils->GetFrameToRightOf(aFrame, firstFrameOnLine, numFramesOnLine);
     } else { // RTL
       frame = bidiUtils->GetFrameToLeftOf(aFrame, firstFrameOnLine, numFramesOnLine);
     }
   }
   
-  PRInt32 numLines;
-  iter->GetNumLines(&numLines);
+  PRInt32 numLines = iter->GetNumLines();
   if (!frame && thisLine < numLines - 1) {
     // Get the first frame of the next line
     iter->GetLine(thisLine + 1, &firstFrameOnLine, &numFramesOnLine, lineBounds, &lineFlags);
     
     if (baseLevel == NSBIDI_LTR) {
       frame = bidiUtils->GetFrameToRightOf(nsnull, firstFrameOnLine, numFramesOnLine);
     } else { // RTL
       frame = bidiUtils->GetFrameToLeftOf(nsnull, firstFrameOnLine, numFramesOnLine);
--- a/layout/generic/nsIFrame.h
+++ b/layout/generic/nsIFrame.h
@@ -80,16 +80,17 @@ class nsPresContext;
 class nsIPresShell;
 class nsIRenderingContext;
 class nsIView;
 class nsIWidget;
 class nsIDOMRange;
 class nsISelectionController;
 class nsBoxLayoutState;
 class nsIBoxLayout;
+class nsILineIterator;
 #ifdef ACCESSIBILITY
 class nsIAccessible;
 #endif
 class nsDisplayListBuilder;
 class nsDisplayListSet;
 class nsDisplayList;
 class gfxSkipChars;
 class gfxSkipCharsIterator;
@@ -2219,16 +2220,24 @@ NS_PTR_TO_INT32(frame->GetProperty(nsGkA
    * GetOverflowRect() of the frame!  Note that it's safe to assume in this
    * method that the frame origin didn't change.  If it did, whoever moved the
    * frame will invalidate as needed anyway.
    */
   void CheckInvalidateSizeChange(const nsRect& aOldRect,
                                  const nsRect& aOldOverflowRect,
                                  const nsSize& aNewDesiredSize);
 
+  /**
+   * Get a line iterator for this frame, if supported.
+   *
+   * @return nsnull if no line iterator is supported.
+   * @note dispose the line iterator using nsILineIterator::DisposeLineIterator
+   */
+  virtual nsILineIterator* GetLineIterator() = 0;
+
 protected:
   // Members
   nsRect           mRect;
   nsIContent*      mContent;
   nsStyleContext*  mStyleContext;
   nsIFrame*        mParent;
   nsIFrame*        mNextSibling;  // singly-linked list of frames
   nsFrameState     mState;
--- a/layout/generic/nsILineIterator.h
+++ b/layout/generic/nsILineIterator.h
@@ -32,57 +32,61 @@
  * and other provisions required by the GPL or the LGPL. If you do not delete
  * the provisions above, a recipient may use your version of this file under
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 #ifndef nsILineIterator_h___
 #define nsILineIterator_h___
 
-#include "nsISupports.h"
-
-/* a6cf90ff-15b3-11d2-932e-00805f8add32 */
-#define NS_ILINE_ITERATOR_IID \
- { 0xa6cf90ff, 0x15b3, 0x11d2,{0x93, 0x2e, 0x00, 0x80, 0x5f, 0x8a, 0xdd, 0x32}}
-
-/* {80AA3D7A-E0BF-4e18-8A82-2110397D7BC4}*/
-#define NS_ILINE_ITERATOR_NAV_IID \
- { 0x80aa3d7a, 0xe0bf, 0x4e18,{0x8a, 0x82, 0x21, 0x10, 0x39, 0x7d, 0x7b, 0xc4}}
+#include "nscore.h"
+#include "nsCoord.h"
 
-// Line iterator API.
-//
-// Lines are numbered from 0 to N, where 0 is the top line and N is
-// the bottom line.
-//
-// NOTE: while you can get this interface by doing a slezy hacky
-// QueryInterface on block frames, it isn't like a normal com
-// interface: it's not reflexive (you can't query back to the block
-// frame) and unlike other frames, it *IS* reference counted so don't
-// forget to NS_RELEASE it when you are done with it!
+class nsIFrame;
+struct nsRect;
 
 // Line Flags (see GetLine below)
 
 // This bit is set when the line is wrapping up a block frame. When
 // clear, it means that the line contains inline elements.
 #define NS_LINE_FLAG_IS_BLOCK           0x1
 
 // This bit is set when the line ends in some sort of break.
 #define NS_LINE_FLAG_ENDS_IN_BREAK      0x4
 
-class nsILineIterator : public nsISupports {
-public:
-  NS_DECLARE_STATIC_IID_ACCESSOR(NS_ILINE_ITERATOR_IID)
+/**
+ * Line iterator API.
+ *
+ * Lines are numbered from 0 to N, where 0 is the top line and N is
+ * the bottom line.
+ *
+ * Obtain this interface from frames via nsIFrame::GetLineIterator.
+ * When you are finished using the iterator, call DisposeLineIterator()
+ * to destroy the iterator if appropriate.
+ */
+class nsILineIterator
+{
+protected:
+  ~nsILineIterator() { }
 
-  // Return the number of lines in the block.
-  NS_IMETHOD GetNumLines(PRInt32* aResult) = 0;
+public:
+  virtual void DisposeLineIterator() = 0;
+
+  /**
+   * The number of lines in the block
+   */
+  virtual PRInt32 GetNumLines() = 0;
 
-  // Return the prevailing direction for the line. aIsRightToLeft will
-  // be set to PR_TRUE if the CSS direction property for the block is
-  // "rtl", otherwise aIsRightToLeft will be set to PR_FALSE.
-  NS_IMETHOD GetDirection(PRBool* aIsRightToLeft) = 0;
+  /**
+   * The prevailing direction of lines.
+   *
+   * @return PR_TRUE if the CSS direction property for the block is
+   *         "rtl", otherwise PR_FALSE
+   */
+  virtual PRBool GetDirection() = 0;
 
   // Return structural information about a line. aFirstFrameOnLine is
   // the first frame on the line and aNumFramesOnLine is the number of
   // frames that are on the line. If the line-number is invalid then
   // aFirstFrameOnLine will be nsnull and aNumFramesOnLine will be
   // zero.
   //
   // For valid line numbers, aLineBounds is set to the bounding box of
@@ -93,29 +97,30 @@ public:
   // In addition, aLineFlags will contain flag information about the
   // line.
   NS_IMETHOD GetLine(PRInt32 aLineNumber,
                      nsIFrame** aFirstFrameOnLine,
                      PRInt32* aNumFramesOnLine,
                      nsRect& aLineBounds,
                      PRUint32* aLineFlags) = 0;
 
-  // Given a frame that's a child of the block, find which line its on
-  // and return that line index into aIndexResult. aIndexResult will
-  // be set to -1 if the frame cannot be found.
-  NS_IMETHOD FindLineContaining(nsIFrame* aFrame,
-                                PRInt32* aLineNumberResult) = 0;
+  /**
+   * Given a frame that's a child of the block, find which line its on
+   * and return that line index. Returns -1 if the frame cannot be found.
+   */
+  virtual PRInt32 FindLineContaining(nsIFrame* aFrame) = 0;
 
-  // Given a Y coordinate relative to the block that provided this
-  // line iterator, find the line that contains the Y
-  // coordinate. Returns -1 in aLineNumberResult if the Y coordinate
-  // is above the first line. Returns N (where N is the number of
-  // lines) if the Y coordinate is below the last line.
-  NS_IMETHOD FindLineAt(nscoord aY,
-                        PRInt32* aLineNumberResult) = 0;
+  /**
+   * Given a Y coordinate relative to the block that provided this
+   * line iterator, return the line that contains the Y
+   * coordinate. Returns -1 in aLineNumberResult if the Y coordinate
+   *  is above the first line. Returns N (where N is the number of
+   * lines) if the Y coordinate is below the last line.
+   */
+  virtual PRInt32 FindLineAt(nscoord aY) = 0;
 
   // Given a line number and an X coordinate, find the frame on the
   // line that is nearest to the X coordinate. The
   // aXIsBeforeFirstFrame and aXIsAfterLastFrame flags are updated
   // appropriately.
   NS_IMETHOD FindFrameAt(PRInt32 aLineNumber,
                          nscoord aX,
                          nsIFrame** aFrameFound,
@@ -131,20 +136,35 @@ public:
   //  If not, return the first and last visual frames
   NS_IMETHOD CheckLineOrder(PRInt32                  aLine,
                             PRBool                   *aIsReordered,
                             nsIFrame                 **aFirstVisual,
                             nsIFrame                 **aLastVisual) = 0;
 #endif
 };
 
-NS_DEFINE_STATIC_IID_ACCESSOR(nsILineIterator, NS_ILINE_ITERATOR_IID)
+class nsAutoLineIterator
+{
+public:
+  nsAutoLineIterator() : mRawPtr(nsnull) { }
+  nsAutoLineIterator(nsILineIterator *i) : mRawPtr(i) { }
+
+  ~nsAutoLineIterator() {
+    if (mRawPtr)
+      mRawPtr->DisposeLineIterator();
+  }
 
-//special line iterator for keyboard navigation
-class nsILineIteratorNavigator : public nsILineIterator {
-public:
-  NS_DECLARE_STATIC_IID_ACCESSOR(NS_ILINE_ITERATOR_NAV_IID)
+  operator nsILineIterator*() { return mRawPtr; }
+  nsILineIterator* operator->() { return mRawPtr; }
+
+  nsILineIterator* operator=(nsILineIterator* i) {
+    if (mRawPtr)
+      mRawPtr->DisposeLineIterator();
+
+    mRawPtr = i;
+    return i;
+  }
+
+private:
+  nsILineIterator* mRawPtr;
 };
 
-NS_DEFINE_STATIC_IID_ACCESSOR(nsILineIteratorNavigator,
-                              NS_ILINE_ITERATOR_NAV_IID)
-
 #endif /* nsILineIterator_h___ */
--- a/layout/generic/nsLineBox.cpp
+++ b/layout/generic/nsLineBox.cpp
@@ -538,17 +538,21 @@ nsLineIterator::nsLineIterator()
 
 nsLineIterator::~nsLineIterator()
 {
   if (mLines != gDummyLines) {
     delete [] mLines;
   }
 }
 
-NS_IMPL_ISUPPORTS2(nsLineIterator, nsILineIterator, nsILineIteratorNavigator)
+/* virtual */ void
+nsLineIterator::DisposeLineIterator()
+{
+  delete this;
+}
 
 nsresult
 nsLineIterator::Init(nsLineList& aLines, PRBool aRightToLeft)
 {
   mRightToLeft = aRightToLeft;
 
   // Count the lines
   PRInt32 numLines = aLines.size();
@@ -573,36 +577,26 @@ nsLineIterator::Init(nsLineList& aLines,
        ++line)
   {
     *lp++ = line;
   }
   mNumLines = numLines;
   return NS_OK;
 }
 
-NS_IMETHODIMP
-nsLineIterator::GetNumLines(PRInt32* aResult)
+PRInt32
+nsLineIterator::GetNumLines()
 {
-  NS_PRECONDITION(aResult, "null OUT ptr");
-  if (!aResult) {
-    return NS_ERROR_NULL_POINTER;
-  }
-  *aResult = mNumLines;
-  return NS_OK;
+  return mNumLines;
 }
 
-NS_IMETHODIMP
-nsLineIterator::GetDirection(PRBool* aIsRightToLeft)
+PRBool
+nsLineIterator::GetDirection()
 {
-  NS_PRECONDITION(aIsRightToLeft, "null OUT ptr");
-  if (!aIsRightToLeft) {
-    return NS_ERROR_NULL_POINTER;
-  }
-  *aIsRightToLeft = mRightToLeft;
-  return NS_OK;
+  return mRightToLeft;
 }
 
 NS_IMETHODIMP
 nsLineIterator::GetLine(PRInt32 aLineNumber,
                         nsIFrame** aFirstFrameOnLine,
                         PRInt32* aNumFramesOnLine,
                         nsRect& aLineBounds,
                         PRUint32* aLineFlags)
@@ -630,52 +624,45 @@ nsLineIterator::GetLine(PRInt32 aLineNum
     if (line->HasBreakAfter())
       flags |= NS_LINE_FLAG_ENDS_IN_BREAK;
   }
   *aLineFlags = flags;
 
   return NS_OK;
 }
 
-NS_IMETHODIMP
-nsLineIterator::FindLineContaining(nsIFrame* aFrame,
-                                   PRInt32* aLineNumberResult)
+PRInt32
+nsLineIterator::FindLineContaining(nsIFrame* aFrame)
 {
   nsLineBox* line = mLines[0];
   PRInt32 lineNumber = 0;
   while (lineNumber != mNumLines) {
     if (line->Contains(aFrame)) {
-      *aLineNumberResult = lineNumber;
-      return NS_OK;
+      return lineNumber;
     }
     line = mLines[++lineNumber];
   }
-  *aLineNumberResult = -1;
-  return NS_OK;
+  return -1;
 }
 
-NS_IMETHODIMP
-nsLineIterator::FindLineAt(nscoord aY,
-                           PRInt32* aLineNumberResult)
+/* virtual */ PRInt32
+nsLineIterator::FindLineAt(nscoord aY)
 {
   nsLineBox* line = mLines[0];
   if (!line || (aY < line->mBounds.y)) {
-    *aLineNumberResult = -1;
-    return NS_OK;
+    return -1;
   }
   PRInt32 lineNumber = 0;
   while (lineNumber != mNumLines) {
     if ((aY >= line->mBounds.y) && (aY < line->mBounds.YMost())) {
-      *aLineNumberResult = lineNumber;
-      return NS_OK;
+      return lineNumber;
     }
     line = mLines[++lineNumber];
   }
-  *aLineNumberResult = mNumLines;
-  return NS_OK;
+  return mNumLines;
 }
 
 #ifdef IBMBIDI
 NS_IMETHODIMP
 nsLineIterator::CheckLineOrder(PRInt32                  aLine,
                                PRBool                   *aIsReordered,
                                nsIFrame                 **aFirstVisual,
                                nsIFrame                 **aLastVisual)
--- a/layout/generic/nsLineBox.h
+++ b/layout/generic/nsLineBox.h
@@ -1503,58 +1503,49 @@ nsLineList_const_reverse_iterator&
 nsLineList_const_reverse_iterator::operator=(const nsLineList_const_reverse_iterator& aOther)
 {
   ASSIGN_FROM(aOther)
 }
 
 
 //----------------------------------------------------------------------
 
-class nsLineIterator : public nsILineIteratorNavigator {
+class NS_FINAL_CLASS nsLineIterator : public nsILineIterator
+{
 public:
   nsLineIterator();
-  virtual ~nsLineIterator();
+  ~nsLineIterator();
 
-  NS_DECL_ISUPPORTS
+  virtual void DisposeLineIterator();
 
-  NS_IMETHOD GetNumLines(PRInt32* aResult);
-  NS_IMETHOD GetDirection(PRBool* aIsRightToLeft);
+  virtual PRInt32 GetNumLines();
+  virtual PRBool GetDirection();
   NS_IMETHOD GetLine(PRInt32 aLineNumber,
                      nsIFrame** aFirstFrameOnLine,
                      PRInt32* aNumFramesOnLine,
                      nsRect& aLineBounds,
                      PRUint32* aLineFlags);
-  NS_IMETHOD FindLineContaining(nsIFrame* aFrame,
-                                PRInt32* aLineNumberResult);
-  NS_IMETHOD FindLineAt(nscoord aY,
-                        PRInt32* aLineNumberResult);
+  virtual PRInt32 FindLineContaining(nsIFrame* aFrame);
+  virtual PRInt32 FindLineAt(nscoord aY);
   NS_IMETHOD FindFrameAt(PRInt32 aLineNumber,
                          nscoord aX,
                          nsIFrame** aFrameFound,
                          PRBool* aXIsBeforeFirstFrame,
                          PRBool* aXIsAfterLastFrame);
 
   NS_IMETHOD GetNextSiblingOnLine(nsIFrame*& aFrame, PRInt32 aLineNumber);
 #ifdef IBMBIDI
   NS_IMETHOD CheckLineOrder(PRInt32                  aLine,
                             PRBool                   *aIsReordered,
                             nsIFrame                 **aFirstVisual,
                             nsIFrame                 **aLastVisual);
 #endif
   nsresult Init(nsLineList& aLines, PRBool aRightToLeft);
 
-protected:
-  PRInt32 NumLines() const {
-    return mNumLines;
-  }
-
-  nsLineBox* CurrentLine() {
-    return mLines[mIndex];
-  }
-
+private:
   nsLineBox* PrevLine() {
     if (0 == mIndex) {
       return nsnull;
     }
     return mLines[--mIndex];
   }
 
   nsLineBox* NextLine() {
--- a/layout/tables/nsTableRowGroupFrame.cpp
+++ b/layout/tables/nsTableRowGroupFrame.cpp
@@ -59,45 +59,26 @@ nsTableRowGroupFrame::nsTableRowGroupFra
 {
   SetRepeatable(PR_FALSE);
 }
 
 nsTableRowGroupFrame::~nsTableRowGroupFrame()
 {
 }
 
-/* ----------- nsTableRowGroupFrame ---------- */
-nsrefcnt nsTableRowGroupFrame::AddRef(void)
-{
-  return 1;//implementation of nsLineIterator
-}
-
-nsrefcnt nsTableRowGroupFrame::Release(void)
-{
-  return 1;//implementation of nsLineIterator
-}
-
 NS_IMETHODIMP
 nsTableRowGroupFrame::QueryInterface(const nsIID& aIID, void** aInstancePtr)
 {
   NS_PRECONDITION(aInstancePtr, "null out param");
 
   static NS_DEFINE_IID(kITableRowGroupIID, NS_ITABLEROWGROUPFRAME_IID);
   if (aIID.Equals(kITableRowGroupIID)) {
     *aInstancePtr = (void*)this;
     return NS_OK;
   }
-  if (aIID.Equals(NS_GET_IID(nsILineIteratorNavigator))) {
-    *aInstancePtr = static_cast<nsILineIteratorNavigator*>(this);
-    return NS_OK;
-  }
-  if (aIID.Equals(NS_GET_IID(nsILineIterator))) {
-    *aInstancePtr = static_cast<nsILineIterator*>(this);
-    return NS_OK;
-  }
 
   return nsHTMLContainerFrame::QueryInterface(aIID, aInstancePtr);
 }
 
 /* virtual */ PRBool
 nsTableRowGroupFrame::IsContainingBlock() const
 {
   return PR_TRUE;
@@ -1648,33 +1629,28 @@ void nsTableRowGroupFrame::SetContinuous
       mLeftContBorderWidth = aPixelValue;
       return;
     default:
       NS_ERROR("invalid NS_SIDE argument");
   }
 }
 
 //nsILineIterator methods
-NS_IMETHODIMP
-nsTableRowGroupFrame::GetNumLines(PRInt32* aResult)
+PRInt32
+nsTableRowGroupFrame::GetNumLines()
 {
-  NS_ENSURE_ARG_POINTER(aResult);
-  *aResult = GetRowCount();
-  return NS_OK;
+  return GetRowCount();
 }
 
-NS_IMETHODIMP
-nsTableRowGroupFrame::GetDirection(PRBool* aIsRightToLeft)
+PRBool
+nsTableRowGroupFrame::GetDirection()
 {
-  NS_ENSURE_ARG_POINTER(aIsRightToLeft);
-  // rtl is table wide @see nsTableIterator
   nsTableFrame* table = nsTableFrame::GetTableFrame(this);
-  *aIsRightToLeft = (NS_STYLE_DIRECTION_RTL ==
-                     table->GetStyleVisibility()->mDirection);
-  return NS_OK;
+  return (NS_STYLE_DIRECTION_RTL ==
+          table->GetStyleVisibility()->mDirection);
 }
   
 NS_IMETHODIMP
 nsTableRowGroupFrame::GetLine(PRInt32    aLineNumber, 
                               nsIFrame** aFirstFrameOnLine, 
                               PRInt32*   aNumFramesOnLine,
                               nsRect&    aLineBounds, 
                               PRUint32*  aLineFlags)
@@ -1709,38 +1685,35 @@ nsTableRowGroupFrame::GetLine(PRInt32   
       aLineBounds = parent->GetRect();
       return NS_OK;
     }
   }
   NS_ERROR("cellmap is lying");
   return NS_ERROR_FAILURE;
 }
   
-NS_IMETHODIMP
-nsTableRowGroupFrame::FindLineContaining(nsIFrame* aFrame, 
-                                         PRInt32*  aLineNumberResult)
+PRInt32
+nsTableRowGroupFrame::FindLineContaining(nsIFrame* aFrame)
 {
   NS_ENSURE_ARG_POINTER(aFrame);
-  NS_ENSURE_ARG_POINTER(aLineNumberResult);
   
   NS_ASSERTION((aFrame->GetType() == nsGkAtoms::tableRowFrame),
                "RowGroup contains a frame that is not a row");
 
   nsTableRowFrame* rowFrame = (nsTableRowFrame*)aFrame;
-  *aLineNumberResult = rowFrame->GetRowIndex() - GetStartRowIndex();
-
-  return NS_OK;
+  return rowFrame->GetRowIndex() - GetStartRowIndex();
 }
 
-NS_IMETHODIMP
-nsTableRowGroupFrame::FindLineAt(nscoord  aY, 
-                                 PRInt32* aLineNumberResult)
+PRInt32
+nsTableRowGroupFrame::FindLineAt(nscoord  aY)
 {
+  NS_NOTREACHED("Not implemented");
   return NS_ERROR_NOT_IMPLEMENTED;
 }
+
 #ifdef IBMBIDI
 NS_IMETHODIMP
 nsTableRowGroupFrame::CheckLineOrder(PRInt32                  aLine,
                                      PRBool                   *aIsReordered,
                                      nsIFrame                 **aFirstVisual,
                                      nsIFrame                 **aLastVisual)
 {
   *aIsReordered = PR_FALSE;
--- a/layout/tables/nsTableRowGroupFrame.h
+++ b/layout/tables/nsTableRowGroupFrame.h
@@ -91,21 +91,22 @@ struct nsRowGroupReflowState {
  * nsTableRowGroupFrame is the frame that maps row groups 
  * (HTML tags THEAD, TFOOT, and TBODY). This class cannot be reused
  * outside of an nsTableFrame.  It assumes that its parent is an nsTableFrame, and 
  * its children are nsTableRowFrames.
  * 
  * @see nsTableFrame
  * @see nsTableRowFrame
  */
-class nsTableRowGroupFrame : public nsHTMLContainerFrame, public nsILineIteratorNavigator
+class nsTableRowGroupFrame
+  : public nsHTMLContainerFrame
+  , public nsILineIterator
 {
 public:
-  // nsISupports
-  NS_DECL_ISUPPORTS_INHERITED
+  NS_IMETHOD QueryInterface(const nsIID &aIID, void **aInstancePtr);
 
   /** instantiate a new instance of nsTableRowFrame.
     * @param aPresShell the pres shell for this frame
     *
     * @return           the frame that was created
     */
   friend nsIFrame* NS_NewTableRowGroupFrame(nsIPresShell* aPresShell, nsStyleContext* aContext);
   virtual ~nsTableRowGroupFrame();
@@ -222,32 +223,34 @@ public:
     * @param aYTotalOffset the total amount that the rowgroup is shifted up
     * @param aWidth        new width of the rowgroup
     */
   nscoord CollapseRowGroupIfNecessary(nscoord aYTotalOffset,
                                       nscoord aWidth);
 
 // nsILineIterator methods
 public:
+  virtual void DisposeLineIterator() { }
+
   // The table row is the equivalent to a line in block layout. 
   // The nsILineIterator assumes that a line resides in a block, this role is
   // fullfilled by the row group. Rows in table are counted relative to the
   // table. The row index of row corresponds to the cellmap coordinates. The
   // line index with respect to a row group can be computed by substracting the
   // row index of the first row in the row group.
    
   /** Get the number of rows in a row group
-    * @param aResult - pointer that holds the number of lines in a row group
+    * @return the number of lines in a row group
     */
-  NS_IMETHOD GetNumLines(PRInt32* aResult);
+  virtual PRInt32 GetNumLines();
 
   /** @see nsILineIterator.h GetDirection
-    * @param aIsRightToLeft - true if the table is rtl
+    * @return true if the table is rtl
     */
-  NS_IMETHOD GetDirection(PRBool* aIsRightToLeft);
+  virtual PRBool GetDirection();
   
   /** Return structural information about a line. 
     * @param aLineNumber       - the index of the row relative to the row group
     *                            If the line-number is invalid then
     *                            aFirstFrameOnLine will be nsnull and 
     *                            aNumFramesOnLine will be zero.
     * @param aFirstFrameOnLine - the first cell frame that originates in row
     *                            with a rowindex that matches a line number
@@ -259,26 +262,25 @@ public:
   NS_IMETHOD GetLine(PRInt32 aLineNumber,
                      nsIFrame** aFirstFrameOnLine,
                      PRInt32* aNumFramesOnLine,
                      nsRect& aLineBounds,
                      PRUint32* aLineFlags);
   
   /** Given a frame that's a child of the rowgroup, find which line its on.
     * @param aFrame       - frame, should be a row
-    * @param aIndexResult - row index relative to the row group if this a row
-    *                       frame. aIndexResult will be set to -1 if the frame
-    *                       cannot be found.
+    * @return               row index relative to the row group if this a row
+    *                       frame. -1 if the frame cannot be found.
     */
-  NS_IMETHOD FindLineContaining(nsIFrame* aFrame, PRInt32* aLineNumberResult);
+  virtual PRInt32 FindLineContaining(nsIFrame* aFrame);
   
   /** not implemented
     * the function is also not called in our tree
     */
-  NS_IMETHOD FindLineAt(nscoord aY, PRInt32* aLineNumberResult);
+  virtual PRInt32 FindLineAt(nscoord aY);
 
   /** Find the orginating cell frame on a row that is the nearest to the
     * coordinate X.
     * @param aLineNumber          - the index of the row relative to the row group
     * @param aX                   - X coordinate in twips relative to the
     *                               origin of the row group
     * @param aFrameFound          - pointer to the cellframe
     * @param aXIsBeforeFirstFrame - the point is before the first originating
@@ -368,16 +370,18 @@ public:
   
   PRBool IsScrolled() {
     // Note that if mOverflowY is CLIP, so is mOverflowX, and we need to clip the background
     // as if the rowgroup is scrollable.
     return GetStyleContext()->GetPseudoType() == nsCSSAnonBoxes::scrolledContent ||
            GetStyleDisplay()->mOverflowY == NS_STYLE_OVERFLOW_CLIP;
   }
 
+  virtual nsILineIterator* GetLineIterator() { return this; }
+
 protected:
   nsTableRowGroupFrame(nsStyleContext* aContext);
 
   void InitChildReflowState(nsPresContext&     aPresContext, 
                             PRBool             aBorderCollapse,
                             nsHTMLReflowState& aReflowState);
   
   /** implement abstract method on nsHTMLContainerFrame */