Make MemoryCache's LRUList manipulation private

Source/core/fetch/MemoryCache.cpp

Index: Source/core/fetch/MemoryCache.cpp
diff --git a/Source/core/fetch/MemoryCache.cpp b/Source/core/fetch/MemoryCache.cpp
index 208ade0cfca0f96f8504c0522baee41ae5ecab34..4e0d6023a6601d46e792b103183c77e4eb2362d9 100644
--- a/Source/core/fetch/MemoryCache.cpp
+++ b/Source/core/fetch/MemoryCache.cpp
@@ -111,8 +111,7 @@ void MemoryCache::add(Resource* resource)
     ASSERT(WTF::isMainThread());
     ASSERT(!m_resources.contains(resource->url()));
     m_resources.set(resource->url(), MemoryCacheEntry::create(resource));
-    resource->setInCache(true);
-    resource->updateForAccess();
+    update(resource, 0, resource->size(), true);
 
     WTF_LOG(ResourceLoading, "MemoryCache::add Added '%s', resource %p\n", resource->url().string().latin1().data(), resource);
 }
@@ -120,15 +119,17 @@ void MemoryCache::add(Resource* resource)
 void MemoryCache::replace(Resource* newResource, Resource* oldResource)
 {
     evict(oldResource);
-    ASSERT(!m_resources.contains(newResource->url()));
-    m_resources.set(newResource->url(), MemoryCacheEntry::create(newResource));
-    newResource->setInCache(true);
-    insertInLRUList(newResource);
-    int delta = newResource->size();
+    add(newResource);
     if (newResource->decodedSize() && newResource->hasClients())
         insertInLiveDecodedResourcesList(newResource);
-    if (delta)
-        adjustSize(newResource->hasClients(), delta);
+}
+
+bool MemoryCache::contains(Resource* resource)
+{
+    if (resource->url().isNull())
+        return false;
+    MemoryCacheEntry* entry = m_resources.get(resource->url());
+    return entry && entry->m_resource == resource;
 }
 
 Resource* MemoryCache::resourceForURL(const KURL& resourceURL)
@@ -242,7 +243,7 @@ void MemoryCache::pruneDeadResources()
         while (current) {
             // Protect 'previous' so it can't get deleted during destroyDecodedData().
             MemoryCacheEntry* previous = current->m_previousInAllResourcesList;
-            ASSERT(!previous || previous->m_resource->inCache());
+            ASSERT(!previous || contains(previous->m_resource.get()));
             if (!current->m_resource->hasClients() && !current->m_resource->isPreloaded() && current->m_resource->isLoaded()) {
                 // Destroy our decoded data. This will remove us from
                 // m_liveDecodedResources, and possibly move us to a different
@@ -254,7 +255,7 @@ void MemoryCache::pruneDeadResources()
             }
             // Decoded data may reference other resources. Stop iterating if 'previous' somehow got
             // kicked out of cache during destroyDecodedData().
-            if (previous && !previous->m_resource->inCache())
+            if (previous && !contains(previous->m_resource.get()))
                 break;
             current = previous;
         }
@@ -263,13 +264,13 @@ void MemoryCache::pruneDeadResources()
         current = m_allResources[i].m_tail;
         while (current) {
             MemoryCacheEntry* previous = current->m_previousInAllResourcesList;
-            ASSERT(!previous || previous->m_resource->inCache());
+            ASSERT(!previous || contains(previous->m_resource.get()));
             if (!current->m_resource->hasClients() && !current->m_resource->isPreloaded() && !current->m_resource->isCacheValidator()) {
                 evict(current->m_resource.get());
                 if (targetSize && m_deadSize <= targetSize)
                     return;
             }
-            if (previous && !previous->m_resource->inCache())
+            if (previous && !contains(previous->m_resource.get()))
                 break;
             current = previous;
         }
