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

Spec uses incorrect settings object when fetching a module worker #849

Open
mkruisselbrink opened this issue Mar 15, 2016 · 2 comments
Open

Comments

@mkruisselbrink
Copy link
Collaborator

In Update the algorithm uses "job's client" as "settings object" passed to "Fetch a module script tree". This client is whatever context called "register" or "update", and could even be null if this is a regular "soft update" check. Fetch a module script tree then uses this settings object to find the module map in which to look up modules to fetch. Obviously when client is null that wouldn't work at all, but even when client isn't null we shouldn't reuse the module map of the client that installed the service worker, rather we should be using an module map that is owned by the service worker (similar to how regular workers have their own module map).

Other than for the module map this client is also used as the client for the requests for actual scripts, both in classic and module scripts. This effects things like the referrer of the request. Not sure what the correct thing to do there is, but the answer to that might influence to correct fix for the module map issue.

@mkruisselbrink mkruisselbrink added this to the Version 2 milestone Mar 15, 2016
@wanderview
Copy link
Member

Also note the job's client is null when created from Soft Update. Not sure storing the client on the job is a good idea at all. We use its origin in some places, but otherwise its only used by the script loading for environment settings (which seems wrong as you point out).

@mkruisselbrink
Copy link
Collaborator Author

Yeah, the job's client was used in a few more places (by associating it with ServiceWorker and ServiceWorkerRegistation instances), but I removed those in 08a506e because those client associations weren't actually needed. So a possible solution for this bug could indeed be to just get rid of the client of a job entirely.

I wonder if current implementations use the client as referrer of requests to fetch a serviceworker, or if this something where implementations already don't follow the spec (looking at the chrome implementation I don't think we use the client as referrer anywhere). If spec already don't set the referrer of these requests to the client who called register/update, I don't see any reason not to make the spec match the implementations, and get rid of the client in jobs/updating entirely.

Of course we'd then still need a way to pass a module map to "fetch a module script tree" separate from the (possibly null) settings object, or create a dummy settings object just for the purpose of fetching updates.

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

No branches or pull requests

3 participants