Closed Bug 1163412 Opened 9 years ago Closed 8 years ago

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

Categories

(Core :: DOM: Service Workers, defect)

defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox40 --- affected

People

(Reporter: flaki, Assigned: rnath, Mentored)

References

Details

(Whiteboard: [good first bug][lang=C++])

Attachments

(1 file)

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
Flags: needinfo?(nsm.nikhil)
I guess it might block v2. Please, change it if it is not the case. Thanks!
This is a good first bug if someone wants to take it up.
Mentor: nsm.nikhil
Flags: needinfo?(nsm.nikhil)
Whiteboard: [good first bug][lang=C++]
Hi!
I'd like to fix this bug. Any suggestions?
This is a proposed patch. Can someone take a look and let me know if I'm on the right track?
Attachment #8607378 - Flags: feedback?(nsm.nikhil)
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.
Attachment #8607378 - Flags: feedback?(nsm.nikhil) → feedback+
Status: NEW → ASSIGNED
Sorry, but I think we have implemented this in other bugs changing the job queue logic.  Thank you for your help!
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: