- Revision
- 196669
- Author
- s...@apple.com
- Date
- 2016-02-16 15:59:25 -0800 (Tue, 16 Feb 2016)
Log Message
REGRESSION (r190430): 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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (196668 => 196669)
--- trunk/LayoutTests/ChangeLog 2016-02-16 23:47:06 UTC (rev 196668)
+++ trunk/LayoutTests/ChangeLog 2016-02-16 23:59:25 UTC (rev 196669)
@@ -1,3 +1,16 @@
+2016-02-16 Said Abou-Hallawa <sabouhall...@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.
+
2016-02-16 Simon Fraser <simon.fra...@apple.com>
Add tests for iframe and overflow scrollability after navigating back
Added: trunk/LayoutTests/fast/shadow-dom/text-slot-child-crash-expected.txt (0 => 196669)
--- 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)
@@ -0,0 +1 @@
+PASS
Added: trunk/LayoutTests/fast/shadow-dom/text-slot-child-crash.svg (0 => 196669)
--- 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)
@@ -0,0 +1,13 @@
+<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>
Modified: trunk/Source/WebCore/ChangeLog (196668 => 196669)
--- 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)
@@ -1,3 +1,42 @@
+2016-02-16 Said Abou-Hallawa <sabouhall...@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:
+
2016-02-16 Simon Fraser <simon.fra...@apple.com>
Every RenderLayer should not have to remove itself from the scrollableArea set
Modified: trunk/Source/WebCore/style/StyleTreeResolver.cpp (196668 => 196669)
--- 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)
@@ -188,24 +188,17 @@
}
#endif
-void TreeResolver::createRenderer(Element& element, RenderStyle& inheritedStyle, RenderTreePosition& renderTreePosition, RefPtr<RenderStyle>&& resolvedStyle)
+void TreeResolver::createRenderer(Element& element, RenderTreePosition& renderTreePosition, RefPtr<RenderStyle>&& resolvedStyle)
{
- ASSERT(!element.renderer());
+ ASSERT(shouldCreateRenderer(element, renderTreePosition.parent()));
+ ASSERT(resolvedStyle);
- RefPtr<RenderStyle> style = resolvedStyle;
-
- if (!shouldCreateRenderer(element, renderTreePosition.parent()))
- return;
-
- if (!style)
- style = styleForElement(element, inheritedStyle);
-
RenderNamedFlowThread* parentFlowRenderer = 0;
#if ENABLE(CSS_REGIONS)
- parentFlowRenderer = moveToFlowThreadIfNeeded(element, *style);
+ parentFlowRenderer = moveToFlowThreadIfNeeded(element, *resolvedStyle);
#endif
- if (!element.rendererIsNeeded(*style))
+ if (!element.rendererIsNeeded(*resolvedStyle))
return;
renderTreePosition.computeNextSibling(element);
@@ -214,7 +207,7 @@
? RenderTreePosition(*parentFlowRenderer, parentFlowRenderer->nextRendererForElement(element))
: renderTreePosition;
- RenderElement* newRenderer = element.createElementRenderer(style.releaseNonNull(), insertionPosition).leakPtr();
+ RenderElement* newRenderer = element.createElementRenderer(resolvedStyle.releaseNonNull(), insertionPosition).leakPtr();
if (!newRenderer)
return;
if (!insertionPosition.canInsert(*newRenderer)) {
@@ -479,6 +472,8 @@
#if ENABLE(SHADOW_DOM) || ENABLE(DETAILS_ELEMENT)
void TreeResolver::createRenderTreeForSlotAssignees(HTMLSlotElement& slot, RenderStyle& inheritedStyle, RenderTreePosition& renderTreePosition)
{
+ ASSERT(shouldCreateRenderer(slot, renderTreePosition.parent()));
+
if (auto* assignedNodes = slot.assignedNodes()) {
pushEnclosingScope();
for (auto* child : *assignedNodes) {
@@ -500,12 +495,21 @@
void TreeResolver::createRenderTreeRecursively(Element& current, RenderStyle& inheritedStyle, RenderTreePosition& renderTreePosition, RefPtr<RenderStyle>&& resolvedStyle)
{
+ ASSERT(!current.renderer());
+
PostResolutionCallbackDisabler callbackDisabler(m_document);
WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
+ bool shouldCallCreateRenderer = shouldCreateRenderer(current, renderTreePosition.parent());
+
+ RefPtr<RenderStyle> style = resolvedStyle;
+ if (!style)
+ style = styleForElement(current, inheritedStyle);
+
#if ENABLE(SHADOW_DOM) || ENABLE(DETAILS_ELEMENT)
if (is<HTMLSlotElement>(current)) {
- createRenderTreeForSlotAssignees(downcast<HTMLSlotElement>(current), inheritedStyle, renderTreePosition);
+ if (shouldCallCreateRenderer && current.rendererIsNeeded(*style))
+ createRenderTreeForSlotAssignees(downcast<HTMLSlotElement>(current), inheritedStyle, renderTreePosition);
return;
}
#endif
@@ -513,7 +517,8 @@
if (current.hasCustomStyleResolveCallbacks())
current.willAttachRenderers();
- createRenderer(current, inheritedStyle, renderTreePosition, WTFMove(resolvedStyle));
+ if (shouldCallCreateRenderer)
+ createRenderer(current, renderTreePosition, style.releaseNonNull());
if (auto* renderer = current.renderer()) {
SelectorFilterPusher selectorFilterPusher(scope().selectorFilter, current, SelectorFilterPusher::NoPush);
Modified: trunk/Source/WebCore/style/StyleTreeResolver.h (196668 => 196669)
--- 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)
@@ -67,7 +67,7 @@
void resolveBeforeOrAfterPseudoElement(Element&, Change, PseudoId, RenderTreePosition&);
void createRenderTreeRecursively(Element&, RenderStyle&, RenderTreePosition&, RefPtr<RenderStyle>&& resolvedStyle);
- void createRenderer(Element&, RenderStyle& inheritedStyle, RenderTreePosition&, RefPtr<RenderStyle>&& resolvedStyle);
+ void createRenderer(Element&, RenderTreePosition&, RefPtr<RenderStyle>&& resolvedStyle);
void createRenderTreeForBeforeOrAfterPseudoElement(Element&, PseudoId, RenderTreePosition&);
void createRenderTreeForChildren(ContainerNode&, RenderStyle&, RenderTreePosition&);
void createRenderTreeForShadowRoot(ShadowRoot&);
