Title: [259819] trunk
Revision
259819
Author
wenson_hs...@apple.com
Date
2020-04-09 13:16:38 -0700 (Thu, 09 Apr 2020)

Log Message

Add an API test for <https://trac.webkit.org/r259766>
https://bugs.webkit.org/show_bug.cgi?id=210294

Reviewed by Tim Horton.

Source/WebCore:

Avoid trying to place the missing value into paragraphSets in TextManipulationController by bailing if either
the start or end positions are null (while the missing value requires both the start and end to be null, it is
sufficient to bail if either are null because `observeParagraphs` will be a no-op anyways).

See Tools/ChangeLog for more details.

Test: TextManipulation.CompleteTextManipulationAvoidCrashingWhenContentIsRemoved

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

Tools:

Exercise the pathological case fixed in r259766 by inserting and then immediately removing a paragraph element
after starting text manipulation. This test also revealed an existing issue in TextManipulationController, where
we will end up hitting a debug assertion when trying to insert { null position, null position } into a HashMap
underneath `TextManipulationController::scheduleObservartionUpdate`.

* TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:
(TestWebKitAPI::TEST):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (259818 => 259819)


--- trunk/Source/WebCore/ChangeLog	2020-04-09 20:11:24 UTC (rev 259818)
+++ trunk/Source/WebCore/ChangeLog	2020-04-09 20:16:38 UTC (rev 259819)
@@ -1,3 +1,21 @@
+2020-04-09  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Add an API test for <https://trac.webkit.org/r259766>
+        https://bugs.webkit.org/show_bug.cgi?id=210294
+
+        Reviewed by Tim Horton.
+
+        Avoid trying to place the missing value into paragraphSets in TextManipulationController by bailing if either
+        the start or end positions are null (while the missing value requires both the start and end to be null, it is
+        sufficient to bail if either are null because `observeParagraphs` will be a no-op anyways).
+
+        See Tools/ChangeLog for more details.
+
+        Test: TextManipulation.CompleteTextManipulationAvoidCrashingWhenContentIsRemoved
+
+        * editing/TextManipulationController.cpp:
+        (WebCore::TextManipulationController::scheduleObservartionUpdate):
+
 2020-04-09  Ryan Haddad  <ryanhad...@apple.com>
 
         Unreviewed, reverting r259804.

Modified: trunk/Source/WebCore/editing/TextManipulationController.cpp (259818 => 259819)


--- trunk/Source/WebCore/editing/TextManipulationController.cpp	2020-04-09 20:11:24 UTC (rev 259818)
+++ trunk/Source/WebCore/editing/TextManipulationController.cpp	2020-04-09 20:16:38 UTC (rev 259819)
@@ -376,6 +376,9 @@
             auto start = startOfParagraph(firstPositionInOrBeforeNode(element.ptr()));
             auto end = endOfParagraph(lastPositionInOrAfterNode(element.ptr()));
 
+            if (start.isNull() || end.isNull())
+                continue;
+
             auto key = makeHashablePositionRange(start, end);
             if (!paragraphSets.add(key).isNewEntry)
                 continue;

Modified: trunk/Tools/ChangeLog (259818 => 259819)


--- trunk/Tools/ChangeLog	2020-04-09 20:11:24 UTC (rev 259818)
+++ trunk/Tools/ChangeLog	2020-04-09 20:16:38 UTC (rev 259819)
@@ -1,3 +1,18 @@
+2020-04-09  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Add an API test for <https://trac.webkit.org/r259766>
+        https://bugs.webkit.org/show_bug.cgi?id=210294
+
+        Reviewed by Tim Horton.
+
+        Exercise the pathological case fixed in r259766 by inserting and then immediately removing a paragraph element
+        after starting text manipulation. This test also revealed an existing issue in TextManipulationController, where
+        we will end up hitting a debug assertion when trying to insert { null position, null position } into a HashMap
+        underneath `TextManipulationController::scheduleObservartionUpdate`.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:
+        (TestWebKitAPI::TEST):
+
 2020-04-09  Simon Fraser  <simon.fra...@apple.com>
 
         Reset view navigation gesture state between tests

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm (259818 => 259819)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm	2020-04-09 20:11:24 UTC (rev 259818)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm	2020-04-09 20:16:38 UTC (rev 259819)
@@ -843,6 +843,47 @@
     EXPECT_WK_STREQ("<p>helllo, <br><b>worrld</b></p>", [webView stringByEvaluatingJavaScript:@"document.body.innerHTML"]);
 }
 
+TEST(TextManipulation, CompleteTextManipulationAvoidCrashingWhenContentIsRemoved)
+{
+    auto delegate = adoptNS([[TextManipulationDelegate alloc] init]);
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 400, 400)]);
+    [webView _setTextManipulationDelegate:delegate.get()];
+
+    [webView synchronouslyLoadTestPageNamed:@"simple"];
+
+    done = false;
+    [webView _startTextManipulationsWithConfiguration:nil completion:^{
+        done = true;
+    }];
+    TestWebKitAPI::Util::run(&done);
+
+    auto *items = [delegate items];
+    EXPECT_EQ(items.count, 1UL);
+    auto *tokens = items[0].tokens;
+    EXPECT_EQ(tokens.count, 1UL);
+
+    __block bool done = false;
+    [webView performAfterReceivingMessage:@"DoneRemovingParagraph" action:^{
+        done = true;
+    }];
+
+    [webView stringByEvaluatingJavaScript:
+        @"const paragraph = document.createElement('p');"
+        "paragraph.textContent = 'Hello world';"
+        "document.body.appendChild(paragraph);"
+        "setTimeout(() => { paragraph.remove(); webkit.messageHandlers.testHandler.postMessage('DoneRemovingParagraph') })"];
+
+    done = false;
+    [webView _completeTextManipulationForItems:@[(_WKTextManipulationItem *)createItem(items[0].identifier, {
+        { tokens[0].identifier, @"Simple HTML file!" },
+    })] completion:^(NSArray<NSError *> *errors) {
+        EXPECT_EQ(errors, nil);
+        done = true;
+    }];
+
+    EXPECT_WK_STREQ("Simple HTML file!", [webView stringByEvaluatingJavaScript:@"document.body.textContent"]);
+}
+
 TEST(TextManipulation, CompleteTextManipulationShouldPreserveImagesAsExcludedTokens)
 {
     auto delegate = adoptNS([[TextManipulationDelegate alloc] init]);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to