Refactor navigation algorithm.

[dtapuska]

Refactor navigation algorithm.

• Create the document after the global is created.
• Formally calculate sandbox flags and origin before document is created.

---

/browsers.html ( diff )
/browsing-the-web.html ( diff )
/infrastructure.html ( diff )
/origin.html ( diff )
/urls-and-fetching.html ( diff )
/window-object.html ( diff )

[annevk]

My main quibble here is with the determining an origin algorithm, but I also very much like the idea of having that. Also curious what @domenic thinks about this revised setup. (The page load stuff is still a little vague, but this only makes it better I think.)

[domenic]

This is great, great stuff. A big improvement. I pushed a few nit fixes (please remember to pull them), and there are a few small remaining issues threading everything through, but this is working out well.

[domenic]

I did a check on the origin stuff. I noticed the following things:

• In "create a new browsing context" step 4,

  Let origin be the result of determining the origin given browsingContext, "about:blank", sandboxFlags and browsingContext's creator origin.

  previously we handled browsingContext's creator origin being null, with the result being an opaque origin. I don't think we handle this any more; the result is that we return null when we should return an opaque origin.

• javascript/non-initial about:blank:

  • Before:
    • javascript: URLs -> The origin of the active document of the browsing context being navigated when the navigate algorithm was invoked.
    • non-initial about:blank -> The origin of the incumbent settings object when the navigate algorithm was invoked, or, if no script was involved, the origin of the node document of the element that initiated the navigation to that URL.
  • After (assuming invocationOrigin gets threaded through correctly):
    • both -> The origin of the incumbent settings object, or, if no script was involved, source's origin.
  • This is not straightforwardly an obvious change to me. The incumbent settings object's origin seems unlikely to be the same as the active document's origin in cases like a parent navigating its child. And the removal of "when the navigate algorithm was invoked" changes the meaning because by now we've been through several tasks and network delays.
  • To maintain parity with the current spec, I think we'd need to either maintain a bit of hand-waving (continue using "when the navigate algorithm was invoked"), or to be rigorous, save both possibilities early in the original navigate algorithm (not inside "process a navigate response"), thread them through, then choose between them at the later time.
  • Alternately, we could try to simplify, possibly by investigating what browsers actually do here. But that sounds like the kind of scope-creeping work that can derail a good refactor.

[dtapuska]

I've based and also pushed the activeDocumentNavgationOrigin and incubmentNavigationOrigin into the actual navigation algorithm. Hopefully this addresses your concerns.

[dtapuska]

@domenic Can you take a look at the new changes sometime?

[domenic]

Sorry for the delay while traveling. You picked the hard, no-handwaving route, but we need to fully thread through the variables...

[dtapuska]

@domenic I threaded them more to each algorithm although I didn't do the multipart algorithm since it seems to refer to a subnavigation.

[domenic]

This LGTM!!!!

@annevk, would you like to do a final pass? If you have editorial issues, it'd be super-great if you could push them as fixups like I've been doing; now that @dtapuska's done all the hard work here I think we editors should be able to finish up any i-dotting/t-crossing.

[annevk]

I pushed a number of changes you should probably take a look at. I think the setup makes sense and should definitely make further refactoring easier. I'm okay with landing this. Thank you both for your work on writing and reviewing it.

[dtapuska]

lgtm

[triple-underscore]

The description of the sandboxed origin browsing context flag
https://html.spec.whatwg.org/multipage/origin.html#sandboxed-origin-browsing-context-flag
refers to the id=sandboxOrigin deleted by the commit:

This flag forces content into a unique origin, thus preventing it from accessing other content from the same origin.

Perherps it should refer to the first step in
"determine the origin" algorithm?
https://html.spec.whatwg.org/multipage/browsers.html#determining-the-origin