Fix crash bug 368166. r=bernd, sr=roc

layout/tables/nsCellMap.cpp

@@ -397,7 +397,8 @@ nsTableCellMap::GetEffectiveRowSpan(PRInt32 aRowIndex,
     rowIndex -= map->GetRowCount();
     map = map->GetNextSibling();
   }
-  return nsnull;
+  NS_NOTREACHED("Bogus row index?");
+  return 0;
 }
 
 PRInt32 
@@ -414,7 +415,8 @@ nsTableCellMap::GetEffectiveColSpan(PRInt32 aRowIndex,
     rowIndex -= map->GetRowCount();
     map = map->GetNextSibling();
   }
-  return nsnull;
+  NS_NOTREACHED("Bogus row index?");
+  return 0;
 }
 
 nsTableCellFrame* 
@@ -2704,16 +2706,27 @@ CellData* nsCellMap::AllocCellData(nsTableCellFrame* aOrigCell)
 void
 nsCellMapColumnIterator::AdvanceRowGroup()
 {
-  // Set mCurMapContentRowCount to 0 in case mCurMap has no next sibling.  This
-  // can happen if we just handled the last originating cell.  Future calls
-  // will end up with mFoundCells == mOrigCells, but for this one mFoundCells
-  // was definitely not big enough if we got here.
-  mCurMapContentRowCount = 0;
   do {
+    mCurMapStart += mCurMapContentRowCount;  
     mCurMap = mCurMap->GetNextSibling();
-  } while (mCurMap && 0 == (mCurMapContentRowCount = mCurMap->GetRowCount()));
+    if (!mCurMap) {
+      // Set mCurMapContentRowCount and mCurMapRelevantRowCount to 0 in case
+      // mCurMap has no next sibling.  This can happen if we just handled the
+      // last originating cell.  Future calls will end up with mFoundCells ==
+      // mOrigCells, but for this one mFoundCells was definitely not big enough
+      // if we got here.
+      mCurMapContentRowCount = 0;
+      mCurMapRelevantRowCount = 0;
+      break;
+    }
 
-  NS_ASSERTION(mCurMapContentRowCount != 0 || !mCurMap, "How did that happen?");
+    mCurMapContentRowCount = mCurMap->GetRowCount();
+    PRUint32 rowArrayLength = mCurMap->mRows.Length();
+    mCurMapRelevantRowCount = PR_MIN(mCurMapContentRowCount, rowArrayLength);
+  } while (0 == mCurMapRelevantRowCount);
+
+  NS_ASSERTION(mCurMapRelevantRowCount != 0 || !mCurMap,
+               "How did that happen?");
 
   // Set mCurMapRow to 0, since cells can't span across table row groups.
   mCurMapRow = 0;
