Title: [98980] trunk
- Revision
- 98980
- Author
- [email protected]
- Date
- 2011-11-01 11:02:42 -0700 (Tue, 01 Nov 2011)
Log Message
REGRESSION(98738): RenderTableSection::recalcCells does not properly shrink the RowStruct grid
https://bugs.webkit.org/show_bug.cgi?id=71246
Reviewed by Darin Adler.
Source/WebCore:
Tests: fast/table/crash-empty-section-calcBorder.html
fast/table/crash-empty-section-fixed-layout-calcArray.html
The refactoring in r98738 changed the way we handle the size to avoid throwing off
the memory. The new logic would end up never shrinking the grid's size (prior to that
we would grow to the appropriate size and throw the excess capacity with shrinkToFit).
Not shrinking would mean that we would potentially read RowStruct with the default values
(for instance no |rowRenderer|).
addCell will properly grow the grid as needed to accomodate the rows and the protruding
cells with a rowspan so we introduce a variable to keep track of the size needed. At the
end, we just shrink it to this size.
* rendering/RenderTableSection.cpp:
(WebCore::RenderTableSection::recalcCells):
Introduce a variable to keep the grid size and shrink to that size to match the old code.
LayoutTests:
Those tests checks that an empty section would not lead to reading
RowStruct without a |rowRenderer| which would crash.
* fast/table/crash-empty-section-calcBorder-expected.txt: Added.
* fast/table/crash-empty-section-calcBorder.html: Added.
* fast/table/crash-empty-section-fixed-layout-calcArray-expected.txt: Added.
* fast/table/crash-empty-section-fixed-layout-calcArray.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (98979 => 98980)
--- trunk/LayoutTests/ChangeLog 2011-11-01 17:59:49 UTC (rev 98979)
+++ trunk/LayoutTests/ChangeLog 2011-11-01 18:02:42 UTC (rev 98980)
@@ -1,3 +1,18 @@
+2011-11-01 Julien Chaffraix <[email protected]>
+
+ REGRESSION(98738): RenderTableSection::recalcCells does not properly shrink the RowStruct grid
+ https://bugs.webkit.org/show_bug.cgi?id=71246
+
+ Reviewed by Darin Adler.
+
+ Those tests checks that an empty section would not lead to reading
+ RowStruct without a |rowRenderer| which would crash.
+
+ * fast/table/crash-empty-section-calcBorder-expected.txt: Added.
+ * fast/table/crash-empty-section-calcBorder.html: Added.
+ * fast/table/crash-empty-section-fixed-layout-calcArray-expected.txt: Added.
+ * fast/table/crash-empty-section-fixed-layout-calcArray.html: Added.
+
2011-11-01 Andrey Kosyakov <[email protected]>
Unreviewed followup for the previous commit (removing another duplicate line).
Added: trunk/LayoutTests/fast/table/crash-empty-section-calcBorder-expected.txt (0 => 98980)
--- trunk/LayoutTests/fast/table/crash-empty-section-calcBorder-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/table/crash-empty-section-calcBorder-expected.txt 2011-11-01 18:02:42 UTC (rev 98980)
@@ -0,0 +1,5 @@
+Bug 71246: REGRESSION(98738): Multiple crashes in the table rendering code
+
+This test PASSES if it does not CRASH.
+
+
Property changes on: trunk/LayoutTests/fast/table/crash-empty-section-calcBorder-expected.txt
___________________________________________________________________
Added: svn:eol-style
Added: trunk/LayoutTests/fast/table/crash-empty-section-calcBorder.html (0 => 98980)
--- trunk/LayoutTests/fast/table/crash-empty-section-calcBorder.html (rev 0)
+++ trunk/LayoutTests/fast/table/crash-empty-section-calcBorder.html 2011-11-01 18:02:42 UTC (rev 98980)
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+ if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+ function crash() {
+ var firstBody = document.getElementById("firstBody");
+ firstBody.removeChild(firstBody.firstChild);
+ firstBody.offsetTop;
+ }
+
+ window.addEventListener("load", crash, false);
+</script>
+</head>
+<body>
+<p>Bug <a href="" REGRESSION(98738): Multiple crashes in the table rendering code</p>
+<p>This test PASSES if it does not CRASH.</p>
+<table style="border-collapse: collapse">
+ <tbody id="firstBody" style="border: 2px solid green"><tr style="border: 4px solid red"></tr></tbody>
+</table>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/table/crash-empty-section-calcBorder.html
___________________________________________________________________
Added: svn:eol-style
Added: trunk/LayoutTests/fast/table/crash-empty-section-fixed-layout-calcArray-expected.txt (0 => 98980)
--- trunk/LayoutTests/fast/table/crash-empty-section-fixed-layout-calcArray-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/table/crash-empty-section-fixed-layout-calcArray-expected.txt 2011-11-01 18:02:42 UTC (rev 98980)
@@ -0,0 +1,5 @@
+Bug 71246: REGRESSION(98738): Multiple crashes in the table rendering code
+
+This test PASSES if it does not CRASH.
+
+
Property changes on: trunk/LayoutTests/fast/table/crash-empty-section-fixed-layout-calcArray-expected.txt
___________________________________________________________________
Added: svn:eol-style
Added: trunk/LayoutTests/fast/table/crash-empty-section-fixed-layout-calcArray.html (0 => 98980)
--- trunk/LayoutTests/fast/table/crash-empty-section-fixed-layout-calcArray.html (rev 0)
+++ trunk/LayoutTests/fast/table/crash-empty-section-fixed-layout-calcArray.html 2011-11-01 18:02:42 UTC (rev 98980)
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+ if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+ function crash() {
+ var firstBody = document.getElementById("firstBody");
+ firstBody.removeChild(firstBody.firstChild);
+ firstBody.offsetTop;
+ }
+
+ window.addEventListener("load", crash, false);
+</script>
+</head>
+<body>
+<p>Bug <a href="" REGRESSION(98738): Multiple crashes in the table rendering code</p>
+<p>This test PASSES if it does not CRASH.</p>
+<table style="table-layout:fixed; width:0;">
+ <tbody id="firstBody"><tr></tr></tbody>
+</table>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/table/crash-empty-section-fixed-layout-calcArray.html
___________________________________________________________________
Added: svn:eol-style
Modified: trunk/Source/WebCore/ChangeLog (98979 => 98980)
--- trunk/Source/WebCore/ChangeLog 2011-11-01 17:59:49 UTC (rev 98979)
+++ trunk/Source/WebCore/ChangeLog 2011-11-01 18:02:42 UTC (rev 98980)
@@ -1,3 +1,27 @@
+2011-11-01 Julien Chaffraix <[email protected]>
+
+ REGRESSION(98738): RenderTableSection::recalcCells does not properly shrink the RowStruct grid
+ https://bugs.webkit.org/show_bug.cgi?id=71246
+
+ Reviewed by Darin Adler.
+
+ Tests: fast/table/crash-empty-section-calcBorder.html
+ fast/table/crash-empty-section-fixed-layout-calcArray.html
+
+ The refactoring in r98738 changed the way we handle the size to avoid throwing off
+ the memory. The new logic would end up never shrinking the grid's size (prior to that
+ we would grow to the appropriate size and throw the excess capacity with shrinkToFit).
+ Not shrinking would mean that we would potentially read RowStruct with the default values
+ (for instance no |rowRenderer|).
+
+ addCell will properly grow the grid as needed to accomodate the rows and the protruding
+ cells with a rowspan so we introduce a variable to keep track of the size needed. At the
+ end, we just shrink it to this size.
+
+ * rendering/RenderTableSection.cpp:
+ (WebCore::RenderTableSection::recalcCells):
+ Introduce a variable to keep the grid size and shrink to that size to match the old code.
+
2011-11-01 Andrey Kosyakov <[email protected]>
[Chromium] Some media/video-*.html layout tests occasionally crash on WIN GPU
Modified: trunk/Source/WebCore/rendering/RenderTableSection.cpp (98979 => 98980)
--- trunk/Source/WebCore/rendering/RenderTableSection.cpp 2011-11-01 17:59:49 UTC (rev 98979)
+++ trunk/Source/WebCore/rendering/RenderTableSection.cpp 2011-11-01 18:02:42 UTC (rev 98980)
@@ -1122,6 +1122,9 @@
m_cRow = 0;
fillRowsWithDefaultStartingAtPosition(0);
+ // 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;
@@ -1135,13 +1138,18 @@
setRowLogicalHeightToRowStyleLogicalHeightIfNotRelative(m_grid[insertionRow]);
for (RenderObject* cell = row->firstChild(); cell; cell = cell->nextSibling()) {
- if (cell->isTableCell())
- addCell(toRenderTableCell(cell), tableRow);
+ if (!cell->isTableCell())
+ continue;
+
+ RenderTableCell* tableCell = toRenderTableCell(cell);
+ gridSize = max(gridSize, insertionRow + tableCell->rowSpan());
+ addCell(tableCell, tableRow);
}
}
}
- m_grid.shrinkToFit();
+ gridSize = max(gridSize, m_cRow);
+ m_grid.shrink(gridSize);
m_needsCellRecalc = false;
setNeedsLayout(true);
}
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
