Title: [264003] trunk
Revision
264003
Author
[email protected]
Date
2020-07-06 17:54:23 -0700 (Mon, 06 Jul 2020)

Log Message

Web process sometimes crashes when translating an article on spiegel.de
https://bugs.webkit.org/show_bug.cgi?id=214014
<rdar://problem/65099253>

Reviewed by Tim Horton.

Source/WebCore:

The crash happens because we try to make a BoundaryPoint (using `makeBoundaryPoint`) out of an orphaned
`Position` that is anchored before or after the anchor node (more specifically, either `PositionIsBeforeAnchor`
or `PositionIsAfterAnchor`). In `makeBoundaryPoint`, we'll avoid the early `WTF::nullopt` return since the
position is non-null, but then try to access the container node, which is null in this case because the anchor
node has been unparented.

Fix this by not attempting to observe orphaned DOM positions when extracting content for translation.

Test: TextManipulation.StartTextManipulationAvoidCrashWhenExtractingOrphanedPositions

* editing/TextManipulationController.cpp:
(WebCore::TextManipulationController::observeParagraphs):

Tools:

Add a test case that dynamically adds an `object` element, immediately removes it, and then adds a chunk of text
and verifies that the text manipulation delegate method for new content is invoked.

* TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (264002 => 264003)


--- trunk/Source/WebCore/ChangeLog	2020-07-07 00:46:03 UTC (rev 264002)
+++ trunk/Source/WebCore/ChangeLog	2020-07-07 00:54:23 UTC (rev 264003)
@@ -1,3 +1,24 @@
+2020-07-06  Wenson Hsieh  <[email protected]>
+
+        Web process sometimes crashes when translating an article on spiegel.de
+        https://bugs.webkit.org/show_bug.cgi?id=214014
+        <rdar://problem/65099253>
+
+        Reviewed by Tim Horton.
+
+        The crash happens because we try to make a BoundaryPoint (using `makeBoundaryPoint`) out of an orphaned
+        `Position` that is anchored before or after the anchor node (more specifically, either `PositionIsBeforeAnchor`
+        or `PositionIsAfterAnchor`). In `makeBoundaryPoint`, we'll avoid the early `WTF::nullopt` return since the
+        position is non-null, but then try to access the container node, which is null in this case because the anchor
+        node has been unparented.
+
+        Fix this by not attempting to observe orphaned DOM positions when extracting content for translation.
+
+        Test: TextManipulation.StartTextManipulationAvoidCrashWhenExtractingOrphanedPositions
+
+        * editing/TextManipulationController.cpp:
+        (WebCore::TextManipulationController::observeParagraphs):
+
 2020-07-06  John Wilander  <[email protected]>
 
         Follow-up to r263992: Make isKinjaLoginAvatarElement() a free function

Modified: trunk/Source/WebCore/editing/TextManipulationController.cpp (264002 => 264003)


--- trunk/Source/WebCore/editing/TextManipulationController.cpp	2020-07-07 00:46:03 UTC (rev 264002)
+++ trunk/Source/WebCore/editing/TextManipulationController.cpp	2020-07-07 00:54:23 UTC (rev 264003)
@@ -426,7 +426,7 @@
 
 void TextManipulationController::observeParagraphs(const Position& start, const Position& end)
 {
-    if (start.isNull() || end.isNull())
+    if (start.isNull() || end.isNull() || start.isOrphan() || end.isOrphan())
         return;
 
     auto document = makeRefPtr(start.document());

Modified: trunk/Tools/ChangeLog (264002 => 264003)


--- trunk/Tools/ChangeLog	2020-07-07 00:46:03 UTC (rev 264002)
+++ trunk/Tools/ChangeLog	2020-07-07 00:54:23 UTC (rev 264003)
@@ -1,3 +1,16 @@
+2020-07-06  Wenson Hsieh  <[email protected]>
+
+        Web process sometimes crashes when translating an article on spiegel.de
+        https://bugs.webkit.org/show_bug.cgi?id=214014
+        <rdar://problem/65099253>
+
+        Reviewed by Tim Horton.
+
+        Add a test case that dynamically adds an `object` element, immediately removes it, and then adds a chunk of text
+        and verifies that the text manipulation delegate method for new content is invoked.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:
+
 2020-07-06  Carlos Alberto Lopez Perez  <[email protected]>
 
         svn-apply should use verbose mode when adding files to git

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm (264002 => 264003)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm	2020-07-07 00:46:03 UTC (rev 264002)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm	2020-07-07 00:54:23 UTC (rev 264003)
@@ -1050,6 +1050,44 @@
     EXPECT_FALSE(item.tokens[3].isExcluded);
 }
 
+TEST(TextManipulation, StartTextManipulationAvoidCrashWhenExtractingOrphanedPositions)
+{
+    auto delegate = adoptNS([[TextManipulationDelegate alloc] init]);
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 400, 400)]);
+    [webView _setTextManipulationDelegate:delegate.get()];
+
+    [webView synchronouslyLoadHTMLString:@"<p>hello world</p>"];
+
+    done = false;
+    [webView _startTextManipulationsWithConfiguration:nil completion:^{
+        done = true;
+    }];
+    TestWebKitAPI::Util::run(&done);
+
+    auto items = [delegate items];
+    EXPECT_EQ(items.count, 1UL);
+    EXPECT_EQ(items[0].tokens.count, 1UL);
+    EXPECT_WK_STREQ("hello world", items[0].tokens[0].content);
+
+    done = false;
+    delegate.get().itemCallback = ^(_WKTextManipulationItem *item) {
+        done = true;
+    };
+
+    [webView objectByEvaluatingJavaScript:@"(() => {"
+        "    const objectElement = document.createElement('object');"
+        "    document.body.appendChild(objectElement);"
+        "    document.body.scrollTop;"
+        "    objectElement.remove();"
+        "    const text = document.createTextNode('testing');"
+        "    const container = document.createElement('div');"
+        "    container.appendChild(text);"
+        "    document.body.appendChild(container);"
+        "})();"];
+
+    TestWebKitAPI::Util::run(&done);
+}
+
 struct Token {
     NSString *identifier;
     NSString *content;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to