API test WKAttachmentTests.AddAttachmentToConnectedImageElement is a flaky failure...
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Apr 2019 04:47:39 +0000 (04:47 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Apr 2019 04:47:39 +0000 (04:47 +0000)
https://bugs.webkit.org/show_bug.cgi?id=196905
<rdar://problem/49886096>

Reviewed by Tim Horton.

This flaky test exercises a race condition between when attachment insertion updates are dispatched from the web
process to the UI process, and when script is executed via -[WKWebView evaluateJavaScript:completionHandler:].
Since attachment insertion and removal updates from the web process to the UI process are scheduled on a zero-
delay timer, we end up with this sequence of events in the problematic (failure) case:

(a) [UI]    Run script #1 (which calls `HTMLAttachmentElement.getAttachmentIdentifier`)
    ...IPC from UI to web process...
(b) [Web]   Evaluate script #1 in the web process, which schedules attachment updates on a zero-delay timer
    ...IPC from web to UI process...
(c) [UI]    Invoke completion handler for script #1
(d) [UI]    Run script #2 (which calls `document.querySelector('img').attachmentIdentifier`)
    ...IPC from UI to web process...
(e) [Web]   Evaluate script #2 in the web process
(f) [Web]   Zero-delay timer fires and dispatches attachment updates to the UI process

...which means that script #2 will complete before the UI process has received the attachment updates sent in
step (f). However, in the case where the flaky test succeeds, the zero-delay timer in (f) fires *before* script
#2 is run in step (e).

This patch fixes the flaky test by waiting until attachment insertion updates are guaranteed to be received in
the UI process by waiting on a script message posted by the web process, after attachment updates are
dispatched.

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

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@244251 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm

index e65ca6b..c9c72bb 100644 (file)
@@ -1,3 +1,37 @@
+2019-04-14  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        API test WKAttachmentTests.AddAttachmentToConnectedImageElement is a flaky failure on Mac Release builds
+        https://bugs.webkit.org/show_bug.cgi?id=196905
+        <rdar://problem/49886096>
+
+        Reviewed by Tim Horton.
+
+        This flaky test exercises a race condition between when attachment insertion updates are dispatched from the web
+        process to the UI process, and when script is executed via -[WKWebView evaluateJavaScript:completionHandler:].
+        Since attachment insertion and removal updates from the web process to the UI process are scheduled on a zero-
+        delay timer, we end up with this sequence of events in the problematic (failure) case:
+
+        (a) [UI]    Run script #1 (which calls `HTMLAttachmentElement.getAttachmentIdentifier`)
+            ...IPC from UI to web process...
+        (b) [Web]   Evaluate script #1 in the web process, which schedules attachment updates on a zero-delay timer
+            ...IPC from web to UI process...
+        (c) [UI]    Invoke completion handler for script #1
+        (d) [UI]    Run script #2 (which calls `document.querySelector('img').attachmentIdentifier`)
+            ...IPC from UI to web process...
+        (e) [Web]   Evaluate script #2 in the web process
+        (f) [Web]   Zero-delay timer fires and dispatches attachment updates to the UI process
+
+        ...which means that script #2 will complete before the UI process has received the attachment updates sent in
+        step (f). However, in the case where the flaky test succeeds, the zero-delay timer in (f) fires *before* script
+        #2 is run in step (e).
+
+        This patch fixes the flaky test by waiting until attachment insertion updates are guaranteed to be received in
+        the UI process by waiting on a script message posted by the web process, after attachment updates are
+        dispatched.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:
+        (TestWebKitAPI::TEST):
+
 2019-04-14  Aakash Jain  <aakash_jain@apple.com>
 
         Disable Flaky API Test WKAttachmentTests.AddAttachmentToConnectedImageElement
index 2bca364..01c0c25 100644 (file)
@@ -1346,16 +1346,27 @@ TEST(WKAttachmentTests, ChangeFileWrapperForPastedImage)
     [webView waitForImageElementSizeToBecome:CGSizeMake(215, 174)];
 }
 
-TEST(WKAttachmentTests, DISABLED_AddAttachmentToConnectedImageElement)
+TEST(WKAttachmentTests, AddAttachmentToConnectedImageElement)
 {
     auto webView = webViewForTestingAttachments();
     [webView _synchronouslyExecuteEditCommand:@"InsertHTML" argument:@"<img></img>"];
 
+    __block bool doneWaitingForAttachmentInsertion = false;
+    [webView performAfterReceivingMessage:@"inserted" action:^{
+        doneWaitingForAttachmentInsertion = true;
+    }];
+
+    const char *scriptForEnsuringAttachmentIdentifier = \
+        "const identifier = HTMLAttachmentElement.getAttachmentIdentifier(document.querySelector('img'));"
+        "setTimeout(() => webkit.messageHandlers.testHandler.postMessage('inserted'), 0);"
+        "identifier";
+
     ObserveAttachmentUpdatesForScope observer(webView.get());
-    NSString *attachmentIdentifier = [webView stringByEvaluatingJavaScript:@"HTMLAttachmentElement.getAttachmentIdentifier(document.querySelector('img'))"];
+    NSString *attachmentIdentifier = [webView stringByEvaluatingJavaScript:@(scriptForEnsuringAttachmentIdentifier)];
     auto attachment = retainPtr([webView _attachmentForIdentifier:attachmentIdentifier]);
     EXPECT_WK_STREQ(attachmentIdentifier, [attachment uniqueIdentifier]);
     EXPECT_WK_STREQ(attachmentIdentifier, [webView stringByEvaluatingJavaScript:@"document.querySelector('img').attachmentIdentifier"]);
+    Util::run(&doneWaitingForAttachmentInsertion);
     observer.expectAttachmentUpdates(@[ ], @[ attachment.get() ]);
 
     auto firstImage = adoptNS([[NSFileWrapper alloc] initWithURL:testImageFileURL() options:0 error:nil]);