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 745 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() && frame()->loader().client()->isControlledByServiceWorker()) {

[michaeln, 2014/10/09 00:35:26]

ouch, this is gonna hurt performance 

[nhiroki, 2014/10/09 02:34:15]

Yes, that's a sore point of this change. I'm going to make sure the performance
impact.

[dominicc (has gone to gerrit), 2014/10/09 02:45:24]

Actually... why don't we land this as-is and concurrently investigate
performance and alternatives? It represents forward progress. The change is
precise. Changing it or backing it out later won't be hard. This is less subtle
than what AppCache does.

All we need is a good regression test which will support future efforts.

+            ASSERT(policy == Load || policy == Reload);
+            memoryCache()->remove(resource.get());
+        }
     }
 
     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() && frame()->loader().client()->isControlledByServiceWorker())
+        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);
 }
 
 }