@@ -2725,24 +2738,11 @@ nsCellMapColumnIterator::IncrementRow(PRInt32 aIncrement)
   NS_PRECONDITION(aIncrement >= 0, "Bogus increment");
   NS_PRECONDITION(mCurMap, "Bogus mOrigCells?");
   if (aIncrement == 0) {
-    // This will span to the end of the row group.  Note that
-    // mCurMapContentRowCount only includes content rows, so we can just
-    // increment by what's left of it.
-    mRow += (mCurMapContentRowCount - mCurMapRow);
     AdvanceRowGroup();
   }
   else {
-    mRow += aIncrement;
     mCurMapRow += aIncrement;
-    if (mCurMapRow >= mCurMapContentRowCount) {
-      // It's possible that the cell whose rowspan we're incrementing by spans
-      // past the end of the content rows in this rowgroup.  In that case,
-      // mCurMapRow will become strictly bigger than mCurMapContentRowCount.
-      // If that happens, we should subtract the difference from mRow, since
-      // mRow should not count non-content rows.  Of course if the difference
-      // is zero it's safe to subtract it too, so just subtract
-      // unconditionally.
-      mRow -= mCurMapRow - mCurMapContentRowCount;
+    if (mCurMapRow >= mCurMapRelevantRowCount) {
       AdvanceRowGroup();
     }
   }
@@ -2760,7 +2760,7 @@ nsCellMapColumnIterator::GetNextFrame(PRInt32* aRow, PRInt32* aColSpan)
   }
 
   while (1) {
-    NS_ASSERTION(mCurMapRow < mCurMapContentRowCount, "Bogus mOrigCells?");
+    NS_ASSERTION(mCurMapRow < mCurMapRelevantRowCount, "Bogus mOrigCells?");
     // Safe to just get the row (which is faster than calling GetDataAt(), but
     // there may not be that many cells in it, so have to use SafeElementAt for
     // the mCol.
@@ -2789,7 +2789,7 @@ nsCellMapColumnIterator::GetNextFrame(PRInt32* aRow, PRInt32* aColSpan)
     nsTableCellFrame* cellFrame = cellData->GetCellFrame();
     NS_ASSERTION(cellFrame, "Orig data without cellframe?");
     
-    *aRow = mRow;
+    *aRow = mCurMapStart + mCurMapRow;
     PRBool ignoredZeroSpan;
     *aColSpan = mCurMap->GetEffectiveColSpan(*mMap, mCurMapRow, mCol,
                                              ignoredZeroSpan);

layout/tables/nsCellMap.h

@@ -560,7 +560,7 @@ class nsCellMapColumnIterator
 {
 public:
   nsCellMapColumnIterator(const nsTableCellMap* aMap, PRInt32 aCol) :
-    mMap(aMap), mCurMap(aMap->mFirstMap), mRow(0),
+    mMap(aMap), mCurMap(aMap->mFirstMap), mCurMapStart(0),
     mCurMapRow(0), mCol(aCol), mFoundCells(0)
   {
     NS_PRECONDITION(aMap, "Must have map");
@@ -568,11 +568,18 @@ public:
     mOrigCells = aMap->GetNumCellsOriginatingInCol(mCol);
     if (mCurMap) {
       mCurMapContentRowCount = mCurMap->GetRowCount();
-      if (mCurMapContentRowCount == 0 && mOrigCells > 0) {
+      PRUint32 rowArrayLength = mCurMap->mRows.Length();
+      mCurMapRelevantRowCount = PR_MIN(mCurMapContentRowCount, rowArrayLength);
+      if (mCurMapRelevantRowCount == 0 && mOrigCells > 0) {
         // This row group is useless; advance!
         AdvanceRowGroup();
       }
     }
+#ifdef DEBUG
+    else {
+      NS_ASSERTION(mOrigCells == 0, "Why no rowgroups?");
+    }
+#endif
   }
 
   nsTableCellFrame* GetNextFrame(PRInt32* aRow, PRInt32* aColSpan);
@@ -587,10 +594,10 @@ private:
   const nsTableCellMap* mMap;
   const nsCellMap* mCurMap;
 
-  // mRow is the row in the entire nsTableCellMap where we are right now.  This
-  // must be passable to nsTableCellMap::GetDataAt, so must be a _content_ row
-  // index.
-  PRUint32 mRow;
+  // mCurMapStart is the row in the entire nsTableCellMap where
+  // mCurMap starts.  This is used to compute row indices to pass to
+  // nsTableCellMap::GetDataAt, so must be a _content_ row index.
+  PRUint32 mCurMapStart;
   
   // In steady-state mCurMapRow is the row in our current nsCellMap
   // that we'll use the next time GetNextFrame() is called.  Due to
@@ -601,7 +608,15 @@ private:
   const PRInt32 mCol;
   PRUint32 mOrigCells;
   PRUint32 mFoundCells;
+
+  // The number of content rows in mCurMap.  This may be bigger than the number
+  // of "relevant" rows, or it might be smaller.
   PRUint32 mCurMapContentRowCount;
+
+  // The number of "relevant" rows in mCurMap.  That is, the number of rows
+  // which might have an originating cell in them.  Once mCurMapRow reaches
+  // mCurMapRelevantRowCount, we should move to the next map.
+  PRUint32 mCurMapRelevantRowCount;
 };