Title: [127206] trunk
- Revision
- 127206
- Author
- [email protected]
- Date
- 2012-08-30 16:22:56 -0700 (Thu, 30 Aug 2012)
Log Message
Crash in RenderTable::calcBorderEnd
https://bugs.webkit.org/show_bug.cgi?id=95487
Reviewed by Abhishek Arya.
Source/WebCore:
r126590 opened the window for a race condition in RenderObjectChildList::removeChildNode.
This is caused because willBeRemovedFromTree should be strictly following by the actual removal
and wasn't.
This race condition was caused by clearSelection() being called just after willBeRemovedFromTree,
which forced a section's cells recalc and would re-add the soon-to-be-removed child, causing the
crash.
Test: fast/table/crash-clearSelection-collapsed-borders.html
* rendering/RenderObjectChildList.cpp:
(WebCore::RenderObjectChildList::removeChildNode):
Moved the clearSeletion call before willBeRemovedFromTree. Added a warning about running code
after willBeRemovedFromTree and before removing the child from the tree.
LayoutTests:
* fast/table/crash-clearSelection-collapsed-borders-expected.txt: Added.
* fast/table/crash-clearSelection-collapsed-borders.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (127205 => 127206)
--- trunk/LayoutTests/ChangeLog 2012-08-30 23:07:21 UTC (rev 127205)
+++ trunk/LayoutTests/ChangeLog 2012-08-30 23:22:56 UTC (rev 127206)
@@ -1,3 +1,13 @@
+2012-08-30 Julien Chaffraix <[email protected]>
+
+ Crash in RenderTable::calcBorderEnd
+ https://bugs.webkit.org/show_bug.cgi?id=95487
+
+ Reviewed by Abhishek Arya.
+
+ * fast/table/crash-clearSelection-collapsed-borders-expected.txt: Added.
+ * fast/table/crash-clearSelection-collapsed-borders.html: Added.
+
2012-08-30 Philip Rogers <[email protected]>
Unreviewed update to test expectations.
Added: trunk/LayoutTests/fast/table/crash-clearSelection-collapsed-borders-expected.txt (0 => 127206)
--- trunk/LayoutTests/fast/table/crash-clearSelection-collapsed-borders-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/table/crash-clearSelection-collapsed-borders-expected.txt 2012-08-30 23:22:56 UTC (rev 127206)
@@ -0,0 +1,2 @@
+95487: Crash in RenderTable::calcBorderEnd
+This test has PASSED if it didn't crash.
Added: trunk/LayoutTests/fast/table/crash-clearSelection-collapsed-borders.html (0 => 127206)
--- trunk/LayoutTests/fast/table/crash-clearSelection-collapsed-borders.html (rev 0)
+++ trunk/LayoutTests/fast/table/crash-clearSelection-collapsed-borders.html 2012-08-30 23:22:56 UTC (rev 127206)
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<table style="border-collapse: collapse">
+ <td id=c0>
+ <span>
+ <a href="" Crash in RenderTable::calcBorderEnd<br/>
+ This test has PASSED if it didn't crash.
+ </span>
+ </td>
+ <td id=c1></td>
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+
+var selection = window.getSelection();
+selection.setBaseAndExtent(c0, 0, c1, 0);
+c1.style.display = 'none';
+</script>
Modified: trunk/Source/WebCore/ChangeLog (127205 => 127206)
--- trunk/Source/WebCore/ChangeLog 2012-08-30 23:07:21 UTC (rev 127205)
+++ trunk/Source/WebCore/ChangeLog 2012-08-30 23:22:56 UTC (rev 127206)
@@ -1,3 +1,25 @@
+2012-08-30 Julien Chaffraix <[email protected]>
+
+ Crash in RenderTable::calcBorderEnd
+ https://bugs.webkit.org/show_bug.cgi?id=95487
+
+ Reviewed by Abhishek Arya.
+
+ r126590 opened the window for a race condition in RenderObjectChildList::removeChildNode.
+ This is caused because willBeRemovedFromTree should be strictly following by the actual removal
+ and wasn't.
+
+ This race condition was caused by clearSelection() being called just after willBeRemovedFromTree,
+ which forced a section's cells recalc and would re-add the soon-to-be-removed child, causing the
+ crash.
+
+ Test: fast/table/crash-clearSelection-collapsed-borders.html
+
+ * rendering/RenderObjectChildList.cpp:
+ (WebCore::RenderObjectChildList::removeChildNode):
+ Moved the clearSeletion call before willBeRemovedFromTree. Added a warning about running code
+ after willBeRemovedFromTree and before removing the child from the tree.
+
2012-08-30 Geoffrey Garen <[email protected]>
Use one object instead of two for closures, eliminating ScopeChainNode
Modified: trunk/Source/WebCore/rendering/RenderObjectChildList.cpp (127205 => 127206)
--- trunk/Source/WebCore/rendering/RenderObjectChildList.cpp 2012-08-30 23:07:21 UTC (rev 127205)
+++ trunk/Source/WebCore/rendering/RenderObjectChildList.cpp 2012-08-30 23:22:56 UTC (rev 127206)
@@ -85,9 +85,6 @@
if (oldChild->isBox())
toRenderBox(oldChild)->deleteLineBoxWrapper();
- if (!owner->documentBeingDestroyed() && notifyRenderer)
- oldChild->willBeRemovedFromTree();
-
// If oldChild is the start or end of the selection, then clear the selection to
// avoid problems of invalid pointers.
// FIXME: The FrameSelection should be responsible for this when it
@@ -95,7 +92,13 @@
if (!owner->documentBeingDestroyed() && oldChild->isSelectionBorder())
owner->view()->clearSelection();
- // remove the child
+ if (!owner->documentBeingDestroyed() && notifyRenderer)
+ oldChild->willBeRemovedFromTree();
+
+ // WARNING: There should be no code running between willBeRemovedFromTree and the actual removal below.
+ // This is needed to avoid race conditions where willBeRemovedFromTree would dirty the tree's structure
+ // and the code running here would force an untimely rebuilding, leaving |oldChild| dangling.
+
if (oldChild->previousSibling())
oldChild->previousSibling()->setNextSibling(oldChild->nextSibling());
if (oldChild->nextSibling())
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes
