Define top level origin of a document

[shivanigithub]

Adds the definition of the top level origin of a resource request.
This is based on feedback from whatwg/fetch#943

---

💥 Error: write EPROTO 139802362353536:error:1409442E:SSL routines:ssl3_read_bytes:tlsv1 alert protocol version:../deps/openssl/openssl/ssl/s3_pkt.c:1494:SSL alert number 70

139802362353536:error:1409E0E5:SSL routines:ssl3_write_bytes:ssl handshake failure:../deps/openssl/openssl/ssl/s3_pkt.c:659:
💥 ###

PR Preview failed to build. (Last tried on Mar 11, 2020, 8:55 PM UTC).

More

PR Preview relies on a number of web services to run. There seems to be an issue with the following one:

🚨 HTML Diff Service - The HTML Diff Service is used to create HTML diffs of the spec changes suggested in a pull request.

🔗 Related URL

If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.

[annevk]

I don't really know what your timezone is, but it might help if you had a quick chat with @domenic about this. I work Berlin office hours, if that helps.

In short, what's needed here is to define new state on document, analogous to something like "CSP list". And then Fetch and others get to inspect that to make decisions.

[shivanigithub]

I am in Eastern Standard Time. Looks like Domenic is OOO.
Do you mean adding something in this section:
https://html.spec.whatwg.org/multipage/dom.html#the-document-object? If yes, while making the change in this section, does the current thinking in this PR make sense to you: dividing the scenarios as top-frame navigation resource and subframe navigation resource while computing the top-level origin?

[annevk]

I think we need to set the field when creating a document (note that there's a couple places where this happens). And yes, I suspect for now we will need to look at ancestors rather than ancestors passing something down.

[shivanigithub]

The latest patch has the top level origin computation in the navigation section now. I kept it right after "determining the origin" as they both are origin states on the document and "determining the top level origin" is dependent on the value returned from "determining the origin". PTAL, thanks!

[domenic]

I'm back from vacation and trying to help, but I'm not sure I really understand the intent here. I can give minor feedback on this PR, e.g. I'll note that it seems to operate on browsing contexts which are not the same as documents, but I'm not sure what the goal is here.

Perhaps as a step back, @annevk, I'm unsure why you're insisting that this be "state" on the document, instead of an algorithm similar to the one defined here. Currently this one takes a browsing context, but if it got redefined to operate on documents, it seems like it would work reasonably?

I'm also unsure how this is helpful for whatwg/fetch#943. Fetches are generally associated with clients, not with documents (or browsing contexts). For example, what is the desired behavior in a worker (i.e. where the client of the fetch is a worker environment settings object)? It seems like you'd really want to go from client -> HTTP cache, which might again require different techniques. Perhaps even different techniques depending on the type of worker!

[annevk]

It could work reasonably if there's a way to get to a parent document without involving a browsing context as otherwise you might get the wrong top-level document for a bfcache document. (I don't think we have that infrastructure at the moment, but we should add it.)

[shivanigithub]

Had a discussion with @domenic and here are the main points discussed. I am working on creating the next patch based on this.

