1784457 - implement is top-level flag for worker scripts

Status: RESOLVED FIXED

(Core :: DOM: Workers, task)

Description

[Yulia Startsev [:yulia]]

TL;DR -- it makes more sense to put the is top-level bit on the WorkerLoadContext than on the loader itself.

When we are running a worker, we currently distinguish between a main worker script (the one that the worker is loaded with) and subsequent scripts loaded via importScripts or similar. Part of this gated work is implementing step 14 of https://html.spec.whatwg.org/multipage/workers.html#worker-processing-model

However, when modules land, storing the "is top-level" flag duplicates information already present on the ScriptLoadRequest. The loading strategy used by importScripts uses a blocking sync loop which allows the imported script to load, and run, synchronously.

Modules on the other hand are asynchronous, and their order of resolution is not significant. If we use a blocking sync loop, then we would only use it for the load and link portion, while potentially blocking other work as we go deeper into one branch of the module graph. For execution, everything will be executed on the root sync loop anyway, because module initialization and execution is only triggered by the main module running, after everything has already been loaded.

Dynamic modules present yet another execution model, which lies between these two models. Dynamic modules run when the code is reached by execution, they register a promise which will resolve separately, async, but inline. However, the internal modules that make up the dynamic import will follow the same rules as a regular module. However it does set custom properties such as csp that are independent of their parent.

Comment 1

[Yulia Startsev [:yulia]]

Attachment: Bug 1784457 - Remove unused mLoadingWorkerScript atomic boolean from WorkerPrivate; r=asuth

After investigating this a bit, I found that this wasn't used. It seems safe to remove.

Depends on D154385

Comment 2

[Yulia Startsev [:yulia]]

Attachment: Bug 1784457 - Split GetBaseURI into two functions; r=asuth

GetBaseURI will implement the virtual method GetBaseURI from ScriptLoaderInterface
(https://searchfox.org/mozilla-central/rev/6ec440e105c2b75d5cae9d34f957a2f85a106d54/js/loader/ModuleLoaderBase.h#64)
-- but the behavior needed there is only ever for child modules. By splitting this, we remove
contextual information which is only used for the initial script.

Depends on D154519

Comment 3

[Yulia Startsev [:yulia]]

Attachment: Bug 1784457 - Implement IsTopLevel in place of IsMainWorkerModule; r=asuth

This implements the core of the change -- which is implementing an explicit IsTopLevel flag for
WorkerLoadContexts, and removing mIsMainScript from the loader. I've asked for some
clarification on this part of the spec from the whatwg editors, as I think this may be misnamed. It
is likely that what is meant here is IsInitialScript rather than IsTopLevel -- as there is a
note stating that this should be initialized with the agent cluster. Once this is clarified I may
update the name.

Comment 4

[Yulia Startsev [:yulia]]

Attachment: Bug 1784457 - Add documentation to worker kinds; r=asuth

Depends on D147319

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4a04d2b6a13a
Remove unused mLoadingWorkerScript atomic boolean from WorkerPrivate; r=asuth
https://hg.mozilla.org/integration/autoland/rev/9563cc483b5b
Split GetBaseURI into two functions; r=asuth
https://hg.mozilla.org/integration/autoland/rev/e46aad19c241
Implement IsTopLevel in place of IsMainWorkerModule; r=asuth
https://hg.mozilla.org/integration/autoland/rev/493caeca88c6
Add documentation to worker kinds; r=asuth

Comment 6

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/4a04d2b6a13a
https://hg.mozilla.org/mozilla-central/rev/9563cc483b5b
https://hg.mozilla.org/mozilla-central/rev/e46aad19c241
https://hg.mozilla.org/mozilla-central/rev/493caeca88c6