RenderTableSection's recalcCell logic is doing too much work
https://bugs.webkit.org/show_bug.cgi?id=71986
Reviewed by Darin Adler.
No change in behavior expected so no test.
r98738 and r98980 missed the point that Vector::clear() does not touch
the internal buffer. Thus calling clear() would keep the memory and properly
reset the size().
Due to how the code worked, after r98980 the code would end up resetting some
of RowStruct field twice: once when growing and the other one when calling
fillRowsWithDefaultStartingAtPosition.
This change make the code use Vector::clear and simplified the recalcCells
logic knowing that growing the Vector will properly initialize most of the
RowStruct fields (with RowStruct constructor).
* rendering/RenderTableSection.cpp:
(WebCore::RenderTableSection::addChild):
(WebCore::RenderTableSection::addCell):
Updated after ensureRows signature change.
(WebCore::RenderTableSection::ensureRows):
Removed the overflow check as it was making some assumption as to how the
Vector worked. If we want to implement a limit, we may do it differently.
Also inlined the part of fillRowsWithDefaultStartingAtPosition that grows
the columns to match our current representation. The rest of the function's
initialization is handled by the constructor already.
(WebCore::RenderTableSection::recalcCells):
Use clear() as it keeps the buffer and just resets the size. Also shrinkToFit
at the end as we may have lost some rows.
* rendering/RenderTableSection.h:
(WebCore::RenderTableSection::CellStruct::CellStruct):
Fixed the style of the constructor.
(WebCore::RenderTableSection::RowStruct::RowStruct):
Added an explicit constructor (which is equivalent to the default one)
but underlines what we expect.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@99919 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/rendering/RenderTableSection.cpp b/Source/WebCore/rendering/RenderTableSection.cpp
index 5921527..e2747cf 100644
--- a/Source/WebCore/rendering/RenderTableSection.cpp
+++ b/Source/WebCore/rendering/RenderTableSection.cpp
@@ -150,9 +150,7 @@
++m_cRow;
m_cCol = 0;
- // make sure we have enough rows
- if (!ensureRows(m_cRow))
- return;
+ ensureRows(m_cRow);
m_grid[insertionRow].rowRenderer = toRenderTableRow(child);
@@ -174,19 +172,17 @@
RenderBox::removeChild(oldChild);
}
-bool RenderTableSection::ensureRows(unsigned numRows)
+void RenderTableSection::ensureRows(unsigned numRows)
{
- if (numRows > m_grid.size()) {
- size_t maxSize = numeric_limits<size_t>::max() / sizeof(RowStruct);
- if (static_cast<size_t>(numRows) > maxSize)
- return false;
+ if (numRows <= m_grid.size())
+ return;
- unsigned oldSize = m_grid.size();
- m_grid.grow(numRows);
- fillRowsWithDefaultStartingAtPosition(oldSize);
- }
+ unsigned oldSize = m_grid.size();
+ m_grid.grow(numRows);
- return true;
+ unsigned effectiveColumnCount = max(1u, table()->numEffCols());
+ for (unsigned row = oldSize; row < m_grid.size(); ++row)
+ m_grid[row].row.grow(effectiveColumnCount);
}
void RenderTableSection::addCell(RenderTableCell* cell, RenderTableRow* row)
@@ -237,9 +233,7 @@
}
}
- // make sure we have enough rows
- if (!ensureRows(insertionRow + rSpan))
- return;
+ ensureRows(insertionRow + rSpan);
m_grid[insertionRow].rowRenderer = row;
@@ -1130,25 +1124,20 @@
{
ASSERT(m_needsCellRecalc);
// We reset the flag here to ensure that |addCell| works. This is safe to do as
- // fillRowsWithDefaultStartingAtPosition makes sure we match the table's columns
- // representation.
+ // we clear the grid and properly rebuild it during |addCell|.
m_needsCellRecalc = false;
m_cCol = 0;
m_cRow = 0;
- fillRowsWithDefaultStartingAtPosition(0);
+ m_grid.clear();
- // The grid size is at least as big as the number of rows in the markup but can grow bigger
- // if we have a cell protruding because it uses a rowspan spannig out of the table.
- unsigned gridSize = 0;
for (RenderObject* row = firstChild(); row; row = row->nextSibling()) {
if (row->isTableRow()) {
unsigned insertionRow = m_cRow;
m_cRow++;
m_cCol = 0;
- if (!ensureRows(m_cRow))
- break;
-
+ ensureRows(m_cRow);
+
RenderTableRow* tableRow = toRenderTableRow(row);
m_grid[insertionRow].rowRenderer = tableRow;
setRowLogicalHeightToRowStyleLogicalHeightIfNotRelative(m_grid[insertionRow]);
@@ -1158,14 +1147,12 @@
continue;
RenderTableCell* tableCell = toRenderTableCell(cell);
- gridSize = max(gridSize, insertionRow + tableCell->rowSpan());
addCell(tableCell, tableRow);
}
}
}
- gridSize = max(gridSize, m_cRow);
- m_grid.shrink(gridSize);
+ m_grid.shrinkToFit();
setNeedsLayout(true);
}
@@ -1181,21 +1168,6 @@
t->setNeedsSectionRecalc();
}
-void RenderTableSection::fillRowsWithDefaultStartingAtPosition(unsigned startingRow)
-{
- unsigned effectiveColumnCount = max(1u, table()->numEffCols());
- for (unsigned row = startingRow; row < m_grid.size(); ++row) {
- // It may be more efficient to reset the CellStruct individually instead of reallocating
- // the whole buffer in each Row, for now this is good enough and will properly shrink
- // the rows if effectiveColumnCount was decreased.
- m_grid[row].row.clear();
- m_grid[row].row.grow(effectiveColumnCount);
- m_grid[row].rowRenderer = 0;
- m_grid[row].baseline = 0;
- m_grid[row].logicalHeight = Length();
- }
-}
-
unsigned RenderTableSection::numColumns() const
{
unsigned result = 0;
