ASSERT_WITH_SECURITY_IMPLICATION in WebCore::InlineTextBox::paint
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Jan 2014 23:36:04 +0000 (23:36 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Jan 2014 23:36:04 +0000 (23:36 +0000)
https://bugs.webkit.org/show_bug.cgi?id=114586

Reviewed by Dave Hyatt.

Taken mostly from https://chromium.googlesource.com/chromium/blink/+/cb2297db16f2e9328cb4dd8b552093d6b22340a8

If RenderQuote is a subclass of RenderObject, it can't be split by the first-letter CSS pseudoclass.
Instead, we should make it a subclass of RenderElement, so that it can be split properly.

Source/WebCore:

Test: fast/css-generated-content/quote-first-letter.html

* dom/PseudoElement.cpp:
(WebCore::PseudoElement::didRecalcStyle):
* rendering/RenderQuote.cpp:
(WebCore::RenderQuote::RenderQuote):
(WebCore::RenderQuote::willBeDestroyed):
(WebCore::RenderQuote::willBeRemovedFromTree):
(WebCore::RenderQuote::styleDidChange):
(WebCore::RenderQuote::updateText):
(WebCore::RenderQuote::computeText):
(WebCore::RenderQuote::updateDepth):
* rendering/RenderQuote.h:
* rendering/style/ContentData.cpp:
(WebCore::QuoteContentData::createContentRenderer):

LayoutTests:

This adds a test to make sure that the splitting behavior occurs, as well as updates existing tests that
didn't use the splitting behavior.

* fast/css-generated-content/quote-first-letter-expected.html: Added.
* fast/css-generated-content/quote-first-letter.html: Added.
* platform/mac/fast/css-generated-content/005-expected.txt:
* platform/mac/fast/css-generated-content/beforeAfter-interdocument-expected.txt:
* platform/mac/fast/css-generated-content/nested-tables-with-before-after-content-crash-expected.txt:
* platform/mac/fast/css-generated-content/no-openclose-quote-expected.txt:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@162972 268f45cc-cd09-0410-ab3c-d52691b4dbfc

16 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/css-generated-content/quote-first-letter-expected.html [new file with mode: 0644]
LayoutTests/fast/css-generated-content/quote-first-letter.html [new file with mode: 0644]
LayoutTests/platform/mac/fast/css-generated-content/005-expected.txt
LayoutTests/platform/mac/fast/css-generated-content/beforeAfter-interdocument-expected.txt
LayoutTests/platform/mac/fast/css-generated-content/nested-tables-with-before-after-content-crash-expected.txt
LayoutTests/platform/mac/fast/css-generated-content/no-openclose-quote-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/dom/PseudoElement.cpp
Source/WebCore/rendering/RenderImage.cpp
Source/WebCore/rendering/RenderImage.h
Source/WebCore/rendering/RenderQuote.cpp
Source/WebCore/rendering/RenderQuote.h
Source/WebCore/rendering/style/ContentData.cpp
Source/WebCore/rendering/style/RenderStyle.cpp
Source/WebCore/rendering/style/RenderStyle.h

index b287e54..8f9e213 100644 (file)
@@ -1,3 +1,25 @@
+2014-01-23  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        ASSERT_WITH_SECURITY_IMPLICATION in WebCore::InlineTextBox::paint
+        https://bugs.webkit.org/show_bug.cgi?id=114586
+
+        Reviewed by Dave Hyatt.
+
+        Taken mostly from https://chromium.googlesource.com/chromium/blink/+/cb2297db16f2e9328cb4dd8b552093d6b22340a8
+
+        If RenderQuote is a subclass of RenderObject, it can't be split by the first-letter CSS pseudoclass.
+        Instead, we should make it a subclass of RenderElement, so that it can be split properly.
+
+        This adds a test to make sure that the splitting behavior occurs, as well as updates existing tests that
+        didn't use the splitting behavior.
+
+        * fast/css-generated-content/quote-first-letter-expected.html: Added.
+        * fast/css-generated-content/quote-first-letter.html: Added.
+        * platform/mac/fast/css-generated-content/005-expected.txt:
+        * platform/mac/fast/css-generated-content/beforeAfter-interdocument-expected.txt:
+        * platform/mac/fast/css-generated-content/nested-tables-with-before-after-content-crash-expected.txt:
+        * platform/mac/fast/css-generated-content/no-openclose-quote-expected.txt:
+
 2014-01-28  Carlos Alberto Lopez Perez  <clopez@igalia.com>
 
         Unreviewed GTK gardening.
diff --git a/LayoutTests/fast/css-generated-content/quote-first-letter-expected.html b/LayoutTests/fast/css-generated-content/quote-first-letter-expected.html
new file mode 100644 (file)
index 0000000..99159d7
--- /dev/null
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<style>
+    .quote { color: red; }
+    .line { position: absolute; top: 50px; left: 10px; }
+</style>
+
+" "
+
+<span class="line">
+    <span class="quote">'</span>Should not crash or assert and all four quotes should be displayed.'
+</span>
diff --git a/LayoutTests/fast/css-generated-content/quote-first-letter.html b/LayoutTests/fast/css-generated-content/quote-first-letter.html
new file mode 100644 (file)
index 0000000..167709f
--- /dev/null
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<style>
+    .first-letter:first-letter { color: red; }
+    span { position: absolute; top: 50px; left: 10px; }
+</style>
+
+<q>
+    <span>
+        <q>Should not crash or assert and all four quotes should be displayed.</q>
+    </span>
+</q>
+
+<script>
+document.body.offsetTop;
+document.querySelector("span").className = 'first-letter';
+</script>
index 3b3627f..73734c1 100644 (file)
@@ -7,7 +7,8 @@ layer at (0,0) size 800x600
         RenderInline {Q} at (0,0) size 158x18
           RenderInline (generated) at (0,0) size 7x18
             RenderQuote at (0,0) size 7x18
-              text run at (0,0) width 7: "\""
+              RenderText at (0,0) size 7x18
+                text run at (0,0) width 7: "\""
           RenderText {#text} at (7,0) size 151x18
             text run at (7,0) width 151: "Quotes should surround"
       RenderBlock (anonymous) at (0,34) size 784x0
@@ -17,6 +18,7 @@ layer at (0,0) size 800x600
           RenderText {#text} at (0,0) size 53x18
             text run at (0,0) width 53: "this text."
           RenderInline (generated) at (0,0) size 7x18
-            RenderQuote at (53,0) size 7x18
-              text run at (53,0) width 7: "\""
+            RenderQuote at (0,0) size 7x18
+              RenderText at (53,0) size 7x18
+                text run at (53,0) width 7: "\""
         RenderText {#text} at (0,0) size 0x0
index 81aadb9..6186952 100644 (file)
@@ -19,7 +19,8 @@ layer at (0,0) size 800x600
           RenderInline {Q} at (0,0) size 158x18
             RenderInline (generated) at (0,0) size 7x18
               RenderQuote at (0,0) size 7x18
-                text run at (0,0) width 7: "\""
+                RenderText at (0,0) size 7x18
+                  text run at (0,0) width 7: "\""
             RenderText {#text} at (7,0) size 151x18
               text run at (7,0) width 151: "Quotes should surround"
         RenderBlock (anonymous) at (0,34) size 784x0
@@ -29,5 +30,6 @@ layer at (0,0) size 800x600
             RenderText {#text} at (0,0) size 53x18
               text run at (0,0) width 53: "this text."
             RenderInline (generated) at (0,0) size 7x18
-              RenderQuote at (53,0) size 7x18
-                text run at (53,0) width 7: "\""
+              RenderQuote at (0,0) size 7x18
+                RenderText at (53,0) size 7x18
+                  text run at (53,0) width 7: "\""
index 85095e2..8639d2f 100755 (executable)
@@ -16,9 +16,11 @@ layer at (0,0) size 800x600
                     RenderTableCell (anonymous) at (0,0) size 32x16 [r=0 c=0 rs=1 cs=1]
                       RenderInline (generated) at (0,0) size 16x16
                         RenderQuote at (0,0) size 16x16
-                          text run at (0,0) width 16: "\""
+                          RenderText at (0,0) size 16x16
+                            text run at (0,0) width 16: "\""
                       RenderInline (generated) at (0,0) size 16x16
-                        RenderQuote at (16,0) size 16x16
-                          text run at (16,0) width 16: "\""
+                        RenderQuote at (0,0) size 16x16
+                          RenderText at (16,0) size 16x16
+                            text run at (16,0) width 16: "\""
       RenderText {#text} at (0,0) size 0x0
       RenderText {#text} at (0,0) size 0x0
index dfa38bc..e44a230 100644 (file)
@@ -7,11 +7,11 @@ layer at (0,0) size 800x600
         RenderInline (generated) at (0,0) size 8x18
           RenderText at (0,0) size 8x18
             text run at (0,0) width 8: "*"
-          RenderQuote at (0,0) size 0x0
+          RenderQuote at (0,0) size 0x18
         RenderText {#text} at (8,0) size 425x18
           text run at (8,0) width 114: "This is some text. "
           text run at (122,0) width 311: "It should have a single asterisk before and after it."
         RenderInline (generated) at (0,0) size 8x18
           RenderText at (433,0) size 8x18
             text run at (433,0) width 8: "*"
-          RenderQuote at (0,0) size 0x0
+          RenderQuote at (0,0) size 0x18
index 6503373..5e518dc 100644 (file)
@@ -1,3 +1,31 @@
+2014-01-23  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        ASSERT_WITH_SECURITY_IMPLICATION in WebCore::InlineTextBox::paint
+        https://bugs.webkit.org/show_bug.cgi?id=114586
+
+        Reviewed by Dave Hyatt.
+
+        Taken mostly from https://chromium.googlesource.com/chromium/blink/+/cb2297db16f2e9328cb4dd8b552093d6b22340a8
+
+        If RenderQuote is a subclass of RenderObject, it can't be split by the first-letter CSS pseudoclass.
+        Instead, we should make it a subclass of RenderElement, so that it can be split properly.
+
+        Test: fast/css-generated-content/quote-first-letter.html
+
+        * dom/PseudoElement.cpp:
+        (WebCore::PseudoElement::didRecalcStyle):
+        * rendering/RenderQuote.cpp:
+        (WebCore::RenderQuote::RenderQuote):
+        (WebCore::RenderQuote::willBeDestroyed):
+        (WebCore::RenderQuote::willBeRemovedFromTree):
+        (WebCore::RenderQuote::styleDidChange):
+        (WebCore::RenderQuote::updateText):
+        (WebCore::RenderQuote::computeText):
+        (WebCore::RenderQuote::updateDepth):
+        * rendering/RenderQuote.h:
+        * rendering/style/ContentData.cpp:
+        (WebCore::QuoteContentData::createContentRenderer):
+
 2014-01-28  Antti Koivisto  <antti@apple.com>
 
         Document::topDocument() should return a reference
index 7dc629e..29259e2 100644 (file)
@@ -112,9 +112,10 @@ void PseudoElement::didRecalcStyle(Style::Change)
     RenderObject* renderer = this->renderer();
     for (RenderObject* child = renderer->nextInPreOrder(renderer); child; child = child->nextInPreOrder(renderer)) {
         // We only manage the style for the generated content which must be images or text.
-        if (!child->isRenderImage())
+        if (!child->isRenderImage() && !child->isQuote())
             continue;
-        toRenderImage(*child).setStyle(RenderImage::createStyleInheritingFromPseudoStyle(renderer->style()));
+        PassRef<RenderStyle> createdStyle = RenderStyle::createStyleInheritingFromPseudoStyle(renderer->style());
+        toRenderElement(*child).setStyle(std::move(createdStyle));
     }
 }
 
index d25349d..86e387e 100644 (file)
@@ -142,18 +142,6 @@ RenderImage::~RenderImage()
     imageResource().shutdown();
 }
 
-PassRef<RenderStyle> RenderImage::createStyleInheritingFromPseudoStyle(const RenderStyle& pseudoStyle)
-{
-    ASSERT(pseudoStyle.styleType() == BEFORE || pseudoStyle.styleType() == AFTER);
-
-    // Images are special and must inherit the pseudoStyle so the width and height of
-    // the pseudo element doesn't change the size of the image. In all other cases we
-    // can just share the style.
-    auto style = RenderStyle::create();
-    style.get().inheritFrom(&pseudoStyle);
-    return style;
-}
-
 // If we'll be displaying either alt text or an image, add some padding.
 static const unsigned short paddingWidth = 4;
 static const unsigned short paddingHeight = 4;
index f7ba538..cc94408 100644 (file)
@@ -39,9 +39,6 @@ public:
     RenderImage(Document&, PassRef<RenderStyle>, StyleImage* = nullptr);
     virtual ~RenderImage();
 
-    // Create a RenderStyle for generated content by inheriting from a pseudo style.
-    static PassRef<RenderStyle> createStyleInheritingFromPseudoStyle(const RenderStyle&);
-
     RenderImageResource& imageResource() { return *m_imageResource; }
     const RenderImageResource& imageResource() const { return *m_imageResource; }
     CachedImage* cachedImage() const { return imageResource().cachedImage(); }
index 52fa63c..a41a122 100644 (file)
 #include "RenderQuote.h"
 
 #include "QuotesData.h"
+#include "RenderTextFragment.h"
 #include "RenderView.h"
 
 using namespace WTF::Unicode;
 
 namespace WebCore {
 
-RenderQuote::RenderQuote(Document& document, QuoteType quote)
-    : RenderText(document, emptyString())
+RenderQuote::RenderQuote(Document& document, PassRef<RenderStyle> style, QuoteType quote)
+    : RenderInline(document, std::move(style))
     , m_type(quote)
     , m_depth(-1)
     , m_next(0)
@@ -50,19 +51,19 @@ RenderQuote::~RenderQuote()
 void RenderQuote::willBeDestroyed()
 {
     detachQuote();
-    RenderText::willBeDestroyed();
+    RenderInline::willBeDestroyed();
 }
 
 void RenderQuote::willBeRemovedFromTree()
 {
-    RenderText::willBeRemovedFromTree();
+    RenderInline::willBeRemovedFromTree();
     detachQuote();
 }
 
 void RenderQuote::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
 {
-    RenderText::styleDidChange(diff, oldStyle);
-    setText(originalText(), true);
+    RenderInline::styleDidChange(diff, oldStyle);
+    updateText();
 }
 
 const unsigned maxDistinctQuoteCharacters = 16;
@@ -336,7 +337,27 @@ static inline StringImpl* apostropheString()
     return apostropheString;
 }
 
-String RenderQuote::originalText() const
+void RenderQuote::updateText()
+{
+    String text = computeText();
+    if (m_text == text)
+        return;
+
+    while (RenderObject* child = firstChild())
+        child->destroy();
+
+    if (text == emptyString() || text == String()) {
+        m_text = String();
+        return;
+    }
+
+    m_text = text;
+
+    RenderTextFragment* fragment = new RenderTextFragment(document(), m_text.impl());
+    addChild(fragment);
+}
+
+String RenderQuote::computeText() const
 {
     if (m_depth < 0)
         return emptyString();
@@ -452,7 +473,7 @@ void RenderQuote::updateDepth()
     if (m_depth == depth)
         return;
     m_depth = depth;
-    setText(originalText(), true);
+    updateText();
 }
 
 } // namespace WebCore
index 009b264..ea2a83c 100644 (file)
 #ifndef RenderQuote_h
 #define RenderQuote_h
 
-#include "RenderText.h"
+#include "RenderInline.h"
 
 namespace WebCore {
 
-class RenderQuote final : public RenderText {
+class RenderQuote final : public RenderInline {
 public:
-    RenderQuote(Document&, QuoteType);
+    RenderQuote(Document&, PassRef<RenderStyle>, QuoteType);
     virtual ~RenderQuote();
 
     void attachQuote();
@@ -40,10 +40,11 @@ private:
     virtual void willBeDestroyed() override;
     virtual const char* renderName() const override { return "RenderQuote"; }
     virtual bool isQuote() const override { return true; };
-    virtual String originalText() const override;
     virtual void styleDidChange(StyleDifference, const RenderStyle*) override;
     virtual void willBeRemovedFromTree() override;
 
+    String computeText() const;
+    void updateText();
     void updateDepth();
 
     QuoteType m_type;
@@ -51,6 +52,7 @@ private:
     RenderQuote* m_next;
     RenderQuote* m_previous;
     bool m_isAttached;
+    String m_text;
 };
 
 RENDER_OBJECT_TYPE_CASTS(RenderQuote, isQuote())
index 2fc5d77..b0c469b 100644 (file)
@@ -49,7 +49,7 @@ std::unique_ptr<ContentData> ContentData::clone() const
 
 RenderPtr<RenderObject> ImageContentData::createContentRenderer(Document& document, const RenderStyle& pseudoStyle) const
 {
-    auto image = createRenderer<RenderImage>(document, RenderImage::createStyleInheritingFromPseudoStyle(pseudoStyle), m_image.get());
+    auto image = createRenderer<RenderImage>(document, RenderStyle::createStyleInheritingFromPseudoStyle(pseudoStyle), m_image.get());
     image->initializeStyle();
     image->setAltText(altText());
     return std::move(image);
@@ -67,9 +67,11 @@ RenderPtr<RenderObject> CounterContentData::createContentRenderer(Document& docu
     return createRenderer<RenderCounter>(document, *m_counter);
 }
 
-RenderPtr<RenderObject> QuoteContentData::createContentRenderer(Document& document, const RenderStyle&) const
+RenderPtr<RenderObject> QuoteContentData::createContentRenderer(Document& document, const RenderStyle& pseudoStyle) const
 {
-    return createRenderer<RenderQuote>(document, m_quote);
+    auto quote = createRenderer<RenderQuote>(document, RenderStyle::createStyleInheritingFromPseudoStyle(pseudoStyle), m_quote);
+    quote->initializeStyle();
+    return std::move(quote);
 }
 
 } // namespace WebCore
index f171803..71c6376 100644 (file)
@@ -107,6 +107,18 @@ PassRef<RenderStyle> RenderStyle::clone(const RenderStyle* other)
     return adoptRef(*new RenderStyle(*other));
 }
 
+PassRef<RenderStyle> RenderStyle::createStyleInheritingFromPseudoStyle(const RenderStyle& pseudoStyle)
+{
+    ASSERT(pseudoStyle.styleType() == BEFORE || pseudoStyle.styleType() == AFTER);
+
+    // Images are special and must inherit the pseudoStyle so the width and height of
+    // the pseudo element doesn't change the size of the image. In all other cases we
+    // can just share the style.
+    auto style = RenderStyle::create();
+    style.get().inheritFrom(&pseudoStyle);
+    return style;
+}
+
 ALWAYS_INLINE RenderStyle::RenderStyle()
     : m_box(defaultStyle().m_box)
     , visual(defaultStyle().visual)
index 510d417..73a5350 100644 (file)
@@ -352,6 +352,9 @@ public:
     static PassRef<RenderStyle> createAnonymousStyleWithDisplay(const RenderStyle* parentStyle, EDisplay);
     static PassRef<RenderStyle> clone(const RenderStyle*);
 
+    // Create a RenderStyle for generated content by inheriting from a pseudo style.
+    static PassRef<RenderStyle> createStyleInheritingFromPseudoStyle(const RenderStyle& pseudoStyle);
+
     enum IsAtShadowBoundary {
         AtShadowBoundary,
         NotAtShadowBoundary,