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

Add a speculative flag to Request as well as related processing #881

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

yoavweiss
Copy link
Collaborator

@yoavweiss yoavweiss commented Mar 19, 2019

As discussed on w3c/resource-hints#74 and on whatwg/html#4115, this PR adds a speculative flag to Request, which makes sure the request:

  • Survives navigations
  • Is low priority
  • Can be ignored by the UA (haven't added that part yet)
  • Adds the related request headers

I intend (as part of whatwg/html#4115) to then make sure prefetch requests are sent with this flag set.


Preview | Diff

@yoavweiss
Copy link
Collaborator Author

  • Can be ignored by the UA (haven't added that part yet)

@annevk - any advice on how such language can be formulated?

@yoavweiss yoavweiss changed the title Add a speculative flag to Reqeust as well as related processing Add a speculative flag to Request as well as related processing Mar 20, 2019
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

I suppose you could return early somehow for a speculative fetch. What does ignoring mean though? Not invoking fetch at all?

fetch.bs Outdated Show resolved Hide resolved
@@ -5628,6 +5638,7 @@ interface Request {
readonly attribute RequestRedirect redirect;
readonly attribute DOMString integrity;
readonly attribute boolean keepalive;
readonly attribute boolean speculative;
Copy link
Member

Choose a reason for hiding this comment

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

This would only result in setting the header? You'd still get the response and such?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, that's the current intention. Even though prefetch is destined for the next navigation, the current renderer can still get the response (e.g. onload fires for it).

Copy link
Member

Choose a reason for hiding this comment

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

Well, but this adds it to fetch() as a new feature. Was that intended? Or should this only be accessible via <link>?

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 thought it'd be needed to make those headers survive SWs (although that may vary based on where #880 ends up)

Copy link
Member

Choose a reason for hiding this comment

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

But this would also allow creation. Surviving SW should happen based on copying private state I think.

Copy link
Member

Choose a reason for hiding this comment

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

^That sounds right to me. This is similar to the idea that priority is a concept that gets preserved here but is never exposed to the Fetch API.

fetch.bs Show resolved Hide resolved
Copy link
Collaborator Author

@yoavweiss yoavweiss left a comment

Choose a reason for hiding this comment

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

Thanks for reviewing! :)

fetch.bs Outdated Show resolved Hide resolved
@@ -5628,6 +5638,7 @@ interface Request {
readonly attribute RequestRedirect redirect;
readonly attribute DOMString integrity;
readonly attribute boolean keepalive;
readonly attribute boolean speculative;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, that's the current intention. Even though prefetch is destined for the next navigation, the current renderer can still get the response (e.g. onload fires for it).

fetch.bs Show resolved Hide resolved
<li><p>If a <var>request</var>'s <a for=request>speculative flag</a> is set, then the user agent
may <a for=fetch lt=terminated>terminate</a> the <a>fetch</a> and return an <a>aborted network
error</a>, in cases where it's reasonable to believe it will result in better user experience.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@annevk I wasn't sure what to write here, so started by writing something. It feels a bit fuzzy to be part of the processing model, so advice on making it better would be appreciated

Copy link
Member

Choose a reason for hiding this comment

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

How concrete is it in the implementation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Currently I'm not aware of implementations that do that at the moment. When discussing whatwg/html#4115 @youennf wanted to give UAs the concrete possibility to do so, while staying conformant.

@yoavweiss
Copy link
Collaborator Author

I suppose you could return early somehow for a speculative fetch. What does ignoring mean though? Not invoking fetch at all?

I added a step to the processing model, aborting early at the UA's discretion. I think it'd be better to notify the developer that prefetch is being ignored.

@youennf
Copy link
Collaborator

youennf commented Mar 22, 2019

Overall, I would tend to go with the minimum proposal to have a functional targeting navigation loads.
Some additional questions/observations:

  1. Is prefetch sometimes/mainly/solely targeting (top) navigation loads.
    It seems to be the case since we want prefetch (at least navigation cross origin prefetch) to escape cache partitioning for instance.
    Preload, on the other hand, is about subresource loads, and can be used for all other cases.
    I like the difference as this allows to express that:
  • a web page might know that it will probably navigate to another page (prefetch)
  • a web page might know its own resources (preloads)
  • a web page might not try to (in fact fail to) determine the resources of the potential next page and try to over-optimize the next page, especially third-party ones.

If we narrow down the scope of prefetch to navigation loads, we might be able to simplify things without loosing too much functionality.

  1. Does it make sense for prefetch loads to go through SW of the prefetching page?
    The response might depend on 1. Let's assume we target navigation loads.
    For third-party navigations, SW interception benefit is unclear, downside is increased processing/latency.
    For same-origin navigations, there might be some theoretical benefits.
    That said, the service worker intercepting the prefetch might not be the one intercepting the navigation load. Preloads might be good enough there.

We also do not want to have the prefetch load go to the SW of the future navigation page.

  1. Should a SW that intercepts the navigation load know that a prefetch is available?
    We might expect the navigation load to go through SW before using the prefetch cache.
    It could be handy for the SW to say: yes, go use the prefetch cache.
    Might be dealt separately.

  2. Do we need the boolean in the Request interface?
    If we allow prefetch loads SW interception, probably. Otherwise, question 5.

  3. Do we want JS to be able to create Request with this flag set?
    If we narrow down the scope to navigation loads, I am not sure allowing fetch() with a speculative load is very useful. One use case might be to emulate form submissions.
    Not sure the complexity of handling long living requests with body is worth this usecase.

  4. Should responses be made available to the web page doing the prefetch?
    onload/onerror might be fine to have for .
    This does not require to provide the response to the webpage.
    In general, we should be worried of communicating third party responses to the web page if mode is not cors. But cors might not be always suitable for prefetch. no-cors might not be suitable either, for instance due to CORP checks. Maybe prefetch would be better served with its own mode.

@yoavweiss
Copy link
Collaborator Author

yoavweiss commented Apr 15, 2019

@youennf thanks for your comments! :) (and apologies for my slowness, travel is my typical excuse)

Regarding 1-3, they are defined in whatwg/html#4115
Do you think we also need to add similar restrictions in Fetch?

Regarding 4 and 5, we can probably avoid those for the time being. The tradeoff here would be that prefetches would be somewhat magical, but maybe we can tackle that later?

Regarding 6, agree that there's no need to communicate the actual response bytes to the prefetching page. Not sure how to spell that out as a spec concept though.

@youennf youennf mentioned this pull request May 17, 2019
@youennf
Copy link
Collaborator

youennf commented May 17, 2019

To improve efficiency, a prefetch could also be allowed to consult the HTTP cache and/or prefetch cache.
That said, the onload/onerror event handlers may allow the web page triggering the prefetch to infer some user specific information. This might need some analysis.

@jkarlin
Copy link

jkarlin commented May 17, 2019

Is prefetch sometimes/mainly/solely targeting (top) navigation loads.
It seems to be the case since we want prefetch (at least navigation cross origin prefetch) to escape cache partitioning for instance.

There are many cases where prefetch is used to load subresources. For instance, there are sites that will create a <link rel=prefetch> to critical-path subresources when you click on a link in order to save a RTT. I'd really like us to solve prefetch for both x-origin documents and subresources.

@youennf
Copy link
Collaborator

youennf commented May 17, 2019

Subresources could potentially be optimized with link preloads in the prefetched resource or web packaging.

@jkarlin
Copy link

jkarlin commented May 17, 2019

Link preloads in the document are still a full RTT slower than prefetching the subresource at navigation start.

@youennf
Copy link
Collaborator

youennf commented May 17, 2019

Sure. In general though, it seems rather brittle for a page to try optimizing the loading of a cross-origin page that might change any minute. Also, if the subresource is already in the cache (say a JS library that is needed for all pages of the prefetched origin), that might actually slow things down to prefetch it if the subresource is already in the cache and prefetch is not allowed to look at the cache.
We can probably start with something simple that solve some of the main use cases and improve upon it.

@jkarlin
Copy link

jkarlin commented May 17, 2019

Right. We'd need the prefetch to specify a cache key (destination origin) to use for the subresource, otherwise the prefetch could be wasted if the cache already has the resource

@@ -1077,6 +1077,9 @@ explicitly set <a for=/>request</a>'s
<p id=keep-alive-flag>A <a for=/>request</a> has an associated
<dfn for=request export>keepalive flag</dfn>. Unless stated otherwise it is unset.

<p id=speculative-flag>A <a for=/>request</a> has an associated
<dfn for=request export>speculative flag</dfn>. Unless stated otherwise it is unset.
Copy link
Member

Choose a reason for hiding this comment

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

Probably this should be a boolean, as I think we're leaning more towards explicit booleans as opposed to "flags"s these days?

@jkarlin
Copy link

jkarlin commented Jun 24, 2019

Sure. In general though, it seems rather brittle for a page to try optimizing the loading of a cross-origin page that might change any minute. Also, if the subresource is already in the cache (say a JS library that is needed for all pages of the prefetched origin), that might actually slow things down to prefetch it if the subresource is already in the cache and prefetch is not allowed to look at the cache.
We can probably start with something simple that solve some of the main use cases and improve upon it.

Sorry, I meant to reply to this earlier. This sounds good to me.

Base automatically changed from master to main January 15, 2021 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants