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.

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

Don't associate a job with a client, instead just have a referrer. #889

Closed
wants to merge 1 commit into from
Closed

Don't associate a job with a client, instead just have a referrer. #889

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 13 additions & 12 deletions spec/service_worker/index.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,7 @@ spec: webidl; urlPrefix: https://heycam.github.io/webidl/
<li>Let <var>newestWorker</var> be the result of running <a href="#get-newest-worker-algorithm">Get Newest Worker</a> algorithm passing <var>registration</var> as its argument.</li>
<li>If <var>newestWorker</var> is null, reject <var>p</var> with an "{{InvalidStateError}}" exception and abort these steps.</li>
<li>If the <a>context object</a>'s <a>relevant settings object</a>'s <a>global object</a> <var>globalObject</var> is a {{ServiceWorkerGlobalScope}} object, and <var>globalObject</var>'s associated <a href="#dfn-service-worker-global-scope-service-worker">service worker</a>'s <a href="#dfn-state">state</a> is <em>installing</em>, reject <var>p</var> with an "{{InvalidStateError}}" exception and abort these steps.</li>
<li>Let <var>job</var> be the result of running <a href="#create-job-algorithm">Create Job</a> with <em>update</em>, <var>registration</var>'s <a href="#dfn-scope-url">scope url</a>, <var>newestWorker</var>'s <a href="#dfn-script-url">script url</a>, <var>p</var>, and the <a>context object</a>'s <a>relevant settings object</a> <var>client</var>.</li>
<li>Let <var>job</var> be the result of running <a href="#create-job-algorithm">Create Job</a> with <em>update</em>, <var>registration</var>'s <a href="#dfn-scope-url">scope url</a>, <var>newestWorker</var>'s <a href="#dfn-script-url">script url</a>, <var>p</var>, and the <a>context object</a>'s <a>relevant settings object</a>'s <a>creation URL</a>.</li>
<li>Set <var>job</var>'s <a>worker type</a> to <var>newestWorker</var>'s <a for="service worker">type</a>.</li>
<li>Invoke <a href="#schedule-job-algorithm">Schedule Job</a> with <var>job</var>.</li>
<li>Return <var>p</var>.</li>
Expand All @@ -634,7 +634,7 @@ spec: webidl; urlPrefix: https://heycam.github.io/webidl/

<ol>
<li>Let <var>p</var> be a <a>promise</a>.</li>
<li>Let <var>job</var> be the result of running <a href="#create-job-algorithm">Create Job</a> with <em>unregister</em>, the <a href="#dfn-scope-url">scope url</a> of the <a href="#dfn-service-worker-registration-interface-service-worker-registration">service worker registration</a>, null, <var>p</var>, and the <a>context object</a>'s <a>relevant settings object</a> <var>client</var>.</li>
<li>Let <var>job</var> be the result of running <a href="#create-job-algorithm">Create Job</a> with <em>unregister</em>, the <a href="#dfn-scope-url">scope url</a> of the <a href="#dfn-service-worker-registration-interface-service-worker-registration">service worker registration</a>, null, <var>p</var>, and the <a>context object</a>'s <a>relevant settings object</a>'s <a>creation URL</a>.</li>
<li>Invoke <a href="#schedule-job-algorithm">Schedule Job</a> with <var>job</var>.</li>
<li>Return <var>p</var>.</li>
</ol>
Expand Down Expand Up @@ -789,7 +789,7 @@ spec: webidl; urlPrefix: https://heycam.github.io/webidl/
<li>If <var>scopeURL</var> is failure, reject <var>p</var> with a <code>TypeError</code> and abort these steps.</li>
<li>If <var>scopeURL</var>'s <a for="url">scheme</a> is not one of "<code>http</code>" and "<code>https</code>", reject <var>p</var> with a <code>TypeError</code> and abort these steps.</li>
<li>If any of the strings in <var>scopeURL</var>'s <a for="url">path</a> contains either <a>ASCII case-insensitive</a> "<code>%2f</code>" or <a>ASCII case-insensitive</a> "<code>%5c</code>", reject <var>p</var> with a <code>TypeError</code> and abort these steps.</li>
<li>Let <var>job</var> be the result of running <a href="#create-job-algorithm">Create Job</a> with <em>register</em>, <var>scopeURL</var>, <var>scriptURL</var>, <var>p</var>, and <var>client</var>.</li>
<li>Let <var>job</var> be the result of running <a href="#create-job-algorithm">Create Job</a> with <em>register</em>, <var>scopeURL</var>, <var>scriptURL</var>, <var>p</var>, and <var>client</var>'s <a>creation URL</a>.</li>
<li>Set <var>job</var>'s <a>worker type</a> to <var>options</var>.{{RegistrationOptions/type}}.</li>
<li>Invoke <a href="#schedule-job-algorithm">Schedule Job</a> with <var>job</var>.</li>
<li>Return <var>p</var>.</li>
Expand Down Expand Up @@ -2882,7 +2882,7 @@ spec: webidl; urlPrefix: https://heycam.github.io/webidl/

<P>A <a href="#dfn-job">job</a> has a <dfn id="dfn-job-worker-type">worker type</dfn> ("<code>classic</code>" or "<code>module</code>").</P>

<P>A <a href="#dfn-job">job</a> has a <dfn id="dfn-job-client">client</dfn> (a <a href="#dfn-service-worker-client">service worker client</a>). It is initially null.</P>
<P>A <a>job</a> has a <dfn for=job>referrer</dfn> (a <a for=url>URL</a> or null).</p>

<P>A <a href="#dfn-job">job</a> has a <dfn id="dfn-job-promise" lt="job promise">promise</dfn> (a <a>promise</a>). It is initially null.</P>

Expand All @@ -2908,7 +2908,7 @@ spec: webidl; urlPrefix: https://heycam.github.io/webidl/
<dd><var>scopeURL</var>, a <a for="url">URL</a></dd>
<dd><var>scriptURL</var>, a <a for="url">URL</a></dd>
<dd><var>promise</var>, a <a>promise</a></dd>
<dd><var>client</var>, a <a href="#dfn-service-worker-client">service worker client</a></dd>
<dd><var>referrer</var>, a <a for=url>URL</a></dd>
<dt>Output</dt>
<dd><var>job</var>, a <a href="#dfn-job">job</a></dd>
</dl>
Expand All @@ -2918,7 +2918,7 @@ spec: webidl; urlPrefix: https://heycam.github.io/webidl/
<li>Set <var>job</var>'s <a href="#dfn-job-scope-url">scope url</a> to <var>scopeURL</var>.</li>
<li>Set <var>job</var>'s <a href="#dfn-job-script-url">script url</a> to <var>scriptURL</var>.</li>
<li>Set <var>job</var>'s <a href="#dfn-job-promise">promise</a> to <var>promise</var>.</li>
<li>Set <var>job</var>'s <a href="#dfn-job-client">client</a> to <var>client</var>.</li>
<li>Set <var>job</var>'s <a for=job>referrer</a> to <var>referrer</var>.</li>
<li>Return <var>job</var>.</li>
</ol>
</section>
Expand Down Expand Up @@ -3044,13 +3044,13 @@ spec: webidl; urlPrefix: https://heycam.github.io/webidl/
<li>Invoke <a href="#finish-job-algorithm">Finish Job</a> with <var>job</var> and abort these steps.</li>
</ol>
</li>
<li>If the <a for="resource">origin</a> of <var>job</var>'s <a href="#dfn-job-script-url">script url</a> is not <var>job</var>'s <a href="#dfn-job-client">client</a>'s <a for="resource">origin</a>, then:
<li>If the <a for="resource">origin</a> of <var>job</var>'s <a href="#dfn-job-script-url">script url</a> is not <var>job</var>'s <a for=job>referrer</a>'s <a for="resource">origin</a>, then:
<ol>
<li>Invoke <a href="#reject-job-promise-algorithm">Reject Job Promise</a> with <var>job</var> and a "{{SecurityError}}" exception.</li>
<li>Invoke <a href="#finish-job-algorithm">Finish Job</a> with <var>job</var> and abort these steps.</li>
</ol>
</li>
<li>If the <a for="resource">origin</a> of <var>job</var>'s <a href="#dfn-job-scope-url">scope url</a> is not <var>job</var>'s <a href="#dfn-job-client">client</a>'s <a for="resource">origin</a>, then:
<li>If the <a for="resource">origin</a> of <var>job</var>'s <a href="#dfn-job-scope-url">scope url</a> is not <var>job</var>'s <a for=job>referrer</a>'s <a for="resource">origin</a>, then:
<ol>
<li>Invoke <a href="#reject-job-promise-algorithm">Reject Job Promise</a> with <var>job</var> and a "{{SecurityError}}" exception.</li>
<li>Invoke <a href="#finish-job-algorithm">Finish Job</a> with <var>job</var> and abort these steps.</li>
Expand Down Expand Up @@ -3109,10 +3109,11 @@ spec: webidl; urlPrefix: https://heycam.github.io/webidl/
<li>Switching on <var>job</var>'s <a>worker type</a>, run these substeps with the following options:
<dl>
<dt><em>"<code>classic</code>"</em></dt>
<dd><p><a>Fetch a classic worker script</a> given <var>job</var>’s <a>serialized</a> <a href="#dfn-job-script-url">script url</a>, <var>job</var>’s <a href="#dfn-job-client">client</a>, and "<code>serviceworker</code>".</p></dd>
<dd><p><a>Fetch a classic worker script</a> given <var>job</var>’s <a>serialized</a> <a href="#dfn-job-script-url">script url</a>, <var>job</var>’s <a for=job>referrer</a>, null, and "<code>serviceworker</code>".</p></dd>
<dt><em>"<code>module</code>"</em></dt>
<dd><p><a>Fetch a module script tree</a> given <var>job</var>’s <a>serialized</a> <a href="#dfn-job-script-url">script url</a>, "<code>omit</code>", "<code>serviceworker</code>", and <var>job</var>’s <a href="#dfn-job-client">client</a>.</p></dd>
<dd><p><a>Fetch a module script tree</a> given <var>job</var>’s <a>serialized</a> <a href="#dfn-job-script-url">script url</a>, "<code>omit</code>", "<code>serviceworker</code>", and some settings object.</p></dd>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@domenic any suggestions on how to best address this? For dedicated and shared workers a settings object is created for the worker before "fetch a module script tree" is invoked, and then that settings object is used to get a module map to store the fetched modules on. For service workers we generally don't create a settings object until the worker actually is started.

