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

Sort out settings object for service worker module scripts #1013

Open
jungkees opened this issue Nov 24, 2016 · 4 comments · May be fixed by #1294
Open

Sort out settings object for service worker module scripts #1013

jungkees opened this issue Nov 24, 2016 · 4 comments · May be fixed by #1294

Comments

@jungkees
Copy link
Collaborator

SW has different script execution model from other workers. SW's script fetching and running execute in separate places with a different lifecycle.

Fetch a module worker script graph requires a settings object in the fetching phase to pare the scripts. (Fetch a classic worker script also requires this argument as well but seems not using for parsing in the fetching phase.)

We need to fix it such that module scripts parsed during the fetching phase can be used in subsequent Run Service Worker script executions.

Issue reported in #1012 (review).

@jungkees jungkees added this to the Version 1 milestone Nov 24, 2016
jungkees added a commit that referenced this issue Nov 24, 2016
Fix arguments to "fetch a classic worker script"

Left issue: #1013.
@jungkees
Copy link
Collaborator Author

jungkees commented Dec 13, 2016

Soft Update is using null for a job's client that's later passed to Fetch a classic worker script and Fetch a module worker script graph. This should be addressed, too.

@jungkees
Copy link
Collaborator Author

jungkees commented Mar 14, 2017

@domenic, @annevk,

I've been thinking about the environment settings object for service workers. Currently we create a settings object for a service worker whenever Run Service Worker needs a cold start. So, it's basically 1:1:1 to the realm and the global object as for other environments. However, having thought about it more, I think it should rather be bound to the service worker's lifetime instead of the realm and global object's lifetime. It seems most of the settings object's fields can be (or need to be) maintained across multiple global objects' creations and terminations. Moreover, fetching a * worker script requires a settings object, which isn't what service worker's Update algorithm strictly follows at the moment. (Update step 7 now passes a to-be-created environment settings object to fetch a * worker script.)

That considered, I propose we create an environment settings object for a service worker in Update algorithm before invoking fetch a * worker script. This settings object is associated with the service worker's script during fetch a * worker script execution. And Run Service Worker algorithm later uses this service worker's script's settings object to run the script with additional set up steps that associate the settings object with the created realm and the global object.

One tricky part is fetch a module worker script graph > .. > create a module script > ParseModule() requires a realm while service worker's Update doesn't create a realm. In my understanding, the only reason that the realm is passed is to set the created script's module record's [[Realm]] internal slot, right? If so, can I set/pass the environment settings object's Realm to undefined at this stage? And later override the value to the actual realm created in Run Service Worker?

To sum, I propose we redefine the lifetime of service worker's environment settings object to satisfy the service worker's processing model. It would look something like:

Update

  • Create an environment settings object with its realm initialized to undefined.
    • Realm execution context returns null.
    • Module map is initial empty map.
    • Create an event loop [1]
  • Fetch a * worker script with the created settings object

Run Service Worker

  • Create a parallel context
  • Invoke InitializeHostDefinedRealm() - i.e. Create a realm
  • Create a global object
  • Set up the settings object with service worker's script's settings object and the created realm
    • Set script's module record's [[Realm]] to the given realm
    • Set realm's [[HostDefined]] field to the given settings object

[1] If it makes sense to create the event loop in Update and use it until the registration is cleared, we don't even have to back up the tasks to registration's task queue in Terminate Service Worker. Does it make sense?

WDYT?

@domenic
Copy link
Contributor

domenic commented Mar 16, 2017

Hmm. I think it would be pretty bad to break the 1:1:1 correspondence. A lot of the platform assumes that.

Also, note what "fetch a worker script" uses the settings object for: it uses it to determine the referrer and such. So I don't think the service worker should be fetching itself, should it? It should be the settings object of the page itself that is doing navigator.serviceWorker.register(). In other words, I think you need a separate settings object for the fetching, and for the running of scripts. The fetching should be "outside", e.g. the initiating document. Whereas the script running should be inside the service worker.

This might be different for future updates. But at least for the initial navigator.serviceWorker.register() it seems necessary.

One tricky part is fetch a module worker script graph > .. > create a module script > ParseModule() requires a realm while service worker's Update doesn't create a realm. In my understanding, the only reason that the realm is passed is to set the created script's module record's [[Realm]] internal slot, right? If so, can I set/pass the environment settings object's Realm to undefined at this stage? And later override the value to the actual realm created in Run Service Worker?

Something like that might work, although we'd need to modify the spec to allow not passing in a realm (we can't just treat undefined as if it's a realm). But I wonder if after considering the above we can find some way to avoid this. I'd be very surprised if any implementation actually parses service worker scripts before creating an instance of the JS engine for the service worker scripts to run in. And the spec should ideally not do something so different from implementations.

jungkees added a commit that referenced this issue Mar 28, 2018
Service workers have a different script execution model from other
workers. Update algorithm fetches a service worker script, and Run
Service Worker algorithm runs the script when installing it and whenever
functional events and message events need to be dispatched.

This change:
 - Passes null as the value of script settings object/module map
   settings object argument to fetch a classic worker script/fetch a
   module worker script graph algorithm, respectively, called in Update
   algorithm, instead of the placeholder argument "the to-be-created
   environment settings object".
 - Sets the script's settings to the environment settings object created
   in Run Service Worker algorithm and the script's record.[[Realm]] to
   that settings object's Realm.

Fixes #1013.
jungkees added a commit that referenced this issue Jul 15, 2019
The Update algorithm step 9 has two issues:
 - Using to-be-created environment settings object instead of a concrete
 environment settings object
 - Passing null value of job's client to the fetch a script algorithms in HTML

This change leaves notes on these issues to track them in the nightly version
and moves them to the Version 2 milestone in the github repository.

For #1013.
jakearchibald pushed a commit that referenced this issue Jul 19, 2019
The Update algorithm step 9 has two issues:
 - Using to-be-created environment settings object instead of a concrete
 environment settings object
 - Passing null value of job's client to the fetch a script algorithms in HTML

This change leaves notes on these issues to track them in the nightly version
and moves them to the Version 2 milestone in the github repository.

For #1013.
@jungkees
Copy link
Collaborator Author

Changing the milestone to V2 after adding notes and issues: 2e2d872.

@jungkees jungkees modified the milestones: Version 1, Version 2 Jul 19, 2019
jakearchibald pushed a commit to CYBAI/ServiceWorker that referenced this issue Aug 9, 2019
The Update algorithm step 9 has two issues:
 - Using to-be-created environment settings object instead of a concrete
 environment settings object
 - Passing null value of job's client to the fetch a script algorithms in HTML

This change leaves notes on these issues to track them in the nightly version
and moves them to the Version 2 milestone in the github repository.

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

Successfully merging a pull request may close this issue.

3 participants