Fetcher: Disable memory caching when Service Worker handles a request

Source/core/fetch/ResourceFetcher.cpp

Index: Source/core/fetch/ResourceFetcher.cpp
diff --git a/Source/core/fetch/ResourceFetcher.cpp b/Source/core/fetch/ResourceFetcher.cpp
index d2761cd2cddc17b34eb0117d803247edc33b0d42..5cfeb3029c5228aa57a0bdb5c170659d4f6afe95 100644
--- a/Source/core/fetch/ResourceFetcher.cpp
+++ b/Source/core/fetch/ResourceFetcher.cpp
@@ -770,6 +770,17 @@ ResourcePtr<Resource> ResourceFetcher::requestResource(Resource::Type type, Fetc
         ASSERT(policy != Use || m_documentLoader->substituteData().isValid());
         ASSERT(policy != Revalidate);
         memoryCache()->remove(resource.get());
+    } else {
+        // Remove a resource to be handled by Service Worker from the cache to
+        // prevent reuse because Service Worker can serve an arbitrary resource
+        // for an URL and pollute a memory cache entry.
+        // FIXME: isControlledByServiceWorker() always returns false on main
+        // resource request, but main resource is always removed from the cache
+        // as the above comment (http://crbug.com/388375).
+        if (frame() && m_documentLoader && frame()->loader().client()->isControlledByServiceWorker(*m_documentLoader)) {
+            ASSERT(policy == Load || policy == Reload);
+            memoryCache()->remove(resource.get());

[Nate Chapin, 2014/10/09 17:56:22]

Re: "(*) I'm not sure we can fetch a resource to be controlled by SW with
Load/Reload
policy without deleting an existing cache. If possible, we could avoid
performance impact on non-sw-controlled page."

Load means there is nothing to replace, so there's no harmful eviction happening
to begin with.

For Reload, you *might* be able to get the behavior you want by keeping a
ResourcePtr to the old resource, and re-add()ing it here. I haven't tested it,
though, so it's quite possible that will have undesirable side effects.

[nhiroki, 2014/10/10 06:20:37]

Thank you for your detailed answer! I'll take a look in a separate CL.

+        }
     }
 
     requestLoadStarted(resource.get(), request, policy == Use ? ResourceLoadingFromCache : ResourceLoadingFromNetwork);
@@ -928,6 +939,11 @@ ResourceFetcher::RevalidationPolicy ResourceFetcher::determineRevalidationPolicy
     if (!existingResource)
         return Load;
 
+    // FIXME: Currently caching for a resource to be handled by Service Worker
+    // is disabled (http://crbug.com/388375).
+    if (frame() && m_documentLoader && frame()->loader().client()->isControlledByServiceWorker(*m_documentLoader))
+        return Reload;
+
     // We already have a preload going for this URL.
     if (fetchRequest.forPreload() && existingResource->isPreloaded())
         return Use;