Protect CachedResource in more places

https://bugs.webkit.org/show_bug.cgi?id=259331
rdar://80172764

Reviewed by David Kilzer.

Protect CachedResource in more places. In particular, a call to MemoryCache::remove()
may delete the resource it is passed so it is important to protect the resource if
we're going to use it after.

* Source/WebCore/loader/SubresourceLoader.cpp:
(WebCore::SubresourceLoader::didFail):
* Source/WebCore/loader/cache/CachedFont.cpp:
(WebCore::CachedFont::setErrorAndDeleteData):
* Source/WebCore/loader/cache/CachedImage.cpp:
(WebCore::CachedImage::updateBufferInternal):
* Source/WebCore/loader/cache/CachedResource.cpp:
(WebCore::CachedResource::redirectReceived):
* Source/WebCore/loader/cache/CachedResourceHandle.cpp:
(WebCore::CachedResourceHandleBase::CachedResourceHandleBase):
* Source/WebCore/loader/cache/CachedResourceHandle.h:
(WebCore::CachedResourceHandle::CachedResourceHandle):
* Source/WebCore/loader/cache/MemoryCache.cpp:
(WebCore::MemoryCache::revalidationSucceeded):
(WebCore::MemoryCache::resourceForRequestImpl):
(WebCore::MemoryCache::forEachSessionResource):
(WebCore::MemoryCache::remove):
(WebCore::MemoryCache::removeResourcesWithOrigin):
(WebCore::MemoryCache::removeResourcesWithOrigins):
(WebCore::MemoryCache::getStatistics):
* Source/WebCore/loader/cache/MemoryCache.h:

Canonical link: https://commits.webkit.org/266163@main

Source/WebCore/loader/SubresourceLoader.cpp

@@ -783,6 +783,7 @@ void SubresourceLoader::didFail(const ResourceError& error)
         return;
 
     ASSERT(!reachedTerminalState());
+    CachedResourceHandle protectedResource { m_resource.get() };
     LOG(ResourceLoading, "Failed to load '%s'.\n", m_resource->url().string().latin1().data());
 
     if (m_frame->document() && error.isAccessControl() && error.domain() != InspectorNetworkAgent::errorDomain() && m_resource->type() != CachedResource::Type::Ping)

Source/WebCore/loader/cache/CachedFont.cpp

