Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A way to immediately unregister a service worker #614

Open
KenjiBaheux opened this issue Feb 2, 2015 · 59 comments

Comments

@KenjiBaheux
Copy link
Collaborator

commented Feb 2, 2015

Spin off from #468.
Alex:

I a conversation I had with @metromoxie makes me think we should also add a special none or blank
value to Service-Worker-Allowed to disable SWs entirely. Many users want a similar kill-switch.

I'd be happy with a different header to kill things, but something seems necessary.

Anne:

I had the impression unregister() was the way to remove a single service worker and there would be no protocol equivalent.

If we want a protocol solution to removing a service worker, using HTTP 410 seems much more applicable.

Using none as keyword to forbid service workers of any kind seems fine although it's a bit unclear to me how you disambiguate that from a relative URL. Would we require URLs in that header to start with a slash?

@KenjiBaheux

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 2, 2015

@metromoxie

This comment has been minimized.

Copy link

commented Feb 7, 2015

@annevk I think unregister only allows the you to do so at your current scope, o it seems like a 'kill switch' in the header gives the added benefit of the server being able, for any URL, to specify "no service workers at all".

Requiring absolute paths in Service-Worker-Allowed seems reasonable to me since I believe in the spec 'scope' is already required to be absolute anyway (http://www.w3.org/TR/service-workers/#unregister-algorithm for example). On the other hand, @slightlyoff's suggestion of a blank value also seems reasonable to me.

@annevk

This comment has been minimized.

Copy link
Member

commented Feb 9, 2015

What is quoted above is out of context as I was responding to a suggestion of using none to remove the current worker, which was a suggestion I disagreed with per that comment.

@jakearchibald

This comment has been minimized.

Copy link
Contributor

commented Feb 9, 2015

I believe in the spec 'scope' is already required to be absolute anyway

Nah, they can be relative (to the page).

The kill switch idea came up before and #236 was created as a result. Would that work here? You could serve a SW that contained only:

self.registration.unregister({immediate: true})

…which would unregister but also un-claim all pages immediately. Alternatively, clients.release() as an explicit opposite to 'claim'.

@metromoxie

This comment has been minimized.

Copy link

commented Feb 10, 2015

I don't think that's sufficient. I'd want to be able to just serve headers that tell the client that Service Workers are not allowed so that any content with HTTP headers, even if it's not going to run JavaScript, can specify that the origin should not have ServiceWorkers.

I'd also love to have it be a preventative measure as well, and prevent registration of ServiceWorkers, but that may be more ambitious/complicated than I think. For example, if Dropbox knows that it should never have a ServiceWorker, they should be able to effectively "turn Service Workers off" for their origin by always specifying said header with all requests.

@annevk

This comment has been minimized.

Copy link
Member

commented Feb 10, 2015

Disabling SWs altogether and preventing installation seems like a CSP directive.

Disabling just one SW through HTTP should use HTTP status codes on its script URL.

@mattto

This comment has been minimized.

Copy link
Member

commented Feb 23, 2015

If the motivation of the kill-switch is a sure-fire way to immediately shutdown SW in an emergency (say you roll out a respondWith('hello world') SW), these don't seem sufficient yet:

  • unregister({immediate: true}) won't be run (your page is just 'hello world')
  • HTTP status 410 on the script URL could take 24 hours before it's fixed

But maybe 24 hours is the best guarantee the SW spec should provide? Sites that are worried can set no-cache or max-age.

@mattto

This comment has been minimized.

Copy link
Member

commented Feb 23, 2015

Ah, I see the SW would run self.registration.unregister({immediate: true}), so that also takes 24 hours.

I implementing both unregister({immediate: true}) and HTTP status 410 would be OK. The protocol solution is a bigger hammer in case the browser's having trouble even spinning up workers or its registration job queue is wedged.

@mattto

This comment has been minimized.

Copy link
Member

commented Feb 24, 2015

Could it make sense for the Update algorithm to do a HEAD request and more aggressively than the 24 hour cache check? For example every 1 hour or even every time? This would be a powerful kill-switch, no waiting for 24 hours.

@kinu

This comment has been minimized.

Copy link
Contributor

commented Feb 24, 2015

@mattto Let me make sure... when you say 'every 1 hour' you mean 'when navigation happens after 1 hour+ since the last update check runs', right? I'd be worried about the battery consumption on mobile devices.

@mattto

This comment has been minimized.

Copy link
Member

commented Feb 24, 2015

Sorry that wasn't clear. My idea was in the Update algorithm, do a HEAD request before fetching the script. The HEAD request could be done every time, or else limited by something like once every hour (similar to how fetching the script bypasses the cache only once every 24 hours). Update can be invoked via by SoftUpdate, which the UA can run at anytime (Chrome schedules one to happen soon after every navigation to a page in scope).

In Chrome's case, I think it shouldn't be too battery/network costly since you're already hitting network by navigating to the site.

@jungkees jungkees added this to the Version 1 milestone Mar 23, 2015
@mattto

This comment has been minimized.

Copy link
Member

commented Apr 3, 2015

FYI I think the current APIs let you implement the equivalent of an unregister({immediate:true}) worker:

skipWaiting();
onactivate = function(e) {
  e.waitUntil(clients.claim()
    .then(function() { return self.registration.unregister(); }
    .then(function() { return Promise.reject(); })));
};

A more sugary syntax may still be good.

@mattto

This comment has been minimized.

Copy link
Member

commented Apr 23, 2015

Nevermind, as per issue #659 the spec will likely change so that waitUntil(reject) is equivalent to waitUntil(resolve) in onactivate. So I think current APIs don't yet support unregister({immediate:true}) or cllients.release().

@jakearchibald

This comment has been minimized.

Copy link
Contributor

commented May 19, 2015

I'm a little confused by the 24hr thing. A SW is checked for updates on navigation, so:

  1. User navigates to broken page
  2. SW serves up "hello world"
  3. SW checks for updates, finds update, skips waiting, and tells clients to reload (#681 (comment))

Step 2 would only be skipped if the ServiceWorker was served with a max-age and was still considered fresh.

For this use-case, I don't see a benefit to unregistering rather than fixing the SW. If an evil user has managed to install a SW on another site, the other site probably wants to unregister it pretty fast, but that's still just:

  1. User navigates to hacked page
  2. SW serves up "l33t hacked"
  3. SW checks for updates, finds update, skips waiting, unregisters, and tells clients to reload (#681 (comment))

So the new SW would be:

self.oninstall = _ => self.skipWaiting();
self.onactivate = _ => {
  self.registration.unregister()
    .then(_ => client.getAll())
    .then(clients => clients.map(c => c.navigate()));
};
@jakearchibald jakearchibald modified the milestones: Version 2, Version 1 Jul 21, 2015
@jakearchibald

This comment has been minimized.

Copy link
Contributor

commented Jul 21, 2015

Moving this to v2, as we have ways of doing this without a specific API.

In cases where the SW is totally broken, this could happen:

  • User navigates to page
  • SW is broken, so it serves a network error
  • Browser may make a skip-service-worker request, if that works they can load the page uncontrolled

Continual failures may even result in the UA unregistering the SW.

@jakearchibald

This comment has been minimized.

Copy link
Contributor

commented Jul 21, 2015

At the f2f we also had discussion about how developers could detect that their responses weren't valid. We had the idea that fetchEvent.respondWith() could return a promise that rejects if the promise passed to respondWith

  • resolves to a network error
  • rejects
  • is converted to a network error by fetch (eg due to opaqueness where opaque is inappropriate)
@mattto

This comment has been minimized.

Copy link
Member

commented Jul 22, 2015

Hi Jake can we track the respondWith() return promise as a different issue? The idea also came up in a codereview from @mikewest: https://codereview.chromium.org/1228233007/#msg9

@gauntface

This comment has been minimized.

Copy link

commented Dec 11, 2015

Just wanted to add an extra use case.

I've writing some unit tests for SW fetch events and hit a scenario where registering and unregistering SW's between tests and essentially there was no way to completely remove the currently controlling sw since the redundant SW is brought back to life if the same SW is used between tests.

@jakearchibald jakearchibald changed the title A header/something to kill SWs A way to immediately unregister a service worker Mar 31, 2017
@jakearchibald

This comment has been minimized.

Copy link
Contributor

commented Mar 31, 2017

I think this makes WPTs awkward too.

@wanderview

This comment has been minimized.

Copy link
Member

commented Mar 31, 2017

Would an API performing the Clear-Site-Data header operation serve this purpose?

@jakearchibald

This comment has been minimized.

Copy link
Contributor

commented Mar 31, 2017

Yeah, that spec used to have an API defined, but it doesn't anymore. It'd also clear a whole lot more, but maybe that's ok.

@jungkees

This comment has been minimized.

Copy link
Collaborator

commented Jun 14, 2019

We've wanted something like unregister({ uncontrol: true }) for a while now, so making CSD behave like that seems ok.

Deleting registration record and unclaiming clients seem two distinct operations to me. We can add unregister({uncontrol: true}) as a feature later (after v1), but the default behavior of unregister() won't do unclaim. Shouldn't we have a separate CSD directive for unclaim? I thought CSD: "executionContext" would do for it, but it seems there are issues: w3c/webappsec-clear-site-data#59. Will folks continue to work on "executionContext"?

@asakusuma

This comment has been minimized.

Copy link

commented Jun 18, 2019

Unclaiming just means that the service worker no longer handles events for that client, correct?

If that is true, then I'm not sure we need a non-unclaiming CSD implementation. I would expect that CSD would unclaim. If we were to have multiple CSD directives for service workers, I would want delineation between stopping new events being handled by the service worker, vs going a step further and killing any in-flight events being handled by the service worker. In general, I would prefer to have the most extreme measure be the default MVP for a CSD service worker directive.

Stepping back, I see CSD as a safety mechanism for a serious problem, where collateral damage to the UX is OK. In these situations, an abrupt loss of any functionality provided by the service worker is going to preferable to a bad service worker continuing to operate unabated. Developers can always implement their own unregister() based mechanisms for less serious situations.

Also, @mattto, didn't you say Chrome already 'unclaims' if things become corrupt? It seems reasonable to spec that somehow.

@jakearchibald How are we defining "corrupt"?

asakusuma added a commit to asakusuma/ServiceWorker that referenced this issue Jun 21, 2019
So that the Clear Site Data spec can reference the algorithm and use
it to clear out the service worker, insteaed of unregister().

Context:
w3c#614 (comment)

Eventually will resolve:
w3c/webappsec-clear-site-data#54
@jakearchibald

This comment has been minimized.

Copy link
Contributor

commented Jun 21, 2019

Stepping back, I see CSD as a safety mechanism for a serious problem, where collateral damage to the UX is OK. In these situations, an abrupt loss of any functionality provided by the service worker is going to preferable to a bad service worker continuing to operate unabated.

Strong agree. It's a kill switch.

@jakearchibald How are we defining "corrupt"?

If Chrome fails to read the registration/serviceworker from disk.

@asakusuma

This comment has been minimized.

Copy link

commented Jul 24, 2019

@jungkees @jakearchibald it just occurred to me that the current approach for CSD will also remove push notification subscriptions. While this might make sense in some scenarios, it's way overkill in others, and presents an unnecessary UX problem in making everyone re-subscribe to notifications. If there's a critical caching bug, we want to be able to use CSD without having to ask everyone to re-subscribe again.

@asakusuma

This comment has been minimized.

Copy link

commented Jul 24, 2019

I think there should be a distinction between "I don't want a service worker anymore" and "this specific service worker isn't working but I still want one when there's a working one available". "Unregister" appears to address the former, but we don't seem to have a 1st class construct for the later.

@asutherland

This comment has been minimized.

Copy link

commented Jul 25, 2019

At least in Firefox, the user permission for an origin to send push notifications is decoupled from any specific registration. (Similarly, if a user expresses a desire to never allow push notifications for an origin, that is persisted. That way a site can't just try and unregister and register to get more tries to prompt the user.)

@asakusuma

This comment has been minimized.

Copy link

commented Jul 25, 2019

@asutherland I like that approach. So for Firefox are subscriptions still associated with a registration? Given an origin that has already received permission for push notifications, after CSD: storage is applied, if the application wants to resume notifications, does the application simply call subscribe() again, and permission is assumed, without having to ask the user again?

Another relevant section of the spec:

User agents MUST acquire consent for permission through a user interface for each call to the subscribe() method, unless a previous permission grant has been persisted, or a prearranged trust relationship applies. Permissions that are preserved beyond the current browsing session MUST be revocable.

@asutherland

This comment has been minimized.

Copy link

commented Jul 25, 2019

The subscriptions are associated with specific registrations yes. (Technically, they're indirectly keyed by the registration scope, but that will likely change.)

The origin-scoped permission is not cleared by Clear-Site-Data, and so when subscribe() is called, if the permission was previously granted (and not subsequently revoked by the user), subscribe will succeed without prompting.

@asakusuma

This comment has been minimized.

Copy link

commented Jul 25, 2019

@asutherland awesome, thank you for the clarification.

@mattto do you know what Chrome does with permissions when CSD is used? Will Chrome re-use existing permissions if subscribe() is called again after CSD?

@jakearchibald

This comment has been minimized.

Copy link
Contributor

commented Sep 15, 2019

For TPAC:

  • I took a lot at spec'ing this, and it was hard. I just need to do the work. It's necessary for clear-site-data.
@asakusuma

This comment has been minimized.

Copy link

commented Sep 15, 2019

I created an issue related to the logistics of applying Clear-Site-Data: #1471

@jakearchibald

This comment has been minimized.

Copy link
Contributor

commented Sep 16, 2019

TPAC:

  • This could take longer to spec than implement.
  • Do tests first, so implementation can happen in parallel with spec.
asakusuma added a commit to asakusuma/wpt that referenced this issue Sep 18, 2019
Adds a failing test, asserting that pages actively controlled by a service worker become uncontrolled after the Clear-Site-Data storage directive.

We agreed on this behavior in a spec issue and confirmed the need for this test at the TPAC Service Workers F2F.

w3c/ServiceWorker#614
F2F notes: https://docs.google.com/document/d/1_Qfw5m3BJEaL1xIzTJd41HXjgJ9gq7mroBDXqSJIzic/edit
asakusuma added a commit to asakusuma/wpt that referenced this issue Sep 18, 2019
…e directive

Adds a failing test, asserting that pages actively controlled by a service worker become uncontrolled after the Clear-Site-Data storage directive.

We agreed on this behavior in a spec issue and confirmed the need for this test at the TPAC Service Workers F2F.

w3c/ServiceWorker#614
F2F notes: https://docs.google.com/document/d/1_Qfw5m3BJEaL1xIzTJd41HXjgJ9gq7mroBDXqSJIzic/edit
@asakusuma

This comment has been minimized.

Copy link

commented Sep 18, 2019

@jakearchibald took a pass at a failing test for verifying that clients are uncontrolled after Clear-Site-Data web-platform-tests/wpt#19132

asakusuma added a commit to asakusuma/wpt that referenced this issue Sep 18, 2019
…e directive

Adds a failing test, asserting that pages actively controlled by a service worker become uncontrolled after the Clear-Site-Data storage directive.

We agreed on this behavior in a spec issue and confirmed the need for this test at the TPAC Service Workers F2F.

w3c/ServiceWorker#614
F2F notes: https://docs.google.com/document/d/1_Qfw5m3BJEaL1xIzTJd41HXjgJ9gq7mroBDXqSJIzic/edit
asakusuma added a commit to asakusuma/wpt that referenced this issue Sep 18, 2019
…e directive

Adds a failing test, asserting that pages actively controlled by a service worker become uncontrolled after the Clear-Site-Data storage directive.

We agreed on this behavior in a spec issue and confirmed the need for this test at the TPAC Service Workers F2F.

w3c/ServiceWorker#614
F2F notes: https://docs.google.com/document/d/1_Qfw5m3BJEaL1xIzTJd41HXjgJ9gq7mroBDXqSJIzic/edit
asakusuma added a commit to asakusuma/wpt that referenced this issue Sep 18, 2019
…e directive

Adds a failing test, asserting that pages actively controlled by a service worker become uncontrolled after the Clear-Site-Data storage directive.

We agreed on this behavior in a spec issue and confirmed the need for this test at the TPAC Service Workers F2F.

w3c/ServiceWorker#614
F2F notes: https://docs.google.com/document/d/1_Qfw5m3BJEaL1xIzTJd41HXjgJ9gq7mroBDXqSJIzic/edit
asakusuma added a commit to asakusuma/wpt that referenced this issue Sep 18, 2019
…e directive

Adds a failing test, asserting that pages actively controlled by a service worker become uncontrolled after the Clear-Site-Data storage directive.

We agreed on this behavior in a spec issue and confirmed the need for this test at the TPAC Service Workers F2F.

w3c/ServiceWorker#614
F2F notes: https://docs.google.com/document/d/1_Qfw5m3BJEaL1xIzTJd41HXjgJ9gq7mroBDXqSJIzic/edit
asakusuma added a commit to asakusuma/wpt that referenced this issue Sep 18, 2019
…e directive

Adds a failing test, asserting that pages actively controlled by a service worker become uncontrolled after the Clear-Site-Data storage directive.

We agreed on this behavior in a spec issue and confirmed the need for this test at the TPAC Service Workers F2F.

w3c/ServiceWorker#614
F2F notes: https://docs.google.com/document/d/1_Qfw5m3BJEaL1xIzTJd41HXjgJ9gq7mroBDXqSJIzic/edit
asakusuma added a commit to asakusuma/wpt that referenced this issue Sep 27, 2019
…e directive

Adds a failing test, asserting that pages actively controlled by a service worker become uncontrolled after the Clear-Site-Data storage directive.

We agreed on this behavior in a spec issue and confirmed the need for this test at the TPAC Service Workers F2F.

w3c/ServiceWorker#614
F2F notes: https://docs.google.com/document/d/1_Qfw5m3BJEaL1xIzTJd41HXjgJ9gq7mroBDXqSJIzic/edit
asakusuma added a commit to asakusuma/wpt that referenced this issue Sep 30, 2019
…e directive

Adds a failing test, asserting that pages actively controlled by a service worker become uncontrolled after the Clear-Site-Data storage directive.

We agreed on this behavior in a spec issue and confirmed the need for this test at the TPAC Service Workers F2F.

w3c/ServiceWorker#614
F2F notes: https://docs.google.com/document/d/1_Qfw5m3BJEaL1xIzTJd41HXjgJ9gq7mroBDXqSJIzic/edit
asakusuma added a commit to asakusuma/wpt that referenced this issue Oct 1, 2019
…e directive

Adds a failing test, asserting that pages actively controlled by a service worker become uncontrolled after the Clear-Site-Data storage directive.

We agreed on this behavior in a spec issue and confirmed the need for this test at the TPAC Service Workers F2F.

w3c/ServiceWorker#614
F2F notes: https://docs.google.com/document/d/1_Qfw5m3BJEaL1xIzTJd41HXjgJ9gq7mroBDXqSJIzic/edit
wanderview added a commit to web-platform-tests/wpt that referenced this issue Oct 7, 2019
…e directive (#19132)

Adds a failing test, asserting that pages actively controlled by a service worker become uncontrolled after the Clear-Site-Data storage directive.

We agreed on this behavior in a spec issue and confirmed the need for this test at the TPAC Service Workers F2F.

w3c/ServiceWorker#614
F2F notes: https://docs.google.com/document/d/1_Qfw5m3BJEaL1xIzTJd41HXjgJ9gq7mroBDXqSJIzic/edit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.