@@ -300,42 +301,28 @@ bool MemoryCache::evict(Resource* resource)
     WTF_LOG(ResourceLoading, "Evicting resource %p for '%s' 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>.
-    if (resource->inCache()) {
-        // Remove from the appropriate LRU list.
-        removeFromLRUList(resource);
+    if (contains(resource)) {
+        update(resource, resource->size(), 0, false);
         removeFromLiveDecodedResourcesList(resource);
-        adjustSize(resource->hasClients(), -static_cast<ptrdiff_t>(resource->size()));
 
         // Remove from the resource map.
         m_resources.remove(resource->url());
-        resource->setInCache(false);
-    } else {
-        ASSERT(!m_resources.get(resource->url()) || m_resources.get(resource->url())->m_resource != resource);
     }
 
     return resource->deleteIfPossible();
 }
 
-MemoryCache::LRUList* MemoryCache::lruListFor(MemoryCacheEntry* entry)
+MemoryCache::LRUList* MemoryCache::lruListFor(unsigned accessCount, size_t size)
 {
-    unsigned accessCount = std::max(entry->m_resource->accessCount(), 1U);
-    unsigned queueIndex = WTF::fastLog2(entry->m_resource->size() / accessCount);
+    ASSERT(accessCount > 0);
+    unsigned queueIndex = WTF::fastLog2(size / accessCount);
     if (m_allResources.size() <= queueIndex)
         m_allResources.grow(queueIndex + 1);
     return &m_allResources[queueIndex];
 }
 
-void MemoryCache::removeFromLRUList(Resource* resource)
+void MemoryCache::removeFromLRUList(MemoryCacheEntry* entry, LRUList* list)
 {
-    MemoryCacheEntry* entry = m_resources.get(resource->url());
-    ASSERT(entry->m_resource == resource);
-
-    LRUList* list = lruListFor(entry);
-    MemoryCacheEntry* next = entry->m_nextInAllResourcesList;
-    MemoryCacheEntry* previous = entry->m_previousInAllResourcesList;
-    if (!next && !previous && list->m_head != entry)
-        return;
-
 #if !ASSERT_DISABLED
     // Verify that we are in fact in this list.
     bool found = false;
@@ -348,6 +335,8 @@ void MemoryCache::removeFromLRUList(Resource* resource)
     ASSERT(found);
 #endif
 
+    MemoryCacheEntry* next = entry->m_nextInAllResourcesList;
+    MemoryCacheEntry* previous = entry->m_previousInAllResourcesList;
     entry->m_nextInAllResourcesList = 0;
     entry->m_previousInAllResourcesList = 0;
 
@@ -362,28 +351,20 @@ void MemoryCache::removeFromLRUList(Resource* resource)
         list->m_head = next;
 }
 
-void MemoryCache::insertInLRUList(Resource* resource)
+void MemoryCache::insertInLRUList(MemoryCacheEntry* entry, LRUList* list)
 {
-    MemoryCacheEntry* entry = m_resources.get(resource->url());
-    ASSERT(entry->m_resource == resource);
-
-    // Make sure we aren't in some list already.
     ASSERT(!entry->m_nextInAllResourcesList && !entry->m_previousInAllResourcesList);
-    ASSERT(resource->inCache());
-
-    LRUList* list = lruListFor(entry);
 
     entry->m_nextInAllResourcesList = list->m_head;
-    if (list->m_head)
-        list->m_head->m_previousInAllResourcesList = entry;
     list->m_head = entry;
 
-    if (!entry->m_nextInAllResourcesList)
+    if (entry->m_nextInAllResourcesList)
+        entry->m_nextInAllResourcesList->m_previousInAllResourcesList = entry;
+    else
         list->m_tail = entry;
 
 #if !ASSERT_DISABLED
     // Verify that we are in now in the list like we should be.
-    list = lruListFor(entry);
     bool found = false;
     for (MemoryCacheEntry* current = list->m_head; current; current = current->m_nextInAllResourcesList) {
         if (current == entry) {
@@ -470,8 +451,8 @@ void MemoryCache::insertInLiveDecodedResourcesList(Resource* resource)
 bool MemoryCache::isInLiveDecodedResourcesList(Resource* resource)
 {
     MemoryCacheEntry* entry = m_resources.get(resource->url());
-    ASSERT(entry->m_resource == resource);
-    return entry ? entry->m_inLiveDecodedResourcesList : false;
+    ASSERT(entry && entry->m_resource == resource);
+    return entry->m_inLiveDecodedResourcesList;
 }
 
 void MemoryCache::addToLiveResourcesSize(Resource* resource)
@@ -488,9 +469,22 @@ void MemoryCache::removeFromLiveResourcesSize(Resource* resource)
     m_deadSize += resource->size();
 }
 
-void MemoryCache::adjustSize(bool live, ptrdiff_t delta)
+void MemoryCache::update(Resource* resource, size_t oldSize, size_t newSize, bool wasAccessed)
 {
-    if (live) {
+    ASSERT(contains(resource));
+    MemoryCacheEntry* entry = m_resources.get(resource->url());
+
+    // The object must now be moved to a different queue, since either its size or its accessCount has been changed,
+    // and both of those are used to determine which LRU queue the resource should be in.
+    if (oldSize)
+        removeFromLRUList(entry, lruListFor(entry->m_accessCount, oldSize));
+    if (wasAccessed)
+        entry->m_accessCount++;
+    if (newSize)
+        insertInLRUList(entry, lruListFor(entry->m_accessCount, newSize));
+
+    ptrdiff_t delta = newSize - oldSize;
+    if (resource->hasClients()) {
         ASSERT(delta >= 0 || m_liveSize >= static_cast<size_t>(-delta) );
         m_liveSize += delta;
     } else {