Merge r249954 - [First-letter] Use WeakPtr for the first-letter inser…

…tion point. https://bugs.webkit.org/show_bug.cgi?id=201842 <rdar://problem/51373788> Reviewed by Antti Koivisto. Source/WebCore: The about-to-be-removed first letter renderer's sibling could potentially be destroyed too as the result of the anonymous subtree collapsing logic (when the next sibling is a generated anonymous block and it is not needed anymore.) Test: fast/text/first-letter-with-columns-crash.html * rendering/updating/RenderTreeBuilderFirstLetter.cpp: (WebCore::RenderTreeBuilder::FirstLetter::updateStyle): LayoutTests: * fast/text/first-letter-with-columns-crash-expected.txt: Added. * fast/text/first-letter-with-columns-crash.html: Added.
WebKit · Sep 23, 2019 · d79c511 · d79c511
1 parent 47f2f0a
commit d79c511
Show file tree

Hide file tree

Showing 5 changed files with 52 additions and 2 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,14 @@
+2019-09-17  Zalan Bujtas  <zalan@apple.com>
+
+        [First-letter] Use WeakPtr for the first-letter insertion point.
+        https://bugs.webkit.org/show_bug.cgi?id=201842
+        <rdar://problem/51373788>
+
+        Reviewed by Antti Koivisto.
+
+        * fast/text/first-letter-with-columns-crash-expected.txt: Added.
+        * fast/text/first-letter-with-columns-crash.html: Added.
+
 2019-09-03  Devin Rousso  <drousso@apple.com>
 
         REGRESSION (r249078): Flaky crash in com.apple.JavaScriptCore: Inspector::InjectedScriptModule::ensureInjected

diff --git a/LayoutTests/fast/text/first-letter-with-columns-crash-expected.txt b/LayoutTests/fast/text/first-letter-with-columns-crash-expected.txt
@@ -0,0 +1 @@
+First letter -PASS if no crash.
diff --git a/LayoutTests/fast/text/first-letter-with-columns-crash.html b/LayoutTests/fast/text/first-letter-with-columns-crash.html
@@ -0,0 +1,23 @@
+<style>
+:first-letter {
+    float: right;
+    content: url()
+}
+
+body {
+    columns: 2;
+}
+</style>
+<body>First letter -PASS if no crash.<span id=outer><span id=inner>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+outer.addEventListener("DOMSubtreeModified", function() {
+    document.execCommand(false);
+    document.body.style.setProperty("-webkit-columns","initial");
+    inner.setAttribute("foobar","");
+    document.body.style.setProperty("-webkit-writing-mode","vertical-lr");
+
+});
+outer.setAttribute("foobar","");
+</script>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,18 @@
+2019-09-17  Zalan Bujtas  <zalan@apple.com>
+
+        [First-letter] Use WeakPtr for the first-letter insertion point.
+        https://bugs.webkit.org/show_bug.cgi?id=201842
+        <rdar://problem/51373788>
+
+        Reviewed by Antti Koivisto.
+
+        The about-to-be-removed first letter renderer's sibling could potentially be destroyed too as the result of the anonymous subtree collapsing logic (when the next sibling is a generated anonymous block and it is not needed anymore.)
+
+        Test: fast/text/first-letter-with-columns-crash.html
+
+        * rendering/updating/RenderTreeBuilderFirstLetter.cpp:
+        (WebCore::RenderTreeBuilder::FirstLetter::updateStyle):
+
 2019-09-13  Chris Dumez  <cdumez@apple.com>
 
         Crash under WebCore::firstPositionInNode()

diff --git a/Source/WebCore/rendering/updating/RenderTreeBuilderFirstLetter.cpp b/Source/WebCore/rendering/updating/RenderTreeBuilderFirstLetter.cpp
@@ -185,15 +185,15 @@ void RenderTreeBuilder::FirstLetter::updateStyle(RenderBlock& firstLetterBlock,
             m_builder.attach(*newFirstLetter, WTFMove(toMove));
         }
 
-        RenderObject* nextSibling = firstLetter->nextSibling();
         if (RenderTextFragment* remainingText = downcast<RenderBoxModelObject>(*firstLetter).firstLetterRemainingText()) {
             ASSERT(remainingText->isAnonymous() || remainingText->textNode()->renderer() == remainingText);
             // Replace the old renderer with the new one.
             remainingText->setFirstLetter(*newFirstLetter);
             newFirstLetter->setFirstLetterRemainingText(*remainingText);
         }
+        WeakPtr<RenderObject> nextSibling = makeWeakPtr(firstLetter->nextSibling());
         m_builder.destroy(*firstLetter);
-        m_builder.attach(*firstLetterContainer, WTFMove(newFirstLetter), nextSibling);
+        m_builder.attach(*firstLetterContainer, WTFMove(newFirstLetter), nextSibling.get());
         return;
     }