1163412 - There should be no such thing as "fail to activate" for a Service Worker script

Status: RESOLVED WORKSFORME

(Core :: DOM: Service Workers, defect)

Description

[Szmozsánszky István [:flaki]]

Browsing the spec issue #659 (There should be no such thing as "fail to activate") based on the disussion:
https://github.com/slightlyoff/ServiceWorker/issues/659#issuecomment-95471388

...and according to the revised Activation algorithm (15.4)
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#activation-algorithm

...shouldn't the ServiceWorkerRegistrationInfo::FinishActivate() method in dom/workers/ServiceWorkerManager.cpp
https://dxr.mozilla.org/mozilla-central/source/dom/workers/ServiceWorkerManager.cpp#1934

...set the worker state to "Activated", regardless of the activation result "aSuccess"?

In other words (from Chrome's impl.): "e.waitUntil(reject) is the same as e.waitUntil(resolve)"
https://code.google.com/p/chromium/issues/detail?id=480050

Comment 1

[Noemí Freire (:noemi)]

I guess it might block v2. Please, change it if it is not the case. Thanks!

Comment 2

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

This is a good first bug if someone wants to take it up.

Comment 3

[Nitul Datt]

Hi!
I'd like to fix this bug. Any suggestions?

Comment 4

[Ryan Nath [:rnath]]

Attachment: Bug1163412_revA.patch

This is a proposed patch. Can someone take a look and let me know if I'm on the right track?

Comment 5

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Comment on attachment 8607378 [details] [diff] [review]
Bug1163412_revA.patch

Review of attachment 8607378 [details] [diff] [review]:
-----------------------------------------------------------------

Please add a test too. The test will be soemthing like:
1) In the test html file, register a SW for a scope
2) In the directory of that scope, have a ready.html file or similar (there are similar files in dom/worker/test/serviceworkers that can be re-used). This waits on navigator.serviceWorker.ready.then(r => parent.postMessage("Activated!")
3) Have the SW script be:
onactivate = function(e) {
  e.waitUntil(Promise.reject());
}

onfetch = function(e) {
  e.respondWith(new Response("hello"));
}

4) make sure a fetch() to a controlled scope results in hello response.

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

Sorry, but I think we have implemented this in other bugs changing the job queue logic.  Thank you for your help!