Diff
Modified: trunk/Source/WebCore/ChangeLog (278118 => 278119)
--- trunk/Source/WebCore/ChangeLog 2021-05-26 18:52:23 UTC (rev 278118)
+++ trunk/Source/WebCore/ChangeLog 2021-05-26 19:19:28 UTC (rev 278119)
@@ -1,3 +1,55 @@
+2021-05-26 Antti Koivisto <an...@apple.com>
+
+ Don't hang onto expired resources without validation headers in memory cache
+ https://bugs.webkit.org/show_bug.cgi?id=226083
+ rdar://78035612
+
+ Reviewed by Chris Dumez.
+
+ They consume memory while only really being useful for history navigation.
+ Disk cache can handle the relatively rare cases where that is useful and not
+ covered by the page cache.
+
+ * loader/ResourceTimingInformation.cpp:
+ (WebCore::ResourceTimingInformation::addResourceTiming):
+ (WebCore::ResourceTimingInformation::removeResourceTiming):
+
+ Add way to remove entries from the map.
+
+ * loader/ResourceTimingInformation.h:
+ * loader/cache/CachedResource.cpp:
+ (WebCore::CachedResource::removeClient):
+
+ Removal from cache is now handled in deleteIfPossible() so any path that gets the resource to
+ canDelete() state will do cache removal check.
+ Since deleteIfPossible is invoked by removeClient the existing behavior is covered.
+
+ (WebCore::CachedResource::deleteIfPossible):
+
+ Remove expired resources without validation headers from the cache if the resource is otherwise deletable.
+ Also eagerly remove no-store http resources (previously only https was removed).
+
+ (WebCore::CachedResource::deleteThis):
+
+ Factor into a function.
+
+ * loader/cache/CachedResource.h:
+ * loader/cache/CachedResourceLoader.cpp:
+ (WebCore::CachedResourceLoader::requestResource):
+
+ Ensure existing resource is removed from cache adding a new one. This was causing confusion with inCache state.
+
+ (WebCore::CachedResourceLoader::garbageCollectDocumentResources):
+
+ Don't remove entries with active loader. They are needed for resource timing.
+ Don't leave removed entries to ResourceTimingInformation. They are not guaranteed to stay alive after
+ being removed from m_documentResources.
+
+ * loader/cache/MemoryCache.cpp:
+ (WebCore::MemoryCache::remove):
+
+ Call CachedResource::deleteThis() directly.
+
2021-05-26 Chris Dumez <cdu...@apple.com>
Drop SecurityPolicyViolationEvent's blockedURL & documentURL attributes
Modified: trunk/Source/WebCore/loader/ResourceTimingInformation.cpp (278118 => 278119)
--- trunk/Source/WebCore/loader/ResourceTimingInformation.cpp 2021-05-26 18:52:23 UTC (rev 278118)
+++ trunk/Source/WebCore/loader/ResourceTimingInformation.cpp 2021-05-26 19:19:28 UTC (rev 278119)
@@ -86,6 +86,11 @@
info.added = Added;
}
+void ResourceTimingInformation::removeResourceTiming(CachedResource& resource)
+{
+ m_initiatorMap.remove(&resource);
+}
+
void ResourceTimingInformation::storeResourceTimingInitiatorInformation(const CachedResourceHandle<CachedResource>& resource, const AtomString& initiatorName, Frame* frame)
{
ASSERT(resource.get());
Modified: trunk/Source/WebCore/loader/ResourceTimingInformation.h (278118 => 278119)
--- trunk/Source/WebCore/loader/ResourceTimingInformation.h 2021-05-26 18:52:23 UTC (rev 278118)
+++ trunk/Source/WebCore/loader/ResourceTimingInformation.h 2021-05-26 19:19:28 UTC (rev 278119)
@@ -41,6 +41,7 @@
static bool shouldAddResourceTiming(CachedResource&);
void addResourceTiming(CachedResource&, Document&, ResourceTiming&&);
+ void removeResourceTiming(CachedResource&);
void storeResourceTimingInitiatorInformation(const CachedResourceHandle<CachedResource>&, const AtomString&, Frame*);
private:
@@ -49,6 +50,7 @@
AtomString name;
AlreadyAdded added;
};
+ // FIXME: This shoudn't use raw pointer as identifier (though it is not dereferenced).
HashMap<CachedResource*, InitiatorInfo> m_initiatorMap;
};
Modified: trunk/Source/WebCore/loader/cache/CachedResource.cpp (278118 => 278119)
--- trunk/Source/WebCore/loader/cache/CachedResource.cpp 2021-05-26 18:52:23 UTC (rev 278118)
+++ trunk/Source/WebCore/loader/cache/CachedResource.cpp 2021-05-26 19:19:28 UTC (rev 278119)
@@ -552,11 +552,6 @@
didRemoveClient(client);
}
- if (deleteIfPossible()) {
- // `this` object is dead here.
- return;
- }
-
if (hasClients())
return;
@@ -565,6 +560,12 @@
memoryCache.removeFromLiveResourcesSize(*this);
memoryCache.removeFromLiveDecodedResourcesList(*this);
}
+
+ if (deleteIfPossible()) {
+ // `this` object is dead here.
+ return;
+ }
+
if (!m_switchingClientsToRevalidatedResource)
allClientsRemoved();
destroyDecodedDataIfNeeded();
@@ -572,13 +573,6 @@
if (!allowsCaching())
return;
- if (response().cacheControlContainsNoStore() && url().protocolIs("https")) {
- // RFC2616 14.9.2:
- // "no-store: ... MUST make a best-effort attempt to remove the information from volatile storage as promptly as possible"
- // "... History buffers MAY store such responses as part of their normal operation."
- // We allow non-secure content to be reused in history, but we do not allow secure content to be reused.
- memoryCache.remove(*this);
- }
memoryCache.pruneSoon();
}
@@ -604,21 +598,49 @@
bool CachedResource::deleteIfPossible()
{
- if (canDelete()) {
- LOG(ResourceLoading, "CachedResource %p deleteIfPossible - can delete, in cache %d", this, inCache());
- if (!inCache()) {
- InspectorInstrumentation::willDestroyCachedResource(*this);
- delete this;
+ if (!canDelete()) {
+ LOG(ResourceLoading, "CachedResource %p deleteIfPossible - can't delete (hasClients %d loader %p preloadCount %u handleCount %u resourceToRevalidate %p proxyResource %p)", this, hasClients(), m_loader.get(), m_preloadCount, m_handleCount, m_resourceToRevalidate, m_proxyResource);
+ return false;
+ }
+
+ LOG(ResourceLoading, "CachedResource %p deleteIfPossible - can delete, in cache %d", this, inCache());
+
+ if (!inCache()) {
+ deleteThis();
+ return true;
+ }
+
+ auto shouldRemoveFromCache = [&] {
+ // We may still keeps some of these cases in disk cache for history navigation.
+ if (response().cacheControlContainsNoStore())
return true;
- }
- if (m_data)
- m_data->hintMemoryNotNeededSoon();
+ if (isExpired() && !canUseCacheValidator())
+ return true;
+ return false;
+ }();
+
+ if (shouldRemoveFromCache) {
+ // Deletes this.
+ MemoryCache::singleton().remove(*this);
+ return true;
}
- LOG(ResourceLoading, "CachedResource %p deleteIfPossible - can't delete (hasClients %d loader %p preloadCount %u handleCount %u resourceToRevalidate %p proxyResource %p)", this, hasClients(), m_loader.get(), m_preloadCount, m_handleCount, m_resourceToRevalidate, m_proxyResource);
+ if (m_data)
+ m_data->hintMemoryNotNeededSoon();
+
return false;
}
+void CachedResource::deleteThis()
+{
+ RELEASE_ASSERT(canDelete());
+ RELEASE_ASSERT(!inCache());
+
+ InspectorInstrumentation::willDestroyCachedResource(*this);
+
+ delete this;
+}
+
void CachedResource::setDecodedSize(unsigned size)
{
if (size == m_decodedSize)
Modified: trunk/Source/WebCore/loader/cache/CachedResource.h (278118 => 278119)
--- trunk/Source/WebCore/loader/cache/CachedResource.h 2021-05-26 18:52:23 UTC (rev 278118)
+++ trunk/Source/WebCore/loader/cache/CachedResource.h 2021-05-26 19:19:28 UTC (rev 278119)
@@ -310,6 +310,8 @@
private:
class Callback;
+ void deleteThis();
+
bool addClientToSet(CachedResourceClient&);
void decodedDataDeletionTimerFired();
Modified: trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp (278118 => 278119)
--- trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp 2021-05-26 18:52:23 UTC (rev 278118)
+++ trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp 2021-05-26 19:19:28 UTC (rev 278119)
@@ -992,8 +992,10 @@
memoryCache.remove(*resource);
FALLTHROUGH;
case Load:
- if (resource)
+ if (resource) {
logMemoryCacheResourceRequest(&frame, DiagnosticLoggingKeys::memoryCacheEntryDecisionKey(), DiagnosticLoggingKeys::unusedKey());
+ memoryCache.remove(*resource);
+ }
resource = loadResource(type, page.sessionID(), WTFMove(request), cookieJar, page.settings());
break;
case Revalidate:
@@ -1443,11 +1445,14 @@
typedef Vector<String, 10> StringVector;
StringVector resourcesToDelete;
- for (auto& resource : m_documentResources) {
- LOG(ResourceLoading, " cached resource %p - hasOneHandle %d", resource.value.get(), resource.value->hasOneHandle());
+ for (auto& resourceEntry : m_documentResources) {
+ auto& resource = *resourceEntry.value;
+ LOG(ResourceLoading, " cached resource %p - hasOneHandle %d", &resource, resource.hasOneHandle());
- if (resource.value->hasOneHandle())
- resourcesToDelete.append(resource.key);
+ if (resource.hasOneHandle() && !resource.loader() && !resource.isPreloaded()) {
+ resourcesToDelete.append(resourceEntry.key);
+ m_resourceTimingInfo.removeResourceTiming(resource);
+ }
}
for (auto& resource : resourcesToDelete)
Modified: trunk/Source/WebCore/loader/cache/MemoryCache.cpp (278118 => 278119)
--- trunk/Source/WebCore/loader/cache/MemoryCache.cpp 2021-05-26 18:52:23 UTC (rev 278118)
+++ trunk/Source/WebCore/loader/cache/MemoryCache.cpp 2021-05-26 19:19:28 UTC (rev 278119)
@@ -122,7 +122,7 @@
resource.setInCache(true);
resourceAccessed(resource);
-
+
LOG(ResourceLoading, "MemoryCache::add Added '%.255s', resource %p\n", resource.url().string().latin1().data(), &resource);
return true;
}
@@ -416,7 +416,8 @@
}
}
- resource.deleteIfPossible();
+ if (resource.canDelete())
+ resource.deleteThis();
}
auto MemoryCache::lruListFor(CachedResource& resource) -> LRUList&
