LayoutTests:
authorantti <antti@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 16 Aug 2007 16:13:28 +0000 (16:13 +0000)
committerantti <antti@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 16 Aug 2007 16:13:28 +0000 (16:13 +0000)
        Reviewed by Maciej.

        Test for <rdar://problem/5388936>
        Crash while setting display:none for a table cell with selection

        * fast/table/destroy-cell-with-selection-crash-expected.txt: Added.
        * fast/table/destroy-cell-with-selection-crash.html: Added.

WebCore:

        Reviewed by Maciej.

        Fix <rdar://problem/5388936>
        Crash while setting display:none for a table cell with selection

        Super class destroy() could (through some selection code in removeChild()) trigger section recalc
        in middle of RenderTableCell::destroy(), cleaning section dirty bit. This would later crash in
        layout since cell grid would still have refence to the dead cell.

        Ensure table sections are dirty when leaving destroy method.

        I can't figure out tests for row and section changes but they look like
        they could crash in similar way as cell.

        * rendering/RenderTableCell.cpp:
        (WebCore::RenderTableCell::destroy):
        * rendering/RenderTableRow.cpp:
        (WebCore::RenderTableRow::destroy):
        * rendering/RenderTableSection.cpp:
        (WebCore::RenderTableSection::destroy):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@25106 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/table/destroy-cell-with-selection-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/table/destroy-cell-with-selection-crash.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/rendering/RenderTableCell.cpp
WebCore/rendering/RenderTableRow.cpp
WebCore/rendering/RenderTableSection.cpp

index 9f81e4945e077ce2678634d0ba036eb4ee443b1e..0871a980a2238bf82cf7494da2077d5adecbf386 100644 (file)
@@ -1,3 +1,13 @@
+2007-08-15  Antti Koivisto  <antti@apple.com>
+
+        Reviewed by Maciej.
+        
+        Test for <rdar://problem/5388936>
+        Crash while setting display:none for a table cell with selection
+
+        * fast/table/destroy-cell-with-selection-crash-expected.txt: Added.
+        * fast/table/destroy-cell-with-selection-crash.html: Added.
+
 2007-08-15  Justin Garcia  <justin.garcia@apple.com>
 
         Reviewed by Darin.
diff --git a/LayoutTests/fast/table/destroy-cell-with-selection-crash-expected.txt b/LayoutTests/fast/table/destroy-cell-with-selection-crash-expected.txt
new file mode 100644 (file)
index 0000000..e86fa3c
--- /dev/null
@@ -0,0 +1,2 @@
+Test doing display:none for a cell in table with selection. This passes if it does not crash.
+
diff --git a/LayoutTests/fast/table/destroy-cell-with-selection-crash.html b/LayoutTests/fast/table/destroy-cell-with-selection-crash.html
new file mode 100644 (file)
index 0000000..8010ef7
--- /dev/null
@@ -0,0 +1,17 @@
+<script>
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+</script>
+Test doing display:none for a cell in table with selection. This passes if it does not crash.
+<table style="border-collapse: collapse">
+    <td id=c0></td>
+    <td id=c1></td>
+</table>
+<script>
+var c0 = document.getElementById('c0');
+var c1 = document.getElementById('c1');
+var selection = window.getSelection();
+selection.setBaseAndExtent(c0, 0, c1, 0);
+c1.style.display = 'none';
+</script>
+
index 0bcfa9eacbacef315b0c85cb10edff9b44d4966d..9d9b84bf9080312139ced8eda19ad837b53e4699 100644 (file)
@@ -1,3 +1,26 @@
+2007-08-15  Antti Koivisto  <antti@apple.com>
+
+        Reviewed by Maciej.
+        
+        Fix <rdar://problem/5388936>
+        Crash while setting display:none for a table cell with selection
+        
+        Super class destroy() could (through some selection code in removeChild()) trigger section recalc 
+        in middle of RenderTableCell::destroy(), cleaning section dirty bit. This would later crash in 
+        layout since cell grid would still have refence to the dead cell.
+        
+        Ensure table sections are dirty when leaving destroy method.
+        
+        I can't figure out tests for row and section changes but they look like
+        they could crash in similar way as cell.
+
+        * rendering/RenderTableCell.cpp:
+        (WebCore::RenderTableCell::destroy):
+        * rendering/RenderTableRow.cpp:
+        (WebCore::RenderTableRow::destroy):
+        * rendering/RenderTableSection.cpp:
+        (WebCore::RenderTableSection::destroy):
+
 2007-08-15  Maciej Stachowiak  <mjs@apple.com>
 
         Reviewed by Geoff.
index 9b50032e0f005c1e76f6ba51dd14a6dc05bbc38e..5d2cb3d0b2c4f664146ffa964125499911b39398 100644 (file)
@@ -54,10 +54,12 @@ RenderTableCell::RenderTableCell(Node* node)
 
 void RenderTableCell::destroy()
 {
-    if (parent() && section())
-        section()->setNeedsCellRecalc();
+    RenderTableSection* recalcSection = parent() ? section() : 0;
 
     RenderBlock::destroy();
+
+    if (recalcSection)
+        recalcSection->setNeedsCellRecalc();
 }
 
 void RenderTableCell::updateFromElement()
index 921ad61ead44ca6f64f7f93b59af6318a653a60a..c5c529aad41e66fbc81d0742b90f6ecf6ec07c35 100644 (file)
@@ -46,10 +46,12 @@ RenderTableRow::RenderTableRow(Node* node)
 
 void RenderTableRow::destroy()
 {
-    if (RenderTableSection* s = section())
-        s->setNeedsCellRecalc();
+    RenderTableSection* recalcSection = section();
     
     RenderContainer::destroy();
+    
+    if (recalcSection)
+        recalcSection->setNeedsCellRecalc();
 }
 
 void RenderTableRow::setStyle(RenderStyle* newStyle)
index 6ed94c5477a420cadea3d8395b7bbadc32556b1b..b00b184d764d6b5fed8f718aa5084c067f673b50 100644 (file)
@@ -72,12 +72,14 @@ RenderTableSection::~RenderTableSection()
 
 void RenderTableSection::destroy()
 {
+    RenderTable* recalcTable = table();
+    
+    RenderContainer::destroy();
+    
     // recalc cell info because RenderTable has unguarded pointers
     // stored that point to this RenderTableSection.
-    if (table())
-        table()->setNeedsSectionRecalc();
-
-    RenderContainer::destroy();
+    if (recalcTable)
+        recalcTable->setNeedsSectionRecalc();
 }
 
 void RenderTableSection::setStyle(RenderStyle* newStyle)