Skip to content

Commit

Permalink
API test WKAttachmentTests.AddAttachmentToConnectedImageElement is a …
Browse files Browse the repository at this point in the history
…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):


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@244251 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
wenson_hsieh@apple.com committed Apr 15, 2019
1 parent dd436c2 commit fd02200
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 2 deletions.
34 changes: 34 additions & 0 deletions Tools/ChangeLog
Original file line number Diff line number Diff line change
@@ -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
Expand Down
15 changes: 13 additions & 2 deletions Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1346,16 +1346,27 @@ @implementation FileWrapper
[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]);
Expand Down

0 comments on commit fd02200

Please sign in to comment.