Add request's reserved client and target client id #383

jungkees · 2016-09-13T12:02:09Z

Add request's reserved client and target client id

This patch defines the request's reserved client and the target client
id. The reserved client of a request is set to an environment or an
environment settings object during a navigation or a worker creation
request, respectively. The target client id of a request is set to the
target browsing context's active document's environment settings
object's id during a navigation. The given values are primarily used
to provide the reserved client's id and target client's id to service
workers' fetch event handlers.

This removes the request's target browsing context as the reserved
client provides the same information.

Related issue: w3c/ServiceWorker#870
Related change: whatwg/html#1776

This patch fixes the client matching flow and exposes reserved client and target client information by adding necessary hooks to fetch and HTML. Related issue: #870 Call flow: #870 (comment) Expected behavior: #870 (comment) Related changes: - HTML: whatwg/html#1776 - Fetch: whatwg/fetch#383 * Work on Handle Fetch is done. Work on the client API is in progress.

jungkees · 2016-09-13T12:17:25Z

@annevk, please take a look in line with the related changes to HTML and SW.

annevk · 2016-09-13T13:21:39Z

So how can this be an environment settings object? Isn't this information what's used for the eventual creation of one?

annevk · 2016-09-13T13:22:06Z

Also, if a reserved client knows the target browsing context, should we still provide that as a slot?

jungkees · 2016-09-13T14:27:55Z

So how can this be an environment settings object? Isn't this information what's used for the eventual creation of one?

I defined an algorithm setting up and returning an environment settings object that has different algorithms than those of browsing context/worker's environment settings object: whatwg/html@df5262b#diff-36cd38f49b9afa08222c0dc9ebfe35ebR77922
The reserved client's id will be used to set the actual environment settings object's id when it's created later. Also, I wanted it to be a type of an environment settings object as we should be able to enumerate these client objects in the service worker steps: get(id), matchAll(), etc.

Also, if a reserved client knows the target browsing context, should we still provide that as a slot?

The reserved client's target browsing context just returns request's target browsing context which was set in https://html.spec.whatwg.org/#process-a-navigate-fetch step 2. The reason why reserved client should hold this information is that when a service worker gets this client later from clients.get(id) or clients.matchAll(), it won't have any reference to the original request. (We plan to use the state of the active document of the target browsing context for WinodwClient's properties associated with the reserved client.)

annevk · 2016-09-13T14:50:04Z

This setup sounds a little weird. This object is passed along with a request but it echoes data from a request? Why wouldn't we just store that data on the object and create it together with the request?

jungkees · 2016-09-13T15:41:31Z

Do you mean we may use the target browsing context passed to "process a navigate fetch" directly to set up the object and delete the target browsing context internal slot from request? If so, I think that will work. FYI, this is the added step to set up the reserved object: whatwg/html@df5262b#diff-36cd38f49b9afa08222c0dc9ebfe35ebR82116 If I understood correctly, we may pass the target browsing context to this algorithm.

annevk · 2016-09-14T07:23:38Z

I think what I'm saying is that if this object lives on a request, a request should not be used to create it.

jungkees · 2016-09-14T12:15:45Z

I understood. So, rather than passing a request to create an object, I'll change the object creation algorithm to take a request's url and target browsing context. Does that make sense?

annevk · 2016-09-14T12:38:24Z

I guess. It seems a little weird to duplicate the URL field. I wonder if we're not better off if we keep some of these concepts distinct and not try to group them all together.

An alternative would be that we just add some properties to request (perhaps grouped together) and at a different stage use those to create an environment settings object.

jungkees · 2016-09-14T13:09:42Z

It seems a little weird to duplicate the URL field.

request' url and request's reserved client's creation URL?

I wonder if we're not better off if we keep some of these concepts distinct and not try to group them all together.

Could you be more specific with the idea? I'm curious if you don't want to use an environment settings object to represent a reserved client in the first place?

An alternative would be that we just add some properties to request (perhaps grouped together) and at a different stage use those to create an environment settings object.

Which environment settings object do you mean? One for reserved client? Or one with the resulting fetched resource?

annevk · 2016-09-14T13:42:54Z

request' url and request's reserved client's creation URL?

Right. Also, what happens if we hit a redirect?

I'm curious if you don't want to use an environment settings object to represent a reserved client in the first place?

Yeah, it doesn't seem like it's entirely the right fit. Maybe it's better if we had a client concept that's either an environment settings object or a "reserved client", or some such.

Before this patch, we didn't have a concept that holds reserved client's information referenced from a service worker during a navigation (more precisely before the actual global object and its relevant environment settings object for the resulting resource is created). This patch defines an "environment" concept (a supertype object of an environment settings object) that has its id, creation URL and target browsing context such that service workers' FetchEvent hanlder can reference this client information before the corresponding environmet setings object is actually created. The changes include: - Create an environment (a reserved client) during a navigation - Set request's reserved client id such that fetch (and Handle Fetch) can use it - Set request's target client id such that fetch (and Handle Fetch) can use it - Add Window object's active worker internal slot - Add WorkerGlobalObject's active worker internal slot - Add "match service worker registration" logic to attach controller Related issue: w3c/ServiceWorker#870 Call flow: w3c/ServiceWorker#870 (comment) Expected behavior: w3c/ServiceWorker#870 (comment) Related changes: - SW (WIP): w3c/ServiceWorker@df39d89 - Fetch: whatwg/fetch#383

This patch fixes the client matching flow and exposes reserved client and target client information by adding necessary hooks to fetch and HTML. Related issue: #870 Call flow: #870 (comment) Expected behavior: #870 (comment) Related changes: - HTML: whatwg/html#1776 - Fetch: whatwg/fetch#383 * Work on Handle Fetch is done. Work on the client API is in progress.

Before this patch, we didn't have a concept that holds reserved client's information referenced from a service worker during a navigation (more precisely before the actual global object and its relevant environment settings object for the resulting resource is created). This patch defines an "environment" concept (a supertype object of an environment settings object) that has its id, creation URL and target browsing context such that service workers' FetchEvent hanlder can reference this client information before the corresponding environmet setings object is actually created. The changes include: - Create an environment (a reserved client) during a navigation - Set request's reserved client id such that fetch (and Handle Fetch) can use it - Set request's target client id such that fetch (and Handle Fetch) can use it - Add Window object's active worker internal slot - Add WorkerGlobalObject's active worker internal slot - Add "match service worker registration" logic to attach controller Related issue: w3c/ServiceWorker#870 Call flow: w3c/ServiceWorker#870 (comment) Expected behavior: w3c/ServiceWorker#870 (comment) Related changes: - SW (WIP): w3c/ServiceWorker@df39d89 - Fetch: whatwg/fetch#383

jungkees · 2016-10-13T04:37:35Z

@annevk, I updated it back to use a reserved client (an environment) instead of just a reserved client id. Please check out whatwg/html#1776 (comment)

Also, the last patch suggests we remove the target browsing context internal slot of the request. I presumed there's no usage of accessing this state yet and reserved client's target browsing context may be used for that purposed if necessary onwards.

This patch fixes the client matching flow and exposes reserved client and target client information by adding necessary hooks to fetch and HTML. Related issue: #870 Call flow: #870 (comment) Expected behavior: #870 (comment) Related changes: - HTML: whatwg/html#1776 - Fetch: whatwg/fetch#383 * Work on Handle Fetch is done. Work on the client API is in progress.

annevk · 2016-10-13T08:26:18Z

Overview.src.html

@@ -662,6 +662,15 @@ <h4 id=terminology-headers>Headers</h4>
 <span data-anolis-spec=html>environment settings object</span>).

 <p>A <span title=concept-request>request</span> has an associated
+<dfn title=concept-request-reserved-client data-export data-dfn-for=request>reserved client</dfn>
+(null or an <span data-anolis-spec=html>environment</span> or an
+<span data-anolis-spec=html>environment settings object</span>). Unless stated otherwise it is null.


Preferred way is: (null, an x, or an xx)

annevk · 2016-10-13T08:26:51Z

Overview.src.html

+
+<p>A <span title=concept-request>request</span> has an associated
+<dfn title=concept-request-target-client-id data-export data-dfn-for=request>target client id</dfn>
+(null or a string). Unless stated otherwise it is null.


Can't we just use the empty string here? It's usually better to not change data types if not necessary.

(That is, any JavaScript API for this should probably just return the empty string, and not return either a string or null.)

Right. Missed that point. Addressed.

annevk · 2016-10-13T09:36:41Z

