Hm, this is an interesting question.  Since RFC7234 says it should fail to match, and pushed streams can be considered a kind of in-memory cache, I think it would be reasonable to refuse a pushed stream with Vary:* response header.

OK, so it's less bad than what I originally titled this as, since for regular HTTP traffic Vary:* is handled in HttpResponseHeaders::GetFreshnessLifetimes, here:

https://cs.chromium.org/chromium/src/net/http/http_response_headers.cc?rcl=3c45abea5a4e62a95c08331d5d88b4c8b1e8e3bb&l=958

Looking from the additional comments in the extracted code in e-mail vs. what's in git, it seems like whatever 
force-cache is sets SKIP_CACHE_VALIDATION, which overrides age check (which includes Vary:*) but not the other Vary checks.

... Seems like I need to step back and get a better perspective on both this force-cache thing as well as SKIP_CACHE_VALIDATION. The comments in load_flags_list.h also probably need some updating --- right now it just says this:

// This is a back/forward style navigation where the cached content should
// be preferred over any protocol specific cache validation.
LOAD_FLAG(SKIP_CACHE_VALIDATION, 1 << 2)

... which doesn't sound like it could be triggered by something like JavaScript at all.

https://fetch.spec.whatwg.org/#concept-request-cache-mode

"force-cache"
Fetch uses any response in the HTTP cache matching the request, not paying attention to staleness. If there was no response, it creates a normal request and updates the HTTP cache with the response.

This does make the distinction between Vary: and age/expiration pretty clear, and I think makes a certain amount of sense, though this would be a behavior change for Vary: * on back button, which worries me.

So after speaking with my colleagues, the consensus seems to be that we either want to split the flag or an OK from loading/renderer folks to change the behavior of Vary:* for back/forward.

So the question is if in back/forward case we want to keep ignoring the Vary:* case (e.g. not requiring validation) or not? I think yes in ideal case, I checked how other browser's b/f cache works but they don't generaly seem to check Vary headers there.

However back to the reality if Vary:* is not that popular (I checked a few super popular pages but wasn't able to find) I think it might be just okay to change the behavior now, and see if it makes any visible perf hits.  If it does we can consider splitting the flag.  (/cc kouhei@, kbx@, please give your thoughts if you have different opinions)

So as a sidenote to all of this: we don't set vary_mismatch_ for Vary:* so we use Last-Modified for it. This arose previously for other Vary: headers in  crbug.com/79758 , and I think the argument made there holds for Vary:*, via the now finalized RFC7232 (rather than httpbis drafts of that bugs time):

"   The "If-Modified-Since" header field makes a GET or HEAD request
   method conditional on the selected representation's modification date
   being more recent than the date provided in the field-value.
"
As this talks about "selected representation", and Vary:* prevents a candidate from being selected, the header is unusable for validation in this case. 

Also I guess LOAD_SKIP_VARY_CHECK doesn't affect Vary:* ATM...

We should split the flag (i.e. don't change how we handle Vary: * in the b/f case)

Here is why: Speed and Predictability. 

For speed, we can assess the impact from a related scenario: No-store. Chrome currently respect No-store on b/f navigations. We've added breakdown loading metrics to understand the cost of doing so:
 - The No-store b/f loading metrics are 1.5x to 2x slower than other b/f navigations.

Sure, Vary: * might be niche and the speed benefits might be weak overall. However, there is value in having browsers aligned on their behavior (i.e. the Predictability argument).


How much work / complexity would splitting the flag be?
Is this blocking something? Originally, this bug seemed related to H2 push predictability issues. It seems that it's not anymore, right?

If we split the flag, how does it look like from service workers?

re: comment #8

1) You mention predictability; does it mean you know what the other implementations do with Vary:*? I guess I should just test it myself, given timezones... I should note that since updating the bug I figured out a way where current behavior potentially breaks things: basically just have to XHR a different variant from a page after the one visited with Vary:*

2) haven't looked at how pervasive the flag use is yet.

3) Re: what this is blocking: sorry I didn't give context, this showed up on a Fetch 'force-cache' testcase; I was asking about H2 since it also has Vary handling.

So I think I can make us behave differently on back + Vary: * than at least Firefox 56 and Safari 11, as follows.

1) Create page.html with this content:
<a href="page2.html">Click here, and then wait a bit, and click back to test</a>

And page2.html with this content:
<script>
var xhr = new XMLHttpRequest();
xhr.open("GET", "page.html");
xhr.setRequestHeader("Serve-As", "text");
xhr.send(null);
</script>

2) Configure Apache 2.4 with this:

