Title: [228606] trunk
- Revision
- 228606
- Author
- [email protected]
- Date
- 2018-02-18 10:44:34 -0800 (Sun, 18 Feb 2018)
Log Message
[RenderTreeBuilder] REGRESSION(r228238) Detach renderer before destroying its subtree.
https://bugs.webkit.org/show_bug.cgi?id=182908
<rdar://problem/37619394>
Reviewed by Antti Koivisto.
Source/WebCore:
Prior to r228238 we first detached the to-be-destroyed renderer and then
started nuking its descendants. r228238 changed the order and now the descendants are
destroyed while they are still attached to the tree. Apparently some of the takeChild()
normalization logic gets triggered now that the renderers still have access to their previous/next
siblings. This is unexpected and it shouldn't matter whether the subtree is still attached.
Let's revert it to the original order for now (see webkit.org/b/182909).
Test: fast/block/crash-when-subtree-is-still-attached.html
* rendering/RenderElement.cpp:
(WebCore::RenderElement::removeAndDestroyChild):
LayoutTests:
* fast/block/crash-when-subtree-is-still-attached-expected.txt: Added.
* fast/block/crash-when-subtree-is-still-attached.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (228605 => 228606)
--- trunk/LayoutTests/ChangeLog 2018-02-18 17:51:49 UTC (rev 228605)
+++ trunk/LayoutTests/ChangeLog 2018-02-18 18:44:34 UTC (rev 228606)
@@ -1,3 +1,14 @@
+2018-02-18 Zalan Bujtas <[email protected]>
+
+ [RenderTreeBuilder] REGRESSION(r228238) Detach renderer before destroying its subtree.
+ https://bugs.webkit.org/show_bug.cgi?id=182908
+ <rdar://problem/37619394>
+
+ Reviewed by Antti Koivisto.
+
+ * fast/block/crash-when-subtree-is-still-attached-expected.txt: Added.
+ * fast/block/crash-when-subtree-is-still-attached.html: Added.
+
2018-02-16 Ryan Haddad <[email protected]>
Unreviewed, rolling out r228575.
Added: trunk/LayoutTests/fast/block/crash-when-subtree-is-still-attached-expected.txt (0 => 228606)
--- trunk/LayoutTests/fast/block/crash-when-subtree-is-still-attached-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/block/crash-when-subtree-is-still-attached-expected.txt 2018-02-18 18:44:34 UTC (rev 228606)
@@ -0,0 +1,3 @@
+PASS if
+no crash.
+
Added: trunk/LayoutTests/fast/block/crash-when-subtree-is-still-attached.html (0 => 228606)
--- trunk/LayoutTests/fast/block/crash-when-subtree-is-still-attached.html (rev 0)
+++ trunk/LayoutTests/fast/block/crash-when-subtree-is-still-attached.html 2018-02-18 18:44:34 UTC (rev 228606)
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html>
+<body>
+<b>PASS if<div></div> no crash.<div></div>foobar</b><div></div>
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+document.designMode = "on";
+document.execCommand("SelectAll");
+document.execCommand("RemoveFormat", true, "Arial");
+</script>
+</body>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (228605 => 228606)
--- trunk/Source/WebCore/ChangeLog 2018-02-18 17:51:49 UTC (rev 228605)
+++ trunk/Source/WebCore/ChangeLog 2018-02-18 18:44:34 UTC (rev 228606)
@@ -1,3 +1,23 @@
+2018-02-18 Zalan Bujtas <[email protected]>
+
+ [RenderTreeBuilder] REGRESSION(r228238) Detach renderer before destroying its subtree.
+ https://bugs.webkit.org/show_bug.cgi?id=182908
+ <rdar://problem/37619394>
+
+ Reviewed by Antti Koivisto.
+
+ Prior to r228238 we first detached the to-be-destroyed renderer and then
+ started nuking its descendants. r228238 changed the order and now the descendants are
+ destroyed while they are still attached to the tree. Apparently some of the takeChild()
+ normalization logic gets triggered now that the renderers still have access to their previous/next
+ siblings. This is unexpected and it shouldn't matter whether the subtree is still attached.
+ Let's revert it to the original order for now (see webkit.org/b/182909).
+
+ Test: fast/block/crash-when-subtree-is-still-attached.html
+
+ * rendering/RenderElement.cpp:
+ (WebCore::RenderElement::removeAndDestroyChild):
+
2018-02-18 Charlie Turner <[email protected]>
[GStreamer] Push smaller buffers from HTTP source
Modified: trunk/Source/WebCore/rendering/RenderElement.cpp (228605 => 228606)
--- trunk/Source/WebCore/rendering/RenderElement.cpp 2018-02-18 17:51:49 UTC (rev 228605)
+++ trunk/Source/WebCore/rendering/RenderElement.cpp 2018-02-18 18:44:34 UTC (rev 228606)
@@ -475,16 +475,20 @@
void RenderElement::removeAndDestroyChild(RenderTreeBuilder& builder, RenderObject& oldChild)
{
- if (is<RenderElement>(oldChild)) {
- auto& child = downcast<RenderElement>(oldChild);
- while (child.firstChild()) {
- auto& firstChild = *child.firstChild();
- if (auto* node = firstChild.node())
- node->setRenderer(nullptr);
- child.removeAndDestroyChild(builder, firstChild);
- }
+ auto toDestroy = builder.takeChild(*this, oldChild);
+ // We need to detach the subtree first so that the descendants don't have
+ // access to previous/next sublings at takeChild().
+ // FIXME: webkit.org/b/182909.
+ if (!is<RenderElement>(toDestroy.get()))
+ return;
+
+ auto& child = downcast<RenderElement>(*toDestroy.get());
+ while (child.firstChild()) {
+ auto& firstChild = *child.firstChild();
+ if (auto* node = firstChild.node())
+ node->setRenderer(nullptr);
+ child.removeAndDestroyChild(builder, firstChild);
}
- auto toDestroy = builder.takeChild(*this, oldChild);
}
RenderObject* RenderElement::attachRendererInternal(RenderPtr<RenderObject> child, RenderObject* beforeChild)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