We should wait with this until HTML lands right? Otherwise the cross-references will lead nowhere. But it looks good. The only thing worth doing might be adding a note to each field mentioning that they are only useful for navigation (and worker?) requests and can be left alone otherwise.

jungkees · 2016-10-13T09:42:38Z

We should wait with this until HTML lands right? Otherwise the cross-references will lead nowhere.

Right. HTML also cross-references to fetch (request's reserved client and target clientid). So, ideally they should land at the same time when both of them are good-to-go.

The only thing worth doing might be adding a note to each field

Will do!

This patch fixes the client matching flow and exposes reserved client and target client information by adding necessary hooks to fetch and HTML. Related issue: #870 Call flow: #870 (comment) Expected behavior: #870 (comment) Related changes: - HTML: whatwg/html#1776 - Fetch: whatwg/fetch#383 * Work on Handle Fetch is done. Work on the client API is in progress.

annevk · 2016-10-24T07:41:42Z

Overview.src.html

+<span data-anolis-spec=html>id</span> of the <span data-anolis-spec=html>target browsing
+context</span>'s <span data-anolis-spec=html>active document</span>'s
+<span data-anolis-spec=html>environment settings object</span> captured before entering the
+<span title=concept-fetch>fetch</span> algorithm.


I was about to merge, but I don't understand this. The target client ID isn't that a reserved ID of sorts?

We discussed this during the f2f last month. This is used to expose the target client's id to fetch event's handler without the handle fetch having to cross the process border to get the target browsing context's document's environment's id from its own steps. The reserved client's target browsing context is used within SW's clients' method (like clients.get(reservedClientId)) to get the target's active document's related window properties later.

So to speak, it's used to initialize fetchEvent.targetClientId without crossing the process border during running the handle fetch steps.

But this ID goes away, right? This is the ID of the global that disappears somewhere during the navigation. (Disappears into history, that is.)

Right. But having discussed with @jakearchibald, this is also a value we'd want to expose to devs to help maintaining their Caches (i.e. for the per-client cache control).

Okay, thanks for confirming. Once the HTML PR has a commit message and is landed I'll merge this with a couple of minor tweaks.

Sounds great. Thanks!

jungkees · 2016-10-24T11:15:16Z

@annevk, I updated the commit message of this PR in the OP, too.

Before this patch, there has been no concept that holds reserved client's information referenced from a service worker during a navigation (more precisely before the actual global object and its relevant environment settings object for the resulting resource is created). This patch defines an "environment" concept (a supertype object of an environment settings object) that has an id, a creation URL, a target browsing context, and an active service worker such that service workers' FetchEvent handlers can reference this client information before the corresponding environment settings object is actually created. The changes include: - Create an environment (a reserved client) during a navigation; for worker creation requests, create an environment settings object and set the fields correspondingly - Set request's reserved client to the created environment or environment settings object such that fetch (and Handle Fetch) can use it - Set request's target client id such that fetch (and Handle Fetch) can use it Note: The reserved client's active service worker is set, if available, during the fetch (in service worker's Handle Fetch algorithm). Related issue: w3c/ServiceWorker#870 Related change: whatwg/fetch#383

This patch defines request's reserved client id and target client id that are set to a reserved environment's id and the target browsing context's active document's settings object's id respectively during a navigation. The given values are primarily used to provide the reserved client's id and target client's id to service workers' fetch event handlers. Related issue: w3c/ServiceWorker#870 Call flow: w3c/ServiceWorker#870 (comment) Expected behavior: w3c/ServiceWorker#870 (comment) Related changes: - HTML: whatwg/html#1776 - SW (WIP): w3c/ServiceWorker@df39d89

The changes in HTML navigation algorithm creates an environment, store a matched active service worker in the environment, and passes the environment to fetch (in request's reserved client.) The access to this environment is needed to dispatch a FetchEvent to the matched service worker.

By adding the reserved client internal slot, the target browsing context in a request becomes redundant. * If request's target browsing context has other planned usages, ignore this patch.

annevk · 2016-10-24T11:58:55Z

Thanks @jungkees, all good now.

jungkees · 2016-10-24T12:02:57Z

Looks good. Thanks!

Before this patch, there has been no concept that holds reserved client's information referenced from a service worker during a navigation (more precisely before the actual global object and its relevant environment settings object for the resulting resource is created). This patch defines an "environment" concept (a supertype object of an environment settings object) that has an id, a creation URL, a target browsing context, and an active service worker such that service workers' FetchEvent handlers can reference this client information before the corresponding environment settings object is actually created. The changes include: - Create an environment (a reserved client) during a navigation; for worker creation requests, create an environment settings object and set the fields correspondingly - Set request's reserved client to the created environment or environment settings object such that fetch (and Handle Fetch) can use it - Set request's target client id such that fetch (and Handle Fetch) can use it Note: The reserved client's active service worker is set, if available, during the fetch (in service worker's Handle Fetch algorithm). Related issue: w3c/ServiceWorker#870 Related change: whatwg/fetch#383

