- Revision
- 188640
- Author
- cdu...@apple.com
- Date
- 2015-08-19 09:50:50 -0700 (Wed, 19 Aug 2015)
Log Message
WebKit may keep outdated entry in the disk cache after a reload
https://bugs.webkit.org/show_bug.cgi?id=148137
<rdar://problem/22299547>
Reviewed by Antti Koivisto.
Source/WebKit2:
WebKit would keep outdated entry in the disk cache after a reload
in the following scenario:
1. We have an entry in the cache
2. The user reloads
3. We get a fresh resource from the network but this one is not cacheable
In this case, we now remove the stale entry from the cache to make sure
it is not served to following requests (e.g. history navigations).
* NetworkProcess/NetworkResourceLoader.cpp:
(WebKit::NetworkResourceLoader::didFinishLoading):
Remove the entry from the cache if its redirection is no longer
cacheable.
* NetworkProcess/cache/NetworkCache.cpp:
(WebKit::NetworkCache::Cache::store):
If we make the decision not to store the response, then remove the
entry in the cache for this resource if it exists.
(WebKit::NetworkCache::Cache::remove):
* NetworkProcess/cache/NetworkCache.h:
Add new remove() overload taking a ResourceRequest argument so the
call site does not have the compute the key.
* NetworkProcess/cache/NetworkCacheStorage.cpp:
(WebKit::NetworkCache::Storage::removeFromPendingWriteOperations):
(WebKit::NetworkCache::Storage::remove):
* NetworkProcess/cache/NetworkCacheStorage.h:
When we're asked to remove an entry with a given key, also remove
it from the pending write operations. This pre-existing bug would
prevent the new layout test from passing.
LayoutTests:
Add layout test to make sure that stale disk cached entries are removed
when it becomes uncacheable.
* http/tests/cache/disk-cache/resource-becomes-uncacheable-expected.txt: Added.
* http/tests/cache/disk-cache/resource-becomes-uncacheable.html: Added.
* http/tests/cache/disk-cache/resources/generate-response-optionally-cacheable.cgi: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (188639 => 188640)
--- trunk/LayoutTests/ChangeLog 2015-08-19 16:14:04 UTC (rev 188639)
+++ trunk/LayoutTests/ChangeLog 2015-08-19 16:50:50 UTC (rev 188640)
@@ -1,3 +1,18 @@
+2015-08-19 Chris Dumez <cdu...@apple.com>
+
+ WebKit may keep outdated entry in the disk cache after a reload
+ https://bugs.webkit.org/show_bug.cgi?id=148137
+ <rdar://problem/22299547>
+
+ Reviewed by Antti Koivisto.
+
+ Add layout test to make sure that stale disk cached entries are removed
+ when it becomes uncacheable.
+
+ * http/tests/cache/disk-cache/resource-becomes-uncacheable-expected.txt: Added.
+ * http/tests/cache/disk-cache/resource-becomes-uncacheable.html: Added.
+ * http/tests/cache/disk-cache/resources/generate-response-optionally-cacheable.cgi: Added.
+
2015-08-19 Brian Burg <bb...@apple.com>
Web Inspector: split TestStub.js into multiple files and modernize it
Added: trunk/LayoutTests/http/tests/cache/disk-cache/resource-becomes-uncacheable-expected.txt (0 => 188640)
--- trunk/LayoutTests/http/tests/cache/disk-cache/resource-becomes-uncacheable-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/cache/disk-cache/resource-becomes-uncacheable-expected.txt 2015-08-19 16:50:50 UTC (rev 188640)
@@ -0,0 +1,21 @@
+Tests that resources are removed from the cache if they become uncacheable
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Resource should be in the cache now.
+Load resource again using default cache policy.
+PASS internals.xhrResponseSource(xhr) is "Disk cache"
+
+Now load again the same resource, ignoring the cached data.
+This time the resource will not be cacheable
+PASS internals.xhrResponseSource(xhr) is "Network"
+
+Stale resource should have been removed from the cache.
+
+Now try to load the resource from the cache.
+PASS internals.xhrResponseSource(xhr) is "Network"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/http/tests/cache/disk-cache/resource-becomes-uncacheable.html (0 => 188640)
--- trunk/LayoutTests/http/tests/cache/disk-cache/resource-becomes-uncacheable.html (rev 0)
+++ trunk/LayoutTests/http/tests/cache/disk-cache/resource-becomes-uncacheable.html 2015-08-19 16:50:50 UTC (rev 188640)
@@ -0,0 +1,57 @@
+<script src=""
+<script src=""
+<body>
+<script>
+window.jsTestIsAsync = true;
+
+description("Tests that resources are removed from the cache if they become uncacheable");
+
+var uniqueTestId = Math.floor((Math.random() * 1000000000000));
+
+function loadResource(cacheable, onload)
+{
+ internals.clearMemoryCache();
+
+ xhr = new XMLHttpRequest();
+ xhr._onload_ = onload;
+ xhr.open("GET", "resources/generate-response-optionally-cacheable.cgi?uniqueId=" + uniqueTestId, true);
+
+ xhr.setRequestHeader("X-Cacheable", cacheable ? "true" : "false");
+ xhr.send(null);
+}
+
+loadResource(true, function() {
+ // Wait a bit so things settle down in the disk cache.
+ setTimeout(function () {
+ debug("Resource should be in the cache now.");
+ debug("Load resource again using default cache policy.")
+ loadResource(true, function() {
+ shouldBeEqualToString("internals.xhrResponseSource(xhr)", "Disk cache");
+
+ debug("");
+ debug("Now load again the same resource, ignoring the cached data.");
+ internals.setOverrideCachePolicy("ReloadIgnoringCacheData");
+
+ debug("This time the resource will not be cacheable");
+ loadResource(false, function() {
+ shouldBeEqualToString("internals.xhrResponseSource(xhr)", "Network");
+
+ setTimeout(function() {
+ debug("");
+ debug("Stale resource should have been removed from the cache.");
+
+ debug("");
+ debug("Now try to load the resource from the cache.");
+ internals.setOverrideCachePolicy("UseProtocolCachePolicy");
+ loadResource(false, function() {
+ shouldBeEqualToString("internals.xhrResponseSource(xhr)", "Network");
+ finishJSTest();
+ });
+ }, 100);
+ });
+ });
+ }, 100);
+});
+
+</script>
+<script src=""
Added: trunk/LayoutTests/http/tests/cache/disk-cache/resources/generate-response-optionally-cacheable.cgi (0 => 188640)
--- trunk/LayoutTests/http/tests/cache/disk-cache/resources/generate-response-optionally-cacheable.cgi (rev 0)
+++ trunk/LayoutTests/http/tests/cache/disk-cache/resources/generate-response-optionally-cacheable.cgi 2015-08-19 16:50:50 UTC (rev 188640)
@@ -0,0 +1,15 @@
+#!/usr/bin/perl -w
+
+use CGI;
+use HTTP::Date;
+
+my $query = new CGI;
+
+print "Status: 200\n";
+if ($query->http && $query->http("X-Cacheable") eq "true") {
+ print "Expires: " . HTTP::Date::time2str(time() + 100) . "\n";
+} else {
+ print "Cache-Control: no-store\n";
+}
+
+print "\n";
Property changes on: trunk/LayoutTests/http/tests/cache/disk-cache/resources/generate-response-optionally-cacheable.cgi
___________________________________________________________________
Added: svn:executable
Modified: trunk/Source/WebKit2/ChangeLog (188639 => 188640)
--- trunk/Source/WebKit2/ChangeLog 2015-08-19 16:14:04 UTC (rev 188639)
+++ trunk/Source/WebKit2/ChangeLog 2015-08-19 16:50:50 UTC (rev 188640)
@@ -1,3 +1,43 @@
+2015-08-19 Chris Dumez <cdu...@apple.com>
+
+ WebKit may keep outdated entry in the disk cache after a reload
+ https://bugs.webkit.org/show_bug.cgi?id=148137
+ <rdar://problem/22299547>
+
+ Reviewed by Antti Koivisto.
+
+ WebKit would keep outdated entry in the disk cache after a reload
+ in the following scenario:
+ 1. We have an entry in the cache
+ 2. The user reloads
+ 3. We get a fresh resource from the network but this one is not cacheable
+
+ In this case, we now remove the stale entry from the cache to make sure
+ it is not served to following requests (e.g. history navigations).
+
+ * NetworkProcess/NetworkResourceLoader.cpp:
+ (WebKit::NetworkResourceLoader::didFinishLoading):
+ Remove the entry from the cache if its redirection is no longer
+ cacheable.
+
+ * NetworkProcess/cache/NetworkCache.cpp:
+ (WebKit::NetworkCache::Cache::store):
+ If we make the decision not to store the response, then remove the
+ entry in the cache for this resource if it exists.
+
+ (WebKit::NetworkCache::Cache::remove):
+ * NetworkProcess/cache/NetworkCache.h:
+ Add new remove() overload taking a ResourceRequest argument so the
+ call site does not have the compute the key.
+
+ * NetworkProcess/cache/NetworkCacheStorage.cpp:
+ (WebKit::NetworkCache::Storage::removeFromPendingWriteOperations):
+ (WebKit::NetworkCache::Storage::remove):
+ * NetworkProcess/cache/NetworkCacheStorage.h:
+ When we're asked to remove an entry with a given key, also remove
+ it from the pending write operations. This pre-existing bug would
+ prevent the new layout test from passing.
+
2015-08-18 Zan Dobersek <zdober...@igalia.com>
[GLib] GMainLoopSource should receive the std::function<> objects through rvalue references
Modified: trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp (188639 => 188640)
--- trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp 2015-08-19 16:14:04 UTC (rev 188639)
+++ trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp 2015-08-19 16:50:50 UTC (rev 188640)
@@ -348,6 +348,9 @@
loader->send(Messages::NetworkProcessConnection::DidCacheResource(loader->originalRequest(), mappedBody.shareableResourceHandle, loader->sessionID()));
#endif
});
+ } else if (!hasCacheableRedirect) {
+ // Make sure we don't keep a stale entry in the cache.
+ NetworkCache::singleton().remove(originalRequest());
}
}
#endif
Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp (188639 => 188640)
--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp 2015-08-19 16:14:04 UTC (rev 188639)
+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp 2015-08-19 16:50:50 UTC (rev 188640)
@@ -405,10 +405,14 @@
StoreDecision storeDecision = makeStoreDecision(originalRequest, response);
if (storeDecision != StoreDecision::Yes) {
LOG(NetworkCache, "(NetworkProcess) didn't store, storeDecision=%d", storeDecision);
- if (m_statistics) {
- auto key = makeCacheKey(originalRequest);
+ auto key = makeCacheKey(originalRequest);
+
+ // Make sure we don't keep a stale entry in the cache.
+ remove(key);
+
+ if (m_statistics)
m_statistics->recordNotCachingResponse(key, storeDecision);
- }
+
return;
}
@@ -454,6 +458,11 @@
m_storage->remove(key);
}
+void Cache::remove(const WebCore::ResourceRequest& request)
+{
+ remove(makeCacheKey(request));
+}
+
void Cache::traverse(std::function<void (const Entry*)>&& traverseHandler)
{
ASSERT(isEnabled());
Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCache.h (188639 => 188640)
--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCache.h 2015-08-19 16:14:04 UTC (rev 188639)
+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCache.h 2015-08-19 16:50:50 UTC (rev 188640)
@@ -99,6 +99,7 @@
void traverse(std::function<void (const Entry*)>&&);
void remove(const Key&);
+ void remove(const WebCore::ResourceRequest&);
void clear();
void clear(std::chrono::system_clock::time_point modifiedSince, std::function<void ()>&& completionHandler);
Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.cpp (188639 => 188640)
--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.cpp 2015-08-19 16:14:04 UTC (rev 188639)
+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.cpp 2015-08-19 16:50:50 UTC (rev 188640)
@@ -494,14 +494,31 @@
return { headerData };
}
+bool Storage::removeFromPendingWriteOperations(const Key& key)
+{
+ auto end = m_pendingWriteOperations.end();
+ for (auto it = m_pendingWriteOperations.begin(); it != end; ++it) {
+ if ((*it)->record.key == key) {
+ m_pendingWriteOperations.remove(it);
+ return true;
+ }
+ }
+ return false;
+}
+
void Storage::remove(const Key& key)
{
ASSERT(RunLoop::isMain());
+ if (!mayContain(key))
+ return;
+
// We can't remove the key from the Bloom filter (but some false positives are expected anyway).
// For simplicity we also don't reduce m_approximateSize on removals.
// The next synchronization will update everything.
+ removeFromPendingWriteOperations(key);
+
serialBackgroundIOQueue().dispatch([this, key] {
WebCore::deleteFile(recordPathForKey(key));
m_blobStorage.remove(bodyPathForKey(key));
Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.h (188639 => 188640)
--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.h 2015-08-19 16:14:04 UTC (rev 188639)
+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.h 2015-08-19 16:50:50 UTC (rev 188640)
@@ -122,6 +122,7 @@
void readRecord(ReadOperation&, const Data&);
void updateFileModificationTime(const String& path);
+ bool removeFromPendingWriteOperations(const Key&);
WorkQueue& ioQueue() { return m_ioQueue.get(); }
WorkQueue& backgroundIOQueue() { return m_backgroundIOQueue.get(); }
