Diff
Modified: trunk/LayoutTests/ChangeLog (100050 => 100051)
--- trunk/LayoutTests/ChangeLog 2011-11-12 01:21:44 UTC (rev 100050)
+++ trunk/LayoutTests/ChangeLog 2011-11-12 01:26:22 UTC (rev 100051)
@@ -1,3 +1,21 @@
+2011-11-11 Julien Chaffraix <[email protected]>
+
+ Crash in styleDidChange when changing a table cell's height.
+ <http://webkit.org/b/72004>
+
+ Reviewed by Darin Adler.
+
+ * fast/table/crash-table-cell-change-height-expected.txt: Added.
+ * fast/table/crash-table-cell-change-height.html: Added.
+
+ * fast/table/table-cell-change-height-with-needsCellRecalc-section-expected.txt: Added.
+ * fast/table/table-cell-change-height-with-needsCellRecalc-section.html: Added.
+ * fast/table/table-cell-change-last-row-with-needsCellRecalc-section-expected.txt: Added.
+ * fast/table/table-cell-change-last-row-with-needsCellRecalc-section.html: Added.
+ * fast/table/table-row-change-height-with-needsCellRecalc-section-expected.txt: Added.
+ * fast/table/table-row-change-height-with-needsCellRecalc-section.html: Added.
+ Those tests checks that we don't try to access bad rows when the section needs a recalc.
+
2011-11-11 Shinya Kawanaka <[email protected]>
Implement legacy text check emulation in unified text check interface.
Added: trunk/LayoutTests/fast/table/crash-table-cell-change-height-expected.txt (0 => 100051)
--- trunk/LayoutTests/fast/table/crash-table-cell-change-height-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/table/crash-table-cell-change-height-expected.txt 2011-11-12 01:26:22 UTC (rev 100051)
@@ -0,0 +1,2 @@
+Bug 72004: Crash in styleDidChange when changing a table cell's height.
+If this test does not crash, it has PASSED.
Property changes on: trunk/LayoutTests/fast/table/crash-table-cell-change-height-expected.txt
___________________________________________________________________
Added: svn:eol-style
Added: trunk/LayoutTests/fast/table/crash-table-cell-change-height.html (0 => 100051)
--- trunk/LayoutTests/fast/table/crash-table-cell-change-height.html (rev 0)
+++ trunk/LayoutTests/fast/table/crash-table-cell-change-height.html 2011-11-12 01:26:22 UTC (rev 100051)
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+<body>
+<table>
+<tr id="row">
+<th id="header" height="50">If this test does not crash, it has PASSED.</th>
+<th>Bug 72004: Crash in styleDidChange when changing a table cell's height.<br></th>
+</tr>
+</table>
+<script>
+if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+var row = document.getElementById("row");
+var header = document.getElementById("header");
+row.parentNode.appendChild(header);
+</script>
+<script>
+header.setAttribute("height", 1);
+</script>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/table/crash-table-cell-change-height.html
___________________________________________________________________
Added: svn:executable
Added: svn:eol-style
Added: trunk/LayoutTests/fast/table/table-cell-change-height-with-needsCellRecalc-section-expected.txt (0 => 100051)
--- trunk/LayoutTests/fast/table/table-cell-change-height-with-needsCellRecalc-section-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/table/table-cell-change-height-with-needsCellRecalc-section-expected.txt 2011-11-12 01:26:22 UTC (rev 100051)
@@ -0,0 +1,2 @@
+Bug 72004: Crash in styleDidChange when changing a table cell's height.
+If this test does not crash, it has PASSED.
Property changes on: trunk/LayoutTests/fast/table/table-cell-change-height-with-needsCellRecalc-section-expected.txt
___________________________________________________________________
Added: svn:eol-style
Added: trunk/LayoutTests/fast/table/table-cell-change-height-with-needsCellRecalc-section.html (0 => 100051)
--- trunk/LayoutTests/fast/table/table-cell-change-height-with-needsCellRecalc-section.html (rev 0)
+++ trunk/LayoutTests/fast/table/table-cell-change-height-with-needsCellRecalc-section.html 2011-11-12 01:26:22 UTC (rev 100051)
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html>
+<body>
+<table id="table">
+<tr>
+<th id="header" height="50">Bug 72004: Crash in styleDidChange when changing a table cell's height.<br></th>
+<th>If this test does not crash, it has PASSED.</th>
+</tr>
+</table>
+<script>
+if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+var table = document.getElementById("table");
+var row = document.createElement("tr");
+table.appendChild(row);
+</script>
+<script>
+// This sets the recalcCells bit on the section.
+row.parentNode.removeChild(row);
+
+// Change on cell's height.
+document.getElementById("header").setAttribute("height", 1);
+</script>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/table/table-cell-change-height-with-needsCellRecalc-section.html
___________________________________________________________________
Added: svn:executable
Added: svn:eol-style
Added: trunk/LayoutTests/fast/table/table-cell-change-last-row-with-needsCellRecalc-section-expected.txt (0 => 100051)
--- trunk/LayoutTests/fast/table/table-cell-change-last-row-with-needsCellRecalc-section-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/table/table-cell-change-last-row-with-needsCellRecalc-section-expected.txt 2011-11-12 01:26:22 UTC (rev 100051)
@@ -0,0 +1,2 @@
+Bug 72004: Crash in styleDidChange when changing a table cell's height.
+If this test does not crash, it has PASSED.
Property changes on: trunk/LayoutTests/fast/table/table-cell-change-last-row-with-needsCellRecalc-section-expected.txt
___________________________________________________________________
Added: svn:eol-style
Added: trunk/LayoutTests/fast/table/table-cell-change-last-row-with-needsCellRecalc-section.html (0 => 100051)
--- trunk/LayoutTests/fast/table/table-cell-change-last-row-with-needsCellRecalc-section.html (rev 0)
+++ trunk/LayoutTests/fast/table/table-cell-change-last-row-with-needsCellRecalc-section.html 2011-11-12 01:26:22 UTC (rev 100051)
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+<body>
+<table id="table">
+<tr>
+<th id="header" height="50">Bug 72004: Crash in styleDidChange when changing a table cell's height.<br></th>
+<th>If this test does not crash, it has PASSED.</th>
+</tr>
+</table>
+<script>
+if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+var table = document.getElementById("table");
+var header = document.getElementById("header");
+
+var row = document.createElement("tr");
+table.insertBefore(row, table.firstChild);
+</script>
+<script>
+// This sets the recalcCells bit on the section.
+row.parentNode.removeChild(row);
+
+// Change on cell's height.
+header.setAttribute("height", 1);
+</script>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/table/table-cell-change-last-row-with-needsCellRecalc-section.html
___________________________________________________________________
Added: svn:executable
Added: svn:eol-style
Added: trunk/LayoutTests/fast/table/table-row-change-height-with-needsCellRecalc-section-expected.txt (0 => 100051)
--- trunk/LayoutTests/fast/table/table-row-change-height-with-needsCellRecalc-section-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/table/table-row-change-height-with-needsCellRecalc-section-expected.txt 2011-11-12 01:26:22 UTC (rev 100051)
@@ -0,0 +1,2 @@
+Bug 72004: Crash in styleDidChange when changing a table cell's height.
+If this test does not crash, it has PASSED.
Property changes on: trunk/LayoutTests/fast/table/table-row-change-height-with-needsCellRecalc-section-expected.txt
___________________________________________________________________
Added: svn:eol-style
Added: trunk/LayoutTests/fast/table/table-row-change-height-with-needsCellRecalc-section.html (0 => 100051)
--- trunk/LayoutTests/fast/table/table-row-change-height-with-needsCellRecalc-section.html (rev 0)
+++ trunk/LayoutTests/fast/table/table-row-change-height-with-needsCellRecalc-section.html 2011-11-12 01:26:22 UTC (rev 100051)
@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+ .fixedHeight { height: 100px; }
+</style>
+</head>
+<body>
+<table id="table">
+<tr id="row">
+<th>Bug 72004: Crash in styleDidChange when changing a table cell's height.<br></th>
+<th>If this test does not crash, it has PASSED.</th>
+</tr>
+</table>
+<script>
+if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+var table = document.getElementById("table");
+var row = document.createElement("tr");
+table.appendChild(row);
+
+</script>
+<script>
+// This sets the recalcCells bit on the section.
+row.parentNode.removeChild(row);
+
+// Change the remaining row's height.
+document.getElementById("row").setAttribute("class", "fixedHeight");
+</script>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/table/table-row-change-height-with-needsCellRecalc-section.html
___________________________________________________________________
Added: svn:executable
Added: svn:eol-style
Modified: trunk/Source/WebCore/ChangeLog (100050 => 100051)
--- trunk/Source/WebCore/ChangeLog 2011-11-12 01:21:44 UTC (rev 100050)
+++ trunk/Source/WebCore/ChangeLog 2011-11-12 01:26:22 UTC (rev 100051)
@@ -1,3 +1,34 @@
+2011-11-11 Julien Chaffraix <[email protected]>
+
+ Crash in styleDidChange when changing a table cell's height.
+ <http://webkit.org/b/72004>
+
+ Reviewed by Darin Adler.
+
+ Tests: fast/table/crash-table-cell-change-height.html
+ fast/table/table-cell-change-height-with-needsCellRecalc-section.html
+ fast/table/table-cell-change-last-row-with-needsCellRecalc-section.html
+ fast/table/table-row-change-height-with-needsCellRecalc-section.html
+
+ r99212 removed our call to recalcCells when the logical height changed. This
+ means that we could end up with a cells without having set its row() properly
+ which would crash.
+
+ The change exposes the was-the-row-properly-set information so that we don't
+ try to access a bogus index. This should work properly even if the section
+ needs a cell recalc as the current section's row structure would still be
+ accurate in this case.
+
+ * rendering/RenderTableCell.cpp:
+ (WebCore::RenderTableCell::styleDidChange): Check that our row index
+ was properly set before calling
+
+ * rendering/RenderTableCell.h:
+ (WebCore::RenderTableCell::rowWasSet): Helper function to detect
+ if we have set up our a row.
+ (WebCore::RenderTableCell::row): Use the previous function in the
+ ASSERT.
+
2011-11-11 Shinya Kawanaka <[email protected]>
Implement legacy text check emulation in unified text check interface.
Modified: trunk/Source/WebCore/rendering/RenderTableCell.cpp (100050 => 100051)
--- trunk/Source/WebCore/rendering/RenderTableCell.cpp 2011-11-12 01:21:44 UTC (rev 100050)
+++ trunk/Source/WebCore/rendering/RenderTableCell.cpp 2011-11-12 01:26:22 UTC (rev 100051)
@@ -317,7 +317,7 @@
RenderBlock::styleDidChange(diff, oldStyle);
setHasBoxDecorations(true);
- if (parent() && section() && oldStyle && style()->height() != oldStyle->height())
+ if (parent() && section() && oldStyle && style()->height() != oldStyle->height() && rowWasSet())
section()->rowLogicalHeightChanged(row());
// If border was changed, notify table.
Modified: trunk/Source/WebCore/rendering/RenderTableCell.h (100050 => 100051)
--- trunk/Source/WebCore/rendering/RenderTableCell.h 2011-11-12 01:21:44 UTC (rev 100050)
+++ trunk/Source/WebCore/rendering/RenderTableCell.h 2011-11-12 01:26:22 UTC (rev 100051)
@@ -71,9 +71,10 @@
m_row = row;
}
+ bool rowWasSet() const { return m_row != unsetRowIndex; }
unsigned row() const
{
- ASSERT(m_row != unsetRowIndex);
+ ASSERT(rowWasSet());
return m_row;
}
