Populate document state origin/initiator origin upon creation

[domfarolino]

This fixes #9460 by supplying an initiator origin (or explicitly null) everywhere we create a new document state.

Specifically:

• Top-level traversal case: initiator origin is null or the new document's origin, depending on the opener null-ness. Null in the no-opener case is obvious. In the opener-exists case, we set the initiator origin to the document's origin because the origin of the new document is inherited from the "initiator" (which in this case is specifically the creator), so the two origins are really the same conceptually
• Create-new-child case: both origin and initiator origin are the new document's origin
• javascript: URL case: This one is a little confusing. There are two options for the initiator origin: (1) get it from the document state that we're replacing, or (2) get it from the initiatorOrigin ¹ that is passed in. These are different because even though we know that initiatorOrigin is "same origin-domain" with the target navigable's document's origin, the two origins may not be "same origin" (since origins can be the former without being the latter). Right now, the document's origin is pulled directly from initiatorOrigin, which as mentioned, is "same origin-domain" with the target navigable's origin, but may be "cross-origin" in general. When setting the new document state's initiator origin, this PR follows suit with the origin; instead of preserving the old document state's initiator origin, we just set it to initiatorOrigin, meaning after a javascript: navigation all of the following are identical:
  • The origin of the document initiating the javascript: URL navigation
  • The new document's origin
  • The new document state's origin
  • The new document state's initiator origin
• #create-navigation-params-by-fetching redirect case: This PR preserves oldDocState's initiator origin to the new document state that we make for redirects. The reason is because (1) there seems to be no good reason to make it null (implicitly by not assigning it), and (2) since I think it should be an origin, it should either be (a) the old document state's initiator origin of this whole navigation, or (b) something like responseOrigin, which doesn't feel right at all. In particular, non-fetch scheme params's initiator origin member (note) mentions that it is distinguished from a document state's initiator origin because document state's initiator origin doesn't get re-computed with each redirect in a change, while non-fetch scheme navigation params's initiator origin does — and is the last fetch-scheme origin in the chain ² —, making responseOrigin feel doubly wrong here.

Aside: I'm not quite sure if this should be considered editorial or not. Technically this PR fixes a small normative oversight from #6315, but I'm sure if we can write WPTs for this. This requires triggering an initially-created about:blank Document's destruction so we could test the full about:blank Document repopulation case, but I don't think we can do tab restores in WPTs. For a second I was thinking we could have a page served with Cache-Control: no-store open an about:blank iframe, navigate away, and then go backward, but that would literally just recreate the entire iframe from scratch, following not the repopulation case but the whole iframe insertion steps and everything (since we don't spec any iframe reparenting). So I'm thinking it might be OK to land this without tests, but an editor should really weigh in (and I can restore the usual PR template accordingly).

---

In this PR, I began to consider making the following changes. Making:

1. The non-fetch scheme navigation params's initiator origin member be nullable, since I think it can be null in the case where we navigate directly to a non-fetch scheme URL with no opener
2. The origin parameter (initiatorOrigin) in (1) "#hand-off-to-external-software", (2) "#loading-a-document", and (3) "#navigate-multipart-x-mixed-replace" to be an origin-or-null, since that parameter seems to come from (1) above, which is nullable.

I decided against making these changes in this PR because the way the current spec works I believe, makes it impossible for document state's initiator origin to not be populated in the navigate-a-new-top-level-traversable-with-no-opener-to-a-non-fetch-scheme-URL-case, since #navigate always has a sourceDocument. In that case, I think we should actually make the changes I describe in this subsection as part of #9133.

---

/browsing-the-web.html ( diff )
/document-sequences.html ( diff )

Footnotes

1. This PR also fixes a small bug where in the javascript: URL case, we referenced initiatorOriginSnapshot instead of initiatorOrigin. ↩

2. At least that is the intention. You'll note however, that what we set https://html.spec.whatwg.org/#populating-a-session-history-entry:non-fetch-scheme-params-initiator-origin-2 to is completely bogus! It's not even a variable. This should be fixed independently: https://github.com/whatwg/html/issues/9517. ↩

[domenic]

This mostly LGTM, but I don't feel confident we've fixed #9460 entirely anymore. See my comment over there.

We can merge this and work on the rest in a separate PR if you'd like, or maybe fix the undefined document bug in one PR, then fix all the document state stuff in a second PR.

Also, I'll note that I was a bit surprised that initiator origin = origin in both cases. In particular, I'm not sure it's what we want for "create a new top-level traversable" with null opener. In that case, maybe the initiator origin should be null, instead of being a new opaque origin (i.e., being the [=URL/origin=] of about:blank).

For "create a child navigable" it makes sense, but it's kind of non-obvious that document's origin === element's node document's origin. It might be clearer to use the latter explicitly.

[domfarolino]

Initiator origin's nullability is confusing to me, as it doesn't seem to need to be null at all at the moment — every usage of it appears to expect it to be non-null, unless you consider the fact that the "same origin" check allows operating on null origins. Anyways, the following two statements are at odds to me:

Also, I'll note that I was a bit surprised that initiator origin = origin in both cases. In particular, I'm not sure it's what we want for "create a new top-level traversable" with null opener. In that case, maybe the initiator origin should be null, instead of being a new opaque origin (i.e., being the [=URL/origin=] of about:blank).

From #9460 (comment):

Should we fix all of these at once? Maybe we could make origin / initiator origin non-nullable at the same time?

I guess my preference here would be to make initiator origin non-null and then just fix all of the instances of creating a new document state to manually populate initiator origin. On the other hand, implementations seem to have this as nullable for the no-initiator case which seems sensible: https://source.chromium.org/chromium/chromium/src/+/main:content/browser/renderer_host/frame_navigation_entry.h;l=155-160;drc=2ae79e6051049907d8e4bebf24a8ada79557d8c0. This makes me lean towards keeping this member nullable, so maybe that's the way to go... The only consequence of this I think would be https://html.spec.whatwg.org/#hand-off-to-external-software would have to decide what to do with a null initiatorOrigin (and we'd actually have to make that parameter accept null, etc). And this is what at least Chromium does anyways — handles null and opaque origins separately from tuple origins, and displays a different message to the user for that case.

[domenic]

Yeah, I think I changed my mind while composing that comment. A null initiator origin makes sense to me. This is quite related to #9133, I think.

[domfarolino]

OK I think I'm happy with the changes I've made to this PR. Please read the new OP that describes everything in detail including some follow-up changes that could be made once we sort out sourceDocument in all cases.

[domenic]

Great stuff, thanks for working through all this! I agree with your assessments.