MemoryCache: Enable MemoryCache to have multiple isolated resource maps

Source/core/fetch/MemoryCache.cpp

 /*
     Copyright (C) 1998 Lars Knoll (knoll@mpi-hd.mpg.de)
     Copyright (C) 2001 Dirk Mueller (mueller@kde.org)
     Copyright (C) 2002 Waldo Bastian (bastian@kde.org)
     Copyright (C) 2004, 2005, 2006, 2007, 2008 Apple Inc. All rights reserved.
 
     This library is free software; you can redistribute it and/or
     modify it under the terms of the GNU Library General Public
     License as published by the Free Software Foundation; either
     version 2 of the License, or (at your option) any later version.
@@ skipping 135 matching lines @@
     // Strip away fragment identifier from HTTP URLs.
     // Data URLs must be unmodified. For file and custom URLs clients may expect resources
     // to be unique even when they differ by the fragment identifier only.
     if (!originalURL.protocolIsInHTTPFamily())
         return originalURL;
     KURL url = originalURL;
     url.removeFragmentIdentifier();
     return url;
 }
 
+String MemoryCache::defaultCacheIdentifier()
+{
+    return emptyString();
+}
+
+MemoryCache::ResourceMap* MemoryCache::getResourceMap(const String& cacheIdentifier)
+{
+    if (!m_resources.contains(cacheIdentifier))
+        m_resources.add(cacheIdentifier, adoptPtrWillBeNoop(new ResourceMap()));

[Mike West, 2014/10/24 12:42:59]

`add()` can fail, can't it? That could cause a null-deref on line 173.

[nhiroki, 2014/10/27 14:30:12]

Hmmm... I think a null-deref never happens because "add()" can fail (can do
nothing) only if "m_resources" has already contained a key "cacheIdentifier" and
it's confirmed at line 163. You mean other cases? (eg. out-of-memory??)

+    return m_resources.get(cacheIdentifier);
+}
+
 void MemoryCache::add(Resource* resource)
 {
     ASSERT(WTF::isMainThread());
     ASSERT(resource->url().isValid());
-    RELEASE_ASSERT(!m_resources.contains(resource->url()));
-    m_resources.set(resource->url().string(), MemoryCacheEntry::create(resource));
+    ResourceMap* resources = getResourceMap(resource->cacheIdentifier());
+    RELEASE_ASSERT(!resources->contains(resource->url()));
+    resources->set(resource->url(), MemoryCacheEntry::create(resource));
     update(resource, 0, resource->size(), true);
 
     WTF_LOG(ResourceLoading, "MemoryCache::add Added '%s', resource %p\n", resource->url().string().latin1().data(), resource);
 }
 
 void MemoryCache::replace(Resource* newResource, Resource* oldResource)
 {
-    if (MemoryCacheEntry* oldEntry = m_resources.get(oldResource->url()))
+    ASSERT(newResource->cacheIdentifier() == oldResource->cacheIdentifier());
+    ResourceMap* resources = getResourceMap(oldResource->cacheIdentifier());
+    if (MemoryCacheEntry* oldEntry = resources->get(oldResource->url()))
         evict(oldEntry);
     add(newResource);
     if (newResource->decodedSize() && newResource->hasClients())
-        insertInLiveDecodedResourcesList(m_resources.get(newResource->url()));
+        insertInLiveDecodedResourcesList(resources->get(newResource->url()));
 }
 
 void MemoryCache::remove(Resource* resource)
 {
     // The resource may have already been removed by someone other than our caller,
     // who needed a fresh copy for a reload.
     if (!contains(resource))

[Mike West, 2014/10/24 12:42:59]

Rather than doing the lookup twice, it might make sense to phrase it as:

`if (ResourceCacheEntry* entry = getEntryForResource(resource)) { ... }`

[nhiroki, 2014/10/27 14:30:13]

Good point! Removed redundant contains() calls.

         return;
-    evict(m_resources.get(resource->url()));
+    evict(getEntryForResource(resource));
 }
 
 bool MemoryCache::contains(const Resource* resource) const
 {
     if (resource->url().isNull())
         return false;
-    const MemoryCacheEntry* entry = m_resources.get(resource->url());
+    ResourceMap* resources = m_resources.get(resource->cacheIdentifier());

[Mike West, 2014/10/24 12:43:00]

It's odd to have `getResourceMap` and not use it here. I understand why; you
don't want the side-effect of creating the resourceMap if it doesn't exist.
Perhaps you could rename `getResourceMap` to `ensureResourceMap` or something
similar to resolve the confusion?

[nhiroki, 2014/10/27 14:30:12]

Yes, that's right.
Sounds reasonable. Renamed it to ensureResourceMap.

+    if (!resources)
+        return false;
+    MemoryCacheEntry* entry = resources->get(resource->url());
     return entry && entry->m_resource == resource;
 }
 
-Resource* MemoryCache::resourceForURL(const KURL& resourceURL)
+Resource* MemoryCache::resourceForURL(const String& cacheIdentifier, const KURL& resourceURL)
 {
     ASSERT(WTF::isMainThread());
+    ResourceMap* resources = getResourceMap(cacheIdentifier);
     KURL url = removeFragmentIdentifierIfNeeded(resourceURL);
-    MemoryCacheEntry* entry = m_resources.get(url);
+    MemoryCacheEntry* entry = resources->get(url);
     if (!entry)
         return 0;
     Resource* resource = entry->m_resource.get();
     if (resource && !resource->lock()) {
         ASSERT(!resource->hasClients());
         bool didEvict = evict(entry);
         ASSERT_UNUSED(didEvict, didEvict);
         return 0;
     }
     return resource;
 }
 
+WillBeHeapVector<Member<Resource>> MemoryCache::resourcesForURL(const KURL& resourceURL)
+{
+    ASSERT(WTF::isMainThread());
+    KURL url = removeFragmentIdentifierIfNeeded(resourceURL);
+    WillBeHeapVector<Member<Resource>> results;
+    for (const auto& resources : m_resources) {

[Mike West, 2014/10/24 12:42:59]

"ResourceMap" seems clearer than "auto".

[nhiroki, 2014/10/27 14:30:12]

"m_resources" is a HashMap object, so this "auto" is not inferred as
"ResourceMap" but "ResourceMapIndex::ValueType". You mean ...

for (const OwnPtr<ResourceMap>& resources : m_resources.values()) {...}

? Or am I missing something?

+        MemoryCacheEntry* entry = resources.value->get(url);
+        if (entry)
+            results.append(entry->m_resource.get());
+    }
+    return results;
+}
+
 size_t MemoryCache::deadCapacity() const
 {
     // Dead resource capacity is whatever space is not occupied by live resources, bounded by an independent minimum and maximum.
     size_t capacity = m_capacity - std::min(m_liveSize, m_capacity); // Start with available capacity.
     capacity = std::max(capacity, m_minDeadCapacity); // Make sure it's above the minimum.
     capacity = std::min(capacity, m_maxDeadCapacity); // Make sure it's below the maximum.
     return capacity;
 }
 
 size_t MemoryCache::liveCapacity() const
@@ skipping 147 matching lines @@
     ASSERT(WTF::isMainThread());
 
     Resource* resource = entry->m_resource.get();
     bool canDelete = resource->canDelete();
     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>.
     update(resource, resource->size(), 0, false);
     removeFromLiveDecodedResourcesList(entry);
 
-    ResourceMap::iterator it = m_resources.find(resource->url());
-    ASSERT(it != m_resources.end());
+    ResourceMap* resources = getResourceMap(resource->cacheIdentifier());
+    ResourceMap::iterator it = resources->find(resource->url());
+    ASSERT(it != resources->end());
 #if ENABLE(OILPAN)
     MemoryCacheEntry* entryPtr = it->value;
 #else
     OwnPtr<MemoryCacheEntry> entryPtr;
     entryPtr.swap(it->value);
 #endif
-    m_resources.remove(it);
+    resources->remove(it);
 #if ENABLE(OILPAN)
     if (entryPtr)
         entryPtr->dispose();
 #endif
     return canDelete;
 }
 
+MemoryCacheEntry* MemoryCache::getEntryForResource(const Resource* resource) const
+{
+    if (resource->url().isNull())

[Mike West, 2014/10/24 12:43:00]

isNull or isEmpty?

[nhiroki, 2014/10/27 14:30:13]

Done.

+        return nullptr;
+    ResourceMap* resources = m_resources.get(resource->cacheIdentifier());
+    if (!resources)
+        return nullptr;
+    MemoryCacheEntry* entry = resources->get(resource->url());
+    if (!entry || entry->m_resource != resource)
+        return nullptr;
+    return entry;
+}
+
 MemoryCacheLRUList* MemoryCache::lruListFor(unsigned accessCount, size_t size)
 {
     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(MemoryCacheEntry* entry, MemoryCacheLRUList* list)
@@ skipping 112 matching lines @@
     m_liveSize += resource->size();
     m_deadSize -= resource->size();
 }
 
 void MemoryCache::makeDead(Resource* resource)
 {
     if (!contains(resource))
         return;
     m_liveSize -= resource->size();
     m_deadSize += resource->size();
-    removeFromLiveDecodedResourcesList(m_resources.get(resource->url()));
+    removeFromLiveDecodedResourcesList(getEntryForResource(resource));
 }
 
 void MemoryCache::update(Resource* resource, size_t oldSize, size_t newSize, bool wasAccessed)
 {
     if (!contains(resource))
         return;
-    MemoryCacheEntry* entry = m_resources.get(resource->url());
+    MemoryCacheEntry* entry = getEntryForResource(resource);
 
     // 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 {
         ASSERT(delta >= 0 || m_deadSize >= static_cast<size_t>(-delta) );
         m_deadSize += delta;
     }
 }
 
 void MemoryCache::updateDecodedResource(Resource* resource, UpdateReason reason, MemoryCacheLiveResourcePriority priority)
 {
     if (!contains(resource))
         return;
-    MemoryCacheEntry* entry = m_resources.get(resource->url());
+    MemoryCacheEntry* entry = getEntryForResource(resource);
 
     removeFromLiveDecodedResourcesList(entry);
     if (priority != MemoryCacheLiveResourcePriorityUnknown && priority != entry->m_liveResourcePriority)
         entry->m_liveResourcePriority = priority;
     if (resource->decodedSize() && resource->hasClients())
         insertInLiveDecodedResourcesList(entry);
 
     if (reason != UpdateForAccess)
         return;
 
     double timestamp = resource->isImage() ? FrameView::currentFrameTimeStamp() : 0.0;
     if (!timestamp)
         timestamp = currentTime();
     entry->m_lastDecodedAccessTime = timestamp;
 }
 
 MemoryCacheLiveResourcePriority MemoryCache::priority(Resource* resource) const
 {
     if (!contains(resource))
         return MemoryCacheLiveResourcePriorityUnknown;
-    MemoryCacheEntry* entry = m_resources.get(resource->url());
+    MemoryCacheEntry* entry = getEntryForResource(resource);
     return entry->m_liveResourcePriority;
 }
 
 void MemoryCache::removeURLFromCache(ExecutionContext* context, const KURL& url)
 {
     if (context->isWorkerGlobalScope()) {
         WorkerGlobalScope* workerGlobalScope = toWorkerGlobalScope(context);
         workerGlobalScope->thread()->workerLoaderProxy().postTaskToLoader(createCrossThreadTask(&removeURLFromCacheInternal, url));
         return;
     }
     removeURLFromCacheInternal(context, url);
 }
 
 void MemoryCache::removeURLFromCacheInternal(ExecutionContext*, const KURL& url)
 {
-    if (Resource* resource = memoryCache()->resourceForURL(url))
+    auto resources = memoryCache()->resourcesForURL(url);
+    for (const auto& resource : resources)

[Mike West, 2014/10/24 12:42:59]

"Resource" seems clearer than "auto".

[nhiroki, 2014/10/27 14:30:12]

Done.

         memoryCache()->remove(resource);
 }
 
 void MemoryCache::TypeStatistic::addResource(Resource* o)
 {
     bool purged = o->wasPurged();
     bool purgeable = o->isPurgeable() && !purged;
     size_t pageSize = (o->encodedSize() + o->overheadSize() + 4095) & ~4095;
     count++;
     size += purged ? 0 : o->size();
     liveSize += o->hasClients() ? o->size() : 0;
     decodedSize += o->decodedSize();
     encodedSize += o->encodedSize();
     encodedSizeDuplicatedInDataURLs += o->url().protocolIsData() ? o->encodedSize() : 0;
     purgeableSize += purgeable ? pageSize : 0;
     purgedSize += purged ? pageSize : 0;
 }
 
 MemoryCache::Statistics MemoryCache::getStatistics()
 {
     Statistics stats;
-    for (const auto& resourceIter : m_resources) {
-        Resource* resource = resourceIter.value->m_resource.get();
-        switch (resource->type()) {
-        case Resource::Image:
-            stats.images.addResource(resource);
-            break;
-        case Resource::CSSStyleSheet:
-            stats.cssStyleSheets.addResource(resource);
-            break;
-        case Resource::Script:
-            stats.scripts.addResource(resource);
-            break;
-        case Resource::XSLStyleSheet:
-            stats.xslStyleSheets.addResource(resource);
-            break;
-        case Resource::Font:
-            stats.fonts.addResource(resource);
-            break;
-        default:
-            stats.other.addResource(resource);
-            break;
+    for (const auto& resources : m_resources) {

[Mike West, 2014/10/24 12:42:59]

"ResourceMap* resourceMap" seems clearer.

[nhiroki, 2014/10/27 14:30:13]

Ditto ("const OwnPtr<ResourceMap>& resources"?).

+        for (const auto& resourceIter : *resources.value) {

[Mike West, 2014/10/24 12:42:59]

Resource rather than auto.

[nhiroki, 2014/10/27 14:30:12]

Ditto.

+            Resource* resource = resourceIter.value->m_resource.get();
+            switch (resource->type()) {
+            case Resource::Image:
+                stats.images.addResource(resource);
+                break;
+            case Resource::CSSStyleSheet:
+                stats.cssStyleSheets.addResource(resource);
+                break;
+            case Resource::Script:
+                stats.scripts.addResource(resource);
+                break;
+            case Resource::XSLStyleSheet:
+                stats.xslStyleSheets.addResource(resource);
+                break;
+            case Resource::Font:
+                stats.fonts.addResource(resource);
+                break;
+            default:
+                stats.other.addResource(resource);
+                break;
+            }
         }
     }
     return stats;
 }
 
 void MemoryCache::evictResources()
 {
     for (;;) {
-        ResourceMap::iterator i = m_resources.begin();
-        if (i == m_resources.end())
+        ResourceMapIndex::iterator indexIter = m_resources.begin();
+        if (indexIter == m_resources.end())
             break;
-        evict(i->value.get());
+        ResourceMap* resources = indexIter->value.get();
+        for (;;) {

[Mike West, 2014/10/24 12:42:59]

Would you mind changing this, and line 693, to `while (true)`? The latter is
much more common in Blink code.

[nhiroki, 2014/10/27 14:30:13]

Done.

+            ResourceMap::iterator resourceIter = resources->begin();
+            if (resourceIter == resources->end())
+                break;
+            evict(resourceIter->value.get());
+        }
+        m_resources.remove(indexIter);
     }
 }
 
 void MemoryCache::prune(Resource* justReleasedResource)
 {
     TRACE_EVENT0("renderer", "MemoryCache::prune()");
 
     if (m_inPruneResources)
         return;
     if (m_liveSize + m_deadSize <= m_capacity && m_maxDeadCapacity && m_deadSize <= m_maxDeadCapacity) // Fast path.
@@ skipping 24 matching lines @@
     if (m_prunePending && m_deadSize > m_maxDeferredPruneDeadCapacity && justReleasedResource) {
         // The following eviction does not respect LRU order, but it can be done
         // immediately in constant time, as opposed to pruneDeadResources, which
         // we would rather defer because it is O(N), which would make tear-down of N
         // objects O(N^2) if we pruned immediately. This immediate eviction is a
         // safeguard against runaway memory consumption by dead resources
         // while a prune is pending.
         // Main Resources in the cache are only substitue data that was
         // precached and should not be evicted.
         if (contains(justReleasedResource) && justReleasedResource->type() != Resource::MainResource)
-            evict(m_resources.get(justReleasedResource->url()));
+            evict(getEntryForResource(justReleasedResource));
 
         // As a last resort, prune immediately
         if (m_deadSize > m_maxDeferredPruneDeadCapacity)
             pruneNow(currentTime);
     }
 }
 
 void MemoryCache::willProcessTask()
 {
 }
@@ skipping 81 matching lines @@
                 printf("(%.1fK, %.1fK, %uA, %dR, %d, %d); ", current->decodedSize() / 1024.0f, (current->encodedSize() + current->overheadSize()) / 1024.0f, current->accessCount(), current->hasClients(), current->isPurgeable(), current->wasPurged());
 
             current = prev;
         }
     }
 }
 
 #endif // MEMORY_CACHE_STATS
 
 } // namespace blink