Header set Vary "*"
Header set Cache-Control "max-age=6000"
Header unset ETag
Header unset Last-Modified
<If "%{HTTP:Serve-As} == 'text'">
    Header set "Content-Type" "text/plain"
</If>

(This probably isn't the right way of doing it, but I am not familiar with the actual content-negotiation stuff, and didn't want a 304 
on the XHR; part of it is to bypass our bug I mentioned in comment #6)

3) Open page.html as served, click on the link to go to page2.html, wait a tiny bit, click back. Boom, we are showing the page as text. Firefox and Safari won't. (Don't have access to Edge at work, and couldn't get IE to my workstation through the fancy auth setup).

Firefox and Safari both have a b/f cache which result in having them ignore a lot more things. 

Given the expected low usage, I don't have a preference for a particular behavior. However, the less differences there is between browsers the better. 
Options:

 1. If there is a "mostly shared" behavior, we should try to align ourselves with it and update the spec accordingly.

 2. Follow/update the spec, convince the other browser vendors to align to the spec.

+annevk

If we end up splitting the flag, I would love to see the newly introduced flag in the spec.

I am not aware of there being anything spec'ed with respect to cache behavior for back/forward --- though it's possible I missed it since the History & Navigation stuff in HTML5 is quite complex, or perhaps am I looking in the wrong place?

It's specified as keeping Document objects alive for longer basically. A history entry ends up with such an object rather than just a URL. But I'm pretty sure interaction with the HTTP cache is undefined at the moment (other than using the defaults).

If back/forward indeed need special HTTP cache behavior that would be something to add to how navigation/history performs fetching (and Fetch might need the necessary features added).

Sorry for the long delay.

After talking with kenjibaheux@ and tyoshino@, we agreed that kinuko@'s #5 (i.e., changing back-forward behavior) was reasonable. morlovich@, will the implementation be hard?

We are planning to send an intent-to-ship tomorrow while leaving this issue open. If you have any concerns please let me know.

Thanks!

No, implementation is straightforward (and would be a simplification of an already-done CL); how would the intent-to-ship affect the timeline in this case?

I would say:

 Chrome has a minor issue with "Vary: *" in the "force-cache" and "only-if-cached" case
  - the impact should be negligible (Vary: * is mostly used in conjunction with other headers that trigger the intended behavior).
  - the issue should be resolved in the next release.
  - the new behavior shouldn't break existing website given the first point and how it would aligns us to other browsers and the spec.



If this issue is fixed by the next branch point (the end of Nov I guess) that'd be great but the next milestone might be acceptable. I'll cc you to the intent thread.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/722dc1135f660708e66ca7f47c16256426838f5f

commit 722dc1135f660708e66ca7f47c16256426838f5f
Author: Maks Orlovich <morlovich@chromium.org>
Date: Fri Nov 10 00:29:30 2017

Move Vary:* handling to HttpVaryData from HttpResponseHeaders::GetFreshnessLifetimes

This changes the behavior under LOAD_SKIP_CACHE_VALIDATION to handle Vary:* consistently
with other Vary headers. It also makes sense organizationally, since Vary is a separate
concept from freshness in the spec, and this change fixes two buglets that previously
affected Vary:* because it was handled in effectively the "wrong" spot:
1) We will no longer incorrectly use Last-Modified for validation of Vary:*
2) LOAD_SKIP_VARY_CHECK works for it now.

Bug:  778681 
Change-Id: Ie6d854d55cfc7ad46ac2db054f42a2e72a4f08fe
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Reviewed-on: https://chromium-review.googlesource.com/760539
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Commit-Queue: Maks Orlovich <morlovich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515366}
[modify] https://crrev.com/722dc1135f660708e66ca7f47c16256426838f5f/net/http/http_cache_unittest.cc
[modify] https://crrev.com/722dc1135f660708e66ca7f47c16256426838f5f/net/http/http_response_headers.cc
[modify] https://crrev.com/722dc1135f660708e66ca7f47c16256426838f5f/net/http/http_vary_data.cc
[modify] https://crrev.com/722dc1135f660708e66ca7f47c16256426838f5f/net/http/http_vary_data.h
[modify] https://crrev.com/722dc1135f660708e66ca7f47c16256426838f5f/net/http/http_vary_data_unittest.cc
[modify] https://crrev.com/722dc1135f660708e66ca7f47c16256426838f5f/net/spdy/chromium/spdy_network_transaction_unittest.cc
[delete] https://crrev.com/f673873becabef1261ec837c4509c3b22b805ea6/third_party/WebKit/LayoutTests/external/wpt/fetch/api/request/request-cache-force-cache-expected.txt