search

w3c / push-api

Push API
https://w3c.github.io/push-api/
Other
146 stars 40 forks source link

event.waitUntil(registration.subscribe()) would cause a deadlock #221

Closed mfalken closed 7 years ago

mfalken commented 8 years ago

The steps for subscribe wait for the installing or waiting version to become active before resolving the promise. This seems to mean oninstall = e => { e.waitUntil(self.registration.subscribe()); } would cause a deadlock (though the UA may timeout the event and fail the installation). Maybe subscribe() should reject if called without an active worker? I believe Chrome's current implementation does this.

beverloo commented 7 years ago

Good catch! The spec was changed in favor of developer convenience, this is an unintentional side-effect.

@kitcambridge, would you know what Firefox does here? We haven't implemented this and still reject when calling subscribe() on a Service Worker that's not activated.

ghost commented 7 years ago

Oof, I don't think we enforce the active worker requirement at all. If the origin has permission, Firefox will happily create the subscription and resolve the promise. https://bugzilla.mozilla.org/show_bug.cgi?id=1244349 is the closest bug for adding that check.

beverloo commented 7 years ago

That's unfortunate! Is there any way for you to add telemetry so that we learn whether it's safe to require an active worker in the spec? (And for Firefox to match it.)

jakearchibald commented 7 years ago

Requiring an active worker feels important, otherwise you can subscribe for push at a time that the registration may fail and be discarded.

I'm not bothered if it waits or throws, but we should make sure all service worker APIs do the same thing.

beverloo commented 7 years ago

@jakearchibald fixed this in 9349b6616d991aff7396083d6048a04642abee2e