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

Spec the use of No-Vary-Search with prerender #310

Merged
merged 4 commits into from
May 7, 2024

Conversation

kjmcnee
Copy link
Collaborator

@kjmcnee kjmcnee commented Apr 19, 2024

The NVS hint from the speculation rule, if any, is now passed to the prerender creation.

The map of URLs to prerendering traversables is replaced with a list of a struct containing additional information about the prerender to support more flexible matching.

When the initial navigation of a prerendering traversable commits, we record its No-Vary-Search value.

When getting a prerender to activate, we now wait on potentially multiple ongoing prerenders that could match based on the NVS hint. This logic is similar to the prefetch case.

During activation, we update the prerender's URL if there is a difference in the prerendered document's URL and the URL navigated to in the case of inexact matching due to NVS.

@kjmcnee kjmcnee requested review from jeremyroman and domenic April 19, 2024 20:25
Copy link

prerendering.bs Outdated Show resolved Hide resolved
prerendering.bs Outdated Show resolved Hide resolved
prerendering.bs Outdated Show resolved Hide resolved
prerendering.bs Outdated Show resolved Hide resolved
prerendering.bs Outdated Show resolved Hide resolved
@@ -259,23 +304,48 @@ Every {{Document}} has an <dfn for="Document">activation start time</dfn>, which

<p class="note">Currently, cross-site prerendering is not specified or implemented, although we have various ideas about how it could work in this repository's explainers.

1. If |referrerDoc|'s [=Document/prerendering traversables map=][|startingURL|] [=map/exists=], then return.
1. [=list/For each=] |record| of |referrerDoc|'s [=Document/prerender records=]:
1. If |record|'s [=prerender record/starting URL=] is |startingURL|, then return.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an interesting question here -- what if we already have a prerender record which is an NVS match (either hinted as such or confirmed)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the prefetch side, it doesn't look like we spec any deduplication. I suppose it could be implicit from the prefetch steps themselves being optional?

Implementation-wise, currently both prefetch and prerender only dedupe based on the exact URL.

So I suppose the options are:

  1. Remove the explicit deduplication from this prerender spec
  2. Add explicit deduplication based on exact matches to the prefetch spec
  3. Add explicit deduplication based on NVS matches to the prefetch and prerender specs and update the chromium implementation
  4. Add explicit deduplication based on exact matches to the prefetch spec and add explicitly optional NVS based deduplication to prefetch and prerender specs

Can we think of a scenario where duplicate requests would be desirable for a UA to allow? If not, option 3 seems appropriate. WDYT?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can imagine a scenario (while the site can adapt to the query param being different, an exact match provides a "better" response which is fully ready faster), but I'm not sure it's worth indexing on. I have a leaning toward 3 as well, but maybe this should fork into an issue in case we want to weigh this a little more (and ask others)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed #315

prerendering.bs Outdated Show resolved Hide resolved
prerendering.bs Outdated Show resolved Hide resolved

1. Let |navigation| be |successorDocument|'s [=relevant global object=]'s [=navigation API=].

1. Let |continue| be the result of <a>firing a push/replace/reload <code>navigate</code> event</a> at |navigation| given "[=NavigationType/replace=]", |activationURL|, and true.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't figure out how to make the regular bikeshed linking work when there's a slash in the text of the definition, so I've ended up using the manual syntax.

@kjmcnee
Copy link
Collaborator Author

kjmcnee commented May 6, 2024

Sorry for the delay. I think this is ready for further review. PTAL.

@kjmcnee kjmcnee requested a review from jeremyroman May 6, 2024 21:01
@kjmcnee kjmcnee merged commit 17b54d4 into WICG:main May 7, 2024
2 checks passed
@kjmcnee kjmcnee deleted the NVS-prerender branch May 7, 2024 20:21
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

Successfully merging this pull request may close these issues.

3 participants