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

Configure script's settings and realm for service workers #1294

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jungkees
Copy link
Collaborator

@jungkees jungkees commented 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.


Preview | Diff

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
Copy link
Collaborator Author

@domenic, passing null to those algorithms breaks the current create a classic script and create a module script algorithm steps. I'm not sure if I still need to create an environment settings object and pass it although the script's settings will be replaced by the another settings object in Run Service Worker eventually.

As far as I can tell, script's settings and script's record.[[Realm]] are referenced when the script is evaluated. So, even if we don't set them in the fetching phase, it wouldn't make any side effects, right?

@domenic
Copy link
Contributor

domenic commented Apr 6, 2018

Hmm, very interesting. It looks like it would be OK to set these to null for create a classic/module script, and have you patch them up later. We just need to fix the HTML algorithms to support null. I can do that. But first I am concerned about another thing...

The fetch a module worker script graph algorithm seems to depend on the module map settings object for other purposes, for example, preventing multiple concurrent fetches of the same URL. I am worried about how that would be implemented given that the realm is not yet created.

This is similar to what I said in #1013 (comment):

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.

Maybe implementers can comment on how they implement service worker script parsing and realm initialization?

Base automatically changed from master to main February 4, 2021 19:56
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

Successfully merging this pull request may close these issues.

Sort out settings object for service worker module scripts
2 participants