@@ -98,6 +98,7 @@ void CachedFont::finishLoading(const FragmentedSharedBuffer* data, const Network
 
 void CachedFont::setErrorAndDeleteData()
 {
+    CachedResourceHandle protectedThis { *this };
     setEncodedSize(0);
     error(Status::DecodeError);
     if (inCache())

Source/WebCore/loader/cache/CachedImage.cpp

@@ -483,6 +483,7 @@ inline void CachedImage::clearImage()
 
 void CachedImage::updateBufferInternal(const FragmentedSharedBuffer& data)
 {
+    CachedResourceHandle protectedThis { *this };
     m_data = const_cast<FragmentedSharedBuffer*>(&data);
     setEncodedSize(m_data->size());
     createImage();

Source/WebCore/loader/cache/CachedResource.cpp

@@ -451,6 +451,7 @@ Seconds CachedResource::freshnessLifetime(const ResourceResponse& response) cons
 
 void CachedResource::redirectReceived(ResourceRequest&& request, const ResourceResponse& response, CompletionHandler<void(ResourceRequest&&)>&& completionHandler)
 {
+    CachedResourceHandle protectedThis { *this };
     CACHEDRESOURCE_RELEASE_LOG("redirectReceived:");
 
     // Remove redirect urls from the memory cache if they contain a fragment.

Source/WebCore/loader/cache/CachedResourceHandle.cpp

@@ -35,6 +35,12 @@ CachedResourceHandleBase::CachedResourceHandleBase()
 {
 }
 
+CachedResourceHandleBase::CachedResourceHandleBase(CachedResource& resource)
+    : m_resource(&resource)
+{
+    m_resource->registerHandle(this);
+}
+
 CachedResourceHandleBase::CachedResourceHandleBase(CachedResource* resource)
     : m_resource(resource)
 {

Source/WebCore/loader/cache/CachedResourceHandle.h

@@ -45,7 +45,8 @@ class WEBCORE_EXPORT CachedResourceHandleBase {
 
 protected:
     CachedResourceHandleBase();
-    CachedResourceHandleBase(CachedResource*);
+    explicit CachedResourceHandleBase(CachedResource*);
+    explicit CachedResourceHandleBase(CachedResource&);
     CachedResourceHandleBase(const CachedResourceHandleBase&);
 
     void setResource(CachedResource*);
@@ -61,6 +62,7 @@ class WEBCORE_EXPORT CachedResourceHandleBase {
 template <class R> class CachedResourceHandle : public CachedResourceHandleBase {
 public: 
     CachedResourceHandle() { }
+    CachedResourceHandle(R& res) : CachedResourceHandleBase(res) { }
     CachedResourceHandle(R* res) : CachedResourceHandleBase(res) { }
     CachedResourceHandle(const CachedResourceHandle<R>& o) : CachedResourceHandleBase(o) { }
     template<typename U> CachedResourceHandle(const CachedResourceHandle<U>& o) : CachedResourceHandleBase(o.get()) { }

Source/WebCore/loader/cache/MemoryCache.cpp

@@ -145,6 +145,7 @@ void MemoryCache::revalidationSucceeded(CachedResource& revalidatingResource, co
     RELEASE_ASSERT(isMainThread());
     ASSERT(response.source() == ResourceResponse::Source::MemoryCacheAfterValidation);
     ASSERT(revalidatingResource.resourceToRevalidate());
+    CachedResourceHandle protectedRevalidatingResource { revalidatingResource };
     CachedResource& resource = *revalidatingResource.resourceToRevalidate();
     ASSERT(!resource.inCache());
     ASSERT(resource.isLoaded());
@@ -201,7 +202,7 @@ CachedResource* MemoryCache::resourceForRequestImpl(const ResourceRequest& reque
     URL url = removeFragmentIdentifierIfNeeded(request.url());
 
     auto key = std::make_pair(url, request.cachePartition());
-    return resources.get(key);
+    return resources.get(key).get();
 }
 
 unsigned MemoryCache::deadCapacity() const 
@@ -254,8 +255,10 @@ void MemoryCache::forEachSessionResource(PAL::SessionID sessionID, const Functio
     if (it == m_sessionResources.end())
         return;
 
-    for (auto& resource : copyToVector(it->value->values()))
-        function(*resource);
+    for (auto& resource : copyToVector(it->value->values())) {
+        if (resource)
+            function(*resource);
+    }
 }
 
 void MemoryCache::destroyDecodedDataForAllImages()
@@ -423,6 +426,8 @@ void MemoryCache::setCapacities(unsigned minDeadBytes, unsigned maxDeadBytes, un
 void MemoryCache::remove(CachedResource& resource)
 {
     RELEASE_ASSERT(isMainThread());
+    CachedResourceHandle protectedResource { resource };
+
     LOG(ResourceLoading, "Evicting resource %p for '%.255s' from cache", &resource, resource.url().string().latin1().data());
     // The resource may have already been removed by someone other than our caller,
     // who needed a fresh copy for a reload. See <http://bugs.webkit.org/show_bug.cgi?id=12479#c6>.
@@ -452,9 +457,6 @@ void MemoryCache::remove(CachedResource& resource)
         }
     }
     RELEASE_ASSERT(!resource.inCache());
-
-    if (resource.canDelete())
-        resource.deleteThis();
 }
 
 auto MemoryCache::lruListFor(CachedResource& resource) -> LRUList&
@@ -531,23 +533,25 @@ bool MemoryCache::inLiveDecodedResourcesList(CachedResource& resource) const
 void MemoryCache::removeResourcesWithOrigin(const SecurityOrigin& origin, const String& cachePartition)
 {
     RELEASE_ASSERT(isMainThread());
-    Vector<CachedResource*> resourcesWithOrigin;
+    Vector<WeakPtr<CachedResource>> resourcesWithOrigin;
     for (auto& resources : m_sessionResources.values()) {
         for (auto& keyValue : *resources) {
             auto& resource = *keyValue.value;
             auto& partitionName = keyValue.key.second;
             if (partitionName == cachePartition) {
-                resourcesWithOrigin.append(&resource);
+                resourcesWithOrigin.append(resource);
                 continue;
             }
             auto resourceOrigin = SecurityOrigin::create(resource.url());
             if (resourceOrigin->equal(&origin))
-                resourcesWithOrigin.append(&resource);
+                resourcesWithOrigin.append(resource);
         }
     }
 
-    for (auto* resource : resourcesWithOrigin)
-        remove(*resource);
+    for (auto& resource : resourcesWithOrigin) {
+        if (resource)
+            remove(*resource);
+    }
 }
 
 void MemoryCache::removeResourcesWithOrigin(const SecurityOrigin& origin)
@@ -576,20 +580,22 @@ void MemoryCache::removeResourcesWithOrigins(PAL::SessionID sessionID, const Has
     for (auto& origin : origins)
         originPartitions.add(ResourceRequest::partitionName(origin->host()));
 
-    Vector<CachedResource*> resourcesToRemove;
+    Vector<WeakPtr<CachedResource>> resourcesToRemove;
     for (auto& keyValuePair : *resourceMap) {
         auto& resource = *keyValuePair.value;
         auto& partitionName = keyValuePair.key.second;
         if (originPartitions.contains(partitionName)) {
-            resourcesToRemove.append(&resource);
+            resourcesToRemove.append(resource);
             continue;
         }
         if (origins.contains(SecurityOrigin::create(resource.url()).ptr()))
-            resourcesToRemove.append(&resource);
+            resourcesToRemove.append(resource);
     }
 
-    for (auto& resource : resourcesToRemove)
-        remove(*resource);
+    for (auto& resource : resourcesToRemove) {
+        if (resource)
+            remove(*resource);
+    }
 }
 
 void MemoryCache::getOriginsWithCache(SecurityOriginSet& origins)
@@ -697,7 +703,7 @@ MemoryCache::Statistics MemoryCache::getStatistics()
     Statistics stats;
 
     for (auto& resources : m_sessionResources.values()) {
-        for (auto* resource : resources->values()) {
+        for (auto& resource : resources->values()) {
             switch (resource->type()) {
             case CachedResource::Type::ImageResource:
                 stats.images.addResource(*resource);

Source/WebCore/loader/cache/MemoryCache.h

@@ -168,7 +168,7 @@ class MemoryCache {
     WEBCORE_EXPORT void pruneLiveResourcesToSize(unsigned targetSize, bool shouldDestroyDecodedDataForAllLiveResources = false);
 
 private:
-    using CachedResourceMap = HashMap<std::pair<URL, String /* partitionName */>, CachedResource*>;
+    using CachedResourceMap = HashMap<std::pair<URL, String /* partitionName */>, WeakPtr<CachedResource>>;
     using LRUList = WeakListHashSet<CachedResource>;
 
     MemoryCache();