Also I would somewhat expect the main script of a module worker to be fetched with the document/worker that created the worker as referrer, similar to how class workers use that as referrer. With the current spec of "Fetch a module script tree" that isn't possible/what happens though. All modules, including the main module of a worker are fetched with the worker itself as referrer.

Copy link
Contributor

Choose a reason for hiding this comment

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

You can't start a worker until its global scope/settings object have been created; it doesn't really make sense. Why do you think it's necessary in this case?

It sounds like there's a bug in the referrer handling though; filing an issue at whatwg/html would be much appreciated!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You can't start a worker until its global scope/settings object have been created; it doesn't really make sense. Why do you think it's necessary in this case?

Currently service workers do: fetch, create settings object, start, while other workers do: create settings object, fetch, start. So in both cases start happens after creating a settings object, it's just the fetching that happens differently.

The way the algorithms are currently written we don't create a settings object until after an update check has decided that a new script was indeed downloaded and we really need to start the worker. But yeah, it might actually be a lot clearer/easier to understand if we always create a settings object/global scope for the service worker before we initiate any kind of fetch/update check. If the update check fails we just throw the settings object away, and if the update check succeeded we can then reuse it to fire the install event at etc. Additionally at the end of a successful update check we can store the settings object's module map, similarly to how we currently store the workers script (and when we have to restart the same worker we can prepopulate the settings object's module map with that same module map). That would also address a couple of other places where the spec currently doesn't make sense around module type workers.

Actually rewriting things like that would be a fairly major undertaking, but probably the best way to actually make this all make sense...

It sounds like there's a bug in the referrer handling though; filing an issue at whatwg/html would be much appreciated!

Will do.

</dl>
<p class=issue>we need some kind of settings object for the module map to be stored in.</p>
<p>To <a>set up the request</a> given <var>request</var>, run the following steps:</p>
<ol>
<li>Append `<code>Service-Worker</code>`/`<code>script</code>` to <var>request</var>'s <a for="request">header list</a>.
Expand Down Expand Up @@ -3243,7 +3244,7 @@ spec: webidl; urlPrefix: https://heycam.github.io/webidl/
</dl>
<ol>
<li>Let <var>installFailed</var> be false.</li>
<li>Let <var>newestWorker</var> be the result of running <a href="#get-newest-worker-algorithm">Get Newest Worker</a> algorithm passing <var>registration</var> as its argument.</li>
<li>Let <var>newestWorker</var> be the result of running <a href="#get-newest-worker-algorithm">Get Newest Worker</a> algorithm passing <var>registration</var> as its argument.</li>
<li>Run the <a href="#update-registration-state-algorithm">Update Registration State</a> algorithm passing <var>registration</var>, "<code>installing</code>" and <var>worker</var> as the arguments.</li>
<li>Run the <a href="#update-state-algorithm">Update Worker State</a> algorithm passing <var>registration</var>'s <a href="#dfn-installing-worker">installing worker</a> and <em>installing</em> as the arguments.</li>
<li><a>Assert</a>: <var>job</var>'s <a href="#dfn-job-promise">promise</a> is not null.</li>
Expand Down Expand Up @@ -3777,7 +3778,7 @@ spec: webidl; urlPrefix: https://heycam.github.io/webidl/
<dd>none</dd>
</dl>
<ol>
<li>If the <a for="resource">origin</a> of <var>job</var>'s <a href="#dfn-job-scope-url">scope url</a> is not <var>job</var>'s <a href="#dfn-job-client">client</a>'s <a for="resource">origin</a>, then:
<li>If the <a for="resource">origin</a> of <var>job</var>'s <a href="#dfn-job-scope-url">scope url</a> is not <var>job</var>'s <a for=job>referrer</a>'s <a for="resource">origin</a>, then:
<ol>
<li>Invoke <a href="#reject-job-promise-algorithm">Reject Job Promise</a> with <var>job</var> and a "{{SecurityError}}" exception.</li>
<li>Invoke <a href="#finish-job-algorithm">Finish Job</a> with <var>job</var> and abort these steps.</li>
Expand Down
Loading