[webkit-reviews] review granted: [Bug 180659] Layout Test http/tests/workers/service/postmessage-after-sw-process-crash.https.html is flaky : [Attachment 329027] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 180659: Layout Test
http/tests/workers/service/postmessage-after-sw-process-crash.https.html is
flaky
https://bugs.webkit.org/show_bug.cgi?id=180659

Attachment 329027: Patch

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




--- Comment #3 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 329027
  --> https://bugs.webkit.org/attachment.cgi?id=329027
Patch

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-c
rash.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.