Skip to content

[worklets] Fix module loading when creating a new worklet global scope. #264

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

[worklets] Fix module loading when creating a new worklet global scope. #264

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c2326d4
[worklets] Fix module loading when creating a new worklet global scope.
bfgeek Jul 20, 2016
87d56cd
[worklets] Introduce top level module url list.
bfgeek Jul 21, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 43 additions & 33 deletions worklets/Overview.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,14 +185,13 @@ When a user agent is to <dfn>create a WorkletGlobalScope</dfn>, for a given |wor

1. Let |resolvedModuleURL| be |entry|'s key.

2. Let |script| be the result of <a>fetch a module script tree</a> given
|resolvedModuleURL|, "anonymous" for the <a>CORS setting attribute</a>, and
|settingsObject|.
2. <a>Fetch a module worklet script tree</a> given |resolvedModuleURL|, |outsideSettings|,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slight problem as we don't have access to |outsideSettings| here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this whole section. The module responses map stores every module, including imported ones. And you want to fetch and execute them all? That means that if you do import './a.js' and a.js does import './b.js', then the module responses map will have both a.js and b.js in it. Then you will execute the tree starting at a.js (so both a.js and b.js), but also you will separately execute the tree starting at b.js (so b.js will be executed twice). This seems bad?

What is the intention of this section?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this was wrong. This only should be loading the top level requests.

This section is used for if a user-agent wants to create a new WorkletGlobalScope in addition to the ones it already has. (E.g. it may have killed one from the pool and needs to restart or something).

When instantiating the new WorkletGlobalScope it has to load all the previous top level modules loaded into it.

Added a "top level module url list" to the worklet which the fetch hook now populates. This reads off that to load it's code.

and |settingsObject|.

Note: Worklets follow <a>web workers</a> here in not allowing "use-credientials" for
fetching resources.
3. Let |script| be the result of <a>fetch a module worklet script tree</a> when it
asynchronously completes.

3. <a>Run a module script</a> given |script|.
4. <a>Run a module script</a> given |script|.

### Script settings for worklets ### {#script-settings-for-worklets}

Expand Down Expand Up @@ -247,6 +246,41 @@ When a user agent is to <dfn>set up a worklet environment settings object</dfn>,

Issue: Merge this with https://html.spec.whatwg.org/multipage/workers.html#set-up-a-worker-environment-settings-object

### Fetching a module worklet script tree ### {#fetching-a-module-worklet-script-tree}

When a user agent is to <dfn>fetch a module worklet script tree</dfn>, given |resolvedModuleURL|,
|outsideSettings|, and |insideSettings| it must run the following steps:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: comma after |insideSettings|

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


1. <a>Fetch a module script tree</a> given |resolvedModuleURL|, |outsideSettings|, "script" the
empty string (as no cryptographic nonce is present for worklets), "not parser-inserted", "omit",
and |insideSettings|.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, things have changed here a bit. I think you want fetch a module worker script tree given resolvedModuleURL, outsideSettings, "script", "omit", and insideSettings.

Also, you may want to invent a new destination of "worklet", since there are separate "sharedworker" and "worker" destinations, so it seems like that's the level of granularity we're operating at. That would require a change to Fetch (@annevk).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed. I was discussing this with @mikewest (as destination is just used for the CSP check right?) settled on "script-src" being ok initially.

Let me know if I'm off the track here in what this param does. (traced it through the other day... i think)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. "script-src" is the CSP directive currently used for <script> and importScripts(), whereas workers and shared workers use "child-src". The choice here is also web-exposed when a service worker intercepts the request. Maybe open a follow-up issue so we can get everyone on the same page?


To <a>perform the fetch</a> given |request|, perform the following steps:

1. Let |cache| be the current {{Worklet}}'s <a>module responses map</a>.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"current Worklet" doesn't seem very defined. Probably insideSetting's global object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cache lives of the Worklet object, not the global, but done by passing other parameter into this function.


2. Let |url| be |request|'s <a >url</a>.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: extra space in <a >

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, this goes to the wrong place; you want to go to https://fetch.spec.whatwg.org/#concept-request-url

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done & done.


3. If |cache| contains an entry with key |url| whose value is "fetching", wait
(<a>in parallel</a>) until that entry's value changes, then proceed to the
next step.

4. If |cache| contains an entry with key |url|, asynchronously complete this
algorithm with that entry's value, and abort these steps.

5. Create an entry in |cache| with key |url| and value "fetching".

6. <a>Fetch</a> |request|.

7. Let |response| be the result of <a>fetch</a> when it asynchronously
completes.

8. Set the value of the entry in |cache| whose key is |url| to |response|, and
asynchronously complete this algorithm with |response|.

2. Asynchronously complete this algorithm with the result of <a>fetch a module script tree</a>
when it completes.

Worklet {#worklet-section}
--------------------------

Expand Down Expand Up @@ -302,34 +336,10 @@ the user agent <em>must</em> run the following steps:
1. Let |insideSettings| be the {{WorkletGlobalScope}}'s associated <a>environment
settings object</a>.

2. <a>Fetch a module script tree</a> given |resolvedModuleURL|, "omit", the empty string
(as no cryptographic nonce is present for worklets), "not parser-inserted",
"script", |outsideSettings|, and |insideSettings|.

To <a>perform the request</a> given |request|, perform the following steps:

1. Let |cache| be the current {{Worklet}}'s <a>module responses map</a>.

2. Let |url| be |request|'s <a >url</a>.

3. If |cache| contains an entry with key |url| whose value is "fetching", wait
(<a>in parallel</a>) until that entry's value changes, then proceed to the
next step.

4. If |cache| contains an entry with key |url|, asynchronously complete this
algorithm with that entry's value, and abort these steps.

5. Create an entry in |cache| with key |url| and value "fetching".

6. <a>Fetch</a> |request|.

7. Let |response| be the result of <a>fetch</a> when it asynchronously
completes.

8. Set the value of the entry in |cache| whose key is |url| to |response|, and
asynchronously complete this algorithm with |response|.
2. <a>Fetch a module worklet script tree</a> given |resolvedModuleURL|,
|outsideSettings|, and |settingsObject|.

3. Let |script| be the result of <a>fetch a module script tree</a> when it
3. Let |script| be the result of <a>fetch a module worklet script tree</a> when it
asynchronously completes.

4. <a>Run a module script</a> given |script|.
Expand Down