Service worker registration selection

[jungkees]

When iframe srcdoc document is created during the steps in navigation
algorithm, the document's Window object's environment settings object
should inherit the parent browsing context's active (service) worker.

This PR introduces an active worker for an environment settings object.
This internal slot may be referenced in more places when adding hooks
to service workers.

Fixes #321.

R= @annevk, @mikewest.

[annevk]

New slots for environment settings objects have to be introduced in multiple places, due to workers. They are also supposed to be algorithms that return something, so you might have to store the active worker on Document and WorkerGlobalScope objects.

[jungkees]

As we discussed when having introduced the client concept (both request's client and service worker client), an environment settings object is the best fit for maintaining this value.

I also defined the environment settings object for a service worker here: https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#run-service-worker-algorithm which precludes its own active worker.

Re the algorithm related concern, making it return a service worker seems fine to me. I might have to use service worker instead of active worker for its type definition.

[annevk]

An environment settings object is probably the best place to obtain it, but I don't think it's the best place to store it. Document is much better for preserving state given all the brokenness of browsers.

[annevk]

See also the issues around HTTPS state and CSP list for more background on that design.

[domenic]

What's the status of this? It's our oldest open PR :P. My read is that @annevk has some suggestions on how this should be done and it's up to @jungkees to update the patch with them? @annevk, could you maybe explicitly list the suggestions in a summary reply?

[annevk]

Basically when picking between Window and Document, we seem to usually store state on the Document. I suspect the reasoning here is that window.open() doesn't replace the Document and that Document is replaced when navigating away from an initial about:blank. That, and probably the fact that Document is what is being stored in history.

I'm not a 100% sure about the rationale around this, @Hixie would know better.

That's why I suggested to follow what we did for HTTPS state and CSP list.

[jungkees]

Sorry for the late response. I'll be back on this issue next week.

[jungkees]

Here's the outline of the changes I'd suggest:

• Set an active worker to Document and WorkerGlobalScope
  • Define "active worker" internal slot in Document and WorkerGlobalScope and set the value in:
    • (For a window client) Initialising a new Document object algorithm:
      • Set Document's active worker to response's active worker.
      • For this, I need a concept, response's active worker in response (which is not exposed to script but used by internal algorithms only.)
    • (For a worker client) Run a worker algorithm:
      • Set WorkerGlobalScope's active worker to response's active worker.
      • For this, I need a handle to the response for the fetched script. (maybe some prose will do?)
• iframe with srcdoc case
  • In the process the iframe attributes algorithm's "If the srcdoc attribute is specified case":
    • Set the resulting Document's active worker to its creator Document's active worker.
• Add "active worker", as an algorithm, to environment settings object
  • Add it to the definitions of the set up a browsing context environment settings object and the set up a worker environment settings object.
  • It returns the global object's associated Document's active worker and the global object's active worker, respectively.
• Changes needed for service worker client concept
  A service worker client has been defined as an environment settings object as it covers both window and worker globals. But with the change in HTML that an environment settings object's lifetime tying to a browsing context's lifetime, the service worker client concept for window clients doesn't make sense any more.

@domenic, I wonder if changing to make setting up a new environment settings object to occur for each Window creation would be possible. Otherwise, a service worker client might have to be redefined as a global object (i.e. Window or WorkerGlobalScope).

For Window vs Document concern, I think Window being a client should be okay as a Window and a Document are always 1:1 except with the initial about:blank where the Window is reused when navigated to a new Document. In this case, we can say the Window client is not being controlled by any active worker with it's initial about:blank document.

Comments would be appreciated.

/cc @mkruisselbrink

[domenic]

For this, I need a concept, response's active worker in response (which is not exposed to script but used by internal algorithms only.)

@annevk, does that sound reasonable?

For this, I need a handle to the response for the fetched script. (maybe some prose will do?)

Yeah, we need that in general. See #853. I might change all the top-level script-fetching algorithms to return some sort of (script, response) tuple. I'll work on that in the next few days.

I wonder if changing to make setting up a new environment settings object to occur for each Window creation would be possible. Otherwise, a service worker client might have to be redefined as a global object (i.e. Window or WorkerGlobalScope).

Oh, shit. It seems like I have made a number of changes to the spec (module and script execution related) without realizing that the same settings object is reused for all Windows in the browsing context. This is bad. I need to think about this a bit more.

You said this changed? ("... the change in HTML that an environment settings object's lifetime tying to a browsing context's lifetime ...") Do you know when?

[jungkees]

You said this changed? ("... the change in HTML that an environment settings object's lifetime tying to a browsing context's lifetime ...") Do you know when?

I think it was around this commit: cf0355d

Before that, it was like:

"Script settings for browsing contexts"
Whenever a new Window object is created, the user agent must:
  ..
  2. Create an environment settings object whose algorithms are defined as follows:

Yeah, an environment settings object tying to a Window will make it nicely cover the references to both a window environment and an worker environment.

[domenic]

So I will dig into this in detail tomorrow, but it does look like that change was ... unintentional? Or at least done without full understanding of the consequences. I think I got confused with how InitializeHostDefinedRealm() is done once per browsing context, and thought that meant that there should be one realm per browsing context, instead of one realm per global. I am 80% sure that one realm per browsing context does not make sense.

[annevk]

@jungkees can you explain why a response would need an active worker slot? It seems we should know the worker from the point of view of the document/worker, no? What scenario requires it to come from the response?

[jungkees]

I think it's needed for the initial non-subresource request matching. In order to set the active worker for a new Document in Initialising a new Document object, there should be a slot that carries this info, i.e. which service worker this Document will be controlled by. I thought an option would be the Handle Fetch set the active worker, the matched SW for the client request, to the response and return it to fetch, and Initialising a new Document object later can use that info to set the active worker.

[annevk]

Can't the document figure out itself which service worker it should be associated with? Not sure why we'd route that through Fetch. Also, it would be observable once the document is initialized, no, so you can't really wait for Fetch?

[jungkees]

The current Handle Fetch step 12.5 "Set client’s active worker to registration’s active worker." doesn't make sense as the Document hasn't been created by that time. That Document will be created in navigate when fetch completes with the response gotten from Handle Fetch. So, I plan to move the Handle Fetch step 12.5 to somewhere in the Initialising a new Document object algorithm.

[bzbarsky]

I think Window being a client should be okay as a Window and a Document are always 1:1 except with the initial about:blank

Just as a note: this is not true: document.open leaves the same Document but creates a new Window.

[mkruisselbrink]

Agreed that having the document figure out what worker it should be associated doesn't work, as it is fetched before the document is created. So whatever worker fetch selects to fetch a document with will have to somehow be passed on to the Document when it is later created (you don't want to document to try to figure it out itself, and possibly end up with a different service worker controlling it then was used to load the document).

