Support IsolatedCodeCache for service worker pass-through responses.

Previously code caching of scripts served from service workers was only
supported if the Response was produced from cache_storage.  If the
service worker implemented a pass-through handler like:

  evt.respondWith(fetch(evt.request))

Then the v8 compiler could not store or load code cache for the script.

With IsolatedCodeCache shipping it becomes much simpler to now
support this feature.  This CL enables support by:

1) Fetching the code cache for service worker controlled scripts.
2) Allowing the code cache to be used for pass-through responses.
3) Storing any code cache produced for pass-through responses.

We still explicitly disable code cache for service worker handlers that
produce either:

a) Synthetic `new Response()` objects.
b) Response objects produced by `fetch()` to a URL different from the
   original request URL.

Bug: 917414
Change-Id: I4efdc852a27069d2937056af0133a986e745b2be
Reviewed-on: https://chromium-review.googlesource.com/c/1394740Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621993}

third_party/blink/renderer/platform/loader/fetch/cached_metadata_handler.cc

@@ -33,6 +33,8 @@ CachedMetadataSenderImpl::CachedMetadataSenderImpl(
       response_time_(response.ResponseTime()),
       code_cache_type_(code_cache_type) {
   DCHECK(response.CacheStorageCacheName().IsNull());
+  DCHECK(!response.WasFetchedViaServiceWorker() ||
+         response.IsServiceWorkerPassThrough());
 }
 
 void CachedMetadataSenderImpl::Send(const uint8_t* data, size_t size) {
@@ -89,14 +91,32 @@ std::unique_ptr<CachedMetadataSender> CachedMetadataSender::Create(
     const ResourceResponse& response,
     blink::mojom::CodeCacheType code_cache_type,
     scoped_refptr<const SecurityOrigin> requestor_origin) {
-  if (response.WasFetchedViaServiceWorker()) {
-    // TODO(leszeks): Check whether it's correct that |requestor_origin| can be
-    // nullptr.
-    if (!requestor_origin || response.CacheStorageCacheName().IsNull())
+  if (!response.WasFetchedViaServiceWorker()) {
+    return std::make_unique<CachedMetadataSenderImpl>(response,
+                                                      code_cache_type);
+  }
+
+  // If the service worker provided a Response produced from cache_storage,
+  // then we need to use a different code cache sender.
+  if (!response.CacheStorageCacheName().IsNull()) {
+    // TODO(leszeks): Check whether it's correct that |origin| can be nullptr.
+    if (!requestor_origin) {
       return std::make_unique<NullCachedMetadataSender>();
+    }
     return std::make_unique<ServiceWorkerCachedMetadataSender>(
         response, std::move(requestor_origin));
   }
+
+  // If the service worker provides a synthetic `new Response()` or a
+  // Response with a different URL then we disable code caching.  In the
+  // synthetic case there is no actual backing storage.  In the case where
+  // the service worker uses a Response with a different URL we don't
+  // currently have a way to read the code cache since the we begin
+  // loading it based on the request URL before the response is available.
+  if (!response.IsServiceWorkerPassThrough()) {
+    return std::make_unique<NullCachedMetadataSender>();
+  }
+
   return std::make_unique<CachedMetadataSenderImpl>(response, code_cache_type);
 }
 

third_party/blink/renderer/platform/loader/fetch/resource_loader.cc

@@ -348,23 +348,21 @@ bool ResourceLoader::ShouldFetchCodeCache() {
   // with the resource from the cache storage.
   if (request.GetRequestContext() == mojom::RequestContextType::SERVICE_WORKER)
     return false;
-  // These requests are serviced by the service worker. It is possible that the
-  // service worker may not service the request in which case it is serviced
-  // by the network. Assuming those fallback cases are not frequent, we don't
-  // fetch from code cache. We may want to have some actual data, to make an
-  // informed decision.
-  // TODO(crbug.com/895850): Get UMA data to see if this check is necessary.
-  if (ResourceLoader::Context().IsControlledByServiceWorker() ==
-      mojom::ControllerServiceWorkerMode::kControlled)
-    return false;
   if (request.DownloadToBlob())
     return false;
   // Javascript resources have type kScript or kMainResource (for inline
-  // scripts). WebAssembly module resources have type kRaw. Note that since we
-  // can't easily distinguish WebAssembly modules from other raw resources, we
-  // perform a code fetch for all raw resources. These fetches should be cheap,
-  // however, requiring one additional IPC and no browser process disk IO since
-  // the cache index is in memory and the resource key should not be present.
+  // scripts). WebAssembly module resources have type kRaw. Note that we
+  // always perform a code fetch for all of these resources because:
+  //
+  // * It is not easy to distinguish WebAssembly modules from other raw
+  //   resources
+  // * The fetch might be handled by Service Workers, but we can't still know
+  //   if the response comes from the CacheStorage (in such cases its own
+  //   code cache will be used) or not.
+  //
+  // These fetches should be cheap, however, requiring one additional IPC and
+  // no browser process disk IO since the cache index is in memory and the
+  // resource key should not be present.
   return resource_->GetType() == ResourceType::kScript ||
          resource_->GetType() == ResourceType::kMainResource ||
          resource_->GetType() == ResourceType::kRaw;
@@ -816,14 +814,20 @@ void ResourceLoader::DidReceiveResponse(
   const ResourceLoaderOptions& options = resource_->Options();
 
   const ResourceResponse& response = web_url_response.ToResourceResponse();
-  // Service worker script has its own code cache. And also, resources which
-  // are served from CacheStorage via service workers have its own code cache.
-  // We should not use cached code from site isolated GeneratedCodeCache in such
-  // cases.
+
   should_use_isolated_code_cache_ =
       RuntimeEnabledFeatures::IsolatedCodeCacheEnabled() &&
-      !(request_context == mojom::RequestContextType::SERVICE_WORKER ||
-        response.WasFetchedViaServiceWorker());
+      // Service worker script has its own code cache.
+      request_context != mojom::RequestContextType::SERVICE_WORKER &&
+      // And also, resources which are served from CacheStorage via service
+      // workers have its own code cache.
+      response.CacheStorageCacheName().IsNull() &&
+      // Also, we only support code cache for other service worker provided
+      // resources when a direct pass-through fetch handler is used.  If the
+      // service worker synthesizes a new Response or provides a Response
+      // fetched from a different URL, then do not use the code cache.
+      (!response.WasFetchedViaServiceWorker() ||
+       response.IsServiceWorkerPassThrough());
 
   // Perform 'nosniff' checks against the original response instead of the 304
   // response for a successful revalidation.

third_party/blink/renderer/platform/loader/fetch/resource_loader.h

@@ -157,6 +157,7 @@ class PLATFORM_EXPORT ResourceLoader final
 
  private:
   friend class SubresourceIntegrityTest;
+  friend class ResourceLoaderIsolatedCodeCacheTest;
   class CodeCacheRequest;
 
   bool ShouldFetchCodeCache();

third_party/blink/renderer/platform/loader/fetch/resource_loader_test.cc

@@ -191,4 +191,71 @@ TEST_F(ResourceLoaderTest, ResponseType) {
   }
 }
 
+class ResourceLoaderIsolatedCodeCacheTest : public ResourceLoaderTest {
+ protected:
+  bool LoadAndCheckIsolatedCodeCache(ResourceResponse response) {
+    const scoped_refptr<const SecurityOrigin> origin =
+        SecurityOrigin::Create(foo_url_);
+
+    FetchContext* context = MakeGarbageCollected<MockFetchContext>(
+        MockFetchContext::kShouldLoadNewResource, nullptr, origin,
+        std::make_unique<TestWebURLLoaderFactory>());
+    ResourceFetcher* fetcher = MakeGarbageCollected<ResourceFetcher>(context);
+
+    ResourceRequest request;
+    request.SetURL(foo_url_);
+    request.SetRequestContext(mojom::RequestContextType::FETCH);
+
+    FetchParameters fetch_parameters(request);
+    Resource* resource = RawResource::Fetch(fetch_parameters, fetcher, nullptr);
+    ResourceLoader* loader = resource->Loader();
+
+    loader->DidReceiveResponse(WrappedResourceResponse(response));
+    return loader->should_use_isolated_code_cache_;
+  }
+};
+
+TEST_F(ResourceLoaderIsolatedCodeCacheTest, ResponseFromNetwork) {
+  ResourceResponse response(foo_url_);
+  response.SetHTTPStatusCode(200);
+  EXPECT_EQ(true, LoadAndCheckIsolatedCodeCache(response));
+}
+
+TEST_F(ResourceLoaderIsolatedCodeCacheTest,
+       SyntheticResponseFromServiceWorker) {
+  ResourceResponse response(foo_url_);
+  response.SetHTTPStatusCode(200);
+  response.SetWasFetchedViaServiceWorker(true);
+  EXPECT_EQ(false, LoadAndCheckIsolatedCodeCache(response));
+}
+
+TEST_F(ResourceLoaderIsolatedCodeCacheTest,
+       PassThroughResponseFromServiceWorker) {
+  ResourceResponse response(foo_url_);
+  response.SetHTTPStatusCode(200);
+  response.SetWasFetchedViaServiceWorker(true);
+  response.SetURLListViaServiceWorker(Vector<KURL>(1, foo_url_));
+  EXPECT_EQ(true, LoadAndCheckIsolatedCodeCache(response));
+}
+
+TEST_F(ResourceLoaderIsolatedCodeCacheTest,
+       DifferentUrlResponseFromServiceWorker) {
+  ResourceResponse response(foo_url_);
+  response.SetHTTPStatusCode(200);
+  response.SetWasFetchedViaServiceWorker(true);
+  response.SetURLListViaServiceWorker(Vector<KURL>(1, bar_url_));
+  EXPECT_EQ(false, LoadAndCheckIsolatedCodeCache(response));
+}
+
+TEST_F(ResourceLoaderIsolatedCodeCacheTest, CacheResponseFromServiceWorker) {
+  ResourceResponse response(foo_url_);
+  response.SetHTTPStatusCode(200);
+  response.SetWasFetchedViaServiceWorker(true);
+  response.SetCacheStorageCacheName("dummy");
+  // The browser does support code cache for cache_storage Responses, but they
+  // are loaded via a different mechanism.  So the ResourceLoader code caching
+  // value should be false here.
+  EXPECT_EQ(false, LoadAndCheckIsolatedCodeCache(response));
+}
+
 }  // namespace blink

third_party/blink/renderer/platform/loader/fetch/resource_response.cc

@@ -126,6 +126,12 @@ KURL ResourceResponse::ResponseUrl() const {
   return CurrentRequestUrl();
 }
 
+bool ResourceResponse::IsServiceWorkerPassThrough() const {
+  return cache_storage_cache_name_.IsEmpty() &&
+         !url_list_via_service_worker_.IsEmpty() &&
+         ResponseUrl() == CurrentRequestUrl();
+}
+
 const AtomicString& ResourceResponse::MimeType() const {
   return mime_type_;
 }

third_party/blink/renderer/platform/loader/fetch/resource_response.h

@@ -179,6 +179,12 @@ class PLATFORM_EXPORT ResourceResponse final {
   // responded to the request. See the comments for that function.
   KURL ResponseUrl() const;
 
+  // Returns true if this response is the result of a service worker
+  // effectively calling `evt.respondWith(fetch(evt.request))`.  Specifically,
+  // it returns false for synthetic constructed responses, responses fetched
+  // from different URLs, and responses produced by cache_storage.
+  bool IsServiceWorkerPassThrough() const;
+
   const AtomicString& MimeType() const;
   void SetMimeType(const AtomicString&);
 

third_party/blink/renderer/platform/loader/fetch/resource_test.cc

@@ -35,10 +35,23 @@ class MockPlatform final : public TestingPlatformSupportWithMockScheduler {
     cached_urls_.push_back(url);
   }
 
+  void CacheMetadataInCacheStorage(const blink::WebURL& url,
+                                   base::Time,
+                                   const uint8_t*,
+                                   size_t,
+                                   const blink::WebSecurityOrigin&,
+                                   const blink::WebString&) override {
+    cache_storage_cached_urls_.push_back(url);
+  }
+
   const Vector<WebURL>& CachedURLs() const { return cached_urls_; }
+  const Vector<WebURL>& CacheStorageCachedURLs() const {
+    return cache_storage_cached_urls_;
+  }
 
  private:
   Vector<WebURL> cached_urls_;
+  Vector<WebURL> cache_storage_cached_urls_;
 };
 
 ResourceResponse CreateTestResourceResponse() {
@@ -49,7 +62,10 @@ ResourceResponse CreateTestResourceResponse() {
 
 void CreateTestResourceAndSetCachedMetadata(const ResourceResponse& response) {
   const uint8_t kTestData[] = {1, 2, 3, 4, 5};
-  MockResource* resource = MockResource::Create(response.CurrentRequestUrl());
+  ResourceRequest request(response.CurrentRequestUrl());
+  request.SetRequestorOrigin(
+      SecurityOrigin::Create(response.CurrentRequestUrl()));
+  MockResource* resource = MockResource::Create(request);
   resource->SetResponse(response);
   resource->SendCachedMetadata(kTestData, sizeof(kTestData));
   return;
@@ -62,16 +78,68 @@ TEST(ResourceTest, SetCachedMetadata_SendsMetadataToPlatform) {
   ResourceResponse response(CreateTestResourceResponse());
   CreateTestResourceAndSetCachedMetadata(response);
   EXPECT_EQ(1u, mock->CachedURLs().size());
+  EXPECT_EQ(0u, mock->CacheStorageCachedURLs().size());
+}
+
+TEST(
+    ResourceTest,
+    SetCachedMetadata_DoesNotSendMetadataToPlatformWhenFetchedViaServiceWorkerWithSyntheticResponse) {
+  ScopedTestingPlatformSupport<MockPlatform> mock;
+
+  // Equivalent to service worker calling respondWith(new Response(...))
+  ResourceResponse response(CreateTestResourceResponse());
+  response.SetWasFetchedViaServiceWorker(true);
+
+  CreateTestResourceAndSetCachedMetadata(response);
+  EXPECT_EQ(0u, mock->CachedURLs().size());
+  EXPECT_EQ(0u, mock->CacheStorageCachedURLs().size());
+}
+
+TEST(
+    ResourceTest,
+    SetCachedMetadata_SendsMetadataToPlatformWhenFetchedViaServiceWorkerWithPassThroughResponse) {
+  ScopedTestingPlatformSupport<MockPlatform> mock;
+
+  // Equivalent to service worker calling respondWith(fetch(evt.request.url));
+  ResourceResponse response(CreateTestResourceResponse());
+  response.SetWasFetchedViaServiceWorker(true);
+  response.SetURLListViaServiceWorker(
+      Vector<KURL>(1, response.CurrentRequestUrl()));
+
+  CreateTestResourceAndSetCachedMetadata(response);
+  EXPECT_EQ(1u, mock->CachedURLs().size());
+  EXPECT_EQ(0u, mock->CacheStorageCachedURLs().size());
 }
 
 TEST(
     ResourceTest,
-    SetCachedMetadata_DoesNotSendMetadataToPlatformWhenFetchedViaServiceWorker) {
+    SetCachedMetadata_DoesNotSendMetadataToPlatformWhenFetchedViaServiceWorkerWithDifferentURLResponse) {
   ScopedTestingPlatformSupport<MockPlatform> mock;
+
+  // Equivalent to service worker calling respondWith(fetch(some_different_url))
   ResourceResponse response(CreateTestResourceResponse());
   response.SetWasFetchedViaServiceWorker(true);
+  response.SetURLListViaServiceWorker(Vector<KURL>(
+      1, url_test_helpers::ToKURL("https://example.com/different/url")));
+
+  CreateTestResourceAndSetCachedMetadata(response);
+  EXPECT_EQ(0u, mock->CachedURLs().size());
+  EXPECT_EQ(0u, mock->CacheStorageCachedURLs().size());
+}
+
+TEST(
+    ResourceTest,
+    SetCachedMetadata_SendsMetadataToPlatformWhenFetchedViaServiceWorkerWithCacheResponse) {
+  ScopedTestingPlatformSupport<MockPlatform> mock;
+
+  // Equivalent to service worker calling respondWith(cache.match(some_url));
+  ResourceResponse response(CreateTestResourceResponse());
+  response.SetWasFetchedViaServiceWorker(true);
+  response.SetCacheStorageCacheName("dummy");
+
   CreateTestResourceAndSetCachedMetadata(response);
   EXPECT_EQ(0u, mock->CachedURLs().size());
+  EXPECT_EQ(1u, mock->CacheStorageCachedURLs().size());
 }
 
 TEST(ResourceTest, RevalidateWithFragment) {