- Revision
- 127381
- Author
- [email protected]
- Date
- 2012-09-01 12:46:16 -0700 (Sat, 01 Sep 2012)
Log Message
Regression(r126763): Heap-use-after-free in WebCore::nextBreakablePosition
https://bugs.webkit.org/show_bug.cgi?id=95229
Patch by Ned Holbrook <[email protected]> on 2012-09-01
Reviewed by Dan Bernstein.
Source/WebCore:
TextLayout and LazyLineBreakIterator cache a RenderText's string during line breaking but RenderCounter
and RenderQuote can replace that string during preferred width calculation. This patch adds a non-virtual member
function to RenderText named updateTextIfNeeded() that triggers immediate string replacement by calling
the new virtual updateText() if necessary, which in turn calls computePreferredLogicalWidths(). In this way
RenderBlock::LineBreaker::nextLineBreak() can ensure a RenderText's string is current before caching it.
Test: fast/css/content/content-quotes-crash.html
* rendering/RenderBlockLineLayout.cpp:
(WebCore::dirtyLineBoxesForRenderer): Drive-by: replace existing code with the equivalent updateTextIfNeeded().
(WebCore::RenderBlock::LineBreaker::nextLineBreak): Use updateTextIfNeeded() prior to caching RenderText string.
* rendering/RenderCounter.cpp:
(WebCore::RenderCounter::updateText): Call computePreferredLogicalWidths().
(WebCore):
* rendering/RenderCounter.h:
(RenderCounter):
* rendering/RenderQuote.cpp:
(WebCore::RenderQuote::updateText): Call computePreferredLogicalWidths().
(WebCore):
* rendering/RenderQuote.h:
(RenderQuote):
* rendering/RenderText.h:
(WebCore::RenderText::updateTextIfNeeded): Only call virtual updateText() if necessary.
(RenderText):
(WebCore::RenderText::updateText): Add no-op default implementation for new virtual member function.
LayoutTests:
Ensure line breaking doesn't crash due to text of RenderText being changed.
* fast/css/content/content-quotes-crash-expected.txt: Added.
* fast/css/content/content-quotes-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (127380 => 127381)
--- trunk/LayoutTests/ChangeLog 2012-09-01 19:42:46 UTC (rev 127380)
+++ trunk/LayoutTests/ChangeLog 2012-09-01 19:46:16 UTC (rev 127381)
@@ -1,3 +1,15 @@
+2012-09-01 Ned Holbrook <[email protected]>
+
+ Regression(r126763): Heap-use-after-free in WebCore::nextBreakablePosition
+ https://bugs.webkit.org/show_bug.cgi?id=95229
+
+ Reviewed by Dan Bernstein.
+
+ Ensure line breaking doesn't crash due to text of RenderText being changed.
+
+ * fast/css/content/content-quotes-crash-expected.txt: Added.
+ * fast/css/content/content-quotes-crash.html: Added.
+
2012-09-01 Li Yin <[email protected]>
fast/events/message-port-clone.html hits ASSERT in Debug (usually in later tests)
Added: trunk/LayoutTests/fast/css/content/content-quotes-crash-expected.txt (0 => 127381)
--- trunk/LayoutTests/fast/css/content/content-quotes-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/css/content/content-quotes-crash-expected.txt 2012-09-01 19:46:16 UTC (rev 127381)
@@ -0,0 +1,3 @@
+PASS: WebKit didn't crash.
+
+a aA
Added: trunk/LayoutTests/fast/css/content/content-quotes-crash.html (0 => 127381)
--- trunk/LayoutTests/fast/css/content/content-quotes-crash.html (rev 0)
+++ trunk/LayoutTests/fast/css/content/content-quotes-crash.html 2012-09-01 19:46:16 UTC (rev 127381)
@@ -0,0 +1,16 @@
+<html>
+<head>
+ <title></title>
+ <script>
+ if (window.testRunner)
+ testRunner.dumpAsText();
+ </script>
+</head>
+<body>
+<p>
+ PASS: WebKit didn't crash.
+</p>
+<ruby><q style="column-gap:2;">a</ruby>
+ <cite style="word-break: break-all;">a<q style="text-transform:uppercase;">a<sup style="text-overflow:ellipsis;">
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (127380 => 127381)
--- trunk/Source/WebCore/ChangeLog 2012-09-01 19:42:46 UTC (rev 127380)
+++ trunk/Source/WebCore/ChangeLog 2012-09-01 19:46:16 UTC (rev 127381)
@@ -1,3 +1,36 @@
+2012-09-01 Ned Holbrook <[email protected]>
+
+ Regression(r126763): Heap-use-after-free in WebCore::nextBreakablePosition
+ https://bugs.webkit.org/show_bug.cgi?id=95229
+
+ Reviewed by Dan Bernstein.
+
+ TextLayout and LazyLineBreakIterator cache a RenderText's string during line breaking but RenderCounter
+ and RenderQuote can replace that string during preferred width calculation. This patch adds a non-virtual member
+ function to RenderText named updateTextIfNeeded() that triggers immediate string replacement by calling
+ the new virtual updateText() if necessary, which in turn calls computePreferredLogicalWidths(). In this way
+ RenderBlock::LineBreaker::nextLineBreak() can ensure a RenderText's string is current before caching it.
+
+ Test: fast/css/content/content-quotes-crash.html
+
+ * rendering/RenderBlockLineLayout.cpp:
+ (WebCore::dirtyLineBoxesForRenderer): Drive-by: replace existing code with the equivalent updateTextIfNeeded().
+ (WebCore::RenderBlock::LineBreaker::nextLineBreak): Use updateTextIfNeeded() prior to caching RenderText string.
+ * rendering/RenderCounter.cpp:
+ (WebCore::RenderCounter::updateText): Call computePreferredLogicalWidths().
+ (WebCore):
+ * rendering/RenderCounter.h:
+ (RenderCounter):
+ * rendering/RenderQuote.cpp:
+ (WebCore::RenderQuote::updateText): Call computePreferredLogicalWidths().
+ (WebCore):
+ * rendering/RenderQuote.h:
+ (RenderQuote):
+ * rendering/RenderText.h:
+ (WebCore::RenderText::updateTextIfNeeded): Only call virtual updateText() if necessary.
+ (RenderText):
+ (WebCore::RenderText::updateText): Add no-op default implementation for new virtual member function.
+
2012-09-01 Li Yin <[email protected]>
fast/events/message-port-clone.html hits ASSERT in Debug (usually in later tests)
Modified: trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp (127380 => 127381)
--- trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp 2012-09-01 19:42:46 UTC (rev 127380)
+++ trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp 2012-09-01 19:46:16 UTC (rev 127381)
@@ -422,9 +422,9 @@
static inline void dirtyLineBoxesForRenderer(RenderObject* o, bool fullLayout)
{
if (o->isText()) {
- if (o->preferredLogicalWidthsDirty() && (o->isCounter() || o->isQuote()))
- toRenderText(o)->computePreferredLogicalWidths(0); // FIXME: Counters depend on this hack. No clue why. Should be investigated and removed.
- toRenderText(o)->dirtyLineBoxes(fullLayout);
+ RenderText* renderText = toRenderText(o);
+ renderText->updateTextIfNeeded(); // FIXME: Counters depend on this hack. No clue why. Should be investigated and removed.
+ renderText->dirtyLineBoxes(fullLayout);
} else
toRenderInline(o)->dirtyLineBoxes(fullLayout);
}
@@ -2433,7 +2433,8 @@
ASSERT(current.m_pos == t->textLength());
}
- if (renderTextInfo.m_text != t || renderTextInfo.m_lineBreakIterator.string() != t->characters()) {
+ if (renderTextInfo.m_text != t) {
+ t->updateTextIfNeeded();
renderTextInfo.m_text = t;
renderTextInfo.m_layout = f.createLayout(t, width.currentWidth(), collapseWhiteSpace);
renderTextInfo.m_lineBreakIterator.reset(t->characters(), t->textLength(), style->locale());
Modified: trunk/Source/WebCore/rendering/RenderCounter.cpp (127380 => 127381)
--- trunk/Source/WebCore/rendering/RenderCounter.cpp 2012-09-01 19:42:46 UTC (rev 127380)
+++ trunk/Source/WebCore/rendering/RenderCounter.cpp 2012-09-01 19:46:16 UTC (rev 127381)
@@ -538,6 +538,11 @@
return text.impl();
}
+void RenderCounter::updateText()
+{
+ computePreferredLogicalWidths(0);
+}
+
void RenderCounter::computePreferredLogicalWidths(float lead)
{
setTextInternal(originalText());
Modified: trunk/Source/WebCore/rendering/RenderCounter.h (127380 => 127381)
--- trunk/Source/WebCore/rendering/RenderCounter.h 2012-09-01 19:42:46 UTC (rev 127380)
+++ trunk/Source/WebCore/rendering/RenderCounter.h 2012-09-01 19:46:16 UTC (rev 127381)
@@ -48,6 +48,7 @@
virtual bool isCounter() const;
virtual PassRefPtr<StringImpl> originalText() const;
+ virtual void updateText() OVERRIDE;
virtual void computePreferredLogicalWidths(float leadWidth);
// Removes the reference to the CounterNode associated with this renderer.
Modified: trunk/Source/WebCore/rendering/RenderQuote.cpp (127380 => 127381)
--- trunk/Source/WebCore/rendering/RenderQuote.cpp 2012-09-01 19:42:46 UTC (rev 127380)
+++ trunk/Source/WebCore/rendering/RenderQuote.cpp 2012-09-01 19:46:16 UTC (rev 127381)
@@ -243,6 +243,11 @@
return StringImpl::empty();
}
+void RenderQuote::updateText()
+{
+ computePreferredLogicalWidths(0);
+}
+
void RenderQuote::computePreferredLogicalWidths(float lead)
{
if (!m_attached)
Modified: trunk/Source/WebCore/rendering/RenderQuote.h (127380 => 127381)
--- trunk/Source/WebCore/rendering/RenderQuote.h 2012-09-01 19:42:46 UTC (rev 127380)
+++ trunk/Source/WebCore/rendering/RenderQuote.h 2012-09-01 19:46:16 UTC (rev 127381)
@@ -43,6 +43,8 @@
virtual const char* renderName() const OVERRIDE { return "RenderQuote"; };
virtual bool isQuote() const OVERRIDE { return true; };
virtual PassRefPtr<StringImpl> originalText() const OVERRIDE;
+
+ virtual void updateText() OVERRIDE;
virtual void computePreferredLogicalWidths(float leadWidth) OVERRIDE;
// We don't override insertedIntoTree to call attachQuote() as it would be attached
Modified: trunk/Source/WebCore/rendering/RenderText.h (127380 => 127381)
--- trunk/Source/WebCore/rendering/RenderText.h 2012-09-01 19:42:46 UTC (rev 127380)
+++ trunk/Source/WebCore/rendering/RenderText.h 2012-09-01 19:46:16 UTC (rev 127381)
@@ -45,6 +45,12 @@
virtual PassRefPtr<StringImpl> originalText() const;
+ void updateTextIfNeeded()
+ {
+ if (preferredLogicalWidthsDirty())
+ updateText();
+ }
+
void extractTextBox(InlineTextBox*);
void attachTextBox(InlineTextBox*);
void removeTextBox(InlineTextBox*);
@@ -139,6 +145,7 @@
virtual void styleWillChange(StyleDifference, const RenderStyle*) { }
virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle);
+ virtual void updateText() { }
virtual void setTextInternal(PassRefPtr<StringImpl>);
virtual UChar previousCharacter() const;
