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

Impact of browsing context group switch during navigation #5350

Open
camillelamy opened this issue Mar 12, 2020 · 16 comments · May be fixed by #6315
Open

Impact of browsing context group switch during navigation #5350

camillelamy opened this issue Mar 12, 2020 · 16 comments · May be fixed by #6315

Comments

@camillelamy
Copy link
Member

@camillelamy camillelamy commented Mar 12, 2020

#5334 (Cross-Origin Opener Policy) introduces browsing context group switches during the navigation when the response has a particular header. The impact of a browsing context group switch during navigation is not defined, especially when it comes to session history.

Navigating back between browsing context groups is undefined because the spec associates session history with a particular browsing context. @annevk proposed in #5334 to introduce a browsing session concept which would maintain session history for a tab (as browser currently implement history). The browsing session would persist even if we swap browsing context groups during navigation, and would allow to navigate back/forward between them.

One question for back navigations between browsing context groups when we have a browsing session concept is whether we re-use the original ones or create new ones.

Currently, in Chrome's implementation of COOP, the browsing context group causes a severance of all opener relationships. They are not re-created when the user navigates back because there is no mechanism to remember that the back navigation must go to a particular browsing context group.

For example:

  • We have a browsing context group 1, which contains browsing contexts showing the documents a.com and b.com. They can communicate with each other using Window.Proxy.
  • The document a.com navigates to c.com and this navigation requires a browsing context group switch. We create a new browsing context group 2, which contains the document c.com. Browsing context group A now only contains b.com. a.com and c.com documents cannot communicate with each other.
  • The user navigates back. This still requires a browsing context group switch. The behavior we have implemented is to create a new browsing context group 3, which will contain only a.com. The a.com document cannot communicate with the b.com document, even though they could when the user initially navigated to them.

This behavior is simpler to implement, but we worry it might cause breakages when the user navigates back and forward. We would prefer to have the following behavior:

  • the browsing session maintains a list of session history entries which track what was the browsing context group of the session for each entry.
  • when navigating to an existing entry which require a browsing context group switch, we check if the browsing context group still has active browsing contexts. If so, we create a browsing context for the navigation inside this browsing context group.
  • otherwise, we create a new browsing context group with a new browsing context.

Note that we might want to do something similar for window.name.

@camillelamy
Copy link
Member Author

@camillelamy camillelamy commented Mar 12, 2020

@mikewest @hemeryar @ParisMeuleman FYI.

@csreis
Copy link

@csreis csreis commented Mar 13, 2020

Hmm, I'm confused by your description of Chrome's current behavior. Since very early on, Chrome has tracked SiteInstance / BrowsingInstance on NavigationEntry, so the session history does know what browsing context group each joint session history item belongs to. We use this restore documents to their previous groups as the user goes back/forward. Am I misunderstanding the case you're describing?

@domenic
Copy link
Member

@domenic domenic commented Mar 19, 2020

I found #4782 which seems related.

@camillelamy
Copy link
Member Author

@camillelamy camillelamy commented Mar 23, 2020

@csreis I am describing what we have implemented for COOP. I agree that the general case is different.

@rakina
Copy link
Member

@rakina rakina commented Apr 22, 2020

