Changeset 244013 in webkit

Timestamp:

Apr 8, 2019, 5:39:39 AM (7 years ago)

Author:

Carlos Garcia Campos

Message:

Merge r243331 - Do not insert the first-letter anonymous container until after we've constructed the first-letter renderer.
​https://bugs.webkit.org/show_bug.cgi?id=195919
<rdar://problem/48573434>

Reviewed by Brent Fulgham.

Source/WebCore:

When the container is injected too early, we might end up removing it as part of the collapsing logic
while the text renderer is being removed (replaced with the first letter + remaining text).

Test: fast/css/first-letter-and-float-crash.html

• rendering/updating/RenderTreeBuilderFirstLetter.cpp:

(WebCore::RenderTreeBuilder::FirstLetter::createRenderers):

LayoutTests:

• fast/css/first-letter-and-float-crash-expected.txt: Added.
• fast/css/first-letter-and-float-crash.html: Added.
• platform/mac/TestExpectations:

Location:

releases/WebKitGTK/webkit-2.24

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/TestExpectations (modified) (1 diff)
• LayoutTests/fast/css/first-letter-and-float-crash-expected.txt (added)
• LayoutTests/fast/css/first-letter-and-float-crash.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/updating/RenderTreeBuilderFirstLetter.cpp (modified) (3 diffs)

releases/WebKitGTK/webkit-2.24/LayoutTests/ChangeLog

@@ +1 @@
+2019-03-21  Zalan Bujtas  <zalan@apple.com>
+
+        Do not insert the first-letter anonymous container until after we've constructed the first-letter renderer.
+        https://bugs.webkit.org/show_bug.cgi?id=195919
+        <rdar://problem/48573434>
+
+        Reviewed by Brent Fulgham.
+
+        * fast/css/first-letter-and-float-crash-expected.txt: Added.
+        * fast/css/first-letter-and-float-crash.html: Added.
+        * platform/mac/TestExpectations:
+
 2019-03-19  Ryosuke Niwa  <rniwa@webkit.org>
 

releases/WebKitGTK/webkit-2.24/LayoutTests/TestExpectations

@@ -2905 +2905 @@
 imported/w3c/web-platform-tests/css/css-lists/list-marker-with-lineheight-and-overflow-hidden-001.html [ ImageOnlyFailure ]
 imported/w3c/web-platform-tests/css/css-lists/list-with-image-display-changed-001.html [ ImageOnlyFailure ]
+
+[ Debug ] fast/css/first-letter-and-float-crash.html [ Skip ]

releases/WebKitGTK/webkit-2.24/Source/WebCore/ChangeLog

@@ +1 @@
+2019-03-21  Zalan Bujtas  <zalan@apple.com>
+
+        Do not insert the first-letter anonymous container until after we've constructed the first-letter renderer.
+        https://bugs.webkit.org/show_bug.cgi?id=195919
+        <rdar://problem/48573434>
+
+        Reviewed by Brent Fulgham.
+
+        When the container is injected too early, we might end up removing it as part of the collapsing logic
+        while the text renderer is being removed (replaced with the first letter + remaining text).
+
+        Test: fast/css/first-letter-and-float-crash.html
+
+        * rendering/updating/RenderTreeBuilderFirstLetter.cpp:
+        (WebCore::RenderTreeBuilder::FirstLetter::createRenderers):
+
 2019-03-19  Ryosuke Niwa  <rniwa@webkit.org>
 

releases/WebKitGTK/webkit-2.24/Source/WebCore/rendering/updating/RenderTreeBuilderFirstLetter.cpp

@@ -218 +218 @@
     newFirstLetter->setIsFirstLetter();
 
-    auto& firstLetter = *newFirstLetter;
-    m_builder.attach(*firstLetterContainer, WTFMove(newFirstLetter), &currentTextChild);
-
     // The original string is going to be either a generated content string or a DOM node's
     // string. We want the original string before it got transformed in case first-letter has
@@ -254 +251 @@
         auto* textNode = currentTextChild.textNode();
         auto* beforeChild = currentTextChild.nextSibling();
+        auto inlineWrapperForDisplayContents = makeWeakPtr(currentTextChild.inlineWrapperForDisplayContents());
+        auto hasInlineWrapperForDisplayContents = inlineWrapperForDisplayContents.get();
         m_builder.destroy(currentTextChild);
 
@@ -266 +265 @@
 
         RenderTextFragment& remainingText = *newRemainingText;
+        ASSERT_UNUSED(hasInlineWrapperForDisplayContents, hasInlineWrapperForDisplayContents == inlineWrapperForDisplayContents.get());
+        remainingText.setInlineWrapperForDisplayContents(inlineWrapperForDisplayContents.get());
         m_builder.attach(*textContentParent, WTFMove(newRemainingText), beforeChild);
+
+        // FIXME: Make attach the final step so that we don't need to keep firstLetter around.
+        auto& firstLetter = *newFirstLetter;
         remainingText.setFirstLetter(firstLetter);
         firstLetter.setFirstLetterRemainingText(remainingText);
-
-        // construct text fragment for the first letter
+        m_builder.attach(*firstLetterContainer, WTFMove(newFirstLetter), &remainingText);
+
+        // Construct text fragment for the first letter.
         auto letter = createRenderer<RenderTextFragment>(firstLetterBlock.document(), oldText, 0, length);
-
         m_builder.attach(firstLetter, WTFMove(letter));
     }