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

Cancelable traversals: avoiding a slowdown #254

Closed
domenic opened this issue Nov 22, 2022 · 3 comments
Closed

Cancelable traversals: avoiding a slowdown #254

domenic opened this issue Nov 22, 2022 · 3 comments

Comments

@domenic
Copy link
Collaborator

domenic commented Nov 22, 2022

We've started implementing #32 in Chromium. (See especially #32 (comment) for the latest status.)

However, we've run into a small snag, which is that allowing such interception can interfere with performance. To explain this we need a bit of background about how traversal works today in terms of inter-process communication.

Today, with no beforeunload handlers:

  • Something (either browser or render process) initiates a traversal
  • We can directly start fetching the document to be traversed to

Today, with beforeunload handlers:

  • Something (either browser or render process) initiates a traversal
  • We have to ask the renderer process to run its beforeunload handlers. This might stop the traversal.
  • Only after the beforeunload handlers have finished, can we start fetching the document to be traversed to. (We cannot do this in parallel with running the beforeunload handlers, because of side-effecting GETs such as /logout links. If the user says "actually, I don't want to leave this site", it's bad if we've already triggered the server-side processing for /logout.)

You can see how the beforeunload path is slower than the no-beforeunload path, as when the traversal is browser-initiated (e.g. the back button), it has to do some extra round-tripping across processes.

If we allow navigate to cancel traversals, then we start making any page which has navigate handlers behave much more like the beforeunload path. Every browser-initiated traversal requires an extra process round trip to the renderer process, to run the navigate handler JavaScript and see if it called navigateEvent.preventDefault(). This is unfortunate. Note that you pay this cost whenever you have any navigate handler, even if you don't intend to use it for traversal cancelation. Even if you are just using it for analytics, or just using it for SPA routing, your cross-document traversals get slower.

How can we fix this? Here are some ideas I've had so far:

Require an opt-in

Something like

navigation.setTraversalNavigateEventMode("cancelable");

By default, traversal navigate events would not be cancelable. If you opt in by using this mode, your traversals get a bit slower, but you get the extra power to cancel them.

This would send the browser process a message asking it to flip into cancelable-traversal mode, where it always consults the renderer process.

This call might need to be async itself. Or maybe there's some clever queuing strategy where we can let it be sync and ensure any messages of this sort are processed before messages to the browser process requesting a traversal.

Only allow canceling same-document traversals

The original concern here applies to cross-document traversals, where there's potentially an expensive step of hitting the network, which we want to start as soon as possible.

If we restricted cancelable traversal navigate events to only be for same-document traversals, then this problem might disappear.

I'm not 100% sure on this, because slowing down those traversals might also be a bad user experience. But it wouldn't directly impact metrics like LCP, which are relevant to cross-document traversals...

One nice part of this approach is that it gets rid of any overlap between navigate and beforeunload. If you want to prevent cross-document traversals, you need beforeunload. If you want to prevent same-document traversals, you need navigate. This was the original ask in #32, so it seems we'd still be meeting web developer needs. Allowing custom cancelation experiences for same-origin cross-document traversals was a nice bonus, but maybe it isn't actually necessary.

In one sense, this is a nice conservative extension: it solves the minimal use case for now, and leaves us room for future expansion to allow canceling cross-document (maybe even one day cross-origin??) traversals. If we did so, it would probably via an opt-in like the above.

In another sense, this is locking in a potential slowdown for all same-document traversals, which might be bad. So we'd need to be sure we're OK with that before proceeding.

@bathos
Copy link

bathos commented Nov 22, 2022

Thoughts appreciated, especially on the question of whether blocking cross-document traversals is important.

I’ve been doing cross-document “canceling” in userland (i.e., the sum effect was like canceling), but for us it was an uncommon “special” case that wouldn’t arise in a typical session. Although having this be a first-class capability of the API seems ideal to me*, I wouldn’t consider its absence to reduce the value of the navigation API model. Same document is what mattered most by far.

* When it did occur, it was more likely for credentials/authorization to have changed since the old doc was active, with the effect being that the old entry — or rather, the resource its associated URL represented — would no longer have the same representation for the user as it had had when the history entry was last current. Intercepting it was always the safe bet for us for this reason, but this pattern seems probably too specific to the application in question for it to have any bearing.

@posva
Copy link
Contributor

posva commented Dec 13, 2022

I think this is great from an SPA router's perspective even though I think we would end up using onbeforeunload by default, still losing the performance, in leaving navigation guards because I don't imagine users having to care, in the case of unsaved changes in a page, if the navigation is going to a different origin or document or not. They would still prefer writing just one function:

onBeforeRouteLeave((to, from) => {
  if (hasUnsavedChanges && !window.confirm('Do you want to leave?')) return false
  // do nothing and let the navigation happen if the user clicks yes
})

And maybe being able to pass an option to it:

onBeforeRouteLeave((to, from) => {
  // ... same code
}, { sameDocumentOnly: true }) // avoid using onbeforeunload behind the scenes and opt in for the fast version

Because when developing an app, I cannot know for sure if the user is going to leave a page by clicking a regular link, clicking on a favorite (maybe by accident) or just changing the URL without realizing they didn't save their changes.

However, I still think this is great because only a few pages in an application need that kind of guard and I think having to use onbeforeunload on those is fine and worth the perf loss.

I think the main issue from #32 has been solved ✅ , which was being able to have a consistent URL update no matter if the navigation came from within the app or the UI (back button) in same-document (SPA) by being able to delay the URL update until all navigation guards confirm the navigation

IMO having the tradeoff explained in this issue is good as it involves a complex behavior of the browser that developers do not need to fully understand to be able to use the Router API of onBeforeRouteLeave

@domenic
Copy link
Collaborator Author

domenic commented Mar 3, 2023

Thanks everyone for the discussions here! We've settled on doing only same-document for now. @natechapin can you work on updating the explainer? I'm working on reflecting this in the spec at whatwg/html#8502 .

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

No branches or pull requests

3 participants