- Revision
- 188952
- Author
- carlo...@webkit.org
- Date
- 2015-08-26 00:44:58 -0700 (Wed, 26 Aug 2015)
Log Message
Merge r188640 - 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: releases/WebKitGTK/webkit-2.10/LayoutTests/ChangeLog (188951 => 188952)
--- releases/WebKitGTK/webkit-2.10/LayoutTests/ChangeLog 2015-08-26 07:44:24 UTC (rev 188951)
+++ releases/WebKitGTK/webkit-2.10/LayoutTests/ChangeLog 2015-08-26 07:44:58 UTC (rev 188952)
@@ -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-13 Joseph Pecoraro <pecor...@apple.com>
Web Inspector: Reduce flakiness of inspector/indexeddb/requestDatabaseNames
Added: releases/WebKitGTK/webkit-2.10/LayoutTests/http/tests/cache/disk-cache/resource-becomes-uncacheable-expected.txt (0 => 188952)
--- releases/WebKitGTK/webkit-2.10/LayoutTests/http/tests/cache/disk-cache/resource-becomes-uncacheable-expected.txt (rev 0)
+++ releases/WebKitGTK/webkit-2.10/LayoutTests/http/tests/cache/disk-cache/resource-becomes-uncacheable-expected.txt 2015-08-26 07:44:58 UTC (rev 188952)
@@ -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: releases/WebKitGTK/webkit-2.10/LayoutTests/http/tests/cache/disk-cache/resource-becomes-uncacheable.html (0 => 188952)
--- releases/WebKitGTK/webkit-2.10/LayoutTests/http/tests/cache/disk-cache/resource-becomes-uncacheable.html (rev 0)
+++ releases/WebKitGTK/webkit-2.10/LayoutTests/http/tests/cache/disk-cache/resource-becomes-uncacheable.html 2015-08-26 07:44:58 UTC (rev 188952)
@@ -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: releases/WebKitGTK/webkit-2.10/LayoutTests/http/tests/cache/disk-cache/resources/generate-response-optionally-cacheable.cgi (0 => 188952)
--- releases/WebKitGTK/webkit-2.10/LayoutTests/http/tests/cache/disk-cache/resources/generate-response-optionally-cacheable.cgi (rev 0)
+++ releases/WebKitGTK/webkit-2.10/LayoutTests/http/tests/cache/disk-cache/resources/generate-response-optionally-cacheable.cgi 2015-08-26 07:44:58 UTC (rev 188952)
@@ -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: releases/WebKitGTK/webkit-2.10/LayoutTests/http/tests/cache/disk-cache/resources/generate-response-optionally-cacheable.cgi
___________________________________________________________________
Added: svn:executable
Modified: releases/WebKitGTK/webkit-2.10/Source/WebKit2/ChangeLog (188951 => 188952)
--- releases/WebKitGTK/webkit-2.10/Source/WebKit2/ChangeLog 2015-08-26 07:44:24 UTC (rev 188951)
+++ releases/WebKitGTK/webkit-2.10/Source/WebKit2/ChangeLog 2015-08-26 07:44:58 UTC (rev 188952)
@@ -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: releases/WebKitGTK/webkit-2.10/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp (188951 => 188952)
--- releases/WebKitGTK/webkit-2.10/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp 2015-08-26 07:44:24 UTC (rev 188951)
+++ releases/WebKitGTK/webkit-2.10/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp 2015-08-26 07:44:58 UTC (rev 188952)
@@ -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: releases/WebKitGTK/webkit-2.10/Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp (188951 => 188952)
--- releases/WebKitGTK/webkit-2.10/Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp 2015-08-26 07:44:24 UTC (rev 188951)
+++ releases/WebKitGTK/webkit-2.10/Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp 2015-08-26 07:44:58 UTC (rev 188952)
@@ -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: releases/WebKitGTK/webkit-2.10/Source/WebKit2/NetworkProcess/cache/NetworkCache.h (188951 => 188952)
--- releases/WebKitGTK/webkit-2.10/Source/WebKit2/NetworkProcess/cache/NetworkCache.h 2015-08-26 07:44:24 UTC (rev 188951)
+++ releases/WebKitGTK/webkit-2.10/Source/WebKit2/NetworkProcess/cache/NetworkCache.h 2015-08-26 07:44:58 UTC (rev 188952)
@@ -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: releases/WebKitGTK/webkit-2.10/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.cpp (188951 => 188952)
--- releases/WebKitGTK/webkit-2.10/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.cpp 2015-08-26 07:44:24 UTC (rev 188951)
+++ releases/WebKitGTK/webkit-2.10/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.cpp 2015-08-26 07:44:58 UTC (rev 188952)
@@ -496,14 +496,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: releases/WebKitGTK/webkit-2.10/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.h (188951 => 188952)
--- releases/WebKitGTK/webkit-2.10/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.h 2015-08-26 07:44:24 UTC (rev 188951)
+++ releases/WebKitGTK/webkit-2.10/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.h 2015-08-26 07:44:58 UTC (rev 188952)
@@ -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(); }
