Bug 1484126 - part 21: Rewrite some loops in HTMLTableEditor.cpp with CellData::NextColumnIndex() or CellData::NextRowIndex() r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Tue, 16 Oct 2018 01:10:44 +0000
changeset 499846 92c8efea77140bc65e183d4c560550bc91d58eb4
parent 499845 19605351b3612103810730b1ce7fdee53d2e3b33
child 499847 ecc77355ba3ecc4ff1e9e8b8053356724b02428c
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1484126
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1484126 - part 21: Rewrite some loops in HTMLTableEditor.cpp with CellData::NextColumnIndex() or CellData::NextRowIndex() r=m_kato A lot of loops in HTMLTableEditor.cpp scans cells along column-axis or row-axis. In those cases, they need to skip columns/rows which are spanned from prior column/row. So, we can rewrite them with CellData::NextColumnIndex() or CellData::NextRowColumn(). Differential Revision: https://phabricator.services.mozilla.com/D8358
editor/libeditor/HTMLEditor.h
editor/libeditor/HTMLTableEditor.cpp
--- a/editor/libeditor/HTMLEditor.h
+++ b/editor/libeditor/HTMLEditor.h
@@ -1251,16 +1251,25 @@ protected: // Shouldn't be used by frien
     // initialized with 1-0 indexes, effective rowspan is 2.
     int32_t mEffectiveRowSpan;
     int32_t mEffectiveColSpan;
     // mIsSelected is set to true if mElement itself or its parent <tr> or
     // <table> is selected.  Otherwise, e.g., the cell just contains selection
     // range, this is set to false.
     bool mIsSelected;
 
+    CellData()
+      : mRowSpan(-1)
+      , mColSpan(-1)
+      , mEffectiveRowSpan(-1)
+      , mEffectiveColSpan(-1)
+      , mIsSelected(false)
+    {
+    }
+
     /**
      * Those constructors initializes the members with a <table> element and
      * both row and column index to specify a cell element.
      */
     CellData(HTMLEditor& aHTMLEditor,
              Element& aTableElement,
              int32_t aRowIndex,
              int32_t aColumnIndex,
--- a/editor/libeditor/HTMLTableEditor.cpp
+++ b/editor/libeditor/HTMLTableEditor.cpp
@@ -673,73 +673,76 @@ HTMLEditor::InsertTableRowsWithTransacti
   AutoTransactionsConserveSelection dontChangeSelection(*this);
 
   RefPtr<Element> cellForRowParent;
   int32_t cellsInRow = 0;
   if (startRowIndex < tableSize.mRowCount) {
     // We are inserting above an existing row.  Get each cell in the insert
     // row to adjust for colspan effects while we count how many cells are
     // needed.
-    for (int32_t colIndex = 0, actualColSpan = 0;; colIndex += actualColSpan) {
-      CellData cellData(*this, *table, startRowIndex, colIndex, ignoredError);
+    CellData cellData;
+    for (int32_t colIndex = 0;; colIndex = cellData.NextColumnIndex()) {
+      cellData.Update(*this, *table, startRowIndex, colIndex, ignoredError);
       if (cellData.FailedOrNotFound()) {
         break; // Perhaps, we reach end of the row.
       }
 
       // XXX So, this is impossible case. Will be removed.
       if (NS_WARN_IF(!cellData.mElement)) {
-        actualColSpan = 1;
-        continue;
+        break;
       }
 
-      actualColSpan = cellData.mEffectiveColSpan;
-
       if (cellData.IsSpannedFromOtherRow()) {
         // We have a cell spanning this location.  Increase its rowspan.
         // Note that if rowspan is 0, we do nothing since that cell should
         // automatically extend into the new row.
         if (cellData.mRowSpan > 0) {
           SetRowSpan(cellData.mElement,
                      cellData.mRowSpan + aNumberOfRowsToInsert);
         }
         continue;
       }
 
-      cellsInRow += actualColSpan;
+      cellsInRow += cellData.mEffectiveColSpan;
       if (!cellForRowParent) {
-        cellForRowParent = std::move(cellData.mElement);
+        // FYI: Don't use std::move() here since NextColumnIndex() needs it.
+        cellForRowParent = cellData.mElement;
       }
+
+      MOZ_ASSERT(colIndex < cellData.NextColumnIndex());
     }
   } else {
     // We are adding a new row after all others.  If it weren't for colspan=0
     // effect,  we could simply use tableSize.mColumnCount for number of new
     // cells...
     // XXX colspan=0 support has now been removed in table layout so maybe this
     //     can be cleaned up now? (bug 1243183)
     cellsInRow = tableSize.mColumnCount;
 
     // but we must compensate for all cells with rowspan = 0 in the last row.
     const int32_t kLastRowIndex = tableSize.mRowCount - 1;
-    for (int32_t colIndex = 0, actualColSpan = 0;; colIndex += actualColSpan) {
-      CellData cellData(*this, *table, kLastRowIndex, colIndex, ignoredError);
+    CellData cellData;
+    for (int32_t colIndex = 0;; colIndex = cellData.NextColumnIndex()) {
+      cellData.Update(*this, *table, kLastRowIndex, colIndex, ignoredError);
       if (cellData.FailedOrNotFound()) {
         break; // Perhaps, we reach end of the row.
       }
 
-     actualColSpan = cellData.mEffectiveColSpan;
-
       if (!cellData.mRowSpan) {
-        MOZ_ASSERT(cellsInRow >= actualColSpan);
-        cellsInRow -= actualColSpan;
+        MOZ_ASSERT(cellsInRow >= cellData.mEffectiveColSpan);
+        cellsInRow -= cellData.mEffectiveColSpan;
       }
 
       // Save cell from the last row that we will use below
       if (!cellForRowParent && !cellData.IsSpannedFromOtherRow()) {
-        cellForRowParent = std::move(cellData.mElement);
+        // FYI: Don't use std::move() here since NextColumnIndex() needs it.
+        cellForRowParent = cellData.mElement;
       }
+
+      MOZ_ASSERT(colIndex < cellData.NextColumnIndex());
     }
   }
 
   if (NS_WARN_IF(!cellsInRow)) {
     // There is no cell element in the last row??
     return NS_OK;
   }
 
@@ -1844,36 +1847,36 @@ HTMLEditor::SelectBlockOfCells(Element* 
       MOZ_ASSERT(mSelectedCellIndex > 0);
       range = selection->GetRangeAt(mSelectedCellIndex - 1);
     }
   }
 
   nsresult rv = NS_OK;
   IgnoredErrorResult ignoredError;
   for (int32_t row = minRow; row <= maxRow; row++) {
-    for (int32_t col = minColumn, actualColSpan = 0;
+    CellData cellData;
+    for (int32_t col = minColumn;
          col <= maxColumn;
-         col += std::max(actualColSpan, 1)) {
-      CellData cellData(*this, *table, row, col, ignoredError);
+         col = cellData.NextColumnIndex()) {
+      cellData.Update(*this, *table, row, col, ignoredError);
       if (cellData.FailedOrNotFound()) {
         return NS_ERROR_FAILURE;
       }
 
       // Skip cells that already selected or are spanned from previous locations
       // XXX So, we should distinguish whether CellData returns error or just
       //     not found later.
       if (!cellData.mIsSelected && cellData.mElement &&
           !cellData.IsSpannedFromOtherRowOrColumn()) {
         rv = AppendNodeToSelectionAsRange(cellData.mElement);
         if (NS_FAILED(rv)) {
           break;
         }
       }
-
-      actualColSpan = cellData.mEffectiveColSpan;
+      MOZ_ASSERT(col < cellData.NextColumnIndex());
     }
   }
   // NS_OK, otherwise, the last failure of AppendNodeToSelectionAsRange().
   return rv;
 }
 
 NS_IMETHODIMP
 HTMLEditor::SelectAllTableCells()
@@ -1911,38 +1914,38 @@ HTMLEditor::SelectAllTableCells()
   // It is now safe to clear the selection
   // BE SURE TO RESET IT BEFORE LEAVING!
   nsresult rv = ClearSelection();
 
   // Select all cells in the same column as current cell
   bool cellSelected = false;
   IgnoredErrorResult ignoredError;
   for (int32_t row = 0; row < tableSize.mRowCount; row++) {
-    for (int32_t col = 0, actualColSpan = 0;
+    CellData cellData;
+    for (int32_t col = 0;
          col < tableSize.mColumnCount;
-         col += std::max(actualColSpan, 1)) {
-      CellData cellData(*this, *table, row, col, ignoredError);
+         col = cellData.NextColumnIndex()) {
+      cellData.Update(*this, *table, row, col, ignoredError);
       if (NS_WARN_IF(cellData.FailedOrNotFound())) {
         rv = NS_ERROR_FAILURE;
         break;
       }
 
       // Skip cells that are spanned from previous rows or columns
       // XXX So, we should distinguish whether CellData returns error or just
       //     not found later.
       if (cellData.mElement &&
           !cellData.IsSpannedFromOtherRowOrColumn()) {
         rv =  AppendNodeToSelectionAsRange(cellData.mElement);
         if (NS_FAILED(rv)) {
           break;
         }
         cellSelected = true;
       }
-
-      actualColSpan = cellData.mEffectiveColSpan;
+      MOZ_ASSERT(col < cellData.NextColumnIndex());
     }
   }
   // Safety code to select starting cell if nothing else was selected
   if (!cellSelected) {
     return AppendNodeToSelectionAsRange(startCell);
   }
   // NS_OK, otherwise, the error of ClearSelection() when there is no column or
   // the last failure of CellData or AppendNodeToSelectionAsRange().
@@ -1998,38 +2001,38 @@ HTMLEditor::SelectTableRow()
 
   // It is now safe to clear the selection
   // BE SURE TO RESET IT BEFORE LEAVING!
   rv = ClearSelection();
 
   // Select all cells in the same row as current cell
   bool cellSelected = false;
   IgnoredErrorResult ignoredError;
-  for (int32_t col = 0, actualColSpan = 0;
+  CellData cellData;
+  for (int32_t col = 0;
        col < tableSize.mColumnCount;
-       col += std::max(actualColSpan, 1)) {
-    CellData cellData(*this, *table, startRowIndex, col, ignoredError);
+       col = cellData.NextColumnIndex()) {
+    cellData.Update(*this, *table, startRowIndex, col, ignoredError);
     if (NS_WARN_IF(cellData.FailedOrNotFound())) {
       rv = NS_ERROR_FAILURE;
       break;
     }
 
     // Skip cells that are spanned from previous rows or columns
     // XXX So, we should distinguish whether CellData returns error or just
     //     not found later.
     if (cellData.mElement &&
         !cellData.IsSpannedFromOtherRowOrColumn()) {
       rv = AppendNodeToSelectionAsRange(cellData.mElement);
       if (NS_FAILED(rv)) {
         break;
       }
       cellSelected = true;
     }
-
-    actualColSpan = cellData.mEffectiveColSpan;
+    MOZ_ASSERT(col < cellData.NextColumnIndex());
   }
   // Safety code to select starting cell if nothing else was selected
   if (!cellSelected) {
     return AppendNodeToSelectionAsRange(startCell);
   }
   // NS_OK, otherwise, the error of ClearSelection() when there is no column or
   // the last failure of CellData or AppendNodeToSelectionAsRange().
   return rv;
@@ -2080,37 +2083,38 @@ HTMLEditor::SelectTableColumn()
 
   // It is now safe to clear the selection
   // BE SURE TO RESET IT BEFORE LEAVING!
   rv = ClearSelection();
 
   // Select all cells in the same column as current cell
   bool cellSelected = false;
   IgnoredErrorResult ignoredError;
-  for (int32_t row = 0, actualRowSpan = 0;
+  CellData cellData;
+  for (int32_t row = 0;
        row < tableSize.mRowCount;
-       row += std::max(actualRowSpan, 1)) {
-    CellData cellData(*this, *table, row, startColIndex, ignoredError);
+       row = cellData.NextRowIndex()) {
+    cellData.Update(*this, *table, row, startColIndex, ignoredError);
     if (NS_WARN_IF(cellData.FailedOrNotFound())) {
       rv = NS_ERROR_FAILURE;
       break;
     }
 
     // Skip cells that are spanned from previous rows or columns
     // XXX So, we should distinguish whether CellData returns error or just
     //     not found later.
     if (cellData.mElement &&
         !cellData.IsSpannedFromOtherRowOrColumn()) {
       rv = AppendNodeToSelectionAsRange(cellData.mElement);
       if (NS_FAILED(rv)) {
         break;
       }
       cellSelected = true;
     }
-    actualRowSpan = cellData.mEffectiveRowSpan;
+    MOZ_ASSERT(row < cellData.NextRowIndex());
   }
   // Safety code to select starting cell if nothing else was selected
   if (!cellSelected) {
     return AppendNodeToSelectionAsRange(startCell);
   }
   // NS_OK, otherwise, the error of ClearSelection() when there is no row or
   // the last failure of CellData or AppendNodeToSelectionAsRange().
   return rv;
@@ -2296,22 +2300,23 @@ HTMLEditor::SplitCellIntoRows(Element* a
   if (NS_WARN_IF(error.Failed())) {
     return error.StealNSResult();
   }
 
   // Find a cell to insert before or after
   RefPtr<Element> cellElementAtInsertionPoint;
   RefPtr<Element> lastCellFound;
   bool insertAfter = (cellData.mFirst.mColumn > 0);
-  for (int32_t colIndex = 0, actualColSpan2 = 0,
+  CellData cellDataAtInsertionPoint;
+  for (int32_t colIndex = 0,
                rowBelowIndex = cellData.mFirst.mRow + aRowSpanAbove;
        colIndex <= tableSize.mColumnCount;
-       colIndex += std::max(actualColSpan2, 1)) {
-    CellData cellDataAtInsertionPoint(*this, *aTable, rowBelowIndex, colIndex,
-                                      ignoredError);
+       colIndex = cellData.NextColumnIndex()) {
+    cellDataAtInsertionPoint.Update(*this, *aTable, rowBelowIndex, colIndex,
+                                    ignoredError);
     // If we fail here, it could be because row has bad rowspan values,
     // such as all cells having rowspan > 1 (Call FixRowSpan first!).
     // XXX According to the comment, this does not assume that
     //     FixRowSpan() doesn't work well and user can create non-rectangular
     //     table.  So, we should not return error when CellData cannot find
     //     a cell.
     if (NS_WARN_IF(cellDataAtInsertionPoint.FailedOrNotFound())) {
       return NS_ERROR_FAILURE;
@@ -2339,20 +2344,21 @@ HTMLEditor::SplitCellIntoRows(Element* a
       // If cell found is AFTER desired new cell colum,
       //  we have multiple cells with rowspan > 1 that
       //  prevented us from finding a cell to insert after...
       if (cellDataAtInsertionPoint.mFirst.mColumn > cellData.mFirst.mColumn) {
         // ... so instead insert before the cell we found
         insertAfter = false;
         break;
       }
-      lastCellFound = std::move(cellDataAtInsertionPoint.mElement);
+      // FYI: Don't use std::move() here since
+      //      cellDataAtInsertionPoint.NextColumnIndex() needs it.
+      lastCellFound = cellDataAtInsertionPoint.mElement;
     }
-
-    actualColSpan2 = cellDataAtInsertionPoint.mEffectiveColSpan;
+    MOZ_ASSERT(colIndex < cellDataAtInsertionPoint.NextColumnIndex());
   }
 
   if (!cellElementAtInsertionPoint && lastCellFound) {
     // Edge case where we didn't find a cell to insert after
     //  or before because column(s) before desired column
     //  and all columns after it are spanned from above.
     //  We can insert after the last cell we found
     cellElementAtInsertionPoint = std::move(lastCellFound);
@@ -2525,20 +2531,20 @@ HTMLEditor::JoinTableCells(bool aMergeNo
       // Adjust rowcount by number of rows we removed
       lastRowIndex -= currentRowCount - tableSize.mRowCount;
 
       bool cellFoundInRow = false;
       bool lastRowIsSet = false;
       int32_t lastColInRow = 0;
       int32_t firstColInRow = firstSelectedCell.mIndexes.mColumn;
       int32_t colIndex = firstSelectedCell.mIndexes.mColumn;
-      for (int32_t actualColSpan2 = 0;
+      for (CellData cellData;
            colIndex < tableSize.mColumnCount;
-           colIndex += std::max(actualColSpan2, 1)) {
-        CellData cellData(*this, *table, rowIndex, colIndex, ignoredError);
+           colIndex = cellData.NextColumnIndex()) {
+        cellData.Update(*this, *table, rowIndex, colIndex, ignoredError);
         if (NS_WARN_IF(cellData.FailedOrNotFound())) {
           return NS_ERROR_FAILURE;
         }
 
         if (cellData.mIsSelected) {
           if (!cellFoundInRow) {
             // We've just found the first selected cell in this row
             firstColInRow = cellData.mCurrent.mColumn;
@@ -2565,17 +2571,17 @@ HTMLEditor::JoinTableCells(bool aMergeNo
             // Cell is in a column less than current right border in
             //  the third or higher selected row, so stop block at the previous row
             lastRowIndex = std::max(0, cellData.mCurrent.mRow - 1);
             lastRowIsSet = true;
           }
           // We're done with this row
           break;
         }
-        actualColSpan2 = cellData.mEffectiveColSpan;
+        MOZ_ASSERT(colIndex < cellData.NextColumnIndex());
       } // End of column loop
 
       // Done with this row
       if (cellFoundInRow) {
         if (rowIndex == firstSelectedCell.mIndexes.mRow) {
           // First row always initializes the right boundary
           lastColIndex = lastColInRow;
         }
@@ -2601,20 +2607,21 @@ HTMLEditor::JoinTableCells(bool aMergeNo
       }
     }
 
     // The list of cells we will delete after joining
     nsTArray<RefPtr<Element>> deleteList;
 
     // 2nd pass: Do the joining and merging
     for (int32_t rowIndex = 0; rowIndex < tableSize.mRowCount; rowIndex++) {
-      for (int32_t colIndex = 0, actualColSpan2 = 0;
+      CellData cellData;
+      for (int32_t colIndex = 0;
            colIndex < tableSize.mColumnCount;
-           colIndex += std::max(actualColSpan2, 1)) {
-        CellData cellData(*this, *table, rowIndex, colIndex, ignoredError);
+           colIndex = cellData.NextColumnIndex()) {
+        cellData.Update(*this, *table, rowIndex, colIndex, ignoredError);
         if (NS_WARN_IF(cellData.FailedOrNotFound())) {
           return NS_ERROR_FAILURE;
         }
 
         // If this is 0, we are past last cell in row, so exit the loop
         if (!cellData.mEffectiveColSpan) {
           break;
         }
@@ -2663,17 +2670,17 @@ HTMLEditor::JoinTableCells(bool aMergeNo
             // Cell is outside join region -- just merge the contents
             rv =
               MergeCells(firstSelectedCell.mElement, cellData.mElement, false);
             if (NS_WARN_IF(NS_FAILED(rv))) {
               return rv;
             }
           }
         }
-        actualColSpan2 = cellData.mEffectiveColSpan;
+        MOZ_ASSERT(colIndex < cellData.NextColumnIndex());
       }
     }
 
     // All cell contents are merged. Delete the empty cells we accumulated
     // Prevent rules testing until we're done
     AutoTopLevelEditSubActionNotifier maybeTopLevelEditSubAction(
                                         *this, EditSubAction::eDeleteNode,
                                         nsIEditor::eNext);
@@ -2890,20 +2897,21 @@ HTMLEditor::FixBadRowSpan(Element* aTabl
   ErrorResult error;
   TableSize tableSize(*this, *aTable, error);
   if (NS_WARN_IF(error.Failed())) {
     return error.StealNSResult();
   }
 
   int32_t minRowSpan = -1;
   IgnoredErrorResult ignoredError;
-  for (int32_t colIndex = 0, actualColSpan = 0;
+  CellData cellData;
+  for (int32_t colIndex = 0;
        colIndex < tableSize.mColumnCount;
-       colIndex += std::max(actualColSpan, 1)) {
-    CellData cellData(*this, *aTable, aRowIndex, colIndex, ignoredError);
+       colIndex = cellData.NextColumnIndex()) {
+    cellData.Update(*this, *aTable, aRowIndex, colIndex, ignoredError);
     // NOTE: This is a *real* failure.
     // CellData passes if cell is missing from cellmap
     // XXX If <table> has large rowspan value or colspan value than actual
     //     cells, we may hit error.  So, this method is always failed to
     //     "fix" the rowspan...
     if (NS_WARN_IF(cellData.FailedOrNotFound())) {
       return NS_ERROR_FAILURE;
     }
@@ -2914,44 +2922,44 @@ HTMLEditor::FixBadRowSpan(Element* aTabl
       break;
     }
 
     if (cellData.mRowSpan > 0 &&
         !cellData.IsSpannedFromOtherRow() &&
         (cellData.mRowSpan < minRowSpan || minRowSpan == -1)) {
       minRowSpan = cellData.mRowSpan;
     }
-    MOZ_ASSERT(cellData.mEffectiveColSpan > 0);
-    actualColSpan = cellData.mEffectiveColSpan;
-  }
+    MOZ_ASSERT(colIndex < cellData.NextColumnIndex());
+  }
+
   if (minRowSpan > 1) {
     // The amount to reduce everyone's rowspan
     // so at least one cell has rowspan = 1
     int32_t rowsReduced = minRowSpan - 1;
-    for (int32_t colIndex = 0, actualColSpan = 0;
+    CellData cellData;
+    for (int32_t colIndex = 0;
          colIndex < tableSize.mColumnCount;
-         colIndex += std::max(actualColSpan, 1)) {
-      CellData cellData(*this, *aTable, aRowIndex, colIndex, ignoredError);
+         colIndex = cellData.NextColumnIndex()) {
+      cellData.Update(*this, *aTable, aRowIndex, colIndex, ignoredError);
       if (NS_WARN_IF(cellData.FailedOrNotFound())) {
         return NS_ERROR_FAILURE;
       }
 
       // Fixup rowspans only for cells starting in current row
       // XXX So, this does not assume that CellData returns error when just
       //     not found a cell.  Fix this later.
       if (cellData.mElement && cellData.mRowSpan > 0 &&
           !cellData.IsSpannedFromOtherRowOrColumn()) {
         nsresult rv =
           SetRowSpan(cellData.mElement, cellData.mRowSpan - rowsReduced);
         if (NS_WARN_IF(NS_FAILED(rv))) {
           return rv;
         }
       }
-      MOZ_ASSERT(cellData.mEffectiveColSpan > 0);
-      actualColSpan = cellData.mEffectiveColSpan;
+      MOZ_ASSERT(colIndex < cellData.NextColumnIndex());
     }
   }
   tableSize.Update(*this, *aTable, error);
   if (NS_WARN_IF(error.Failed())) {
     return error.StealNSResult();
   }
   aNewRowCount = tableSize.mRowCount;
   return NS_OK;
@@ -2969,20 +2977,21 @@ HTMLEditor::FixBadColSpan(Element* aTabl
   ErrorResult error;
   TableSize tableSize(*this, *aTable, error);
   if (NS_WARN_IF(error.Failed())) {
     return error.StealNSResult();
   }
 
   int32_t minColSpan = -1;
   IgnoredErrorResult ignoredError;
-  for (int32_t rowIndex = 0, actualRowSpan = 0;
+  CellData cellData;
+  for (int32_t rowIndex = 0;
        rowIndex < tableSize.mRowCount;
-       rowIndex += std::max(actualRowSpan, 1)) {
-    CellData cellData(*this, *aTable, rowIndex, aColIndex, ignoredError);
+       rowIndex = cellData.NextRowIndex()) {
+    cellData.Update(*this, *aTable, rowIndex, aColIndex, ignoredError);
     // NOTE: This is a *real* failure.
     // CellData passes if cell is missing from cellmap
     // XXX If <table> has large rowspan value or colspan value than actual
     //     cells, we may hit error.  So, this method is always failed to
     //     "fix" the colspan...
     if (NS_WARN_IF(cellData.FailedOrNotFound())) {
       return NS_ERROR_FAILURE;
     }
@@ -2992,44 +3001,44 @@ HTMLEditor::FixBadColSpan(Element* aTabl
     if (!cellData.mElement) {
       break;
     }
     if (cellData.mColSpan > 0 &&
         !cellData.IsSpannedFromOtherColumn() &&
         (cellData.mColSpan < minColSpan || minColSpan == -1)) {
       minColSpan = cellData.mColSpan;
     }
-    MOZ_ASSERT(cellData.mEffectiveRowSpan > 0);
-    actualRowSpan = cellData.mEffectiveRowSpan;
-  }
+    MOZ_ASSERT(rowIndex < cellData.NextRowIndex());
+  }
+
   if (minColSpan > 1) {
     // The amount to reduce everyone's colspan
     // so at least one cell has colspan = 1
     int32_t colsReduced = minColSpan - 1;
-    for (int32_t rowIndex = 0, actualRowSpan = 0;
+    CellData cellData;
+    for (int32_t rowIndex = 0;
          rowIndex < tableSize.mRowCount;
-         rowIndex += std::max(actualRowSpan, 1)) {
-      CellData cellData(*this, *aTable, rowIndex, aColIndex, ignoredError);
+         rowIndex = cellData.NextRowIndex()) {
+      cellData.Update(*this, *aTable, rowIndex, aColIndex, ignoredError);
       if (NS_WARN_IF(cellData.FailedOrNotFound())) {
         return NS_ERROR_FAILURE;
       }
 
       // Fixup colspans only for cells starting in current column
       // XXX So, this does not assume that CellData returns error when just
       //     not found a cell.  Fix this later.
       if (cellData.mElement && cellData.mColSpan > 0 &&
           !cellData.IsSpannedFromOtherRowOrColumn()) {
         nsresult rv =
           SetColSpan(cellData.mElement, cellData.mColSpan - colsReduced);
         if (NS_WARN_IF(NS_FAILED(rv))) {
           return rv;
         }
       }
-      MOZ_ASSERT(cellData.mEffectiveRowSpan > 0);
-      actualRowSpan = cellData.mEffectiveRowSpan;
+      MOZ_ASSERT(rowIndex < cellData.NextRowIndex());
     }
   }
   tableSize.Update(*this, *aTable, error);
   if (NS_WARN_IF(error.Failed())) {
     return error.StealNSResult();
   }
   aNewColCount = tableSize.mColumnCount;
   return NS_OK;
@@ -3251,37 +3260,34 @@ HTMLEditor::GetNumberOfCellsInRow(Elemen
 {
   IgnoredErrorResult ignoredError;
   TableSize tableSize(*this, aTableElement, ignoredError);
   if (NS_WARN_IF(ignoredError.Failed())) {
     return -1;
   }
 
   int32_t numberOfCells = 0;
-  for (int32_t columnIndex = 0; columnIndex < tableSize.mColumnCount;) {
-    CellData cellData(*this, aTableElement, aRowIndex, columnIndex,
-                      ignoredError);
+  CellData cellData;
+  for (int32_t columnIndex = 0;
+       columnIndex < tableSize.mColumnCount;
+       columnIndex = cellData.NextColumnIndex()) {
+    cellData.Update(*this, aTableElement, aRowIndex, columnIndex, ignoredError);
     // Failure means that there is no more cell in the row.  In this case,
     // we shouldn't return error since we just reach the end of the row.
     // XXX So, this method assumes that CellData won't return error when
     //     just not found.  Fix this later.
     if (cellData.FailedOrNotFound()) {
       break;
     }
 
-    if (cellData.mElement) {
-      // Only count cells that start in row we are working with
-      if (!cellData.IsSpannedFromOtherRow()) {
-        numberOfCells++;
-      }
-      // Next possible location for a cell
-      columnIndex += cellData.mEffectiveColSpan;
-    } else {
-      columnIndex++;
+    // Only count cells that start in row we are working with
+    if (cellData.mElement && !cellData.IsSpannedFromOtherRow()) {
+      numberOfCells++;
     }
+    MOZ_ASSERT(columnIndex < cellData.NextColumnIndex());
   }
   return numberOfCells;
 }
 
 NS_IMETHODIMP
 HTMLEditor::GetTableSize(Element* aTableOrElementInTable,
                          int32_t* aRowCount,
                          int32_t* aColumnCount)
@@ -4233,20 +4239,21 @@ HTMLEditor::AllCellsInRowSelected(Elemen
                                   int32_t aRowIndex,
                                   int32_t aNumberOfColumns)
 {
   if (NS_WARN_IF(!aTable)) {
     return false;
   }
 
   IgnoredErrorResult ignoredError;
-  for (int32_t col = 0, actualColSpan = 0;
+  CellData cellData;
+  for (int32_t col = 0;
        col < aNumberOfColumns;
-       col += std::max(actualColSpan, 1)) {
-    CellData cellData(*this, *aTable, aRowIndex, col, ignoredError);
+       col = cellData.NextColumnIndex()) {
+    cellData.Update(*this, *aTable, aRowIndex, col, ignoredError);
     if (NS_WARN_IF(cellData.FailedOrNotFound())) {
       return false;
     }
 
     // If no cell, we may have a "ragged" right edge, so return TRUE only if
     // we already found a cell in the row.
     // XXX So, this does not assume that CellData returns error when just
     //     not found a cell.  Fix this later.
@@ -4256,36 +4263,36 @@ HTMLEditor::AllCellsInRowSelected(Elemen
 
     // Return as soon as a non-selected cell is found.
     // XXX Odd, this is testing if each cell element is selected.  Why do
     //     we need to warn if it's false??
     if (NS_WARN_IF(!cellData.mIsSelected)) {
       return false;
     }
 
-    MOZ_ASSERT(cellData.mEffectiveColSpan > 0);
-    actualColSpan = cellData.mEffectiveColSpan;
+    MOZ_ASSERT(col < cellData.NextColumnIndex());
   }
   return true;
 }
 
 bool
 HTMLEditor::AllCellsInColumnSelected(Element* aTable,
                                      int32_t aColIndex,
                                      int32_t aNumberOfRows)
 {
   if (NS_WARN_IF(!aTable)) {
     return false;
   }
 
   IgnoredErrorResult ignoredError;
-  for (int32_t row = 0, actualRowSpan = 0;
+  CellData cellData;
+  for (int32_t row = 0;
        row < aNumberOfRows;
-       row += std::max(actualRowSpan, 1)) {
-    CellData cellData(*this, *aTable, row, aColIndex, ignoredError);
+       row = cellData.NextRowIndex()) {
+    cellData.Update(*this, *aTable, row, aColIndex, ignoredError);
     if (NS_WARN_IF(cellData.FailedOrNotFound())) {
       return false;
     }
 
     // If no cell, we must have a "ragged" right edge on the last column so
     // return TRUE only if we already found a cell in the row.
     // XXX So, this does not assume that CellData returns error when just
     //     not found a cell.  Fix this later.
@@ -4295,17 +4302,17 @@ HTMLEditor::AllCellsInColumnSelected(Ele
 
     // Return as soon as a non-selected cell is found.
     // XXX Odd, this is testing if each cell element is selected.  Why do
     //     we need to warn if it's false??
     if (NS_WARN_IF(!cellData.mIsSelected)) {
       return false;
     }
 
-    actualRowSpan = cellData.mEffectiveRowSpan;
+    MOZ_ASSERT(row < cellData.NextRowIndex());
   }
   return true;
 }
 
 bool
 HTMLEditor::IsEmptyCell(dom::Element* aCell)
 {
   MOZ_ASSERT(aCell);