[webkit-reviews] review granted: [Bug 182416] When SW install fails, null out registration.installing before setting worker state to "redundant" : [Attachment 332931] 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 182416: When SW install fails, null out registration.installing before
setting worker state to "redundant"
https://bugs.webkit.org/show_bug.cgi?id=182416

Attachment 332931: Patch

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




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

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

> Source/WebCore/ChangeLog:10
> +	   This does not match the spec but this is what Firefox and Chrome do.
This is also what the

Change makes sense to me. Can we file an issue against the spec?

> LayoutTests/ChangeLog:9
> +	   Add layout test coverage. I have verified that this test is passing
in both Firefox and Chrome.

Since this test is useful to ensure interop with Chrome and Firefox will be
kept, it would be good to make this as a WPT test.
Given how the test is simple, making it a WPT test should be straightforward.

> LayoutTests/http/tests/workers/service/resources/install-fails-worker.js:5
> +	   }, 10);

I am not clear why we need to have a 10 millisecond timer there.
Cannot we just write "event.waitUntil(Promise.reject());"