Row with only spanning cells have a height of 0.

Source/core/rendering/RenderTableSection.cpp

Index: Source/core/rendering/RenderTableSection.cpp
diff --git a/Source/core/rendering/RenderTableSection.cpp b/Source/core/rendering/RenderTableSection.cpp
index da4b53a56e5c1fb5c6908e7bd47bd46eca864cab..cd612e16eacf0542289f1107a3e3f45ee9bd6041 100644
--- a/Source/core/rendering/RenderTableSection.cpp
+++ b/Source/core/rendering/RenderTableSection.cpp
@@ -253,6 +253,28 @@ void RenderTableSection::addCell(RenderTableCell* cell, RenderTableRow* row)
     cell->setCol(table()->effColToCol(col));
 }
 
+unsigned RenderTableSection::rowHasOnlySpanningCells(unsigned row)
+{
+    ASSERT(row < m_grid.size());

[Julien - ping for review, 2013/09/17 16:51:26]

This will be caught by Vector::at() (even in release, out of bounds access are
really nasty) at line 260 as you do m_grid[row].

[a.suchit, 2013/09/18 11:40:38]

returned if row have more than vector size.

+
+    unsigned totalCols = m_grid[row].row.size();
+
+    if (!totalCols)
+        return false;
+
+    for (unsigned col = 0; col < totalCols; col++) {
+        CellStruct rowSpanCell = cellAt(row, col);

[Julien - ping for review, 2013/09/17 16:51:26]

Let's avoid a copy by using a reference here. Also it should probably be const?

[a.suchit, 2013/09/18 11:40:38]

Done.

+        if (rowSpanCell.cells.size()) {
+            if (rowSpanCell.cells[0]->rowSpan() == 1)
+                return false;
+        } else {
+            return false;
+        }

[Julien - ping for review, 2013/09/17 16:51:26]

Let's write this (more readable with the early return):

if (rowSpan.cells.isEmpty()) {
    // Some explanations here for why we consider one empty cell to make the
return false as it's still unclear to me. Please let's make it a why comment.
    return false;
}

if (rowSpanCell.cells[0]->rowSpan() == 1)
    return false;

[a.suchit, 2013/09/18 11:40:38]

Done.

+    }
+
+    return true;
+}
+
 void RenderTableSection::populateSpanningRowsHeightFromCell(RenderTableCell* cell, struct SpanningRowsHeight& spanningRowsHeight)
 {
     const unsigned rowSpan = cell->rowSpan();
@@ -265,6 +287,11 @@ void RenderTableSection::populateSpanningRowsHeightFromCell(RenderTableCell* cel
     for (unsigned row = 0; row < rowSpan; row++) {
         unsigned actualRow = row + rowIndex;
         spanningRowsHeight.rowHeight[row] = m_rowPos[actualRow + 1] - m_rowPos[actualRow] - borderSpacingForRow(actualRow);
+        if (!spanningRowsHeight.rowHeight[row]) {
+            if (rowHasOnlySpanningCells(actualRow)) {
+                spanningRowsHeight.rowWithOnlySpanningCells = true;

[Julien - ping for review, 2013/09/17 16:51:26]

We can remove the inner branch (yay!):

spanningRowsHeight.rowWithOnlySpanningCells |=
rowHasOnlySpanningCells(actualRow);

[a.suchit, 2013/09/18 11:40:38]

Done.

+            }
+        }
         spanningRowsHeight.totalRowsHeight += spanningRowsHeight.rowHeight[row];
         spanningRowsHeight.spanningCellHeightIgnoringBorderSpacing -= borderSpacingForRow(actualRow);
     }
@@ -395,6 +422,49 @@ static bool compareRowSpanCellsInHeightDistributionOrder(const RenderTableCell*
     return false;
 }
 
+unsigned RenderTableSection::calcRowHeightHavingOnlySpanningCells(unsigned row)

[Julien - ping for review, 2013/09/17 16:51:26]

This function is never called.

[a.suchit, 2013/09/18 11:40:38]

It is called by function "updateRowsHeightHavingOnlySpanningCells" in line : 459

+{
+    ASSERT(row < m_grid.size());

[Julien - ping for review, 2013/09/17 16:51:26]

Again, duplicated by Vector::at().

[a.suchit, 2013/09/18 11:40:38]

returned if row have more than vector size.

+    ASSERT(rowHasOnlySpanningCells(row));
+
+    unsigned totalCols = m_grid[row].row.size();
+
+    if (!totalCols)
+        return 0;
+
+    unsigned rowHeight = 0;
+
+    for (unsigned col = 0; col < totalCols; col++) {
+        CellStruct rowSpanCell = cellAt(row, col);
+        if (rowSpanCell.cells.size() && rowSpanCell.cells[0]->rowSpan() > 1)
+            rowHeight = max(rowHeight, rowSpanCell.cells[0]->logicalHeightForRowSizing() / rowSpanCell.cells[0]->rowSpan());
+    }
+
+    return rowHeight;
+}
+
+void RenderTableSection::updateRowsHeightHavingOnlySpanningCells(RenderTableCell* cell, struct SpanningRowsHeight& spanningRowsHeight)
+{
+    ASSERT(spanningRowsHeight.rowHeight.size());
+
+    int accumulatedPositionIncrease = 0;
+    const unsigned rowSpan = cell->rowSpan();
+    const unsigned rowIndex = cell->rowIndex();
+
+    ASSERT(rowSpan == spanningRowsHeight.rowHeight.size());

[Julien - ping for review, 2013/09/17 16:51:26]

Nice!

+
+    for (unsigned row = 0; row < spanningRowsHeight.rowHeight.size(); row++) {
+        unsigned actualRow = row + rowIndex;
+        if (!spanningRowsHeight.rowHeight[row] && rowHasOnlySpanningCells(actualRow)) {
+            spanningRowsHeight.rowHeight[row] = calcRowHeightHavingOnlySpanningCells(actualRow);
+            accumulatedPositionIncrease += spanningRowsHeight.rowHeight[row];
+        }
+        m_rowPos[actualRow + 1] += accumulatedPositionIncrease;
+    }
+
+    spanningRowsHeight.totalRowsHeight += accumulatedPositionIncrease;
+}
+
 // Distribute rowSpan cell height in rows those comes in rowSpan cell based on the ratio of row's height if
 // 1. RowSpan cell height is greater then the total height of rows in rowSpan cell
 void RenderTableSection::distributeRowSpanHeightToRows(SpanningRenderTableCells& rowSpanCells)
@@ -444,8 +514,13 @@ void RenderTableSection::distributeRowSpanHeightToRows(SpanningRenderTableCells&
 
         populateSpanningRowsHeightFromCell(cell, spanningRowsHeight);
 
-        if (!spanningRowsHeight.totalRowsHeight || spanningRowsHeight.spanningCellHeightIgnoringBorderSpacing <= spanningRowsHeight.totalRowsHeight)
+        if (spanningRowsHeight.rowWithOnlySpanningCells)
+            updateRowsHeightHavingOnlySpanningCells(cell, spanningRowsHeight);
+
+        if (!spanningRowsHeight.totalRowsHeight || spanningRowsHeight.spanningCellHeightIgnoringBorderSpacing <= spanningRowsHeight.totalRowsHeight) {
+            extraHeightToPropagate = m_rowPos[rowIndex + rowSpan] - originalBeforePosition;
             continue;
+        }
 
         int totalPercent = 0;
         int totalAutoRowsHeight = 0;