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

should FetchEvent.request.cache reflect non-fetch browser cache controls? #875

Open
wanderview opened this Issue Apr 11, 2016 · 18 comments

Comments

Projects
None yet
6 participants
@wanderview
Copy link
Member

wanderview commented Apr 11, 2016

Currently the Request.cache attribute reflects the request's RequestCache setting. The current specs are a bit vague on this point. Only fetch() actually sets this, but the browser can bypass the http cache for other reasons. For example, if a user presses the refresh button causing the page to reload with http cache revalidation, should the corresponding FetchEvent.request.cache be set to no-cache.

I believe we should reflect these browser cache settings on all FetchEvent's whether they come from fetch() or another source.

@wanderview wanderview added this to the Version 1 milestone Apr 11, 2016

@jakearchibald

This comment has been minimized.

Copy link
Collaborator

jakearchibald commented Apr 11, 2016

We should add a note to fetch to set no-cache on requests if say devtools is blocking the cache.

@ehsan

This comment has been minimized.

Copy link

ehsan commented Apr 11, 2016

Or if the user has disabled the cache, etc. This is user configurable in Firefox.

@wanderview

This comment has been minimized.

Copy link
Member

wanderview commented Apr 12, 2016

The exact value should really match the behavior. For example, our devtools option to bypass http cache actually does 'no-store'. Hitting refresh button without the devtools option set does 'no-cache'.

@slightlyoff slightlyoff added the decided label Apr 12, 2016

@wanderview

This comment has been minimized.

Copy link
Member

wanderview commented Apr 13, 2016

@annevk

This comment has been minimized.

Copy link
Member

annevk commented Apr 14, 2016

If the user hits refresh we change how a fetch() in the page behaves? Or we change how navigation fetches?

@jakearchibald

This comment has been minimized.

Copy link
Collaborator

jakearchibald commented Jul 27, 2016

@annevk both right? Hitting refresh changes the fetching of both the navigation fetch and subresources.

@jakearchibald

This comment has been minimized.

Copy link
Collaborator

jakearchibald commented Jul 27, 2016

Pre F2F notes: Don't think there's anything to discuss unless there are objections. Seems like we should move this to the fetch repo.

@jakearchibald

This comment has been minimized.

Copy link
Collaborator

jakearchibald commented Jul 28, 2016

F2F:

  • Browsers do different things on refresh, that's ok
  • The request object should reflect the intended cache interaction so fetch(event.request) does the right thing.
@annevk

This comment has been minimized.

Copy link
Member

annevk commented Jul 29, 2016

So I'm still wondering whether we should expose this difference through fetch(). Since that means we're effectively changing the mode from default when this happens? What if the mode you used was not default? Is it okay that the page can override the refresh semantics desired by the user?

It's not at all clear to me what the right thing here is.

@wanderview

This comment has been minimized.

Copy link
Member

wanderview commented Aug 2, 2016

This is largely the point of using a service worker, no? The user hits the refresh button triggering no-cache fetches. The SW then responds with data from the Cache API without consulting the server. Or synthesizes a response, etc. The user's request to check with the server is often ignored by design.

If the user does a hard refresh (shift-ctrl-r) then SW is bypassed. The user's intent cannot be changed at this point.

This is a SW intercept design choice, though. The exposed request.cache value is a minor point when you can return completely different responses.

I see no reason to make browser cache overrides magical and hidden.

@jungkees

This comment has been minimized.

Copy link
Collaborator

jungkees commented Aug 2, 2016

I'm not sure whether @annevk's comment and @wanderview's comment are on the same page. To me, what matters in this issue is just to set request's cache mode to a right value for the given request context. E.g., set it to no-cache for refresh in general. That cache mode will be observed only when the request hits the network layer bypassing SW. (If the request hits the network layer and the dev set the optional request.cache, that should override whatever the default behavior.)

For specifying what cache mode is used for refresh, etc., I guess we defer to fetch. But what's discussed in the f2f was that "Browsers do different things on refresh, that's ok". So, we may just close this issue?

@jungkees

This comment has been minimized.

Copy link
Collaborator

jungkees commented Aug 12, 2016

Let's think about an example scenario. A navigation to a main resource (N) that includes a fetch(url, { cache: "no-store" }) call to a subresource (R). Let's call the fetches N and R, respectively.

  • Initial load of N triggers fetches to:
    N - cache mode: "default"
    R - cache mode: "no-store"
    Intercepted by SW: yes - the cache mode values don't matter
  • Refreshing the document triggers fetches to:
    N - cache mode: "default" or "no-cache"? (currently "default"; no step overrides the value)
    R - cache mode: "no-store" or "no-cache"? (currently "no-store"; no step overrides the value)
    Intercepted by SW: yes - the cache mode values don't matter
    When they're bypassed to the network stack: we may expect "no-cache" for both N and R
  • Force refreshing the document triggers fetches to:
    N - cache mode: "default" or "reload"? (currently "default"; no step overrides the value)
    R - cache mode: "no-store" or "reload"? (currently "no-store"; no step overrides the value)
    Intercepted by SW: no
    When they get to the network stack: we may expect "reload" for both N and R

I guess @wanderview originally brought this question? - how to override the cache mode value (or not) and how to spec it?

@wanderview

This comment has been minimized.

Copy link
Member

wanderview commented Aug 12, 2016

Refreshing the document triggers fetches to:
N - cache mode: "default" or "no-cache"? (currently "default"; no step overrides the value)
R - cache mode: "no-store" or "no-cache"? (currently "no-store"; no step overrides the value)
Intercepted by SW: yes - the cache mode values don't matter
When they're bypassed to the network stack: we may expect "no-cache" for both N and R

I would expect R to be no-store in this case. This is what we do in gecko.

Force refreshing the document triggers fetches to:
N - cache mode: "default" or "reload"? (currently "default"; no step overrides the value)
R - cache mode: "no-store" or "reload"? (currently "no-store"; no step overrides the value)
Intercepted by SW: no
When they get to the network stack: we may expect "reload" for both N and R

In gecko we report no-store for both N and R here.

Personally I think default should mean "follow cache-control headers".

I'm not sure where it is spec'd, but I think refresh and hard refresh should do something like:

  • When a page is refreshed set request.cache to no-cache if request.cache is currently default.
  • When a page is hard refreshed set request.cache to no-store in request.cache is currently default.
@jungkees

This comment has been minimized.

Copy link
Collaborator

jungkees commented Sep 19, 2016

I think the expected behavior for "refresh" and "hard refresh" and how to spec them (overriding the cache mode values) have not been decided. I want to bring this to f2f during this TPAC.

/cc @annevk

@jakearchibald

This comment has been minimized.

Copy link
Collaborator

jakearchibald commented Sep 20, 2016

Dupe of #873?

@jungkees

This comment has been minimized.

Copy link
Collaborator

jungkees commented Sep 20, 2016

Yes.

Resolution:

  • The cache mode of the request should reflect the browser's intent

@jungkees jungkees added the decided label Sep 20, 2016

@jungkees

This comment has been minimized.

Copy link
Collaborator

jungkees commented Feb 9, 2017

Also clarify the expected behavior of location.reload in terms of cache control: #873 (comment).

@jungkees

This comment has been minimized.

Copy link
Collaborator

jungkees commented Apr 2, 2018

It's about setting a right cache mode value for various fetching instances. I'm not sure where to move this issue, but this isn't a SW V1 issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment