service worker: Clarify comments about factory bundle.

Bug: 715640
Change-Id: I130337f41b02b9402e4d27dd6e909212768989bf
Reviewed-on: https://chromium-review.googlesource.com/c/1260745Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596538}

content/browser/service_worker/embedded_worker_instance.cc

@@ -81,6 +81,12 @@ void NotifyWorkerVersionDoomedOnUI(int worker_process_id, int worker_route_id) {
       worker_process_id, 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.
+//
+// The network factory does not support reconnection to the network service.
 std::unique_ptr<URLLoaderFactoryBundleInfo> CreateFactoryBundle(
     RenderProcessHost* rph,
     const url::Origin& origin,
@@ -103,7 +109,8 @@ std::unique_ptr<URLLoaderFactoryBundleInfo> CreateFactoryBundle(
           std::move(pair.second);
 
       // To be safe, ignore schemes that aren't allowed to register service
-      // workers. We assume that importScripts should fail on such schemes.
+      // workers. We assume that importScripts and fetch() should fail on such
+      // schemes.
       if (!base::ContainsValue(GetServiceWorkerSchemes(), scheme))
         continue;
       network::mojom::URLLoaderFactoryPtr factory_ptr;
@@ -205,22 +212,32 @@ void SetupOnUIThread(base::WeakPtr<ServiceWorkerProcessManager> process_manager,
         std::move(request));
   }
 
-  // S13nServiceWorker:
-  // Create a default loader for network fallback.
-  // The factory from RPH::CreateURLLoaderFactory() doesn't support
-  // reconnection to the network service after a crash, but it's probably OK
-  // since it's used for a single service worker startup until installation
-  // finishes.
+  // S13nServiceWorker: Create factory bundles for this worker to do loading.
+  // 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 a non-http(s) URL.
+    // 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);
+
+    // The bundle for the browser is passed to ServiceWorkerScriptLoaderFactory
+    // and used to request non-installed service worker scripts. It's OK to not
+    // support reconnection to then network service because it can only used
+    // 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);
+
+    // 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);
   }