So there's been some discussions on Chrome's navigation-dev list around this area. (there's also a doc detailing the differences between spec vs Chrome impl here).

On a very high level, I think this is what we want:

  • Add the possibility of a BCG swap on navigation, which will result in new BCG & BCs with no shared state, severing scripting relationships.
  • Make sure the joint history session and history navigation accommodates multiple BCGs/top-level browsing contexts.
  • Add support for restoring BCG and its state on history navigations.
    • We should reuse the original BCG that we previously used for the page before, if we still have it including when using COOP - I think this is different than the current COOP spec. It is possible for the BCG to be gone already before we navigated back to it.
    • We should restore the state of the BCs (names, scripting relationships, whether it's an auxiliary browsing context/not) on history navigation.
    • Since scripting relationships are tied to browsing contexts (see WindowProxy, opener browsing context), we need to specify a way to update the references in other pages' BCs that pointed to the old destroyed BCs to refer to the recreated browsing contexts, maybe by storing more states in the history entries? This part is still unclear.

@annevk
Copy link
Member

@annevk annevk commented Apr 24, 2020

First of all, appreciate you all putting effort into nailing this down. A lot of this will notably improve security for end users and sites.

The navigation-dev link is a 404.

We should restore the state of the BCs (names, scripting relationships, whether it's an auxiliary browsing context/not) on history navigation.

This is the opposite of the conclusion we reached in https://bugs.chromium.org/p/chromium/issues/detail?id=1043162, right? Reading the Google Doc it strikes me that perhaps @camillelamy was not aware of that discussion?

@rakina
Copy link
Member

@rakina rakina commented Apr 27, 2020

The navigation-dev link is a 404.

Oops, updated!

We should restore the state of the BCs (names, scripting relationships, whether it's an auxiliary browsing context/not) on history navigation.

This is the opposite of the conclusion we reached in https://bugs.chromium.org/p/chromium/issues/detail?id=1043162, right? Reading the Google Doc it strikes me that perhaps @camillelamy was not aware of that discussion?

I think that is actually aligned with the conclusion from https://bugs.chromium.org/p/chromium/issues/detail?id=1043162#c2 ?

The case:

  1. Navigate to A
  2. Navigate from A to B
  3. window.open() => C
  4. Navigate with history.back() from B to A.

A will be in BCG 1, B and C will be in BCG 2. B and C can script each other. A will be in BCG 1 and can't script B nor C?

If previous to step 1 A was able to script D, then when we come back it can script D again.

Am I missing something?

@annevk
Copy link
Member

@annevk annevk commented Apr 27, 2020

So when you go from A to B, you also go from BCG-for-A to BCG-for-B? And you're saying that would always happen, even if BCG-for-A also contained top-level D?

FWIW, @mystor tells me that reviving BCs / WindowProxy objects would be involved for Firefox.

@rakina
Copy link
Member

@rakina rakina commented Apr 27, 2020

So when you go from A to B, you also go from BCG-for-A to BCG-for-B? And you're saying that would always happen, even if BCG-for-A also contained top-level D?

There are certain cases where we will switch BCGs (detailed in the doc), and we won't always switch BCGs. With Chrome's SiteIsolation on desktop we always switch BCGs on cross-site navigations, I think. We're looking into switching BCGs for same-site navigations too to support back-forward cache.

@jakearchibald
Copy link
Collaborator

@jakearchibald jakearchibald commented Sep 15, 2021

From: https://docs.google.com/document/d/1jSOhWc2Uo8g8Wcb94YWQcf6SJgN_Rul6ih4Qt5HelE8/edit

If it’s a history navigation, we will use the browsing context group that we used when we were on that page before

I can't recreate that.

  1. Go to https://random-server-stuff.glitch.me/isolated-nav-test/
  2. Click 1 (target 'foo') - this should open a new aux window.
  3. Go back to the first window.
  4. Click 1 (isolated).
  5. Click back.
  6. Click 2 (target 'foo').

The above opens a new window for me, suggesting it isn't using the same context as before.

Also, I can't figure out the rules in play here:

  1. Go to https://random-server-stuff.glitch.me/isolated-nav-test/.
  2. Click 1 (target 'foo') - this should open a new aux window.
  3. Go back to the first window.
  4. Go offline.
  5. Click 2 - you'll get an error page.
  6. Go online.
  7. Click reload.
  8. Click 2 (target 'foo').

This does not open a new window, suggesting the same browsing context is used.

  1. Go to https://random-server-stuff.glitch.me/isolated-nav-test/?isolate.
  2. Click 1 (isolated) (target 'foo') - this should open a new aux window.
  3. Go back to the first window.
  4. Go offline.
  5. Click 2 (isolated) - you'll get an error page.
  6. Go online.
  7. Click reload.
  8. Click 2 (isolated) (target 'foo').

This opens a new window, suggesting a new browsing context is used.

Maybe this means error pages use the same browsing context if the previous browsing context is not isolated?

@jakearchibald
Copy link
Collaborator

@jakearchibald jakearchibald commented Sep 16, 2021

Maybe this means error pages use the same browsing context if the previous browsing context is not isolated?

A shorter way to say that is "error pages are not isolated", but is that what we want?

@hemeryar
Copy link
Member

@hemeryar hemeryar commented Sep 17, 2021

Hi Jake! While I believe this to be a chrome implementation detail (I don't think error pages are spec'd?), that's correct that we have had conflicting requirements with error pages and COOP. The stand at the moment was to disable COOP on them (https://chromium-review.googlesource.com/c/chromium/src/+/3024337). It's a good starting point, since it links to several real bugs we encountered.

So the answer is, we're not 100% confident that this is the right decision but that's our take on it for now. @antosart for the reference.

@jakearchibald
Copy link
Collaborator

@jakearchibald jakearchibald commented Sep 17, 2021

While I believe this to be a chrome implementation detail (I don't think error pages are spec'd?)

I'm trying to spec the parts of them that are observable, such as this. Ok I'll start with:

Browsing contexts are swapped when traversing from:

  • an isolated document to an isolated document of a different origin
  • from a non-isolated document to an isolated origin document
  • from an isolated document to a non-isolated document

But that leaves this bit from: https://docs.google.com/document/d/1jSOhWc2Uo8g8Wcb94YWQcf6SJgN_Rul6ih4Qt5HelE8/edit

If it’s a history navigation, we will use the browsing context group that we used when we were on that page before

I like that as an idea, but it doesn't seem to be what Chrome or Firefox are doing now. Also, it can create some interesting situations where a WindowProxy can become reconnected after it seems to have closed. Eg:

  1. On a non-isolated page, click <a target="foo" href="not-isolated">…</a>, which opens new-window-1 to a non-isolated page.
  2. Navigate new-window-1 to an isolated page.
  3. Click <a target="foo" href="not-isolated">…</a> again, which opens new-window-2 (since new-window-1 is no longer "foo" due to isolation)
  4. back() on new-window-1.
  5. Click <a target="foo" href="not-isolated">…</a> again.

Which window is 'foo'? It feels like there are two. In Firefox and Chrome, it seems like step 4 doesn't result in reusing the previous browsing context, it'll create a new one, meaning there's still only one navigable with name "foo".

@whatwg whatwg deleted a comment from Jkilla123 Sep 19, 2021
@jakearchibald
Copy link
Collaborator

@jakearchibald jakearchibald commented Sep 21, 2021

Ok, I'm going to spec that error pages are not-isolated, and that BCGs swapped per the previous comment. This means traversing to a history entry may produce a document using a different browsing context than the one the history entry previously used.

@domenic
Copy link
Member

@domenic domenic commented Dec 8, 2021

The above discussion doesn't seem to talk about the bfcache case, i.e.:

  • Start on a.com (non-COI)
  • Go to b.com (COI); we do a browsing context group (BCG) swap
  • Go back to a.com, via bfcache

In this case, since the history traversal comes back to the original document, I'm pretty sure it should come back to the original browsing context (and BCG).

The current spec seems to prohibit bfcache from working across BCG swap boundaries, because it discards the original browsing context (and thus, all documents that live in it) near the end of https://html.spec.whatwg.org/#obtain-browsing-context-navigation . I believe this is incorrect; in fact I hear from @altimin that Chromium only bfcaches across BCG swap boundaries.

In the big ol' session history rewrite (#6315) I plan to change this to not discard the browsing context during swaps, but instead just ensure that the newly-created document points to the post-swap browsing context instead of the original. Then, the old one can stick around (if there are any bfcached documents using it) or get "garbage collected" (if nothing is using it). Please holler if that sounds like the wrong plan.

@annevk
Copy link
Member

@annevk annevk commented Dec 8, 2021

Assuming 1 TLBC per BCG is a requirement for bfcache, I think it's not observable if you preserve the TLBC or recreate it (assuming that you don't use discard semantics when collecting), so that seems fine from a simplicity standpoint. If we revisit 1 TLBC per BCG for bfcache, we should probably also revisit this.

Also, this is another reason to make it clear in step 7 of "The rules for choosing a browsing context" that the BCs need to be in the same BCG for the purposes of being "related enough" so there's no theoretical way to get hold of such a BC.

@domenic domenic linked a pull request Mar 4, 2022 that will close this issue
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

7 participants