1784476 - Ensure that every worker loaded script has the correct mClientInfo

Status: RESOLVED FIXED

(Core :: DOM: Workers, task)

Description

[Yulia Startsev [:yulia]]

TL;DR -- when implementing modules I realized that we need to store ClientInfo independently of the loader.

Previously, in an earlier refactoring -- I noticed that we were setting a ClientInfo in two places, once for use in the channel, which was used by importScripts:

https://searchfox.org/mozilla-central/rev/63d3ddbc815ee433a45a982d2e74e2994ebbc22c/dom/workers/ScriptLoader.cpp#2323-2331

This is passed here: https://searchfox.org/mozilla-central/rev/63d3ddbc815ee433a45a982d2e74e2994ebbc22c/dom/workers/ScriptLoader.cpp#1066-1071

And eventually used only by import scripts here:

https://searchfox.org/mozilla-central/rev/63d3ddbc815ee433a45a982d2e74e2994ebbc22c/dom/workers/ScriptLoader.cpp#236-240

The second place it was set was in a "reservedClientInfo" field, only to be used by main threads:

https://searchfox.org/mozilla-central/rev/63d3ddbc815ee433a45a982d2e74e2994ebbc22c/dom/workers/ScriptLoader.cpp#1102-1117

Since these two were storing the same information, and every creation of import scripts would create a new ClientInfo for that request, I simplified these two fields and went with only having one, stored on the loading. However, there is a subtle difference now, in that we can sometimes land in this code path with a main script:

https://searchfox.org/mozilla-central/rev/63d3ddbc815ee433a45a982d2e74e2994ebbc22c/dom/workers/ScriptLoader.cpp#235-241

This didn't result in any problems or test failures so I left it as it was.

However, Modules make this more complicated! We need to regenerate the client info for child modules, we cannot use one that is set globally on the loader as it will be out of date in comparison to the headers that come back from the load of an initial script. To prep for loading modules, scripts should be aware of what the client load info is, so we need to store it on the WorkerLoadContext.

Comment 1

[Yulia Startsev [:yulia]]

Attachment: Bug 1784476 - Move all required information for constructing ScriptLoadRequests to WorkerScriptLoader constructor; r=asuth

Comment 2

[Yulia Startsev [:yulia]]

Attachment: Bug 1784476 - Move ClientInfo to WorkerLoadContext; r=asuth

Previously, we had the client info for main scripts on the ScriptLoader, and in the ScriptLoadInfo
for non-main scripts. This was a bit confusing, so I moved it all to the ScriptLoader as in the
importScripts case, it is always recreated with each new load. However, for modules this is not
true. In order to make it persistant for main scripts and modules, as well as ensure that it has the
correct data, it should always live on the
WorkerLoadContext.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6e7ce080121d
Move all required information for constructing ScriptLoadRequests to WorkerScriptLoader constructor; r=asuth
https://hg.mozilla.org/integration/autoland/rev/037726440c5f
Move ClientInfo to WorkerLoadContext; r=asuth

Comment 4

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/6e7ce080121d
https://hg.mozilla.org/mozilla-central/rev/037726440c5f