Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Browsing context hierarchy needs to go through the embedding document #5020

Open
annevk opened this issue Oct 18, 2019 · 6 comments
Open

Browsing context hierarchy needs to go through the embedding document #5020

annevk opened this issue Oct 18, 2019 · 6 comments

Comments

@annevk
Copy link
Member

@annevk annevk commented Oct 18, 2019

Grabbing the parent browsing context or top-level browsing context is rather problematic. In particular when then their active document or some such is consulted.

@bzbarsky has raised this many times.

To prevent these kind of issues we should make browsing contexts explicitly point to their embedding element and make all traversal go through that and then define convenient short hands for top-level document, parent document, and such.

This would also help with defining new concepts such as top-level origin, without having to explicitly forward such state downward.

@annevk
Copy link
Member Author

@annevk annevk commented Oct 21, 2019

Example: if you navigate a popup from A to A' and A embeds B and A' does not, when you arrive at A' and look at B's window.parent and window.top they are null in all browsers, but it's not necessarily true that B doesn't have an associated browsing context (depends on bfcache).

This issue is related to browsing context's "sleepy state" not being defined. In particular if you go back from A' to A and A was in the bfcache, the browsing context from B is still there and cannot have been discarded, but this is not really defined either way. In part I suppose whether we need to resolve this depends on how bfcache ends up being designed, but I suspect we will need to solve it given what existing user agents are iterating towards.

annevk added a commit to web-platform-tests/wpt that referenced this issue Oct 21, 2019
annevk added a commit that referenced this issue Nov 15, 2019
annevk added a commit that referenced this issue Nov 21, 2019
annevk added a commit that referenced this issue Nov 22, 2019
This generally sets out the infrastructure for a more secure model for working with browsing contexts, as outlined in #5020.

* Adds browsing context's container
* Adds browsing context's container document
* Makes "update the rendering" account for browsing contexts in shadow trees
* Removes nested through; closes #4409.
@domenic
Copy link
Member

@domenic domenic commented Mar 2, 2022

I'm not sure this is as simple as always going through the embedding document. In particular, the embedding document is cross-process is many cases. So for example, the spec has lots of "ancestor" or "descendant" checks. Such as:

  • Navigation sandboxing depends on whether the source/target of the navigation have an ancestor relationship
  • User activation propagation needs to propagate throughout the tree, to both ancestors and descendants

We cannot go through the document and the browsing context container in such cases, as they're happening "in the browser process" (i.e. in parallel, or in a global way, not connected to any specific document). In implementations, there's a source of truth for what the browsing context hierarchy looks like in this browser process.

So I think we're going to continue needing to use terms like "ancestor" and "descendant" which are defined in similar ways to how they are defined today, and assume there's a global view of the browsing context hierarchy.

That said, we still definitely need to fix the "sleepy state" / bfcache issue you mention, and that can be done.

And, in the simpler cases, we can use the explicit container and parent pointers, instead of the vague global-view ones.

@annevk
Copy link
Member Author

@annevk annevk commented Mar 3, 2022

We are explicitly ignoring the process boundary in many cases, why is it important here? You argued before in favor of ignoring it. I agree it might be better to not ignore it, but that would also require redesign of many things that cross the agent cluster boundary, including objects such as WindowProxy and Location.

@domenic
Copy link
Member

@domenic domenic commented Mar 3, 2022

I think I'm drawing the line at accessing specific HTML elements (e.g., containers) across processes. Doing so is pretty incoherent, and trying to move from the current setup (which roughly matches implementations, i.e. they have a global source-of-truth in the browser process) to one that pokes at elements across processes (which is not something implementations do) is not an improvement, like this issue seems to imply it is.

@annevk
Copy link
Member Author

@annevk annevk commented Mar 3, 2022

Not entirely sure how that is materially different from a WindowProxy object, but okay. I think the problem I outlined still stands though. The global view is not a good solution for nested browsing contexts that might not be part of the current entry of session history.

@domenic
Copy link
Member

@domenic domenic commented Mar 3, 2022

To expand on this, because it's quite subtle:

  • There is a lot of state that is replicated across processes in implementations, or is only accessed in the browser process. Examples of such state are: the current view of the browsing context tree; the URL of each browsing context; whether the browsing context is on the initial about:blank.

    We should be OK-ish with ignoring the process boundary for this stuff. Eventually it would be nice if we had some explicit replication in the spec, similar to what you're mentioning with WindowProxy and Location. We should also slightly prefer to access this stuff from in parallel, or from the "correct" event loop, where possible.

    This can be especially tricky with Document objects, because we store a lot of interesting things on them. (E.g., their initial about:blank-ness, or their current URL). On the one hand, Document is a specific JavaScript object, and accessing its "properties" cross-process or from in-parallel is bad. On the other hand, this is just reflective of the fact that we haven't done all the extra work to split up document into "canonical browser-process document" + "document proxy as viewed by other processes" + "document's properties as viewed by itself". (Where the latter might be even more up-to-date than the browser process, for things like sync navigations.)

  • There is some state which I think we can clearly state is not replicated across processes, and never will be even if we make our spec maximally rigorous. The DOM tree is a great example of this; the DOM is not thread-safe, much less cross-process accessible.

Again, I'm not sure exactly what this means for the specific example in #5020 (comment) . It's just that while working on the session history rewrite, I noticed that it didn't make sense to move from the current ancestor/descendant relationships, to some sort of method of collecting the ancestors/descendants which synchronously traverses the DOM tree across OOPIF boundaries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants