Fetcher: Disable memory caching when Service Worker handles a request

Source/core/fetch/ResourceFetcher.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, 2009, 2010, 2011 Apple Inc. All rights reserved.
     Copyright (C) 2009 Torch Mobile Inc. http://www.torchmobile.com/
 
     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
@@ skipping 752 matching lines @@
 
     // FIXME: Temporarily leave main resource caching disabled for chromium,
     // see https://bugs.webkit.org/show_bug.cgi?id=107962. Before caching main
     // resources, we should be sure to understand the implications for memory
     // use.
     // Remove main resource from cache to prevent reuse.
     if (type == Resource::MainResource) {
         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);
 
     ASSERT(resource->url() == url.string());
     m_documentResources.set(resource->url(), resource);
     return resource;
 }
 
 void ResourceFetcher::resourceTimingReportTimerFired(Timer<ResourceFetcher>* timer)
@@ skipping 138 matching lines @@
     }
 }
 
 ResourceFetcher::RevalidationPolicy ResourceFetcher::determineRevalidationPolicy(Resource::Type type, const FetchRequest& fetchRequest, Resource* existingResource) const
 {
     const ResourceRequest& request = fetchRequest.resourceRequest();
 
     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;
 
     // If the same URL has been loaded as a different type, we need to reload.
     if (existingResource->type() != type) {
         // FIXME: If existingResource is a Preload and the new type is LinkPrefetch
         // We really should discard the new prefetch since the preload has more
         // specific type information! crbug.com/379893
         // fast/dom/HTMLLinkElement/link-and-subresource-test hits this case.
@@ skipping 582 matching lines @@
 
 void ResourceFetcher::trace(Visitor* visitor)
 {
     visitor->trace(m_document);
     visitor->trace(m_loaders);
     visitor->trace(m_multipartLoaders);
     ResourceLoaderHost::trace(visitor);
 }
 
 }