When SW install fails, null out registration.installing before setting worker state to "redundant". This does not match the spec but this is what Firefox and Chrome do. This is also what the web-platform-tests expect.
<rdar://problem/37141997>
Created attachment 332931 [details] Patch
Comment on attachment 332931 [details] 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());"
Comment on attachment 332931 [details] 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? Yes. I'll do that. >> 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. Are you not satisfied with the existing WPT test? I wrote this one for clarity and to cover only the exact case. However, the WPT test does cover this change since it started passing. >> 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());" Ok.
Created attachment 332936 [details] Patch
Comment on attachment 332936 [details] Patch Clearing flags on attachment: 332936 Committed r227997: <https://trac.webkit.org/changeset/227997>
All reviewed patches have been landed. Closing bug.