MemoryCache: make sure that Resources are evicted only when they can be deleted

This CL fixes a problem where Resources can be taken out of the cache but not
deleted (if their handle count is not 0 for example).

BUG=290193

Review URL: https://codereview.chromium.org/179943002

git-svn-id: svn://svn.chromium.org/blink/trunk@170311 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/Source/core/fetch/MemoryCache.cpp

@@ -233,10 +233,11 @@ void MemoryCache::pruneDeadResources()
         MemoryCacheEntry* current = m_allResources[i].m_tail;
         while (current) {
             MemoryCacheEntry* previous = current->m_previousInAllResourcesList;
-            if (current->m_resource->wasPurged()) {
+            if (current->m_resource->wasPurged() && current->m_resource->canDelete()) {
                 ASSERT(!current->m_resource->hasClients());
                 ASSERT(!current->m_resource->isPreloaded());
-                evict(current);
+                bool wasEvicted = evict(current);
+                ASSERT_UNUSED(wasEvicted, wasEvicted);
             }
             current = previous;
         }
@@ -275,8 +276,10 @@ void MemoryCache::pruneDeadResources()
         while (current) {
             MemoryCacheEntry* previous = current->m_previousInAllResourcesList;
             ASSERT(!previous || contains(previous->m_resource.get()));
-            if (!current->m_resource->hasClients() && !current->m_resource->isPreloaded() && !current->m_resource->isCacheValidator()) {
-                evict(current);
+            if (!current->m_resource->hasClients() && !current->m_resource->isPreloaded()
+                && !current->m_resource->isCacheValidator() && current->m_resource->canDelete()) {
+                bool wasEvicted = evict(current);
+                ASSERT_UNUSED(wasEvicted, wasEvicted);
                 if (targetSize && m_deadSize <= targetSize)
                     return;
             }

third_party/WebKit/Source/core/fetch/MemoryCacheTest.cpp

@@ -153,7 +153,7 @@ TEST_F(MemoryCacheTest, DeadResourceEviction)
     const unsigned maxDeadCapacity = 0;
     memoryCache()->setCapacities(minDeadCapacity, maxDeadCapacity, totalCapacity);
 
-    ResourcePtr<Resource> cachedResource =
+    Resource* cachedResource =
         new Resource(ResourceRequest(""), Resource::Raw);
     const char data[5] = "abcd";
     cachedResource->appendData(data, 3);
@@ -164,7 +164,7 @@ TEST_F(MemoryCacheTest, DeadResourceEviction)
     ASSERT_EQ(0u, memoryCache()->deadSize());
     ASSERT_EQ(0u, memoryCache()->liveSize());
 
-    memoryCache()->add(cachedResource.get());
+    memoryCache()->add(cachedResource);
     ASSERT_EQ(cachedResource->size(), memoryCache()->deadSize());
     ASSERT_EQ(0u, memoryCache()->liveSize());
 
@@ -183,8 +183,8 @@ TEST_F(MemoryCacheTest, LiveResourceEvictionAtEndOfTask)
     const unsigned maxDeadCapacity = 0;
     memoryCache()->setCapacities(minDeadCapacity, maxDeadCapacity, totalCapacity);
     const char data[6] = "abcde";
-    ResourcePtr<Resource> cachedDeadResource =
-        new Resource(ResourceRequest("http://foo"), Resource::Raw);
+    Resource* cachedDeadResource =
+        new Resource(ResourceRequest("hhtp://foo"), Resource::Raw);
     cachedDeadResource->appendData(data, 3);
     ResourcePtr<Resource> cachedLiveResource =
         new FakeDecodedResource(ResourceRequest(""), Resource::Raw);
@@ -194,7 +194,7 @@ TEST_F(MemoryCacheTest, LiveResourceEvictionAtEndOfTask)
 
     class Task1 : public blink::WebThread::Task {
     public:
-        Task1(const ResourcePtr<Resource>& live, const ResourcePtr<Resource>& dead)
+        Task1(const ResourcePtr<Resource>& live, Resource* dead)
             : m_live(live)
             , m_dead(dead)
         { }
@@ -209,7 +209,7 @@ TEST_F(MemoryCacheTest, LiveResourceEvictionAtEndOfTask)
             ASSERT_EQ(0u, memoryCache()->deadSize());
             ASSERT_EQ(0u, memoryCache()->liveSize());
 
-            memoryCache()->add(m_dead.get());
+            memoryCache()->add(m_dead);
             memoryCache()->add(m_live.get());
             memoryCache()->insertInLiveDecodedResourcesList(m_live.get());
             ASSERT_EQ(m_dead->size(), memoryCache()->deadSize());
@@ -223,7 +223,8 @@ TEST_F(MemoryCacheTest, LiveResourceEvictionAtEndOfTask)
         }
 
     private:
-        ResourcePtr<Resource> m_live, m_dead;
+        ResourcePtr<Resource> m_live;
+        Resource* m_dead;
     };
 
     class Task2 : public blink::WebThread::Task {

third_party/WebKit/Source/core/fetch/Resource.cpp

@@ -364,6 +364,11 @@ bool Resource::unlock()
     return true;
 }
 
+bool Resource::hasRightHandleCountApartFromCache(unsigned targetCount) const
+{
+    return m_handleCount == targetCount + memoryCache()->contains(this);
+}
+
 void Resource::responseReceived(const ResourceResponse& response)
 {
     setResponse(response);
@@ -403,6 +408,17 @@ void Resource::setCachedMetadata(unsigned dataTypeID, const char* data, size_t s
     blink::Platform::current()->cacheMetadata(m_response.url(), m_response.responseTime(), serializedData.data(), serializedData.size());
 }
 
+bool Resource::canDelete() const
+{
+    return !hasClients() && !m_loader && !m_preloadCount && hasRightHandleCountApartFromCache(0)
+        && !m_protectorCount && !m_resourceToRevalidate && !m_proxyResource;
+}
+
+bool Resource::hasOneHandleApartFromCache() const
+{
+    return hasRightHandleCountApartFromCache(1);
+}
+
 CachedMetadata* Resource::cachedMetadata(unsigned dataTypeID) const
 {
     if (!m_cachedMetadata || m_cachedMetadata->dataTypeID() != dataTypeID)
@@ -771,6 +787,8 @@ void Resource::unregisterHandle(ResourcePtrBase* h)
         unlock();
     } else if (m_handleCount == 1 && memoryCache()->contains(this)) {
         unlock();
+        if (!hasClients())
+            memoryCache()->prune(this);
     }
 }
 

third_party/WebKit/Source/core/fetch/Resource.h

@@ -197,8 +197,8 @@ public:
     // Returns cached metadata of the given type associated with this resource.
     CachedMetadata* cachedMetadata(unsigned dataTypeID) const;
 
-    bool canDelete() const { return !hasClients() && !m_loader && !m_preloadCount && !m_handleCount && !m_protectorCount && !m_resourceToRevalidate && !m_proxyResource; }
-    bool hasOneHandle() const { return m_handleCount == 1; }
+    bool canDelete() const;
+    bool hasOneHandleApartFromCache() const;
 
     // List of acceptable MIME types separated by ",".
     // A MIME type may contain a wildcard, e.g. "text/*".
@@ -342,6 +342,8 @@ private:
 
     bool unlock();
 
+    bool hasRightHandleCountApartFromCache(unsigned targetCount) const;
+
     void failBeforeStarting();
 
     String m_fragmentIdentifierForRequest;

third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp

@@ -1072,7 +1072,7 @@ void ResourceFetcher::garbageCollectDocumentResources()
     StringVector resourcesToDelete;
 
     for (DocumentResourceMap::iterator it = m_documentResources.begin(); it != m_documentResources.end(); ++it) {
-        if (it->value->hasOneHandle())
+        if (it->value->hasOneHandleApartFromCache())
             resourcesToDelete.append(it->key);
     }