Title: [162972] trunk
Revision
162972
Author
mmaxfi...@apple.com
Date
2014-01-28 15:36:04 -0800 (Tue, 28 Jan 2014)

Log Message

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.

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:

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (162971 => 162972)


--- trunk/LayoutTests/ChangeLog	2014-01-28 23:30:29 UTC (rev 162971)
+++ trunk/LayoutTests/ChangeLog	2014-01-28 23:36:04 UTC (rev 162972)
@@ -1,3 +1,25 @@
+2014-01-23  Myles C. Maxfield  <mmaxfi...@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  <clo...@igalia.com>
 
         Unreviewed GTK gardening.

Added: trunk/LayoutTests/fast/css-generated-content/quote-first-letter-expected.html (0 => 162972)


--- trunk/LayoutTests/fast/css-generated-content/quote-first-letter-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/css-generated-content/quote-first-letter-expected.html	2014-01-28 23:36:04 UTC (rev 162972)
@@ -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>

Added: trunk/LayoutTests/fast/css-generated-content/quote-first-letter.html (0 => 162972)


--- trunk/LayoutTests/fast/css-generated-content/quote-first-letter.html	                        (rev 0)
+++ trunk/LayoutTests/fast/css-generated-content/quote-first-letter.html	2014-01-28 23:36:04 UTC (rev 162972)
@@ -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>

Modified: trunk/LayoutTests/platform/mac/fast/css-generated-content/005-expected.txt (162971 => 162972)


--- trunk/LayoutTests/platform/mac/fast/css-generated-content/005-expected.txt	2014-01-28 23:30:29 UTC (rev 162971)
+++ trunk/LayoutTests/platform/mac/fast/css-generated-content/005-expected.txt	2014-01-28 23:36:04 UTC (rev 162972)
@@ -7,7 +7,8 @@
         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 @@
           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

Modified: trunk/LayoutTests/platform/mac/fast/css-generated-content/beforeAfter-interdocument-expected.txt (162971 => 162972)


--- trunk/LayoutTests/platform/mac/fast/css-generated-content/beforeAfter-interdocument-expected.txt	2014-01-28 23:30:29 UTC (rev 162971)
+++ trunk/LayoutTests/platform/mac/fast/css-generated-content/beforeAfter-interdocument-expected.txt	2014-01-28 23:36:04 UTC (rev 162972)
@@ -19,7 +19,8 @@
           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 @@
             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: "\""

Modified: trunk/LayoutTests/platform/mac/fast/css-generated-content/nested-tables-with-before-after-content-crash-expected.txt (162971 => 162972)


--- trunk/LayoutTests/platform/mac/fast/css-generated-content/nested-tables-with-before-after-content-crash-expected.txt	2014-01-28 23:30:29 UTC (rev 162971)
+++ trunk/LayoutTests/platform/mac/fast/css-generated-content/nested-tables-with-before-after-content-crash-expected.txt	2014-01-28 23:36:04 UTC (rev 162972)
@@ -16,9 +16,11 @@
                     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

Modified: trunk/LayoutTests/platform/mac/fast/css-generated-content/no-openclose-quote-expected.txt (162971 => 162972)


--- trunk/LayoutTests/platform/mac/fast/css-generated-content/no-openclose-quote-expected.txt	2014-01-28 23:30:29 UTC (rev 162971)
+++ trunk/LayoutTests/platform/mac/fast/css-generated-content/no-openclose-quote-expected.txt	2014-01-28 23:36:04 UTC (rev 162972)
@@ -7,11 +7,11 @@
         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

Modified: trunk/Source/WebCore/ChangeLog (162971 => 162972)


--- trunk/Source/WebCore/ChangeLog	2014-01-28 23:30:29 UTC (rev 162971)
+++ trunk/Source/WebCore/ChangeLog	2014-01-28 23:36:04 UTC (rev 162972)
@@ -1,3 +1,31 @@
+2014-01-23  Myles C. Maxfield  <mmaxfi...@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  <an...@apple.com>
 
         Document::topDocument() should return a reference

