- Revision
- 165837
- Author
- [email protected]
- Date
- 2014-03-18 13:00:47 -0700 (Tue, 18 Mar 2014)
Log Message
REGRESSION (r162334): RenderTableCol::styleDidChange uses out-of-date table information
https://bugs.webkit.org/show_bug.cgi?id=129561
Reviewed by Antti Koivisto.
Source/WebCore:
Test: fast/table/update-col-width-and-remove-table-cell-crash.html
Fixes an issue where a table column or table column group may query an out-
of-date model of its associated table as part of its process to propagate
style changes to affected table cells.
* rendering/RenderTableCol.cpp:
(WebCore::RenderTableCol::styleDidChange): Ensure that all sections in the table
are up-to-date before querying for a table cell.
* rendering/RenderTableSection.cpp:
(WebCore::RenderTableSection::recalcCells): Update comment to read well. In
particular, remove the reference to RenderTableSection::fillRowsWithDefaultStartingAtPosition()
as this function was removed in <http://trac.webkit.org/changeset/99919>.
(WebCore::RenderTableSection::setNeedsCellRecalc): Clear the grid preemptively to
to ensure that accessors cannot access stale data. We'll build the grid again
in RenderTableSection::recalcCells().
(WebCore::RenderTableSection::numColumns): Add ASSERT(!m_needsCellRecalc) to assert
that the grid cells are up-to-date. That is, we don't need to calculate them again.
* rendering/RenderTableSection.h: Add ASSERT(!m_needsCellRecalc) or call recalcCellsIfNeeded()
before accessing the grid to ensure that it's up-to-date.
LayoutTests:
Add a test to ensure that a table column propagates a style change to applicable
table cells.
* fast/table/update-col-width-and-remove-table-cell-crash-expected.txt: Added.
* fast/table/update-col-width-and-remove-table-cell-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (165836 => 165837)
--- trunk/LayoutTests/ChangeLog 2014-03-18 19:59:05 UTC (rev 165836)
+++ trunk/LayoutTests/ChangeLog 2014-03-18 20:00:47 UTC (rev 165837)
@@ -1,5 +1,18 @@
2014-03-18 Daniel Bates <[email protected]>
+ REGRESSION (r162334): RenderTableCol::styleDidChange uses out-of-date table information
+ https://bugs.webkit.org/show_bug.cgi?id=129561
+
+ Reviewed by Antti Koivisto.
+
+ Add a test to ensure that a table column propagates a style change to applicable
+ table cells.
+
+ * fast/table/update-col-width-and-remove-table-cell-crash-expected.txt: Added.
+ * fast/table/update-col-width-and-remove-table-cell-crash.html: Added.
+
+2014-03-18 Daniel Bates <[email protected]>
+
REGRESSION (r163560): ASSERTION FAILED: childrenInline() in WebCore::RenderSVGText::layout
https://bugs.webkit.org/show_bug.cgi?id=130346
Added: trunk/LayoutTests/fast/table/update-col-width-and-remove-table-cell-crash-expected.txt (0 => 165837)
--- trunk/LayoutTests/fast/table/update-col-width-and-remove-table-cell-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/table/update-col-width-and-remove-table-cell-crash-expected.txt 2014-03-18 20:00:47 UTC (rev 165837)
@@ -0,0 +1 @@
+This test PASSED if it doesn't cause a crash, especially when run with Guard Malloc or MallocScribble enabled.
Added: trunk/LayoutTests/fast/table/update-col-width-and-remove-table-cell-crash.html (0 => 165837)
--- trunk/LayoutTests/fast/table/update-col-width-and-remove-table-cell-crash.html (rev 0)
+++ trunk/LayoutTests/fast/table/update-col-width-and-remove-table-cell-crash.html 2014-03-18 20:00:47 UTC (rev 165837)
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+
+function runTest() {
+ document.getElementById("column1").width = "90";
+ var firstRow = document.getElementById("firstRow");
+ firstRow.removeChild(firstRow.firstElementChild);
+ document.getElementById("row").offsetWidth;
+}
+</script>
+<style>
+.column2 {
+ width: 10px;
+}
+</style>
+</head>
+<body _onload_="runTest()">
+<table>
+ <colgroup>
+ <col id="column1" class="column1">
+ <col class="column2">
+ </colgroup>
+ <tbody>
+ <tr id="firstRow">
+ <td colspan="4"></td>
+ </tr>
+ <tr id="row">
+ <td colspan="4"></td>
+ </tr>
+ </tbody>
+</table>
+<p>This test PASSED if it doesn't cause a crash, especially when run with Guard Malloc or MallocScribble enabled.</p>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (165836 => 165837)
--- trunk/Source/WebCore/ChangeLog 2014-03-18 19:59:05 UTC (rev 165836)
+++ trunk/Source/WebCore/ChangeLog 2014-03-18 20:00:47 UTC (rev 165837)
@@ -1,5 +1,33 @@
2014-03-18 Daniel Bates <[email protected]>
+ REGRESSION (r162334): RenderTableCol::styleDidChange uses out-of-date table information
+ https://bugs.webkit.org/show_bug.cgi?id=129561
+
+ Reviewed by Antti Koivisto.
+
+ Test: fast/table/update-col-width-and-remove-table-cell-crash.html
+
+ Fixes an issue where a table column or table column group may query an out-
+ of-date model of its associated table as part of its process to propagate
+ style changes to affected table cells.
+
+ * rendering/RenderTableCol.cpp:
+ (WebCore::RenderTableCol::styleDidChange): Ensure that all sections in the table
+ are up-to-date before querying for a table cell.
+ * rendering/RenderTableSection.cpp:
+ (WebCore::RenderTableSection::recalcCells): Update comment to read well. In
+ particular, remove the reference to RenderTableSection::fillRowsWithDefaultStartingAtPosition()
+ as this function was removed in <http://trac.webkit.org/changeset/99919>.
+ (WebCore::RenderTableSection::setNeedsCellRecalc): Clear the grid preemptively to
+ to ensure that accessors cannot access stale data. We'll build the grid again
+ in RenderTableSection::recalcCells().
+ (WebCore::RenderTableSection::numColumns): Add ASSERT(!m_needsCellRecalc) to assert
+ that the grid cells are up-to-date. That is, we don't need to calculate them again.
+ * rendering/RenderTableSection.h: Add ASSERT(!m_needsCellRecalc) or call recalcCellsIfNeeded()
+ before accessing the grid to ensure that it's up-to-date.
+
+2014-03-18 Daniel Bates <[email protected]>
+
REGRESSION (r163560): ASSERTION FAILED: childrenInline() in WebCore::RenderSVGText::layout
https://bugs.webkit.org/show_bug.cgi?id=130346
Modified: trunk/Source/WebCore/rendering/RenderTableCol.cpp (165836 => 165837)
--- trunk/Source/WebCore/rendering/RenderTableCol.cpp 2014-03-18 19:59:05 UTC (rev 165836)
+++ trunk/Source/WebCore/rendering/RenderTableCol.cpp 2014-03-18 20:00:47 UTC (rev 165837)
@@ -55,6 +55,7 @@
if (table && !table->selfNeedsLayout() && !table->normalChildNeedsLayout() && oldStyle && oldStyle->border() != style().border())
table->invalidateCollapsedBorders();
else if (oldStyle->width() != style().width()) {
+ table->recalcSectionsIfNeeded();
for (auto& section : childrenOfType<RenderTableSection>(*table)) {
unsigned nEffCols = table->numEffCols();
for (unsigned j = 0; j < nEffCols; j++) {
Modified: trunk/Source/WebCore/rendering/RenderTableSection.cpp (165836 => 165837)
--- trunk/Source/WebCore/rendering/RenderTableSection.cpp 2014-03-18 19:59:05 UTC (rev 165836)
+++ trunk/Source/WebCore/rendering/RenderTableSection.cpp 2014-03-18 20:00:47 UTC (rev 165837)
@@ -1361,9 +1361,9 @@
void RenderTableSection::recalcCells()
{
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 reset the flag here to ensure that addCell() works. This is safe to do because we clear the grid
+ // and update its dimensions to be consistent with the table's column representation before we rebuild
+ // the grid using addCell().
m_needsCellRecalc = false;
m_cCol = 0;
@@ -1403,12 +1403,17 @@
void RenderTableSection::setNeedsCellRecalc()
{
m_needsCellRecalc = true;
+
+ // Clear the grid now to ensure that we don't hold onto any stale pointers (e.g. a cell renderer that is being removed).
+ m_grid.clear();
+
if (RenderTable* t = table())
t->setNeedsSectionRecalc();
}
unsigned RenderTableSection::numColumns() const
{
+ ASSERT(!m_needsCellRecalc);
unsigned result = 0;
for (unsigned r = 0; r < m_grid.size(); ++r) {
Modified: trunk/Source/WebCore/rendering/RenderTableSection.h (165836 => 165837)
--- trunk/Source/WebCore/rendering/RenderTableSection.h 2014-03-18 19:59:05 UTC (rev 165836)
+++ trunk/Source/WebCore/rendering/RenderTableSection.h 2014-03-18 20:00:47 UTC (rev 165837)
@@ -142,15 +142,30 @@
const RenderTableCell* firstRowCellAdjoiningTableStart() const;
const RenderTableCell* firstRowCellAdjoiningTableEnd() const;
- CellStruct& cellAt(unsigned row, unsigned col) { return m_grid[row].row[col]; }
- const CellStruct& cellAt(unsigned row, unsigned col) const { return m_grid[row].row[col]; }
+ CellStruct& cellAt(unsigned row, unsigned col)
+ {
+ recalcCellsIfNeeded();
+ return m_grid[row].row[col];
+ }
+
+ const CellStruct& cellAt(unsigned row, unsigned col) const
+ {
+ ASSERT(!m_needsCellRecalc);
+ return m_grid[row].row[col];
+ }
+
RenderTableCell* primaryCellAt(unsigned row, unsigned col)
{
+ recalcCellsIfNeeded();
CellStruct& c = m_grid[row].row[col];
return c.primaryCell();
}
- RenderTableRow* rowRendererAt(unsigned row) const { return m_grid[row].rowRenderer; }
+ RenderTableRow* rowRendererAt(unsigned row) const
+ {
+ ASSERT(!m_needsCellRecalc);
+ return m_grid[row].rowRenderer;
+ }
void appendColumn(unsigned pos);
void splitColumn(unsigned pos, unsigned first);
@@ -194,7 +209,12 @@
return styleForCellFlow->isLeftToRightDirection() ? outerBorderEnd() : outerBorderStart();
}
- unsigned numRows() const { return m_grid.size(); }
+ unsigned numRows() const
+ {
+ ASSERT(!m_needsCellRecalc);
+ return m_grid.size();
+ }
+
unsigned numColumns() const;
void recalcCells();
void recalcCellsIfNeeded()
@@ -206,7 +226,11 @@
bool needsCellRecalc() const { return m_needsCellRecalc; }
void setNeedsCellRecalc();
- LayoutUnit rowBaseline(unsigned row) { return m_grid[row].baseline; }
+ LayoutUnit rowBaseline(unsigned row)
+ {
+ recalcCellsIfNeeded();
+ return m_grid[row].baseline;
+ }
void rowLogicalHeightChanged(unsigned rowIndex);
@@ -263,7 +287,12 @@
bool hasOverflowingCell() const { return m_overflowingCells.size() || m_forceSlowPaintPathWithOverflowingCell; }
void computeOverflowFromCells(unsigned totalRows, unsigned nEffCols);
- CellSpan fullTableRowSpan() const { return CellSpan(0, m_grid.size()); }
+ CellSpan fullTableRowSpan() const
+ {
+ ASSERT(!m_needsCellRecalc);
+ return CellSpan(0, m_grid.size());
+ }
+
CellSpan fullTableColumnSpan() const { return CellSpan(0, table()->columns().size()); }
// Flip the rect so it aligns with the coordinates used by the rowPos and columnPos vectors.