[annevk]

@mkruisselbrink I don't follow. The worker to use is based on the URL, that is available to both parties. Do implementations really store a reference on the response?

[jungkees]

Just as a note: this is not true: document.open leaves the same Document but creates a new Window.

Yes, that's right. Thanks for clarifying it. So, technically, the service worker client is changing to a new one when document.open() is called, and the new client will still be controlled by the same active worker automatically.

As an aside, I plan to keep the current service worker client definition that it being an environment setttings object when HTML changes it to be tied to the Window object's lifetime.

[annevk]

@jungkees I think it should be tied to documents, otherwise reviving a document from history with a service worker attached wouldn't work, would it?

[jungkees]

I think it would work. Traversing the history of a browsing context to a specific entry is just making that entry's Document object to the active document, isn't it. Also, the fact that a Client object in the API represents a window rather than a document makes it better with a window.

[annevk]

Yes, but then you'd have to setup the Window object correctly again with the correct service worker (where does that come from?). I'm not really convinced that works well or matches how it's implemented today.

[mkruisselbrink]

@mkruisselbrink I don't follow. The worker to use is based on the URL, that is available to both parties. Do implementations really store a reference on the response?

What I was trying to say is that yes, the worker to use is based on the URL, but once a document has selected a worker that same worker will handle all requests from that document (modulo things like Clients.claim). A worker that gets installed after a document is fetched but before the Document is created should not become the controlling worker for that document (again, unless the new worker calls Clients.claim). I would guess implementations just create something resembling the client when initiating the initial request for the document, rather than waiting for the Document instance to be created after fetching is done, rather than storing a reference to the controlling worker on the response.

[annevk]

The thing on the response also fails if you do use "claim" it seems. I think it would be better to try to approach what implementations actually do here.

[jungkees]

Also, could you please use fixup commits going forward? And only rebase at the end. That would make it a little easier to review.

Ah.. Sorry about this. Will do.

[annevk]

Given that about:blank inherits almost everything from the parent, I think it would be very surprising if using about:blank did not inherit the service worker. @wanderview, do you know why it doesn't inherit in implementations despite it being required?

[wanderview]

I don't really know, but I can hazard some guesses:

1. Perhaps its wasn't spec'd when we implemented this part.
2. Its a corner case without a test, so it could have been missed.
3. Its not a useful case for developers, so no one has complained.

I'm struggling to understand why you would want any about: scheme documents to be controlled. What does that actually accomplish?

[wanderview]

Also note that gecko gets other things wrong here. We control documents instead of windows.

[annevk]

Whenever you create an <iframe>, it's created with a document whose URL is about:blank. So if you then start loading resources in it, it would make sense if those reused the service worker, since they already reuse the CSP policy, the referrer policy, the base URL, cookies, etc.

[wanderview]

But whenever a new document is loaded in a frame we have to check for a new service worker anyway. The non-subreaource URL loading in the frame determines the service worker, not the parent.

If it happens to match the parent's scope and gets the same service worker, great, but I don't see any real benefit in controlling about:blank.

If we want about:blank controlled for theoretical spec reasons, OK. But it doesn't seem important for real users, IMO.

[annevk]

I don't follow. When you create an <iframe> you also create a document, one that is fully functional, but whose URL is about:blank. You can then insert elements into that <iframe> and use it, much like you can use <iframe srcdoc>. Why should we cater to the latter but not the former?

[wanderview]

Ok, that sounds like a reasonable thing to do I guess. I wrote a gecko bug:

https://bugzilla.mozilla.org/show_bug.cgi?id=1261403

[jungkees]

@annevk, is it true for auxiliary browsing contexts, too? For instance, the initial about:blank document created in a browsing context opened by window.open() should inherit the creator browsing context's active document's active worker I think. If so, adding the steps for setting the active worker to https://html.spec.whatwg.org/#creating-a-new-browsing-context seems to be appropriate (like what the step 4 in the algorithm does for the origin of a Document). WDYT?

[jungkees]

I'm trying to get my head around the following sequence:

1. An iframe element with no src attribute is inserted into a document.
2. A nested browsing context is created.
3. The step 2 invokes process the iframe attributes for the "first time".
4. The src attribute of the iframe is set to the empty string later.
5. The step 4 triggers the otherwise steps for iframe or frame elements.
6. The step 5 triggers navigate the iframe's browsing context to "about:blank".

Does the navigation above create a new Document again or reuse the Document created when the browsing context was created? If it's the former, I think the steps for setting up an active worker should be added in the document creation time instead of the browsing context creation time (which I thought would be okay in #322 (comment)).

[bzbarsky]

Does the navigation above create a new Document again or reuse the Document

It creates a new Document. It reuses the existing Window, because https://html.spec.whatwg.org/#read-html (called from the navigate algorithm) calls https://html.spec.whatwg.org/#initialise-the-document-object which will reuse the window because https://html.spec.whatwg.org/multipage/embedded-content.html#otherwise-steps-for-iframe-or-frame-elements says to do navigations from initial about:blank with replacement enabled (in a monkeypatch after the algorithm steps) and https://html.spec.whatwg.org/multipage/browsers.html#origin defines the about:blank being loaded to be same-origin as the initial one.

[jungkees]

@annevk, I added a follow-up commit (51e5b39) having addressed the case for local url documents that have their creator. Note that I removed the step I had added in process the iframe attributes in the previous commit as the iframe srcdoc case is now covered in the steps added in initialising a new Document object algorithm. Please take a look.

[annevk]

s/having its/after it had its/

Right?

[jungkees]

Yes, I'll get it replaced as such.

[annevk]

This also needs to be rebased. I think there might be conflicts since I removed the creator document concept so we wouldn't keep documents unnecessarily alive.

[jungkees]

This also needs to be rebased. I think there might be conflicts since I removed the creator document concept so we wouldn't keep documents unnecessarily alive.

I noticed that. I'll rebase it.

[jungkees]

@annevk, Sorry I made it a mess while merging the remote and rebasing on it. So, I manually merged all the changes up to date on top of the latest upstream snapshot and force pushed it as a single commit.

[jungkees]

Please don't merge this yet. Today we had a conversation about the client concept and the related APIs needed again in service worker f2f: w3c/ServiceWorker#870 (comment) We decided to additionally expose fetchEvent.reservedClientId and fetchEvent.targetClientId. This means that the client's lifetime should be longer than that of an environment settings object. So, we'd need to define a service worker client as an independent concept instead of using an environment settings object. I'll come back to this after sorting out the service worker client definition work.

[domenic]

It sounds like this is still undergoing some churn, and the likely solution will be different enough to be a new pull request. I'll close this, if that's OK.

[jungkees]

You're right. I'll start it off in a new PR when ready. Thanks.