<!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>[196669] 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/196669">196669</a></dd>
<dt>Author</dt> <dd>said@apple.com</dd>
<dt>Date</dt> <dd>2016-02-16 15:59:25 -0800 (Tue, 16 Feb 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>REGRESSION (<a href="http://trac.webkit.org/projects/webkit/changeset/190430">r190430</a>): WTFCrashWithSecurityImplication in:void SVGRootInlineBox::layoutCharactersInTextBoxes()
https://bugs.webkit.org/show_bug.cgi?id=154185
Reviewed by Ryosuke Niwa.
Source/WebCore:
This is a regression caused by adding support for HTMLSlotElement. The
crash happens when adding an HTMLSlotElement to anther element which should
not have it as a child like SVGTextElement for example. In this case, we
were creating a RenderText which should not be happen inside an SVG document.
The RenderText::createTextBox() was creating InlineTextBox for the slot's
text and attach it to the SVGRootInlineBox. In layoutCharactersInTextBoxes(),
the assumption is the inline box is either SVGInlineTextBox or SVGInlineFlowBox.
But since we have an InlineTextBox instead, the crash happens when casting
the InlineTextBox to SVGInlineFlowBox.
The fix is for createRenderTreeForSlotAssignees() to not create a renderer
when the parent element should not have a renderer for the this element.
This is the same thing we do for createRenderer() which handles the non
HTMLSlotElement case and which is called also from createRenderTreeRecursively().
Test: fast/shadow-dom/text-slot-child-crash.svg
* style/StyleTreeResolver.cpp:
(WebCore::Style::moveToFlowThreadIfNeeded):
(WebCore::Style::TreeResolver::createRenderer): Delete the check for
shouldCreateRenderer() and handling the case when resolvedStyle is null
since these are handled by the caller createRenderTreeRecursively().
(WebCore::Style::TreeResolver::createRenderTreeForSlotAssignees):
Assert shouldCreateRenderer() is true for this element.
(WebCore::Style::TreeResolver::createRenderTreeRecursively): Don't create
the renderer if shouldCreateRenderer() returns false. Also handle the case
when resolvedStyle is null and pass the new style to createRenderer().
* style/StyleTreeResolver.h:
LayoutTests:
Ensure that adding an HTMLSlotElement with text to an SVGTextElement will
not create a renderer and we won't crash.
* fast/shadow-dom/text-slot-child-crash-expected.txt: Added.
* fast/shadow-dom/text-slot-child-crash.svg: 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="#trunkSourceWebCorestyleStyleTreeResolvercpp">trunk/Source/WebCore/style/StyleTreeResolver.cpp</a></li>
<li><a href="#trunkSourceWebCorestyleStyleTreeResolverh">trunk/Source/WebCore/style/StyleTreeResolver.h</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastshadowdomtextslotchildcrashexpectedtxt">trunk/LayoutTests/fast/shadow-dom/text-slot-child-crash-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastshadowdomtextslotchildcrashsvg">trunk/LayoutTests/fast/shadow-dom/text-slot-child-crash.svg</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (196668 => 196669)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2016-02-16 23:47:06 UTC (rev 196668)
+++ trunk/LayoutTests/ChangeLog 2016-02-16 23:59:25 UTC (rev 196669)
</span><span class="lines">@@ -1,3 +1,16 @@
</span><ins>+2016-02-16 Said Abou-Hallawa <sabouhallawa@apple.com>
+
+ REGRESSION (r190430): WTFCrashWithSecurityImplication in:void SVGRootInlineBox::layoutCharactersInTextBoxes()
+ https://bugs.webkit.org/show_bug.cgi?id=154185
+
+ Reviewed by Ryosuke Niwa.
+
+ Ensure that adding an HTMLSlotElement with text to an SVGTextElement will
+ not create a renderer and we won't crash.
+
+ * fast/shadow-dom/text-slot-child-crash-expected.txt: Added.
+ * fast/shadow-dom/text-slot-child-crash.svg: Added.
+
</ins><span class="cx"> 2016-02-16 Simon Fraser <simon.fraser@apple.com>
</span><span class="cx">
</span><span class="cx"> Add tests for iframe and overflow scrollability after navigating back
</span></span></pre></div>
<a id="trunkLayoutTestsfastshadowdomtextslotchildcrashexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/shadow-dom/text-slot-child-crash-expected.txt (0 => 196669)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/shadow-dom/text-slot-child-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/shadow-dom/text-slot-child-crash-expected.txt 2016-02-16 23:59:25 UTC (rev 196669)
</span><span class="lines">@@ -0,0 +1 @@
</span><ins>+PASS
</ins></span></pre></div>
<a id="trunkLayoutTestsfastshadowdomtextslotchildcrashsvg"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/shadow-dom/text-slot-child-crash.svg (0 => 196669)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/shadow-dom/text-slot-child-crash.svg (rev 0)
+++ trunk/LayoutTests/fast/shadow-dom/text-slot-child-crash.svg 2016-02-16 23:59:25 UTC (rev 196669)
</span><span class="lines">@@ -0,0 +1,13 @@
</span><ins>+<svg xmlns="http://www.w3.org/2000/svg">
+ <text id="text">
+ <tspan>PASS</tspan>
+ </text>
+ <script>
+ if (window.testRunner)
+ testRunner.dumpAsText();
+ var slotElement = document.createElementNS("http://www.w3.org/1999/xhtml", "slot");
+ slotElement.innerHTML = "Some text";
+ var parent = document.getElementById("text");
+ parent.appendChild(slotElement);
+ </script>
+</svg>
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (196668 => 196669)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2016-02-16 23:47:06 UTC (rev 196668)
+++ trunk/Source/WebCore/ChangeLog 2016-02-16 23:59:25 UTC (rev 196669)
</span><span class="lines">@@ -1,3 +1,42 @@
</span><ins>+2016-02-16 Said Abou-Hallawa <sabouhallawa@apple.com>
+
+ REGRESSION (r190430): WTFCrashWithSecurityImplication in:void SVGRootInlineBox::layoutCharactersInTextBoxes()
+ https://bugs.webkit.org/show_bug.cgi?id=154185
+
+ Reviewed by Ryosuke Niwa.
+
+ This is a regression caused by adding support for HTMLSlotElement. The
+ crash happens when adding an HTMLSlotElement to anther element which should
+ not have it as a child like SVGTextElement for example. In this case, we
+ were creating a RenderText which should not be happen inside an SVG document.
+ The RenderText::createTextBox() was creating InlineTextBox for the slot's
+ text and attach it to the SVGRootInlineBox. In layoutCharactersInTextBoxes(),
+ the assumption is the inline box is either SVGInlineTextBox or SVGInlineFlowBox.
+ But since we have an InlineTextBox instead, the crash happens when casting
+ the InlineTextBox to SVGInlineFlowBox.
+
+ The fix is for createRenderTreeForSlotAssignees() to not create a renderer
+ when the parent element should not have a renderer for the this element.
+ This is the same thing we do for createRenderer() which handles the non
+ HTMLSlotElement case and which is called also from createRenderTreeRecursively().
+
+ Test: fast/shadow-dom/text-slot-child-crash.svg
+
+ * style/StyleTreeResolver.cpp:
+ (WebCore::Style::moveToFlowThreadIfNeeded):
+ (WebCore::Style::TreeResolver::createRenderer): Delete the check for
+ shouldCreateRenderer() and handling the case when resolvedStyle is null
+ since these are handled by the caller createRenderTreeRecursively().
+
+ (WebCore::Style::TreeResolver::createRenderTreeForSlotAssignees):
+ Assert shouldCreateRenderer() is true for this element.
+
+ (WebCore::Style::TreeResolver::createRenderTreeRecursively): Don't create
+ the renderer if shouldCreateRenderer() returns false. Also handle the case
+ when resolvedStyle is null and pass the new style to createRenderer().
+
+ * style/StyleTreeResolver.h:
+
</ins><span class="cx"> 2016-02-16 Simon Fraser <simon.fraser@apple.com>
</span><span class="cx">
</span><span class="cx"> Every RenderLayer should not have to remove itself from the scrollableArea set
</span></span></pre></div>
<a id="trunkSourceWebCorestyleStyleTreeResolvercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/style/StyleTreeResolver.cpp (196668 => 196669)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/style/StyleTreeResolver.cpp 2016-02-16 23:47:06 UTC (rev 196668)
+++ trunk/Source/WebCore/style/StyleTreeResolver.cpp 2016-02-16 23:59:25 UTC (rev 196669)
</span><span class="lines">@@ -188,24 +188,17 @@
</span><span class="cx"> }
</span><span class="cx"> #endif
</span><span class="cx">
</span><del>-void TreeResolver::createRenderer(Element& element, RenderStyle& inheritedStyle, RenderTreePosition& renderTreePosition, RefPtr<RenderStyle>&& resolvedStyle)
</del><ins>+void TreeResolver::createRenderer(Element& element, RenderTreePosition& renderTreePosition, RefPtr<RenderStyle>&& resolvedStyle)
</ins><span class="cx"> {
</span><del>- ASSERT(!element.renderer());
</del><ins>+ ASSERT(shouldCreateRenderer(element, renderTreePosition.parent()));
+ ASSERT(resolvedStyle);
</ins><span class="cx">
</span><del>- RefPtr<RenderStyle> style = resolvedStyle;
-
- if (!shouldCreateRenderer(element, renderTreePosition.parent()))
- return;
-
- if (!style)
- style = styleForElement(element, inheritedStyle);
-
</del><span class="cx"> RenderNamedFlowThread* parentFlowRenderer = 0;
</span><span class="cx"> #if ENABLE(CSS_REGIONS)
</span><del>- parentFlowRenderer = moveToFlowThreadIfNeeded(element, *style);
</del><ins>+ parentFlowRenderer = moveToFlowThreadIfNeeded(element, *resolvedStyle);
</ins><span class="cx"> #endif
</span><span class="cx">
</span><del>- if (!element.rendererIsNeeded(*style))
</del><ins>+ if (!element.rendererIsNeeded(*resolvedStyle))
</ins><span class="cx"> return;
</span><span class="cx">
</span><span class="cx"> renderTreePosition.computeNextSibling(element);
</span><span class="lines">@@ -214,7 +207,7 @@
</span><span class="cx"> ? RenderTreePosition(*parentFlowRenderer, parentFlowRenderer->nextRendererForElement(element))
</span><span class="cx"> : renderTreePosition;
</span><span class="cx">
</span><del>- RenderElement* newRenderer = element.createElementRenderer(style.releaseNonNull(), insertionPosition).leakPtr();
</del><ins>+ RenderElement* newRenderer = element.createElementRenderer(resolvedStyle.releaseNonNull(), insertionPosition).leakPtr();
</ins><span class="cx"> if (!newRenderer)
</span><span class="cx"> return;
</span><span class="cx"> if (!insertionPosition.canInsert(*newRenderer)) {
</span><span class="lines">@@ -479,6 +472,8 @@
</span><span class="cx"> #if ENABLE(SHADOW_DOM) || ENABLE(DETAILS_ELEMENT)
</span><span class="cx"> void TreeResolver::createRenderTreeForSlotAssignees(HTMLSlotElement& slot, RenderStyle& inheritedStyle, RenderTreePosition& renderTreePosition)
</span><span class="cx"> {
</span><ins>+ ASSERT(shouldCreateRenderer(slot, renderTreePosition.parent()));
+
</ins><span class="cx"> if (auto* assignedNodes = slot.assignedNodes()) {
</span><span class="cx"> pushEnclosingScope();
</span><span class="cx"> for (auto* child : *assignedNodes) {
</span><span class="lines">@@ -500,12 +495,21 @@
</span><span class="cx">
</span><span class="cx"> void TreeResolver::createRenderTreeRecursively(Element& current, RenderStyle& inheritedStyle, RenderTreePosition& renderTreePosition, RefPtr<RenderStyle>&& resolvedStyle)
</span><span class="cx"> {
</span><ins>+ ASSERT(!current.renderer());
+
</ins><span class="cx"> PostResolutionCallbackDisabler callbackDisabler(m_document);
</span><span class="cx"> WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
</span><span class="cx">
</span><ins>+ bool shouldCallCreateRenderer = shouldCreateRenderer(current, renderTreePosition.parent());
+
+ RefPtr<RenderStyle> style = resolvedStyle;
+ if (!style)
+ style = styleForElement(current, inheritedStyle);
+
</ins><span class="cx"> #if ENABLE(SHADOW_DOM) || ENABLE(DETAILS_ELEMENT)
</span><span class="cx"> if (is<HTMLSlotElement>(current)) {
</span><del>- createRenderTreeForSlotAssignees(downcast<HTMLSlotElement>(current), inheritedStyle, renderTreePosition);
</del><ins>+ if (shouldCallCreateRenderer && current.rendererIsNeeded(*style))
+ createRenderTreeForSlotAssignees(downcast<HTMLSlotElement>(current), inheritedStyle, renderTreePosition);
</ins><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx"> #endif
</span><span class="lines">@@ -513,7 +517,8 @@
</span><span class="cx"> if (current.hasCustomStyleResolveCallbacks())
</span><span class="cx"> current.willAttachRenderers();
</span><span class="cx">
</span><del>- createRenderer(current, inheritedStyle, renderTreePosition, WTFMove(resolvedStyle));
</del><ins>+ if (shouldCallCreateRenderer)
+ createRenderer(current, renderTreePosition, style.releaseNonNull());
</ins><span class="cx">
</span><span class="cx"> if (auto* renderer = current.renderer()) {
</span><span class="cx"> SelectorFilterPusher selectorFilterPusher(scope().selectorFilter, current, SelectorFilterPusher::NoPush);
</span></span></pre></div>
<a id="trunkSourceWebCorestyleStyleTreeResolverh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/style/StyleTreeResolver.h (196668 => 196669)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/style/StyleTreeResolver.h 2016-02-16 23:47:06 UTC (rev 196668)
+++ trunk/Source/WebCore/style/StyleTreeResolver.h 2016-02-16 23:59:25 UTC (rev 196669)
</span><span class="lines">@@ -67,7 +67,7 @@
</span><span class="cx"> void resolveBeforeOrAfterPseudoElement(Element&, Change, PseudoId, RenderTreePosition&);
</span><span class="cx">
</span><span class="cx"> void createRenderTreeRecursively(Element&, RenderStyle&, RenderTreePosition&, RefPtr<RenderStyle>&& resolvedStyle);
</span><del>- void createRenderer(Element&, RenderStyle& inheritedStyle, RenderTreePosition&, RefPtr<RenderStyle>&& resolvedStyle);
</del><ins>+ void createRenderer(Element&, RenderTreePosition&, RefPtr<RenderStyle>&& resolvedStyle);
</ins><span class="cx"> void createRenderTreeForBeforeOrAfterPseudoElement(Element&, PseudoId, RenderTreePosition&);
</span><span class="cx"> void createRenderTreeForChildren(ContainerNode&, RenderStyle&, RenderTreePosition&);
</span><span class="cx"> void createRenderTreeForShadowRoot(ShadowRoot&);
</span></span></pre>
</div>
</div>
</body>
</html>