REGRESSION(r190430): Assertion failure in Text::~Text()

https://bugs.webkit.org/show_bug.cgi?id=153577 Reviewed by Antti Koivisto. Source/WebCore: The bug was caused by destroyRenderTreeIfNeeded exiting early on all HTMLSlotElement as it lacks a render object. Fixed it by explicitly avoiding the early return when child is a HTMLSlotElement. Test: fast/shadow-dom/slot-removal-crash-2.html * dom/ContainerNode.cpp: (WebCore::destroyRenderTreeIfNeeded): LayoutTests: Added a regression test. The test hits an assertion in debug build without the fix. * fast/shadow-dom/slot-removal-crash-2-expected.txt: Added. * fast/shadow-dom/slot-removal-crash-2.html: Added. Canonical link: https://commits.webkit.org/171653@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@195727 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Jan 28, 2016 · e37fd340848087a0ac17e5d3638f7e398e4469d8 · e37fd34
1 parent d48f2b4
commit e37fd340848087a0ac17e5d3638f7e398e4469d8
Showing with 84 additions and 1 deletion.

+12 −0 LayoutTests/ChangeLog

+5 −0 LayoutTests/fast/shadow-dom/slot-removal-crash-2-expected.txt

+50 −0 LayoutTests/fast/shadow-dom/slot-removal-crash-2.html

+15 −0 Source/WebCore/ChangeLog

+2 −1 Source/WebCore/dom/ContainerNode.cpp
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
+2016-01-27  Ryosuke Niwa  <rniwa@webkit.org>
+
+        REGRESSION(r190430): Assertion failure in Text::~Text()
+        https://bugs.webkit.org/show_bug.cgi?id=153577
+
+        Reviewed by Antti Koivisto.
+
+        Added a regression test. The test hits an assertion in debug build without the fix.
+
+        * fast/shadow-dom/slot-removal-crash-2-expected.txt: Added.
+        * fast/shadow-dom/slot-removal-crash-2.html: Added.
+
 2016-01-27  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
         Garbage is displayed when root svg element has mix-blend-mode set

diff --git a/LayoutTests/fast/shadow-dom/slot-removal-crash-2-expected.txt b/LayoutTests/fast/shadow-dom/slot-removal-crash-2-expected.txt
@@ -0,0 +1,5 @@
+Test that removing a slot element with text node does not result in an assertion failure.
+The test passes if WebKit does not hit an assertion.
+PASS.
+
+
diff --git a/LayoutTests/fast/shadow-dom/slot-removal-crash-2.html b/LayoutTests/fast/shadow-dom/slot-removal-crash-2.html
@@ -0,0 +1,50 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>Test that removing a slot element with text node does not result in an assertion failure.<br>
+The test passes if WebKit does not hit an assertion.</p>
+<script>
+
+if (window.testRunner) {
+    testRunner.waitUntilDone();
+    testRunner.dumpAsText();
+}
+
+var iframe = document.createElement('iframe');
+document.body.appendChild(iframe);
+
+var x;
+
+function runTest() {
+    var doc = iframe.contentDocument;
+
+    var host = doc.createElement('div');
+    var shadowRoot = host.attachShadow({mode: 'open'});
+    var slot = doc.createElement('slot');
+    slot.textContent = 'hello';
+    shadowRoot.appendChild(slot);
+    doc.body.appendChild(host);
+
+    setTimeout(function () {
+        x = slot.offsetTop;
+        shadowRoot.removeChild(slot);
+    }, 0);
+}
+
+runTest();
+
+setTimeout(function () {
+    iframe.src = 'about:blank';
+    x = document.body.offsetTop;
+    if (window.GCController)
+        GCController.collect();
+
+    document.querySelector('p').innerHTML += '<br>PASS.';
+
+    if (window.testRunner)
+        testRunner.notifyDone();
+}, 0);
+
+</script>
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,18 @@
+2016-01-27  Ryosuke Niwa  <rniwa@webkit.org>
+
+        REGRESSION(r190430): Assertion failure in Text::~Text()
+        https://bugs.webkit.org/show_bug.cgi?id=153577
+
+        Reviewed by Antti Koivisto.
+
+        The bug was caused by destroyRenderTreeIfNeeded exiting early on all HTMLSlotElement as it lacks a render object.
+        Fixed it by explicitly avoiding the early return when child is a HTMLSlotElement.
+
+        Test: fast/shadow-dom/slot-removal-crash-2.html
+
+        * dom/ContainerNode.cpp:
+        (WebCore::destroyRenderTreeIfNeeded):
+
 2016-01-27  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
         Garbage is displayed when root svg element has mix-blend-mode set

diff --git a/Source/WebCore/dom/ContainerNode.cpp b/Source/WebCore/dom/ContainerNode.cpp
@@ -35,6 +35,7 @@
 #include "GenericCachedHTMLCollection.h"
 #include "HTMLFormControlsCollection.h"
 #include "HTMLOptionsCollection.h"
+#include "HTMLSlotElement.h"
 #include "HTMLTableRowsCollection.h"
 #include "InlineTextBox.h"
 #include "JSLazyEventListener.h"
@@ -99,7 +100,7 @@ void ContainerNode::removeDetachedChildren()
 static inline void destroyRenderTreeIfNeeded(Node& child)
 {
     // FIXME: Get rid of the named flow test.
-    if (!child.renderer() && !child.isNamedFlowContentNode())
+    if (!child.renderer() && !child.isNamedFlowContentNode() && !is<HTMLSlotElement>(child))
         return;
     if (is<Element>(child))
         Style::detachRenderTree(downcast<Element>(child));