Modified: trunk/Source/WebCore/dom/PseudoElement.cpp (162971 => 162972)


--- trunk/Source/WebCore/dom/PseudoElement.cpp	2014-01-28 23:30:29 UTC (rev 162971)
+++ trunk/Source/WebCore/dom/PseudoElement.cpp	2014-01-28 23:36:04 UTC (rev 162972)
@@ -112,9 +112,10 @@
     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));
     }
 }
 

Modified: trunk/Source/WebCore/rendering/RenderImage.cpp (162971 => 162972)


--- trunk/Source/WebCore/rendering/RenderImage.cpp	2014-01-28 23:30:29 UTC (rev 162971)
+++ trunk/Source/WebCore/rendering/RenderImage.cpp	2014-01-28 23:36:04 UTC (rev 162972)
@@ -142,18 +142,6 @@
     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;

Modified: trunk/Source/WebCore/rendering/RenderImage.h (162971 => 162972)


--- trunk/Source/WebCore/rendering/RenderImage.h	2014-01-28 23:30:29 UTC (rev 162971)
+++ trunk/Source/WebCore/rendering/RenderImage.h	2014-01-28 23:36:04 UTC (rev 162972)
@@ -39,9 +39,6 @@
     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(); }

Modified: trunk/Source/WebCore/rendering/RenderQuote.cpp (162971 => 162972)


--- trunk/Source/WebCore/rendering/RenderQuote.cpp	2014-01-28 23:30:29 UTC (rev 162971)
+++ trunk/Source/WebCore/rendering/RenderQuote.cpp	2014-01-28 23:36:04 UTC (rev 162972)
@@ -24,14 +24,15 @@
 #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 @@
 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,8 +337,28 @@
     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();
     bool isOpenQuote = false;
@@ -452,7 +473,7 @@
     if (m_depth == depth)
         return;
     m_depth = depth;
-    setText(originalText(), true);
+    updateText();
 }
 
 } // namespace WebCore

Modified: trunk/Source/WebCore/rendering/RenderQuote.h (162971 => 162972)


--- trunk/Source/WebCore/rendering/RenderQuote.h	2014-01-28 23:30:29 UTC (rev 162971)
+++ trunk/Source/WebCore/rendering/RenderQuote.h	2014-01-28 23:36:04 UTC (rev 162972)
@@ -23,13 +23,13 @@
 #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 @@
     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 @@
     RenderQuote* m_next;
     RenderQuote* m_previous;
     bool m_isAttached;
+    String m_text;
 };
 
 RENDER_OBJECT_TYPE_CASTS(RenderQuote, isQuote())

Modified: trunk/Source/WebCore/rendering/style/ContentData.cpp (162971 => 162972)


--- trunk/Source/WebCore/rendering/style/ContentData.cpp	2014-01-28 23:30:29 UTC (rev 162971)
+++ trunk/Source/WebCore/rendering/style/ContentData.cpp	2014-01-28 23:36:04 UTC (rev 162972)
@@ -49,7 +49,7 @@
 
 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 @@
     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

Modified: trunk/Source/WebCore/rendering/style/RenderStyle.cpp (162971 => 162972)


--- trunk/Source/WebCore/rendering/style/RenderStyle.cpp	2014-01-28 23:30:29 UTC (rev 162971)
+++ trunk/Source/WebCore/rendering/style/RenderStyle.cpp	2014-01-28 23:36:04 UTC (rev 162972)
@@ -107,6 +107,18 @@
     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)

Modified: trunk/Source/WebCore/rendering/style/RenderStyle.h (162971 => 162972)


--- trunk/Source/WebCore/rendering/style/RenderStyle.h	2014-01-28 23:30:29 UTC (rev 162971)
+++ trunk/Source/WebCore/rendering/style/RenderStyle.h	2014-01-28 23:36:04 UTC (rev 162972)
@@ -352,6 +352,9 @@
     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,
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to