1136066 - Service worker registration when already registered doesn't update the worker

Status: RESOLVED INVALID

(Core :: DOM: Core & HTML, defect)

Description

[Albert [:albert]]

Steps to reproduce:

* Run https://acperez.github.io/sw-replace-test/
* Register worker 1.
* Refresh and note that fetch is called.
* Register worker 2 (same scope).
* Check registrations (Get registrations button) and see that worker 2 is registered.
* Refresh
* fetch method of worker 1 is still being called
* If tab is closed and reopened, then method of worker 2 is called.

Expected results:
* Register worker 1.
* Refresh and note that fetch is called.
* Register worker 2 (same scope).
* Check registrations (Get registrations button) and see that worker 2 is registered.
* Refresh.
* fetch method of worker 2 is called.

Comment 1

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

This happens because when the page is refreshed, the 'new' document is being controlled before the older one stops being controlled, leading to the new activation not happening. I'm looking into when we can put calls to Maybe{Start,Stop}Controlling to get the correct ordering.

Comment 2

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

Attachment: Stop controlling a document as soon as it is unloaded

Boris, I'd like to move stop controlling to unloading to fix this ordering and
also be inline with the spec
http://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#on-client-unload-algorithm
which states "The user agent must run these steps, or their equivalent, when
a service worker client unloads by unloading, being killed, or terminating."

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8568578 [details] [diff] [review]
Stop controlling a document as soon as it is unloaded

r-, add -p to your diff options and I'll look again.

Past that, the spec needs to define exactly what it means by "unloading".  Does it mean the "unloading document visibility change steps", does it mean the "unloading document cleanup steps", something else?  Which one it means, exactly, is probably observable.

Comment 4

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

Attachment: Stop controlling a document as soon as it is unloaded.

Sorry, I don't usually use MQ but this time I had to and it doesn't seem to set the option.

Comment 5

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

Filed https://github.com/slightlyoff/ServiceWorker/issues/636

Comment 6

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

Comment on attachment 8568585 [details] [diff] [review]
Stop controlling a document as soon as it is unloaded.

I guess I'll drop review until it is clarified.

Comment 7

[Boris Zbarsky [:bzbarsky]]

I think 

[diff]
showfunc = true

will give you -p bits in mq.

Comment 8

[Jake Archibald [:jakea]]

This appears to be working as expected. Assuming a single tab, when you navigate/refresh, this happens:

1. Page A is in a loaded state
2. Refresh/navigate for Page B
3. Request made to Page B's url
4. If the response replaces Page A, unload A

If the response has a "Content-Disposition" header, A would not be unloaded. Because the request for B goes out before the unload of A, it goes via the currently active service worker.

self.skipWaiting() can also be used to avoid the wait.

This overlap in client lifetime is only really an issue during development & with single tabs. I'm wanting Chrome to add some kind of ServiceWorker refresh button to devtools that does this:

1. Check for ServiceWorker update
2. Install
3. skipWaiting()
4. Refresh controlled pages

Comment 9

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

Based on what Jake says, Gecko does this correctly right now.