- Revision
- 116827
- Author
- commit-qu...@webkit.org
- Date
- 2012-05-11 17:28:32 -0700 (Fri, 11 May 2012)
Log Message
use after free in WebCore::RenderObject::document
https://bugs.webkit.org/show_bug.cgi?id=84891
Patch by David Barton <dbar...@mathscribe.com> on 2012-05-11
Reviewed by Julien Chaffraix.
Source/WebCore:
Change RenderMathMLFenced::addChild() to use the beforeChild parameter. When beforeChild
is 0, insert child renderers before the closing fence, which might not be the same as
this->lastChild(), e.g. possibly due to anonymous blocks or generated content.
Tests: mathml/presentation/mfenced-add-child1-expected.html
mathml/presentation/mfenced-add-child1.html
mathml/presentation/mfenced-add-child2-expected.html
mathml/presentation/mfenced-add-child2.html
* rendering/mathml/RenderMathMLFenced.cpp:
(WebCore::RenderMathMLFenced::RenderMathMLFenced):
(WebCore::RenderMathMLFenced::makeFences):
(WebCore::RenderMathMLFenced::addChild):
* rendering/mathml/RenderMathMLFenced.h:
(RenderMathMLFenced):
LayoutTests:
* mathml/presentation/mfenced-add-child1-expected.html: Added.
* mathml/presentation/mfenced-add-child1.html: Added.
* mathml/presentation/mfenced-add-child2-expected.html: Added.
* mathml/presentation/mfenced-add-child2.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (116826 => 116827)
--- trunk/LayoutTests/ChangeLog 2012-05-12 00:17:45 UTC (rev 116826)
+++ trunk/LayoutTests/ChangeLog 2012-05-12 00:28:32 UTC (rev 116827)
@@ -1,3 +1,15 @@
+2012-05-11 David Barton <dbar...@mathscribe.com>
+
+ use after free in WebCore::RenderObject::document
+ https://bugs.webkit.org/show_bug.cgi?id=84891
+
+ Reviewed by Julien Chaffraix.
+
+ * mathml/presentation/mfenced-add-child1-expected.html: Added.
+ * mathml/presentation/mfenced-add-child1.html: Added.
+ * mathml/presentation/mfenced-add-child2-expected.html: Added.
+ * mathml/presentation/mfenced-add-child2.html: Added.
+
2012-05-10 Timothy Hatcher <timo...@apple.com>
Instrument timer function calls so they show up in the Web Inspector Timeline.
Added: trunk/LayoutTests/mathml/presentation/mfenced-add-child1-expected.html (0 => 116827)
--- trunk/LayoutTests/mathml/presentation/mfenced-add-child1-expected.html (rev 0)
+++ trunk/LayoutTests/mathml/presentation/mfenced-add-child1-expected.html 2012-05-12 00:28:32 UTC (rev 116827)
@@ -0,0 +1,9 @@
+<html>
+<body>
+<p>For <a href="" 84891</a> -
+mfenced add/remove child confusion.</p>
+
+<p>This test passes if the mfenced is completely removed from the render tree,
+so e.g. no parentheses or commas are showing after this paragraph.</p>
+</body>
+</html>
Added: trunk/LayoutTests/mathml/presentation/mfenced-add-child1.html (0 => 116827)
--- trunk/LayoutTests/mathml/presentation/mfenced-add-child1.html (rev 0)
+++ trunk/LayoutTests/mathml/presentation/mfenced-add-child1.html 2012-05-12 00:28:32 UTC (rev 116827)
@@ -0,0 +1,30 @@
+<html>
+<body>
+<p>For <a href="" 84891</a> -
+mfenced add/remove child confusion.</p>
+
+<p>This test passes if the mfenced is completely removed from the render tree,
+so e.g. no parentheses or commas are showing after this paragraph.</p>
+
+<math>
+ <mfenced id="mfenced">
+ <mi id="mi1">b</mi>
+ <mi id="mi2">c</mi>
+ </mfenced>
+</math>
+</body>
+
+<script>
+var mfenced = document.getElementById("mfenced");
+
+document.getElementById("mi1").appendChild(document.createElement("div"));
+mfenced.appendChild(document.createElement("span"));
+
+var head = document.getElementsByTagName("head")[0];
+var style = document.createElement("style");
+style.appendChild(document.createTextNode("script{}"));
+head.appendChild(style);
+
+mfenced.parentNode.removeChild(mfenced);
+</script>
+</html>
Added: trunk/LayoutTests/mathml/presentation/mfenced-add-child2-expected.html (0 => 116827)
--- trunk/LayoutTests/mathml/presentation/mfenced-add-child2-expected.html (rev 0)
+++ trunk/LayoutTests/mathml/presentation/mfenced-add-child2-expected.html 2012-05-12 00:28:32 UTC (rev 116827)
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>For <a href="" 84891</a> -
+mfenced add/remove child confusion.</p>
+
+<p>This test passes if the math element is changed to (x1y).</p>
+
+<math>
+ <mfenced id="mfenced" separators="1">
+ <mi id="mi1">x</mi>
+ <mi id="mi2">y</mi>
+ </mfenced>
+</math>
+</body>
+</html>
Added: trunk/LayoutTests/mathml/presentation/mfenced-add-child2.html (0 => 116827)
--- trunk/LayoutTests/mathml/presentation/mfenced-add-child2.html (rev 0)
+++ trunk/LayoutTests/mathml/presentation/mfenced-add-child2.html 2012-05-12 00:28:32 UTC (rev 116827)
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>For <a href="" 84891</a> -
+mfenced add/remove child confusion.</p>
+
+<p>This test passes if the math element is changed to (x1y).</p>
+
+<math>
+ <mfenced id="mfenced" separators="12"></mfenced>
+ <mi id="mi1">x</mi>
+ <mi id="mi2">y</mi>
+</math>
+
+<script>
+var mfenced = document.getElementById("mfenced");
+var mi1 = document.getElementById("mi1"), mi2 = document.getElementById("mi2");
+mfenced.appendChild(mi2);
+mfenced.insertBefore(mi1, mi2);
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (116826 => 116827)
--- trunk/Source/WebCore/ChangeLog 2012-05-12 00:17:45 UTC (rev 116826)
+++ trunk/Source/WebCore/ChangeLog 2012-05-12 00:28:32 UTC (rev 116827)
@@ -1,3 +1,26 @@
+2012-05-11 David Barton <dbar...@mathscribe.com>
+
+ use after free in WebCore::RenderObject::document
+ https://bugs.webkit.org/show_bug.cgi?id=84891
+
+ Reviewed by Julien Chaffraix.
+
+ Change RenderMathMLFenced::addChild() to use the beforeChild parameter. When beforeChild
+ is 0, insert child renderers before the closing fence, which might not be the same as
+ this->lastChild(), e.g. possibly due to anonymous blocks or generated content.
+
+ Tests: mathml/presentation/mfenced-add-child1-expected.html
+ mathml/presentation/mfenced-add-child1.html
+ mathml/presentation/mfenced-add-child2-expected.html
+ mathml/presentation/mfenced-add-child2.html
+
+ * rendering/mathml/RenderMathMLFenced.cpp:
+ (WebCore::RenderMathMLFenced::RenderMathMLFenced):
+ (WebCore::RenderMathMLFenced::makeFences):
+ (WebCore::RenderMathMLFenced::addChild):
+ * rendering/mathml/RenderMathMLFenced.h:
+ (RenderMathMLFenced):
+
2012-05-11 Anders Carlsson <ander...@apple.com>
Can't scroll on webpage after following links from Blogger
Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLFenced.cpp (116826 => 116827)
--- trunk/Source/WebCore/rendering/mathml/RenderMathMLFenced.cpp 2012-05-12 00:17:45 UTC (rev 116826)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLFenced.cpp 2012-05-12 00:28:32 UTC (rev 116827)
@@ -48,6 +48,7 @@
: RenderMathMLRow(element)
, m_open(OpeningBraceChar)
, m_close(ClosingBraceChar)
+ , m_closeFenceRenderer(0)
{
}
@@ -94,49 +95,67 @@
RenderObject* openFence = new (renderArena()) RenderMathMLOperator(node(), m_open);
openFence->setStyle(createOperatorStyle());
RenderBlock::addChild(openFence, firstChild());
- RenderObject* closeFence = new (renderArena()) RenderMathMLOperator(node(), m_close);
- closeFence->setStyle(createOperatorStyle());
- RenderBlock::addChild(closeFence);
+ m_closeFenceRenderer = new (renderArena()) RenderMathMLOperator(node(), m_close);
+ m_closeFenceRenderer->setStyle(createOperatorStyle());
+ RenderBlock::addChild(m_closeFenceRenderer);
}
-void RenderMathMLFenced::addChild(RenderObject* child, RenderObject*)
+void RenderMathMLFenced::addChild(RenderObject* child, RenderObject* beforeChild)
{
// make the fences if the render object is empty
if (isEmpty())
updateFromElement();
+ // FIXME: Adding or removing a child should possibly cause all later separators to shift places if they're different,
+ // as later child positions change by +1 or -1.
+
+ RenderObject* separatorRenderer = 0;
if (m_separators.get()) {
unsigned int count = 0;
for (Node* position = child->node(); position; position = position->previousSibling()) {
- if (position->nodeType() == Node::ELEMENT_NODE)
+ if (position->isElementNode())
count++;
}
-
- if (count > 1) {
+ if (!beforeChild) {
+ // We're adding at the end (before the closing fence), so a new separator would go before the new child, not after it.
+ --count;
+ }
+ // |count| is now the number of element children that will be before our new separator, i.e. it's the 1-based index of the separator.
+
+ if (count > 0) {
UChar separator;
// Use the last separator if we've run out of specified separators.
- if ((count - 1) >= m_separators.get()->length())
+ if (count > m_separators.get()->length())
separator = (*m_separators.get())[m_separators.get()->length() - 1];
else
- separator = (*m_separators.get())[count - 2];
+ separator = (*m_separators.get())[count - 1];
- RenderObject* separatorObj = new (renderArena()) RenderMathMLOperator(node(), separator);
- separatorObj->setStyle(createOperatorStyle());
- RenderBlock::addChild(separatorObj, lastChild());
+ separatorRenderer = new (renderArena()) RenderMathMLOperator(node(), separator);
+ separatorRenderer->setStyle(createOperatorStyle());
}
}
// If we have a block, we'll wrap it in an inline-block.
if (child->isBlockFlow() && child->style()->display() != INLINE_BLOCK) {
// Block objects wrapper.
-
RenderBlock* block = createAlmostAnonymousBlock(INLINE_BLOCK);
- RenderBlock::addChild(block, lastChild());
- block->addChild(child);
- } else
- RenderBlock::addChild(child, lastChild());
+ block->addChild(child);
+ child = block;
+ }
+
+ if (beforeChild) {
+ // Adding |x| before an existing |y| e.g. in element (y) - first insert our new child |x|, then its separator, to get (x, y).
+ RenderBlock::addChild(child, beforeChild);
+ if (separatorRenderer)
+ RenderBlock::addChild(separatorRenderer, beforeChild);
+ } else {
+ // Adding |y| at the end of an existing element e.g. (x) - insert the separator first before the closing fence, then |y|, to get (x, y).
+ if (separatorRenderer)
+ RenderBlock::addChild(separatorRenderer, m_closeFenceRenderer);
+ RenderBlock::addChild(child, m_closeFenceRenderer);
+ }
}
}
Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLFenced.h (116826 => 116827)
--- trunk/Source/WebCore/rendering/mathml/RenderMathMLFenced.h 2012-05-12 00:17:45 UTC (rev 116826)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLFenced.h 2012-05-12 00:28:32 UTC (rev 116827)
@@ -47,6 +47,8 @@
UChar m_open;
UChar m_close;
RefPtr<StringImpl> m_separators;
+
+ RenderObject* m_closeFenceRenderer;
};
}