Bug 180659 - Layout Test http/tests/workers/service/postmessage-after-sw-process-crash.https.html is flaky
Summary: Layout Test http/tests/workers/service/postmessage-after-sw-process-crash.htt...
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
Keywords: InRadar
Depends on:
Reported: 2017-12-11 10:12 PST by Ryan Haddad
Modified: 2017-12-11 14:50 PST (History)
6 users (show)

See Also:

Patch (8.46 KB, patch)
2017-12-11 13:43 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (8.76 KB, patch)
2017-12-11 14:16 PST, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryan Haddad 2017-12-11 10:12:14 PST
The following layout test is flaky on macOS and iOS WK2:


Probable cause:

Test was added with https://trac.webkit.org/changeset/225622/webkit

Flakiness Dashboard:


--- /Volumes/Data/slave/elcapitan-release-tests-wk2/build/layout-test-results/http/tests/workers/service/postmessage-after-sw-process-crash.https-expected.txt
+++ /Volumes/Data/slave/elcapitan-release-tests-wk2/build/layout-test-results/http/tests/workers/service/postmessage-after-sw-process-crash.https-actual.txt
@@ -3,6 +3,5 @@
 PASS: Service worker received message 'Message 1' from origin ''
 * Simulating Service Worker process crash
 * Sending 'Message 2' to Service Worker
-PASS: Client received message from service worker, origin:
-PASS: Service worker received message 'Message 2' from origin ''
+FAIL: Did not receive message from service worker process after the crash
Comment 1 Chris Dumez 2017-12-11 10:32:16 PST
Will take a look.
Comment 2 Chris Dumez 2017-12-11 13:43:36 PST
Created attachment 329027 [details]
Comment 3 youenn fablet 2017-12-11 14:03:40 PST
Comment on attachment 329027 [details]

View in context: https://bugs.webkit.org/attachment.cgi?id=329027&action=review

> Source/WebKit/StorageProcess/ServiceWorker/WebSWServerConnection.cpp:133
> +    server().runServiceWorkerIfNecessary(destinationIdentifier, [destinationIdentifier, message = message.vector(), sourceIdentifier, sourceData = WTFMove(sourceData)](bool success, auto& contextConnection) mutable {

No need for message to be passed as an r value.

This is so easy to fall into the trap of copying a DataReference and thinking that it will copy the data it refers to.
Maybe DataReference should not be copy constructible, one can still explicitly create a DataReference with one of its other constructor.

> LayoutTests/http/tests/workers/service/resources/postmessage-after-sw-process-crash.js:18
> +        }, 10);

For that kind of pattern, I usually like to do this ping for a number of time and fail afterwards with an explicit message.
This allows making test messages in case of timeouts more precise.
Comment 4 Chris Dumez 2017-12-11 14:16:06 PST
Created attachment 329034 [details]
Comment 5 WebKit Commit Bot 2017-12-11 14:49:13 PST
Comment on attachment 329034 [details]

Clearing flags on attachment: 329034

Committed r225758: <https://trac.webkit.org/changeset/225758>
Comment 6 WebKit Commit Bot 2017-12-11 14:49:15 PST
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2017-12-11 14:50:49 PST