Changeset 276558 in webkit

Timestamp:

Apr 24, 2021, 3:31:57 PM (5 years ago)

Author:

commit-queue@webkit.org

Message:

Crash in BreakBlockquoteCommand::doApply()
​https://bugs.webkit.org/show_bug.cgi?id=224941

Patch by Julian Gonzalez <​julian_a_gonzalez@apple.com> on 2021-04-24
Reviewed by Ryosuke Niwa.

Source/WebCore:

Despite assertions to the contrary, it is possible for there not to be any node
to move into the new blockquote in BreakBlockquoteCommand::doApply() as a result
of layout updates, so remove the assertions and handle this case.

Test: editing/pasteboard/paste-as-quotation-then-paste-crash.html

• editing/BreakBlockquoteCommand.cpp:

(WebCore::BreakBlockquoteCommand::doApply):

LayoutTests:

Add test for this crash, running only on Release for now.
Thanks to Tuomas Karkkainen for its basic structure.

• TestExpectations:
• editing/pasteboard/paste-as-quotation-then-paste-crash-expected.txt: Added.
• editing/pasteboard/paste-as-quotation-then-paste-crash.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/TestExpectations (modified) (1 diff)
• LayoutTests/editing/pasteboard/paste-as-quotation-then-paste-crash-expected.txt (added)
• LayoutTests/editing/pasteboard/paste-as-quotation-then-paste-crash.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/editing/BreakBlockquoteCommand.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-04-24  Julian Gonzalez  <julian_a_gonzalez@apple.com>
+
+        Crash in BreakBlockquoteCommand::doApply()
+        https://bugs.webkit.org/show_bug.cgi?id=224941
+
+        Reviewed by Ryosuke Niwa.
+
+        Add test for this crash, running only on Release for now.
+        Thanks to Tuomas Karkkainen for its basic structure.
+
+        * TestExpectations:
+        * editing/pasteboard/paste-as-quotation-then-paste-crash-expected.txt: Added.
+        * editing/pasteboard/paste-as-quotation-then-paste-crash.html: Added.
+
 2021-04-24  Zalan Bujtas  <zalan@apple.com>
 

trunk/LayoutTests/TestExpectations

@@ -1745 +1745 @@
 webkit.org/b/139634 [ Debug ] fast/selectors/nth-child-of-register-requirement.html [ Slow ]
 webkit.org/b/139634 [ Debug ] fast/selectors/not-backtracking.html [ Slow ]
+
+webkit.org/b/224941 [ Debug ] editing/pasteboard/paste-as-quotation-then-paste-crash.html [ WontFix ]
 
 webkit.org/b/61932 [ Debug ] jquery/manipulation.html [ Slow ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-04-24  Julian Gonzalez  <julian_a_gonzalez@apple.com>
+
+        Crash in BreakBlockquoteCommand::doApply()
+        https://bugs.webkit.org/show_bug.cgi?id=224941
+
+        Reviewed by Ryosuke Niwa.
+
+        Despite assertions to the contrary, it is possible for there not to be any node
+        to move into the new blockquote in BreakBlockquoteCommand::doApply() as a result
+        of layout updates, so remove the assertions and handle this case.
+
+        Test: editing/pasteboard/paste-as-quotation-then-paste-crash.html
+
+        * editing/BreakBlockquoteCommand.cpp:
+        (WebCore::BreakBlockquoteCommand::doApply):
+
 2021-04-24  Antoine Quint  <graouts@webkit.org>
 

trunk/Source/WebCore/editing/BreakBlockquoteCommand.cpp

@@ -110 +110 @@
         Text& textNode = downcast<Text>(*startNode);
         if ((unsigned)pos.deprecatedEditingOffset() >= textNode.length()) {
-            startNode = NodeTraversal::next(*startNode);
-            ASSERT(startNode);
+            if (auto* nextNode = NodeTraversal::next(*startNode))
+                startNode = nextNode;
         } else if (pos.deprecatedEditingOffset() > 0)
             splitTextNode(textNode, pos.deprecatedEditingOffset());
     } else if (pos.deprecatedEditingOffset() > 0) {
-        Node* childAtOffset = startNode->traverseToChildAt(pos.deprecatedEditingOffset());
-        startNode = childAtOffset ? childAtOffset : NodeTraversal::next(*startNode);
-        ASSERT(startNode);
+        if (auto* child = startNode->traverseToChildAt(pos.deprecatedEditingOffset()))
+            startNode = child;
+        else if (auto* next = NodeTraversal::next(*startNode))
+            startNode = next;
     }