Cache entries with 0 cache lifetime and no validation headers are generally not useful.
<rdar://problem/20347160>
What about history navigation and tab restoring after jetsam / crash?
I keep the current policy for main resources.
The current pruning algorithm already takes care of dropping resources that were not reused first. I am slightly worried about being more aggressive here and completely not cache such resources. It will potentially cause us to use the radio more when using history navigations and on tab restore.
That balances against concerns of wasting cache space (so having less space for useful item) and power and performance effects of unnecessary disk writes.
The pruning policy should hopefully address the issue for wasting cache space. This change could negatively affect <rdar://problem/19869257>. In airplane mode, it is not only an efficiency issue but the user would potentially loose useful content as we wouldn't be able to restore it.
Created attachment 249744 [details] patch
Attachment 249744 [details] did not pass style-queue: ERROR: Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp:296: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 1 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.
The patch is still very conservative. Resources marked this uncacheable are generally one time xhr, ad ping images/scripts and similar. They are probably very rarely needed for a reasonably good tab restore. If we find problematic cases we can change the policy.
Comment on attachment 249744 [details] patch r=me
Comment on attachment 249744 [details] patch Attachment 249744 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6148567194402816 New failing tests: http/tests/cache/disk-cache/disk-cache-request-max-stale.html
Created attachment 249752 [details] Archive of layout-test-results from ews104 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Comment on attachment 249744 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=249744&action=review > Source/WebKit2/NetworkProcess/cache/NetworkCache.h:-69 > - NoDueToAttachmentResponse, We need to keep this one. We cannot shift values in this enum as they are stored in integers in the database.
Comment on attachment 249744 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=249744&action=review >> Source/WebKit2/NetworkProcess/cache/NetworkCache.h:-69 >> - NoDueToAttachmentResponse, > > We need to keep this one. We cannot shift values in this enum as they are stored in integers in the database. We should probably add a comment for this enum to explain we cannot do this :) Sorry about that.
Comment on attachment 249744 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=249744&action=review > Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp:274 > + bool hasExipirationHeaders = std::isfinite(response.expires()) || std::isfinite(response.cacheControlMaxAge()); hasExipirationHeaders ? :D typo.
Comment on attachment 249744 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=249744&action=review > Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp:283 > + bool storeUnconditionallyForBackNavigation = originalRequest.priority() == WebCore::ResourceLoadPriorityVeryHigh; I would say "ForHistoryNavigation".
Created attachment 249759 [details] patch
Comment on attachment 249759 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=249759&action=review ok > Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp:281 > + auto currentTime = std::chrono::duration<double>(std::chrono::system_clock::now().time_since_epoch()); auto now = currentTime(); ? c.f. NetworkResourceLoader::didFinishLoading(). > LayoutTests/http/tests/cache/disk-cache/disk-cache-request-max-stale.html:30 > + nit: extra line?
> > + auto currentTime = std::chrono::duration<double>(std::chrono::system_clock::now().time_since_epoch()); > > auto now = currentTime(); ? We should move to std::chrono everywhere.
https://trac.webkit.org/r182152
Created attachment 249766 [details] follow-up to cover no-cache
Comment on attachment 249766 [details] follow-up to cover no-cache r=me
Follow-up for no-cache case: https://trac.webkit.org/r182154