<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[212028] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/212028">212028</a></dd>
<dt>Author</dt> <dd>bfulgham@apple.com</dd>
<dt>Date</dt> <dd>2017-02-09 18:19:04 -0800 (Thu, 09 Feb 2017)</dd>
</dl>
<h3>Log Message</h3>
<pre>Crash in render tree after dynamically mutating the slot value
https://bugs.webkit.org/show_bug.cgi?id=167502
Patch by Ryosuke Niwa <rniwa@webkit.org> on 2017-02-09
Reviewed by Antti Koivisto.
Source/WebCore:
The crash was caused by attributeChanged not destructing the render tree after an assigned element had been
removed from its slot. Since the style resolver can no longer find this element in the flat tree, we need to
delete its render object as if the element had been removed from the DOM tree.
Tests: fast/html/details-summary-slot.html
fast/shadow-dom/shadow-slot-attribute-change-crash.html
* dom/Element.cpp:
(WebCore::Element::attributeChanged):
* html/HTMLSummaryElement.cpp:
(WebCore::SummarySlotElement): Added. Always use the default slot regardless of the slot attribute's value.
(WebCore::HTMLSummaryElement::create): Use SummarySlotElement
LayoutTests:
Added regression tests for the crash, and one for assigning non-empty slot value to a child
of a summary element. The slot attribute should always be ignored since the fact summary
element has its own shadow tree is an implementation detail that should never be exposed.
* fast/html/details-summary-slot-expected.html: Added.
* fast/html/details-summary-slot.html: Added.
* fast/shadow-dom/shadow-slot-attribute-change-crash-expected.txt: Added.
* fast/shadow-dom/shadow-slot-attribute-change-crash.html: Added.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoredomElementcpp">trunk/Source/WebCore/dom/Element.cpp</a></li>
<li><a href="#trunkSourceWebCoredomShadowRooth">trunk/Source/WebCore/dom/ShadowRoot.h</a></li>
<li><a href="#trunkSourceWebCoredomSlotAssignmenth">trunk/Source/WebCore/dom/SlotAssignment.h</a></li>
<li><a href="#trunkSourceWebCorehtmlHTMLSummaryElementcpp">trunk/Source/WebCore/html/HTMLSummaryElement.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfasthtmldetailssummaryslotexpectedhtml">trunk/LayoutTests/fast/html/details-summary-slot-expected.html</a></li>
<li><a href="#trunkLayoutTestsfasthtmldetailssummaryslothtml">trunk/LayoutTests/fast/html/details-summary-slot.html</a></li>
<li><a href="#trunkLayoutTestsfastshadowdomshadowslotattributechangecrashexpectedtxt">trunk/LayoutTests/fast/shadow-dom/shadow-slot-attribute-change-crash-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastshadowdomshadowslotattributechangecrashhtml">trunk/LayoutTests/fast/shadow-dom/shadow-slot-attribute-change-crash.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (212027 => 212028)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2017-02-10 02:17:05 UTC (rev 212027)
+++ trunk/LayoutTests/ChangeLog 2017-02-10 02:19:04 UTC (rev 212028)
</span><span class="lines">@@ -1,3 +1,19 @@
</span><ins>+2017-02-09 Ryosuke Niwa <rniwa@webkit.org>
+
+ Crash in render tree after dynamically mutating the slot value
+ https://bugs.webkit.org/show_bug.cgi?id=167502
+
+ Reviewed by Antti Koivisto.
+
+ Added regression tests for the crash, and one for assigning non-empty slot value to a child
+ of a summary element. The slot attribute should always be ignored since the fact summary
+ element has its own shadow tree is an implementation detail that should never be exposed.
+
+ * fast/html/details-summary-slot-expected.html: Added.
+ * fast/html/details-summary-slot.html: Added.
+ * fast/shadow-dom/shadow-slot-attribute-change-crash-expected.txt: Added.
+ * fast/shadow-dom/shadow-slot-attribute-change-crash.html: Added.
+
</ins><span class="cx"> 2017-02-09 Antti Koivisto <antti@apple.com>
</span><span class="cx">
</span><span class="cx"> Details element doesn't work correctly when mutating content between closing and opening
</span></span></pre></div>
<a id="trunkLayoutTestsfasthtmldetailssummaryslotexpectedhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/html/details-summary-slot-expected.html (0 => 212028)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/html/details-summary-slot-expected.html (rev 0)
+++ trunk/LayoutTests/fast/html/details-summary-slot-expected.html 2017-02-10 02:19:04 UTC (rev 212028)
</span><span class="lines">@@ -0,0 +1,7 @@
</span><ins>+<!DOCTYPE html>
+<html>
+<body>
+<p>This tests sets slot on a child of summary element inside a details element. It should have no effect.</p>
+<details><summary><div>PASS</div></summary></details>
+</body>
+</html>
</ins></span></pre></div>
<a id="trunkLayoutTestsfasthtmldetailssummaryslothtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/html/details-summary-slot.html (0 => 212028)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/html/details-summary-slot.html (rev 0)
+++ trunk/LayoutTests/fast/html/details-summary-slot.html 2017-02-10 02:19:04 UTC (rev 212028)
</span><span class="lines">@@ -0,0 +1,10 @@
</span><ins>+<!DOCTYPE html>
+<html>
+<body>
+<p>This tests sets slot on a child of summary element inside a details element. It should have no effect.</p>
+<details><summary><div>PASS</div></summary></details>
+<script>
+document.querySelector('div').slot = 'foo';
+</script>
+</body>
+</html>
</ins></span></pre></div>
<a id="trunkLayoutTestsfastshadowdomshadowslotattributechangecrashexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/shadow-dom/shadow-slot-attribute-change-crash-expected.txt (0 => 212028)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/shadow-dom/shadow-slot-attribute-change-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/shadow-dom/shadow-slot-attribute-change-crash-expected.txt 2017-02-10 02:19:04 UTC (rev 212028)
</span><span class="lines">@@ -0,0 +1,3 @@
</span><ins>+This tests dynamically mutating the slot value. WebKit should not crash.
+
+PASS - WebKit did not crash
</ins></span></pre></div>
<a id="trunkLayoutTestsfastshadowdomshadowslotattributechangecrashhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/shadow-dom/shadow-slot-attribute-change-crash.html (0 => 212028)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/shadow-dom/shadow-slot-attribute-change-crash.html (rev 0)
+++ trunk/LayoutTests/fast/shadow-dom/shadow-slot-attribute-change-crash.html 2017-02-10 02:19:04 UTC (rev 212028)
</span><span class="lines">@@ -0,0 +1,26 @@
</span><ins>+<!DOCTYPE html>
+<html>
+<body>
+<p>This tests dynamically mutating the slot value. WebKit should not crash.</p>
+<outer-host style="display: block; -webkit-column-count: 2;"><inner-host style="display: block;"><div id="mutateThis">?<section style="column-span: all;"></section></div></inner-host></outer-host>
+<script>
+
+if (window.testRunner)
+ testRunner.dumpAsText();
+
+let outerHost = document.querySelector('outer-host');
+outerHost.attachShadow({mode: 'closed'}).innerHTML = '<slot></slot>';
+
+let innerHost = document.querySelector('inner-host').attachShadow({mode: 'closed'});
+innerHost.innerHTML = '<slot></slot>';
+
+outerHost.getBoundingClientRect();
+mutateThis.slot = "f";
+outerHost.getBoundingClientRect();
+mutateThis.innerText = "";
+
+document.write('PASS - WebKit did not crash');
+
+</script>
+</body>
+</html>
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (212027 => 212028)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2017-02-10 02:17:05 UTC (rev 212027)
+++ trunk/Source/WebCore/ChangeLog 2017-02-10 02:19:04 UTC (rev 212028)
</span><span class="lines">@@ -1,3 +1,23 @@
</span><ins>+2017-02-09 Ryosuke Niwa <rniwa@webkit.org>
+
+ Crash in render tree after dynamically mutating the slot value
+ https://bugs.webkit.org/show_bug.cgi?id=167502
+
+ Reviewed by Antti Koivisto.
+
+ The crash was caused by attributeChanged not destructing the render tree after an assigned element had been
+ removed from its slot. Since the style resolver can no longer find this element in the flat tree, we need to
+ delete its render object as if the element had been removed from the DOM tree.
+
+ Tests: fast/html/details-summary-slot.html
+ fast/shadow-dom/shadow-slot-attribute-change-crash.html
+
+ * dom/Element.cpp:
+ (WebCore::Element::attributeChanged):
+ * html/HTMLSummaryElement.cpp:
+ (WebCore::SummarySlotElement): Added. Always use the default slot regardless of the slot attribute's value.
+ (WebCore::HTMLSummaryElement::create): Use SummarySlotElement
+
</ins><span class="cx"> 2017-02-09 Antti Koivisto <antti@apple.com>
</span><span class="cx">
</span><span class="cx"> Details element doesn't work correctly when mutating content between closing and opening
</span></span></pre></div>
<a id="trunkSourceWebCoredomElementcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/Element.cpp (212027 => 212028)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/Element.cpp 2017-02-10 02:17:05 UTC (rev 212027)
+++ trunk/Source/WebCore/dom/Element.cpp 2017-02-10 02:19:04 UTC (rev 212028)
</span><span class="lines">@@ -1326,7 +1326,7 @@
</span><span class="cx"> else if (name == HTMLNames::slotAttr) {
</span><span class="cx"> if (auto* parent = parentElement()) {
</span><span class="cx"> if (auto* shadowRoot = parent->shadowRoot())
</span><del>- shadowRoot->hostChildElementDidChangeSlotAttribute(oldValue, newValue);
</del><ins>+ shadowRoot->hostChildElementDidChangeSlotAttribute(*this, oldValue, newValue);
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebCoredomShadowRooth"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/ShadowRoot.h (212027 => 212028)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/ShadowRoot.h 2017-02-10 02:17:05 UTC (rev 212027)
+++ trunk/Source/WebCore/dom/ShadowRoot.h 2017-02-10 02:19:04 UTC (rev 212028)
</span><span class="lines">@@ -77,7 +77,7 @@
</span><span class="cx"> void didRemoveAllChildrenOfShadowHost();
</span><span class="cx"> void didChangeDefaultSlot();
</span><span class="cx"> void hostChildElementDidChange(const Element&);
</span><del>- void hostChildElementDidChangeSlotAttribute(const AtomicString& oldValue, const AtomicString& newValue);
</del><ins>+ void hostChildElementDidChangeSlotAttribute(Element&, const AtomicString& oldValue, const AtomicString& newValue);
</ins><span class="cx">
</span><span class="cx"> const Vector<Node*>* assignedNodesForSlot(const HTMLSlotElement&);
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCoredomSlotAssignmenth"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/SlotAssignment.h (212027 => 212028)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/SlotAssignment.h 2017-02-10 02:17:05 UTC (rev 212027)
+++ trunk/Source/WebCore/dom/SlotAssignment.h 2017-02-10 02:19:04 UTC (rev 212028)
</span><span class="lines">@@ -25,6 +25,7 @@
</span><span class="cx">
</span><span class="cx"> #pragma once
</span><span class="cx">
</span><ins>+#include "RenderTreeUpdater.h"
</ins><span class="cx"> #include "ShadowRoot.h"
</span><span class="cx"> #include <wtf/HashMap.h>
</span><span class="cx"> #include <wtf/HashSet.h>
</span><span class="lines">@@ -113,12 +114,13 @@
</span><span class="cx"> m_slotAssignment->hostChildElementDidChange(childElement, *this);
</span><span class="cx"> }
</span><span class="cx">
</span><del>-inline void ShadowRoot::hostChildElementDidChangeSlotAttribute(const AtomicString& oldValue, const AtomicString& newValue)
</del><ins>+inline void ShadowRoot::hostChildElementDidChangeSlotAttribute(Element& element, const AtomicString& oldValue, const AtomicString& newValue)
</ins><span class="cx"> {
</span><del>- if (m_slotAssignment) {
- m_slotAssignment->didChangeSlot(oldValue, *this);
- m_slotAssignment->didChangeSlot(newValue, *this);
- }
</del><ins>+ if (!m_slotAssignment)
+ return;
+ m_slotAssignment->didChangeSlot(oldValue, *this);
+ m_slotAssignment->didChangeSlot(newValue, *this);
+ RenderTreeUpdater::tearDownRenderers(element);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> } // namespace WebCore
</span></span></pre></div>
<a id="trunkSourceWebCorehtmlHTMLSummaryElementcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/html/HTMLSummaryElement.cpp (212027 => 212028)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/html/HTMLSummaryElement.cpp 2017-02-10 02:17:05 UTC (rev 212027)
+++ trunk/Source/WebCore/html/HTMLSummaryElement.cpp 2017-02-10 02:19:04 UTC (rev 212028)
</span><span class="lines">@@ -31,15 +31,26 @@
</span><span class="cx"> #include "PlatformMouseEvent.h"
</span><span class="cx"> #include "RenderBlockFlow.h"
</span><span class="cx"> #include "ShadowRoot.h"
</span><ins>+#include "SlotAssignment.h"
</ins><span class="cx">
</span><span class="cx"> namespace WebCore {
</span><span class="cx">
</span><span class="cx"> using namespace HTMLNames;
</span><span class="cx">
</span><ins>+class SummarySlotElement final : public SlotAssignment {
+private:
+ void hostChildElementDidChange(const Element&, ShadowRoot& shadowRoot) override
+ {
+ didChangeSlot(SlotAssignment::defaultSlotName(), shadowRoot);
+ }
+
+ const AtomicString& slotNameForHostChild(const Node&) const override { return SlotAssignment::defaultSlotName(); }
+};
+
</ins><span class="cx"> Ref<HTMLSummaryElement> HTMLSummaryElement::create(const QualifiedName& tagName, Document& document)
</span><span class="cx"> {
</span><span class="cx"> Ref<HTMLSummaryElement> summary = adoptRef(*new HTMLSummaryElement(tagName, document));
</span><del>- summary->addShadowRoot(ShadowRoot::create(document, ShadowRootMode::UserAgent));
</del><ins>+ summary->addShadowRoot(ShadowRoot::create(document, std::make_unique<SummarySlotElement>()));
</ins><span class="cx"> return summary;
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>