Use outside settings object to fetch workers #1149
Conversation
|
|
||
| <dt>"<code data-x="">module</code>"</dt> | ||
| <dd><span>Fetch a module script tree</span> given <var>url</var>, the value of the <code | ||
| data-x="">credentials</code> member of <var>options</var>, the empty string (as no | ||
| <var>cryptographic nonce</var> is present for workers), "<code data-x="">not | ||
| parser-inserted</code>", <var>destination</var>, and <var>settings object</var>.</dd> | ||
| parser-inserted</code>", <var>destination</var>, and <var>outside settings</var>.</dd> |
There was a problem hiding this comment.
Doesn't "Fetch a module script tree" still need access to both the outside settings object and the settings object for the worker itself? The modules it fetches should be stored in the module map associated with the worker, not the one associated with the parent environment.
There was a problem hiding this comment.
Yes, you're right.
This raises an interesting question. When we fetch a module script tree for a worker, what fetch client should we use for fetching imported modules? The outside one, or the inside one? The outside one was used for fetching the original module, but the inside one seems more like the thing that actually initiated the fetch.
There was a problem hiding this comment.
It certainly seems like it would make sense to mirror what importScripts does, and use the worker as fetch client for purposes of CSP checks, referrers etc.
There was a problem hiding this comment.
Agreed. OK, pushed a second commit which corrects that. Thanks for the catch.
|
This could use editor review BTW. |
domenic
force-pushed
the
fix-worker-client
branch
from
890e5b2 to
f280eae
Compare
As discussed in #1122, #1111, and in w3c/ServiceWorker#889 (comment), a number of problems are caused by the current setup of using the settings object of the worker itself as the feth client. Instead, we use the incumbent settings object to do the fetching. (Incumbent, instead of e.g. current, because almost everything else in the (Shared)Worker constructors uses the incumbent settings object.) Notably, for fetching module script trees, this necessitates separating the fetch client settings object from the one used for the module map. This fixes #1111 since now module workers are fetched with the correct client, and thus automatically get the correct referrer.
| <p>The following algorithms are used when <span data-x="fetch a module script tree">fetching a | ||
| module script tree</span>, and are not meant to be used directly by other specifications (or by | ||
| other parts of this specification).</p> | ||
|
|
||
| <p>To <dfn>fetch the descendants of a module script</dfn> <var>module script</var>, given a |
There was a problem hiding this comment.
Does this not need to be modified too? If a page A fetches a module script B which has C as dependency, the referrer for B is A and C is B. I don't really see where we define that C has B as referrer.
There was a problem hiding this comment.
That is a separate bug which I hope to work on after fixing this one: #1150
There was a problem hiding this comment.
I added that to the commit message.
As discussed in #1122, #1111, and in
w3c/ServiceWorker#889 (comment),
a number of problems are caused by the current setup of using the
settings object of the worker itself as the feth client. Instead, we use
the incumbent settings object to do the fetching. (Incumbent, instead
of e.g. current, because almost everything else in the (Shared)Worker
constructors uses the incumbent settings object.)
This fixes #1111 since now module workers are fetched with the correct
client, and thus automatically get the correct referrer.
/cc @mkruisselbrink.