<!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>[179691] 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/179691">179691</a></dd>
<dt>Author</dt> <dd>zalan@apple.com</dd>
<dt>Date</dt> <dd>2015-02-05 08:50:44 -0800 (Thu, 05 Feb 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>Do not destroy RenderQuote's text fragment child when quotation mark string is changing.
https://bugs.webkit.org/show_bug.cgi?id=141271
rdar://problem/18169375
Reviewed by Antti Koivisto.
Similar approach as https://codereview.chromium.org/679593004/
This patch ensures that laying out a RenderQuote does not force a sibling RenderQuote's
child renderer(RenderText) to be destroyed.
BreakingContext holds a pointer to the next renderer on the line (BreakingContext::m_nextObject).
While laying out the line, initiated by BreakingContext, placing the current renderer could end up destroying the "next" renderer.
This happens when the pseudo after quotation mark(RenderQuote) becomes floated, the sibling <q>'s pseudo
before text needs to be changed (from " to ') so that we don't end up with 2 sets of the same opening
strings.
The fix is to reuse the RenderTextFragment object instead of destroy/recreate it.
Source/WebCore:
Test: fast/css/content/quote-crash-when-floating.html
* rendering/RenderQuote.cpp:
(WebCore::RenderQuote::RenderQuote):
(WebCore::fragmentChild):
(WebCore::RenderQuote::updateText):
* rendering/RenderQuote.h:
* rendering/RenderTextFragment.cpp:
(WebCore::RenderTextFragment::setText):
(WebCore::RenderTextFragment::setContentString):
* rendering/RenderTextFragment.h:
LayoutTests:
* fast/css/content/quote-crash-when-floating-expected.txt: Added.
* fast/css/content/quote-crash-when-floating.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="#trunkSourceWebCorerenderingRenderQuotecpp">trunk/Source/WebCore/rendering/RenderQuote.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderQuoteh">trunk/Source/WebCore/rendering/RenderQuote.h</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderTextFragmentcpp">trunk/Source/WebCore/rendering/RenderTextFragment.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderTextFragmenth">trunk/Source/WebCore/rendering/RenderTextFragment.h</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastcsscontentquotecrashwhenfloatingexpectedtxt">trunk/LayoutTests/fast/css/content/quote-crash-when-floating-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastcsscontentquotecrashwhenfloatinghtml">trunk/LayoutTests/fast/css/content/quote-crash-when-floating.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (179690 => 179691)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2015-02-05 14:31:07 UTC (rev 179690)
+++ trunk/LayoutTests/ChangeLog 2015-02-05 16:50:44 UTC (rev 179691)
</span><span class="lines">@@ -1,3 +1,25 @@
</span><ins>+2015-02-05 Zalan Bujtas <zalan@apple.com>
+
+ Do not destroy RenderQuote's text fragment child when quotation mark string is changing.
+ https://bugs.webkit.org/show_bug.cgi?id=141271
+ rdar://problem/18169375
+
+ Reviewed by Antti Koivisto.
+
+ Similar approach as https://codereview.chromium.org/679593004/
+
+ This patch ensures that laying out a RenderQuote does not force a sibling RenderQuote's
+ child renderer(RenderText) to be destroyed.
+ BreakingContext holds a pointer to the next renderer on the line (BreakingContext::m_nextObject).
+ While laying out the line, initiated by BreakingContext, placing the current renderer could end up destroying the "next" renderer.
+ This happens when the pseudo after quotation mark(RenderQuote) becomes floated, the sibling <q>'s pseudo
+ before text needs to be changed (from " to ') so that we don't end up with 2 sets of the same opening
+ strings.
+ The fix is to reuse the RenderTextFragment object instead of destroy/recreate it.
+
+ * fast/css/content/quote-crash-when-floating-expected.txt: Added.
+ * fast/css/content/quote-crash-when-floating.html: Added.
+
</ins><span class="cx"> 2015-02-05 Gyuyoung Kim <gyuyoung.kim@samsung.com>
</span><span class="cx">
</span><span class="cx"> Unreviewed EFL gardening. Set all tests of svg/W3C-SVG-1.1 and svg/W3C-SVG-1.1-SE to flaky.
</span></span></pre></div>
<a id="trunkLayoutTestsfastcsscontentquotecrashwhenfloatingexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css/content/quote-crash-when-floating-expected.txt (0 => 179691)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/css/content/quote-crash-when-floating-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/css/content/quote-crash-when-floating-expected.txt 2015-02-05 16:50:44 UTC (rev 179691)
</span><span class="lines">@@ -0,0 +1 @@
</span><ins>+PASS if no crash or assert.
</ins></span></pre></div>
<a id="trunkLayoutTestsfastcsscontentquotecrashwhenfloatinghtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css/content/quote-crash-when-floating.html (0 => 179691)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/css/content/quote-crash-when-floating.html (rev 0)
+++ trunk/LayoutTests/fast/css/content/quote-crash-when-floating.html 2015-02-05 16:50:44 UTC (rev 179691)
</span><span class="lines">@@ -0,0 +1,19 @@
</span><ins>+<!DOCTYPE html>
+<html>
+<title>This tests that quotes with overflow and float do not crash.</title>
+<style>
+ q::after {
+ overflow-y: -webkit-paged-x;
+ float: right;
+ }
+</style>
+</head>
+<body>
+PASS if no crash or assert.
+<q></q><q></q>
+<script>
+ if (window.testRunner)
+ testRunner.dumpAsText();
+</script>
+</body>
+</html>
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (179690 => 179691)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2015-02-05 14:31:07 UTC (rev 179690)
+++ trunk/Source/WebCore/ChangeLog 2015-02-05 16:50:44 UTC (rev 179691)
</span><span class="lines">@@ -1,3 +1,34 @@
</span><ins>+2015-02-05 Zalan Bujtas <zalan@apple.com>
+
+ Do not destroy RenderQuote's text fragment child when quotation mark string is changing.
+ https://bugs.webkit.org/show_bug.cgi?id=141271
+ rdar://problem/18169375
+
+ Reviewed by Antti Koivisto.
+
+ Similar approach as https://codereview.chromium.org/679593004/
+
+ This patch ensures that laying out a RenderQuote does not force a sibling RenderQuote's
+ child renderer(RenderText) to be destroyed.
+ BreakingContext holds a pointer to the next renderer on the line (BreakingContext::m_nextObject).
+ While laying out the line, initiated by BreakingContext, placing the current renderer could end up destroying the "next" renderer.
+ This happens when the pseudo after quotation mark(RenderQuote) becomes floated, the sibling <q>'s pseudo
+ before text needs to be changed (from " to ') so that we don't end up with 2 sets of the same opening
+ strings.
+ The fix is to reuse the RenderTextFragment object instead of destroy/recreate it.
+
+ Test: fast/css/content/quote-crash-when-floating.html
+
+ * rendering/RenderQuote.cpp:
+ (WebCore::RenderQuote::RenderQuote):
+ (WebCore::fragmentChild):
+ (WebCore::RenderQuote::updateText):
+ * rendering/RenderQuote.h:
+ * rendering/RenderTextFragment.cpp:
+ (WebCore::RenderTextFragment::setText):
+ (WebCore::RenderTextFragment::setContentString):
+ * rendering/RenderTextFragment.h:
+
</ins><span class="cx"> 2015-02-04 Dean Jackson <dino@apple.com>
</span><span class="cx">
</span><span class="cx"> [Media iOS] Add a debug setting to always show the optimized fullscreen button
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderQuotecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderQuote.cpp (179690 => 179691)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderQuote.cpp 2015-02-05 14:31:07 UTC (rev 179690)
+++ trunk/Source/WebCore/rendering/RenderQuote.cpp 2015-02-05 16:50:44 UTC (rev 179691)
</span><span class="lines">@@ -34,10 +34,7 @@
</span><span class="cx"> RenderQuote::RenderQuote(Document& document, Ref<RenderStyle>&& style, QuoteType quote)
</span><span class="cx"> : RenderInline(document, WTF::move(style))
</span><span class="cx"> , m_type(quote)
</span><del>- , m_depth(-1)
- , m_next(0)
- , m_previous(0)
- , m_isAttached(false)
</del><ins>+ , m_text(emptyString())
</ins><span class="cx"> {
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -333,24 +330,30 @@
</span><span class="cx"> return apostropheString;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+static RenderTextFragment* fragmentChild(RenderObject* lastChild)
+{
+ if (!lastChild)
+ return nullptr;
+
+ if (!is<RenderTextFragment>(lastChild))
+ return nullptr;
+
+ return downcast<RenderTextFragment>(lastChild);
+}
+
</ins><span class="cx"> void RenderQuote::updateText()
</span><span class="cx"> {
</span><span class="cx"> String text = computeText();
</span><span class="cx"> if (m_text == text)
</span><span class="cx"> return;
</span><del>-
- while (RenderObject* child = lastChild())
- child->destroy();
-
- if (text == emptyString() || text == String()) {
- m_text = String();
</del><ins>+ m_text = text;
+ // Start from the end of the child list because, if we've had a first-letter
+ // renderer inserted then the remaining text will be at the end.
+ if (auto* fragment = fragmentChild(lastChild())) {
+ fragment->setContentString(m_text);
</ins><span class="cx"> return;
</span><span class="cx"> }
</span><del>-
- m_text = text;
-
- RenderTextFragment* fragment = new RenderTextFragment(document(), m_text.impl());
- addChild(fragment);
</del><ins>+ addChild(new RenderTextFragment(document(), m_text));
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> String RenderQuote::computeText() const
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderQuoteh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderQuote.h (179690 => 179691)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderQuote.h 2015-02-05 14:31:07 UTC (rev 179690)
+++ trunk/Source/WebCore/rendering/RenderQuote.h 2015-02-05 16:50:44 UTC (rev 179691)
</span><span class="lines">@@ -46,11 +46,11 @@
</span><span class="cx"> void updateText();
</span><span class="cx"> void updateDepth();
</span><span class="cx">
</span><del>- QuoteType m_type;
- int m_depth;
- RenderQuote* m_next;
- RenderQuote* m_previous;
- bool m_isAttached;
</del><ins>+ const QuoteType m_type;
+ int m_depth { -1 };
+ RenderQuote* m_next { nullptr };
+ RenderQuote* m_previous { nullptr };
+ bool m_isAttached { false };
</ins><span class="cx"> String m_text;
</span><span class="cx"> };
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderTextFragmentcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderTextFragment.cpp (179690 => 179691)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderTextFragment.cpp 2015-02-05 14:31:07 UTC (rev 179690)
+++ trunk/Source/WebCore/rendering/RenderTextFragment.cpp 2015-02-05 16:50:44 UTC (rev 179691)
</span><span class="lines">@@ -88,7 +88,6 @@
</span><span class="cx"> m_end = textLength();
</span><span class="cx"> if (!m_firstLetter)
</span><span class="cx"> return;
</span><del>- ASSERT(!m_contentString);
</del><span class="cx"> m_firstLetter->destroy();
</span><span class="cx"> m_firstLetter = 0;
</span><span class="cx"> if (!textNode())
</span><span class="lines">@@ -119,4 +118,10 @@
</span><span class="cx"> return nullptr;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+void RenderTextFragment::setContentString(const String& text)
+{
+ m_contentString = text;
+ setText(text);
+}
+
</ins><span class="cx"> } // namespace WebCore
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderTextFragmenth"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderTextFragment.h (179690 => 179691)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderTextFragment.h 2015-02-05 14:31:07 UTC (rev 179690)
+++ trunk/Source/WebCore/rendering/RenderTextFragment.h 2015-02-05 16:50:44 UTC (rev 179691)
</span><span class="lines">@@ -49,6 +49,7 @@
</span><span class="cx">
</span><span class="cx"> RenderBlock* blockForAccompanyingFirstLetter();
</span><span class="cx">
</span><ins>+ void setContentString(const String& text);
</ins><span class="cx"> StringImpl* contentString() const { return m_contentString.impl(); }
</span><span class="cx">
</span><span class="cx"> virtual void setText(const String&, bool force = false) override;
</span></span></pre>
</div>
</div>
</body>
</html>