jungkees mentioned this pull request Sep 13, 2016

Improve navigate for service worker hooks whatwg/html#1776

Merged

jungkees force-pushed the add-reserved-client branch from 22f9e68 to 4511e2e Compare September 28, 2016 13:27

annevk reviewed Oct 13, 2016

View reviewed changes

jungkees force-pushed the add-reserved-client branch from a39ace9 to 36a3288 Compare October 14, 2016 06:16

annevk mentioned this pull request Oct 24, 2016

Convert to bikeshed #398

Closed

annevk reviewed Oct 24, 2016

View reviewed changes

jungkees changed the title ~~Add request's reserved client~~ Add request's reserved client and target client id Oct 24, 2016

jungkees and others added 6 commits October 24, 2016 13:56

Remove request's target browsing context

adf2aa9

By adding the reserved client internal slot, the target browsing context in a request becomes redundant. * If request's target browsing context has other planned usages, ignore this patch.

Fix type of target client id and editoral miss

caed164

Add notes for reserved client and target client id

c570082

nits

3e175d4

annevk force-pushed the add-reserved-client branch from c66b0f7 to 3e175d4 Compare October 24, 2016 11:57

annevk merged commit fb87b70 into whatwg:master Oct 24, 2016

jungkees deleted the add-reserved-client branch October 24, 2016 12:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add request's reserved client and target client id #383

Add request's reserved client and target client id #383

jungkees commented Sep 13, 2016 •

edited

Loading

jungkees commented Sep 13, 2016

annevk commented Sep 13, 2016

annevk commented Sep 13, 2016

jungkees commented Sep 13, 2016

annevk commented Sep 13, 2016

jungkees commented Sep 13, 2016

annevk commented Sep 14, 2016

jungkees commented Sep 14, 2016

annevk commented Sep 14, 2016

jungkees commented Sep 14, 2016

annevk commented Sep 14, 2016

jungkees commented Oct 13, 2016

annevk Oct 13, 2016

jungkees Oct 13, 2016

annevk Oct 13, 2016

annevk Oct 13, 2016

jungkees Oct 13, 2016

annevk commented Oct 13, 2016

jungkees commented Oct 13, 2016

annevk Oct 24, 2016

jungkees Oct 24, 2016

jungkees Oct 24, 2016 •

edited

Loading

annevk Oct 24, 2016 •

edited

Loading

jungkees Oct 24, 2016

annevk Oct 24, 2016

jungkees Oct 24, 2016

jungkees commented Oct 24, 2016

annevk commented Oct 24, 2016

jungkees commented Oct 24, 2016

Add request's reserved client and target client id #383

Add request's reserved client and target client id #383

Conversation

jungkees commented Sep 13, 2016 • edited Loading

jungkees commented Sep 13, 2016

annevk commented Sep 13, 2016

annevk commented Sep 13, 2016

jungkees commented Sep 13, 2016

annevk commented Sep 13, 2016

jungkees commented Sep 13, 2016

annevk commented Sep 14, 2016

jungkees commented Sep 14, 2016

annevk commented Sep 14, 2016

jungkees commented Sep 14, 2016

annevk commented Sep 14, 2016

jungkees commented Oct 13, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

annevk commented Oct 13, 2016

jungkees commented Oct 13, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jungkees Oct 24, 2016 • edited Loading

Choose a reason for hiding this comment

annevk Oct 24, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jungkees commented Oct 24, 2016

annevk commented Oct 24, 2016

jungkees commented Oct 24, 2016

jungkees commented Sep 13, 2016 •

edited

Loading

jungkees Oct 24, 2016 •

edited

Loading

annevk Oct 24, 2016 •

edited

Loading