• Since the main consumer of the cache partitioning key is fetch and every fetch request has an associated environment settings object, create a new field top-level-origin on the environment settings object. (https://html.spec.whatwg.org/multipage/webappapis.html#environment-settings-object)
• The environment settings object for a document is created on navigation here. Define the top-level-origin here and pass it to the environment settings object. (https://html.spec.whatwg.org/#initialise-the-document-object 5.3)
• The environment settings object for a worker(dedicated/shared) is created here. Define the top-level-origin here and pass it to the environment settings object (https://html.spec.whatwg.org/#run-a-worker 8)
• In the fetch spec, https://fetch.spec.whatwg.org/#concept-http-network-or-cache-fetch, will mention that the http cache associated with the fetch request’s environment settings top-level-origin will be used.

[shivanigithub]

Added the environment settings and document level changes in the latest patch. PTAL, thanks!

[domenic]

Sorry for the delay; I'll try to be more responsive going forward.

[shivanigithub]

Thanks for the feedback.
This got a little delayed but I should be able to get to these this week.

[shivanigithub]

Sorry for the delay. Addressed the feedback now. Thanks!

[domenic]

I fixed some formatting nits; please take a look at the latest commit and be sure to pull it down before adding more.

It seems like the main work left here is that you need to ensure all settings objects have a top-level origin. This includes updating the other call site for "set up a window environment settings object", i.e. making sure new popup windows have a top-level origin set for themselves, and also updating workers.

From what I understand we might not yet know the right answer for workers. That's OK. In that case, we should still have https://html.spec.whatwg.org/#set-up-a-worker-environment-settings-object add a "The top-level origin" entry, but its contents can be a <p class="XXX"> explaining that we are still figuring out what this should be.

[annevk]

I'm curious what you both think about building this on top of #5091's container document. With that we could define a document's parent document and a document's top-level document on top of that and those would provide a bfcache-safe access to the top-level origin (the origin of a document's top-level document).

The main wrinkle is that if you were to perform same origin-domain comparisons with this origin you'd be in a trouble (as mutations to the origin are generally ignored, but observed by that comparison). But I don't think we have use cases for that and we might also wanna call out explicitly to not do that, regardless of whether we use this design or copying down the tree.

We could also use top-level document for other concepts as long as they are immutable. Policies having to be immutable makes this less attractive as a general mechanism I suspect as unfortunately we have many mutable policies. So I guess we'll at least have some copying down, at which point maybe we should try to fit everything into that scheme long term. Hmm. Not sure, would love your input.

[shivanigithub]

@annevk : I'm in the reading #5091 but in the meantime have one question regarding your comment here: https://github.com/whatwg/html/pull/5091#issuecomment-554419563
In the scenario you discussed there, as per the changes in this PR, B's top-level origin will be saved as A1 and not C since B's top-level origin will be computed at the time of navigation commit for B.
Does that sound right?

[shivanigithub]

Thinking a bit more about this, the eager approach seems more aligned with what we want, since we would not like the partition of a frame to change during its lifetime.
Will upload the new patch, although I will be slow since I am OOO for the next few weeks.

[domenic]

I wonder though whether this is intentionally allowed or something that ought to be spec'd

My suspicion is that preventing fetches from such discarded contexts is something that we should work to spec, eventually. In fact I believe we've vaguely known about it for a long time, but nobody has invested the effort to do a proper test matrix of types of fetches × different browsers × types of shutdowns × other factors in order to confidently suggest a way forward. (For example, what about sendBeacon or keepalive fetches, which are intentionally supposed to outlive a document? Or img fetches, which historically have done so and analytics vendors rely on? Maybe we could still disallow starting one of those from a detached document, as long as we allowed it to continue once started...)

However, I don't think it's reasonable to block any of this work on figuring out that long-standing problem.

Thinking a bit more about this, the eager approach seems more aligned with what we want

That works for me! It probably does help sidestep this whole mess in the best way.

My other suggestion was going to be adding some kind of Assert or <p class="XXX"> in the Fetch spec that says something like "this might sometimes be null per spec, but in implementations we suspect such fetches get shut down earlier", and maybe linking to an open bug about that whole problem. But, I think switching to the eager approach will be better.

[shivanigithub]

Changed to using the eager approach as discussed. PTAL, thanks!

[shivanigithub]

Friendly ping.

[domenic]

This looks great. I pushed some minor nit fixes, but this looks really solid to me.

@annevk, can we trouble you for another look?

[shivanigithub]

@annevk : friendly ping.

[shivanigithub]

Thanks Domenic!

[shivanigithub]

@annevk : friendly ping.

[annevk]

Thanks @shivanigithub, this setup seems good. There's a couple of minor issues left however.

[shivanigithub]

Addressed feedback. PTAL, thanks!

[annevk]

Same here, it seems using the container document would be sufficient and require less going up the tree (though it might end up with more words I suppose).

[shivanigithub]

A question on how does "browsing context's top-level browsing context" work? If it's such that every browsing context has a reference to its top-level browsing context then we are not really going up the tree here, but just accessing that reference? Searching other places that invoke "browsing context's top-level browsing context", I see examples like 1, 2, 3, etc.

I am not sure how container document would be better, since container document will lead to the parent and we'll have to continue to iterate till the last parent, right?

[domenic]

I agree; I don't see why container document would be better. Top-level browsing context is defined straightforwardly in a way that bottoms out in container document; it's a nice abstraction on top that saves us some typing. (Explicitly: TLBC uses ancestor which uses child browsing context which uses container document.) I don't see why we would avoid using that abstraction when it gives us exactly what we want.

[annevk]

I think grabbing the active document of the top-level browsing context is the problematic aspect here. It might well be changing due to race conditions. Container document could become null I suppose, but that seems safer than a different document.

[domenic]

I don't see how a race condition could be involved. This step is atomic. If you're worried about things spontaneously changing during a single step, both approaches seem equally problematic.

[annevk]

It's currently atomic because the spec is rather blind to site isolation. It also seems nicer to me if top-level origin works recursively.

[annevk]

I thought about it some more and I'm okay with this being merged as the active document stuff is an existing problem that needs auditing either way and the new instances are fairly isolated and also happen at clear points in time.

[domenic]

Thanks for being flexible :). Is your other comment outstanding, or is this good to merge? And do you have any thoughts about whether we should merge this now/ASAP, vs. waiting for whatwg/fetch#943? I'm unsure myself; it's a standalone piece of infrastructure, but it has no consumers until the Fetch PR is merged.

[annevk]

It's good to merge. I think it's okay to merge now. I suspect there are many more dependencies we haven't identified yet, as browsers do plenty of things differently for third parties these days, and this is the primitive you want to tell a third party from a first party.

[shivanigithub]

Thanks @annevk !