Implement stale-while-revalidate c-c response directive handling in HTTP cache validation code
Categories
(Core :: Networking: HTTP, enhancement, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox68 | --- | fixed |
People
(Reporter: mayhemer, Assigned: mayhemer)
References
(Blocks 2 open bugs)
Details
(Whiteboard: [necko-triaged][wptsync upstream])
Attachments
(1 file, 2 obsolete files)
Bug 1536511 - Cache-control: stale-while-revalidate response header implementation, r=kershaw!,asuth
47 bytes,
text/x-phabricator-request
|
Details | Review |
See https://tools.ietf.org/html/rfc5861#section-3
If c-c: s-w-r is present on the cached response and the entry is not fresh enough but the request manages to fall into the revalidate window, serve from the cache and then start an independent, same principal http channel instructed to revalidate the entry and potentially refresh with a new response in background, given the lowest possible priority.
Tentative P1 as it would be nice to have this in this cycle, Chrome is shiping this in M75 (June 4th).
FYI, google doesn't plan to shorten their max-age time in favor of the revalidation window use.
Assignee | ||
Updated•5 years ago
|
Comment 1•5 years ago
|
||
Honza, I'd like to assign this to you since this is a P1 bug.
If you think someone else should work on this, please put this back to triage list. Thanks.
Assignee | ||
Comment 2•5 years ago
|
||
Assignee | ||
Comment 3•5 years ago
•
|
||
I'm changing a bit how the expiration time is stored on a cache entry to allow calculation of the stale window for most cases it can be calculated (like max-age=0), let's see how the try push goes
https://treeherder.mozilla.org/#/jobs?repo=try&revision=faa68e55f71d9b1fb7b32c66985aafbc37327f48
Assignee | ||
Comment 4•5 years ago
|
||
The expiration time change probably causes a perma fail of:
TEST-UNEXPECTED-FAIL | /fetch/stale-while-revalidate/fetch.tentative.html | Second fetch does not return same response - assert_not_equals: got disallowed value "weyjkbkkgbldasdsozay"
TEST-UNEXPECTED-PASS | /fetch/stale-while-revalidate/stale-image.tentative.html | Cache returns stale resource - expected FAIL
TEST-UNEXPECTED-PASS | /fetch/stale-while-revalidate/stale-script.tentative.html | Cache returns stale resource - expected FAIL
TEST-UNEXPECTED-TIMEOUT | /service-workers/service-worker/client-navigate.https.html | expected OK
Assignee | ||
Comment 5•5 years ago
|
||
(In reply to Honza Bambas (:mayhemer) from comment #4)
The expiration time change probably causes a perma fail of:
ok, I'm apparently blind - those are wpt tests touching exactly this feature. patch will be updated.
Assignee | ||
Comment 6•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 7•5 years ago
|
||
After reading actual changes to the fetch() spec, there are two bits missing in the patch:
- service workers has to be explicitly disabled for the background revalidate request
- the revalidate requests has to be part of the browser context (it is - same principal) but also has to be aborted when the context goes away; hence, we have to add it to the same load group, which (unless I'm mistaken) allows us to use request tailing ensuring way small effect on page load time
going to update
Assignee | ||
Comment 8•5 years ago
|
||
Pushed by honzab.moz@firemni.cz: https://hg.mozilla.org/integration/autoland/rev/728aa342ad35 Cache-control: stale-while-revalidate response header implementation,
Comment 10•5 years ago
|
||
bugherder |
Assignee | ||
Updated•5 years ago
|
Updated•5 years ago
|
Comment 11•5 years ago
|
||
Backout by nbeleuzu@mozilla.com: https://hg.mozilla.org/mozilla-central/rev/34059c6188e5 Backed out changeset 728aa342ad35 for causing crashes on Bug 1545023. a=backout
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 12•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Comment 13•5 years ago
|
||
In case it's useful: in bug 1545023 comment 13, I posted some STR that reliably trigger the crash there ([@ mozilla::net::LoadInfo::LoadInfo]
) in Nightly builds from before this was backed out.
Comment 15•5 years ago
|
||
Pushed by honzab.moz@firemni.cz: https://hg.mozilla.org/integration/autoland/rev/46ecb4e343db Cache-control: stale-while-revalidate response header implementation, r=kershaw,asuth
Comment 16•5 years ago
|
||
bugherder |
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/16529 for changes under testing/web-platform/tests
Comment 19•5 years ago
|
||
Compatibility table links to 995651 https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control#Browser_compatibility
Assignee | ||
Comment 20•5 years ago
|
||
(In reply to gwarser from comment #19)
Compatibility table links to 995651 https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control#Browser_compatibility
Thanks for pointing this out! Created https://github.com/mdn/browser-compat-data/pull/4004
Comment 21•5 years ago
|
||
Honza, can you take a look at this last small change on the spec PR so we can close the loop there?
https://github.com/whatwg/fetch/pull/853#discussion_r278136865
Thanks! (And thanks for the quick implementation!)
Assignee | ||
Comment 22•5 years ago
|
||
(In reply to Ben Kelly [:bkelly, not reviewing] from comment #21)
Honza, can you take a look at this last small change on the spec PR so we can close the loop there?
https://github.com/whatwg/fetch/pull/853#discussion_r278136865
I commented there, but we need some feedback from Anne.
Thanks! (And thanks for the quick implementation!)
There are followups pending, so don't cheer too soon ;)
Upstream PR merged
Description
•