Create non-network factories for subresource loads from a service worker

Previously we skipped to create non-network factories for subresource loading
from a service worker as a performance optimization. It works on
S13nServiceWorker because extensions are handled by a resource dispatcher host.
However, NetworkService doesn't work well because those requests need to be
routed to extensions before going to the NetworkService. This CL fixes by always
creating non-network factories for subresource loads.

Bug: 901443
Change-Id: Ia932dc11352c14be9f33c9e3448891cf66d699e0
Reviewed-on: https://chromium-review.googlesource.com/c/1322258Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606099}

chrome/browser/chrome_do_not_track_browsertest.cc

@@ -170,32 +170,15 @@ IN_PROC_BROWSER_TEST_F(ChromeDoNotTrackTest, FetchFromServiceWorker) {
   const GURL url = embedded_test_server()->GetURL(
       std::string("/workers/fetch_from_service_worker.html"));
   ui_test_utils::NavigateToURL(browser(), url);
-
-  // Wait until service worker become ready.
-  const base::string16 title = base::ASCIIToUTF16("DONE");
-  {
-    content::TitleWatcher watcher(GetWebContents(), title);
-    EXPECT_EQ(title, watcher.WaitAndGetTitle());
-  }
-  ExpectPageTextEq("ready");
+  EXPECT_EQ("ready", EvalJs(GetWebContents(), "setup();"));
 
   const std::string script =
       "fetch_from_service_worker('" + fetch_url.spec() + "');";
-  ASSERT_TRUE(ExecJs(GetWebContents(), script));
-  {
-    content::TitleWatcher watcher(GetWebContents(), title);
-    EXPECT_EQ(title, watcher.WaitAndGetTitle());
-  }
-  ExpectPageTextEq("1");
+  EXPECT_EQ("1", EvalJs(GetWebContents(), script));
 
   // Updating settings should be reflected immediately.
   SetEnableDoNotTrack(false /* enabled */);
-  ASSERT_TRUE(ExecJs(GetWebContents(), script));
-  {
-    content::TitleWatcher watcher(GetWebContents(), title);
-    EXPECT_EQ(title, watcher.WaitAndGetTitle());
-  }
-  ExpectPageTextEq("None");
+  EXPECT_EQ("None", EvalJs(GetWebContents(), script));
 }
 
 }  // namespace

chrome/browser/extensions/fetch_apitest.cc

@@ -164,6 +164,37 @@ IN_PROC_BROWSER_TEST_F(ExtensionFetchTest,
   EXPECT_EQ("text content", fetch_result);
 }
 
+// Calling fetch() from a http(s) service worker context to a
+// chrome-extensions:// URL since the loading path in a service worker is
+// different from pages.
+// This is a regression test for https://crbug.com/901443.
+IN_PROC_BROWSER_TEST_F(
+    ExtensionFetchTest,
+    HostCanFetchWebAccessibleExtensionResource_FetchFromServiceWorker) {
+  TestExtensionDir dir;
+  dir.WriteManifestWithSingleQuotes(
+      "{"
+      "'background': {'scripts': ['bg.js']},"
+      "'manifest_version': 2,"
+      "'name': 'HostCanFetchWebAccessibleExtensionResource_"
+      "FetchFromServiceWorker',"
+      "'version': '1',"
+      "'web_accessible_resources': ['text']"
+      "}");
+  const Extension* extension = WriteFilesAndLoadTestExtension(&dir);
+  ASSERT_TRUE(extension);
+
+  content::WebContents* tab =
+      CreateAndNavigateTab(embedded_test_server()->GetURL(
+          "/workers/fetch_from_service_worker.html"));
+  EXPECT_EQ("ready", content::EvalJs(tab, "setup();"));
+  EXPECT_EQ("text content",
+            content::EvalJs(
+                tab, base::StringPrintf(
+                         "fetch_from_service_worker('%s');",
+                         extension->GetResourceURL("text").spec().c_str())));
+}
+
 IN_PROC_BROWSER_TEST_F(ExtensionFetchTest,
                        HostCannotFetchNonWebAccessibleExtensionResource) {
   TestExtensionDir dir;

chrome/test/data/workers/fetch_from_service_worker.html

 <script>
-let registration = null;
-
-self.onload = async () => {
-  registration = await navigator.serviceWorker.register(
-      'fetch_from_service_worker.js');
-  if (!registration) {
-    document.body.innerText = 'Registration failed';
-    document.title = 'DONE';
-    return;
-  }
-  registration = await navigator.serviceWorker.ready;
-  document.body.innerText = 'ready';
-  document.title = 'DONE';
+async function setup() {
+  await navigator.serviceWorker.register('fetch_from_service_worker.js');
+  await navigator.serviceWorker.ready;
+  return 'ready';
 }
 
 function fetch_from_service_worker(url) {
-  document.title = 'Fetching';
-  document.innerText = '';
-  if (!registration) {
-    document.innerText = 'Registration failed';
-    document.title = 'DONE';
-    return;
-  }
-  const channel = new MessageChannel();
-  channel.port1.onmessage = e => {
-    document.body.innerText = e.data;
-    document.title = 'DONE';
-  };
-  registration.active.postMessage({url: url}, [channel.port2]);
+  return new Promise(async resolve => {
+    const registration = await navigator.serviceWorker.ready;
+    const channel = new MessageChannel();
+    channel.port1.onmessage = e => { resolve(e.data); };
+    registration.active.postMessage({url}, [channel.port2]);
+  });
 }
 </script>

chrome/test/data/workers/fetch_from_service_worker.js

@@ -2,17 +2,20 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-async function MessageHandler(e) {
-  const port = e.ports[0];
-  const response = await fetch(e.data.url);
-  if (!response.ok) {
-    port.postMessage('bad response');
-    return;
+async function handleMessage(e) {
+  try {
+    const response = await fetch(e.data.url);
+    if (!response.ok) {
+      e.ports[0].postMessage('bad response');
+      return;
+    }
+    const text = await response.text();
+    e.ports[0].postMessage(text);
+  } catch (error) {
+    e.ports[0].postMessage(`${error}`);
   }
-  const text = await response.text();
-  port.postMessage(text);
 }
 
 self.addEventListener('message', e => {
-  e.waitUntil(MessageHandler(e));
+  e.waitUntil(handleMessage(e));
 });

content/browser/service_worker/embedded_worker_instance.cc

@@ -96,14 +96,12 @@ void NotifyWorkerVersionDoomedOnUI(int worker_process_id, int worker_route_id) {
 
 // Returns a factory bundle for doing loads on behalf of the specified |rph| and
 // |origin|. The returned bundle has a default factory that goes to network and
-// if |use_non_network_factories| is true it may also include scheme-specific
-// factories that don't go to network.
+// it may also include scheme-specific factories that don't go to network.
 //
 // The network factory does not support reconnection to the network service.
 std::unique_ptr<URLLoaderFactoryBundleInfo> CreateFactoryBundle(
     RenderProcessHost* rph,
-    const url::Origin& origin,
-    bool use_non_network_factories) {
+    const url::Origin& origin) {
   auto factory_bundle = std::make_unique<URLLoaderFactoryBundleInfo>();
   network::mojom::URLLoaderFactoryPtrInfo default_factory_info;
   if (!GetNetworkFactoryCallbackForTest()) {
@@ -118,29 +116,27 @@ std::unique_ptr<URLLoaderFactoryBundleInfo> CreateFactoryBundle(
   }
   factory_bundle->default_factory_info() = std::move(default_factory_info);
 
-  if (use_non_network_factories) {
-    ContentBrowserClient::NonNetworkURLLoaderFactoryMap non_network_factories;
-    GetContentClient()
-        ->browser()
-        ->RegisterNonNetworkSubresourceURLLoaderFactories(
-            rph->GetID(), MSG_ROUTING_NONE, &non_network_factories);
-
-    for (auto& pair : non_network_factories) {
-      const std::string& scheme = pair.first;
-      std::unique_ptr<network::mojom::URLLoaderFactory> factory =
-          std::move(pair.second);
-
-      // To be safe, ignore schemes that aren't allowed to register service
-      // workers. We assume that importScripts and fetch() should fail on such
-      // schemes.
-      if (!base::ContainsValue(GetServiceWorkerSchemes(), scheme))
-        continue;
-      network::mojom::URLLoaderFactoryPtr factory_ptr;
-      mojo::MakeStrongBinding(std::move(factory),
-                              mojo::MakeRequest(&factory_ptr));
-      factory_bundle->scheme_specific_factory_infos().emplace(
-          scheme, factory_ptr.PassInterface());
-    }
+  ContentBrowserClient::NonNetworkURLLoaderFactoryMap non_network_factories;
+  GetContentClient()
+      ->browser()
+      ->RegisterNonNetworkSubresourceURLLoaderFactories(
+          rph->GetID(), MSG_ROUTING_NONE, &non_network_factories);
+
+  for (auto& pair : non_network_factories) {
+    const std::string& scheme = pair.first;
+    std::unique_ptr<network::mojom::URLLoaderFactory> factory =
+        std::move(pair.second);
+
+    // To be safe, ignore schemes that aren't allowed to register service
+    // workers. We assume that importScripts and fetch() should fail on such
+    // schemes.
+    if (!base::ContainsValue(GetServiceWorkerSchemes(), scheme))
+      continue;
+    network::mojom::URLLoaderFactoryPtr factory_ptr;
+    mojo::MakeStrongBinding(std::move(factory),
+                            mojo::MakeRequest(&factory_ptr));
+    factory_bundle->scheme_specific_factory_infos().emplace(
+        scheme, factory_ptr.PassInterface());
   }
   return factory_bundle;
 }
@@ -238,13 +234,7 @@ void SetupOnUIThread(base::WeakPtr<ServiceWorkerProcessManager> process_manager,
   // These bundles don't support reconnection to the network service, see
   // below comments.
   if (blink::ServiceWorkerUtils::IsServicificationEnabled()) {
-    // For performance, we only create the loader factories for non-http(s)
-    // URLs (e.g. chrome-extension://) when the main script URL is
-    // non-http(s). We assume an http(s) service worker cannot
-    // importScripts or fetch() a non-http(s) URL.
-    bool use_non_network_factories = !params->script_url.SchemeIsHTTPOrHTTPS();
-
-    url::Origin origin = url::Origin::Create(params->script_url);
+    const url::Origin origin = url::Origin::Create(params->script_url);
 
     // The bundle for the browser is passed to ServiceWorkerScriptLoaderFactory
     // and used to request non-installed service worker scripts. It's OK to not
@@ -252,16 +242,14 @@ void SetupOnUIThread(base::WeakPtr<ServiceWorkerProcessManager> process_manager,
     // until the service worker reaches the 'installed' state.
     //
     // TODO(falken): Only make this bundle for non-installed service workers.
-    factory_bundle_for_browser =
-        CreateFactoryBundle(rph, origin, use_non_network_factories);
+    factory_bundle_for_browser = CreateFactoryBundle(rph, origin);
 
     // The bundle for the renderer is passed to the service worker, and
     // used for subresource loading from the service worker (i.e., fetch()).
     // It's OK to not support reconnection to the network service because the
     // service worker terminates itself when the connection breaks, so a new
     // instance can be started.
-    factory_bundle_for_renderer =
-        CreateFactoryBundle(rph, origin, use_non_network_factories);
+    factory_bundle_for_renderer = CreateFactoryBundle(rph, origin);
   }
 
   // Register to DevTools and update params accordingly.