Migrating/removing navigation_token (DidCommitProvisionalLoad_Params)

[Rakina Zata Amni]

Hi navigation-dev,

We're currently looking into moving things out of DidCommitProvisionalLoad_Params, including navigation_token. Looks like it's generated by the browser and sent to the renderer along with the CommitNavigation call from the browser (through CommitNavigationParams), or generated in the renderer during commit if the commit call is purely from the renderer (which should only happen for renderer-initiated same-document navigation or initial about:blank page now?). 

It has two uses in the browser side:

1. Passing same_document_navigation_request_ to DidCommitNavigationInternal for browser-initiated same-document navigations.
2. Deleting navigation_request from within DidCommitNavigationInternal, if the navigation_request's navigation_token does not match the navigation_token sent by the renderer. Looks like this is used as protection against cases of renderers claiming they have committed (and also maybe commit races? is that possible?)

It looks like after we remove all the other parts of DidCommit params and start swapping RFHs at CommitNavigation instead, NavigationRequests will also be gone after CommitNavigation (CMIIW) so we probably can remove navigation_token then. Does that sound right?

[Camille Lamy]

Hi Rakina,

I think so. We introduced it because we had issues where we matched the wrong NavigationRequest to the commit that happened. This was prior to introducing the NavigationClient mojo interface which prevents such race conditions in a much more robust manner. So point 2 should already have been taken care of, at that point it's just a question of properly handling browser initiated same-document navigation. They could go through a per NavigationRequest Mojo interface like cross-document ones. We didn't do it at the time because that would mean two different Commit IPCs for same-document navigations: one on NavigationClient for browser-initiated navigation and one on FrameHost for renderer-initiated ones. However that might be the better option for correctness here.

TLDR: you can probably get rid of it right away for anything that isn't a same-document navigation. For same-document browser-intiated navigations you might want to have them commit through the NavigationClient interface instead of using Frame and FrameHost. Then you can probably delete the token.