- Revision
- 179691
- Author
- [email protected]
- Date
- 2015-02-05 08:50:44 -0800 (Thu, 05 Feb 2015)
Log Message
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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (179690 => 179691)
--- trunk/LayoutTests/ChangeLog 2015-02-05 14:31:07 UTC (rev 179690)
+++ trunk/LayoutTests/ChangeLog 2015-02-05 16:50:44 UTC (rev 179691)
@@ -1,3 +1,25 @@
+2015-02-05 Zalan Bujtas <[email protected]>
+
+ 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.
+
2015-02-05 Gyuyoung Kim <[email protected]>
Unreviewed EFL gardening. Set all tests of svg/W3C-SVG-1.1 and svg/W3C-SVG-1.1-SE to flaky.
Added: trunk/LayoutTests/fast/css/content/quote-crash-when-floating-expected.txt (0 => 179691)
--- 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)
@@ -0,0 +1 @@
+PASS if no crash or assert.
Added: trunk/LayoutTests/fast/css/content/quote-crash-when-floating.html (0 => 179691)
--- 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)
@@ -0,0 +1,19 @@
+<!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>
Modified: trunk/Source/WebCore/ChangeLog (179690 => 179691)
--- 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)
@@ -1,3 +1,34 @@
+2015-02-05 Zalan Bujtas <[email protected]>
+
+ 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:
+
2015-02-04 Dean Jackson <[email protected]>
[Media iOS] Add a debug setting to always show the optimized fullscreen button
Modified: trunk/Source/WebCore/rendering/RenderQuote.cpp (179690 => 179691)
--- 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)
@@ -34,10 +34,7 @@
RenderQuote::RenderQuote(Document& document, Ref<RenderStyle>&& style, QuoteType quote)
: RenderInline(document, WTF::move(style))
, m_type(quote)
- , m_depth(-1)
- , m_next(0)
- , m_previous(0)
- , m_isAttached(false)
+ , m_text(emptyString())
{
}
@@ -333,24 +330,30 @@
return apostropheString;
}
+static RenderTextFragment* fragmentChild(RenderObject* lastChild)
+{
+ if (!lastChild)
+ return nullptr;
+
+ if (!is<RenderTextFragment>(lastChild))
+ return nullptr;
+
+ return downcast<RenderTextFragment>(lastChild);
+}
+
void RenderQuote::updateText()
{
String text = computeText();
if (m_text == text)
return;
-
- while (RenderObject* child = lastChild())
- child->destroy();
-
- if (text == emptyString() || text == String()) {
- m_text = String();
+ 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);
return;
}
-
- m_text = text;
-
- RenderTextFragment* fragment = new RenderTextFragment(document(), m_text.impl());
- addChild(fragment);
+ addChild(new RenderTextFragment(document(), m_text));
}
String RenderQuote::computeText() const
Modified: trunk/Source/WebCore/rendering/RenderQuote.h (179690 => 179691)
--- 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)
@@ -46,11 +46,11 @@
void updateText();
void updateDepth();
- QuoteType m_type;
- int m_depth;
- RenderQuote* m_next;
- RenderQuote* m_previous;
- bool m_isAttached;
+ const QuoteType m_type;
+ int m_depth { -1 };
+ RenderQuote* m_next { nullptr };
+ RenderQuote* m_previous { nullptr };
+ bool m_isAttached { false };
String m_text;
};
Modified: trunk/Source/WebCore/rendering/RenderTextFragment.cpp (179690 => 179691)
--- 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)
@@ -88,7 +88,6 @@
m_end = textLength();
if (!m_firstLetter)
return;
- ASSERT(!m_contentString);
m_firstLetter->destroy();
m_firstLetter = 0;
if (!textNode())
@@ -119,4 +118,10 @@
return nullptr;
}
+void RenderTextFragment::setContentString(const String& text)
+{
+ m_contentString = text;
+ setText(text);
+}
+
} // namespace WebCore
Modified: trunk/Source/WebCore/rendering/RenderTextFragment.h (179690 => 179691)
--- 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)
@@ -49,6 +49,7 @@
RenderBlock* blockForAccompanyingFirstLetter();
+ void setContentString(const String& text);
StringImpl* contentString() const { return m_contentString.impl(); }
virtual void setText(const String&, bool force = false) override;
