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

Should update() always reject if there is an installing worker? #1429

Open
Yannic opened this issue Jun 9, 2019 · 5 comments
Open

Should update() always reject if there is an installing worker? #1429

Yannic opened this issue Jun 9, 2019 · 5 comments

Comments

@Yannic
Copy link

Yannic commented Jun 9, 2019

Currently, section 3.2.6 (step 4) of the spec mandates that calling update() rejects immediately if the context object’s relevant settings object's global object globalObject is a ServiceWorkerGlobalScope object, and globalObject's associated service worker's state is installing (i.e. calling update() inside the install handler rejects, compare WPT).

Updating if there is a worker in installing state doesn't really make sense and most likely indicates that sites call update() too often. It may be cleaner to change the spec to always reject if there is an installing worker and not only from within the installing worker.

Given that Firefox doesn't follow the spec here and already reject updates immediately (https://bugzilla.mozilla.org/show_bug.cgi?id=1488792) and Chrome only very recently implemented the current spec (when fixing a bug that prevented updating if there was an installing worker, https://crbug.com/895845), this change shouldn't cause sites to break.

What do people think? @aliams @asutherland @jakearchibald @mattto @wanderview @youennf

@mfalken
Copy link
Member

mfalken commented Jun 10, 2019

Thanks Yannic. Let me merge this with #1155 to keep discussion in one place.

@mfalken mfalken closed this as completed Jun 10, 2019
@mfalken mfalken reopened this Jun 10, 2019
@mfalken
Copy link
Member

mfalken commented Jun 10, 2019

On second thought, let me close that one and keep this one. The other one was about whether to reject if called from an installing worker, this is about whether to reject when there is any installing worker, even if called from another context.

@mfalken
Copy link
Member

mfalken commented Jun 10, 2019

For history, discussions about this step were at: #800 #1155

In my opinion, it's easy to avoid calling update() in an installing worker. It's harder to avoid calling update() when there is any installing worker, so throwing can be surprising and you'll end up with code like: if (!registration.installing) { registration.update(); } which seems cumbersome.

I'm feeling now update() when there is an installing worker should just be coalesced into that ongoing job and dropped silently without triggering a new update.

@jakearchibald
Copy link
Contributor

Agreed.

@Yannic
Copy link
Author

Yannic commented Jun 13, 2019

Merging update() into already running ones should have the same effect. We'll still need the special handling for installing workers, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants