shared worker: Give the renderer a factory bundle when NetworkService…

… is enabled.

Similar to frames, the shared worker needs a factory bundle in order to
load non-NetworkService URLs like chrome-extension://.

This fixes the remaining test failure but we're still missing test
coverage. I think the factory bundle needs to be propagated to
ServiceWorkerSubresourceLoaderFactory for use with network fallback.

Bug: 839982
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I2c489a17fb30364e477d691bc346f7656b07906d
Reviewed-on: https://chromium-review.googlesource.com/1069956
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561423}

content/browser/shared_worker/mock_shared_worker.cc

@@ -4,6 +4,7 @@
 
 #include "content/browser/shared_worker/mock_shared_worker.h"
 
+#include "content/common/url_loader_factory_bundle.h"
 #include "mojo/public/cpp/test_support/test_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
@@ -102,6 +103,7 @@ void MockSharedWorkerFactory::CreateSharedWorker(
         service_worker_provider_info,
     network::mojom::URLLoaderFactoryAssociatedPtrInfo
         script_loader_factory_ptr_info,
+    std::unique_ptr<URLLoaderFactoryBundleInfo> subresource_loaders,
     mojom::SharedWorkerHostPtr host,
     mojom::SharedWorkerRequest request,
     service_manager::mojom::InterfaceProviderPtr interface_provider) {

content/browser/shared_worker/mock_shared_worker.h

@@ -22,6 +22,7 @@
 class GURL;
 
 namespace content {
+class URLLoaderFactoryBundleInfo;
 
 class MockSharedWorker : public mojom::SharedWorker {
  public:
@@ -71,6 +72,7 @@ class MockSharedWorkerFactory : public mojom::SharedWorkerFactory {
           service_worker_provider_info,
       network::mojom::URLLoaderFactoryAssociatedPtrInfo
           script_loader_factory_ptr_info,
+      std::unique_ptr<URLLoaderFactoryBundleInfo> subresource_loaders,
       mojom::SharedWorkerHostPtr host,
       mojom::SharedWorkerRequest request,
       service_manager::mojom::InterfaceProviderPtr interface_provider) override;

content/browser/shared_worker/shared_worker_host.cc

@@ -14,6 +14,8 @@
 #include "content/browser/shared_worker/shared_worker_content_settings_proxy_impl.h"
 #include "content/browser/shared_worker/shared_worker_instance.h"
 #include "content/browser/shared_worker/shared_worker_service_impl.h"
+#include "content/browser/storage_partition_impl.h"
+#include "content/common/url_loader_factory_bundle.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
@@ -121,7 +123,8 @@ void SharedWorkerHost::Start(
     mojom::SharedWorkerFactoryPtr factory,
     mojom::ServiceWorkerProviderInfoForSharedWorkerPtr
         service_worker_provider_info,
-    network::mojom::URLLoaderFactoryAssociatedPtrInfo script_loader_factory) {
+    network::mojom::URLLoaderFactoryAssociatedPtrInfo script_loader_factory,
+    std::unique_ptr<URLLoaderFactoryBundleInfo> factory_bundle) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   AdvanceTo(Phase::kStarted);
 
@@ -151,19 +154,48 @@ void SharedWorkerHost::Start(
       mojom::kNavigation_SharedWorkerSpec, process_id_,
       mojo::MakeRequest(&interface_provider)));
 
+  // NetworkService: Add the network factory to the bundle to pass to the
+  // renderer. The bundle is only provided (along with |script_loader_factory|)
+  // if NetworkService/S13nSW is enabled.
+  DCHECK(!script_loader_factory || factory_bundle);
+  if (factory_bundle) {
+    network::mojom::URLLoaderFactoryPtrInfo network_factory_info;
+    CreateNetworkFactory(mojo::MakeRequest(&network_factory_info));
+    DCHECK(!factory_bundle->default_factory_info());
+    factory_bundle->default_factory_info() = std::move(network_factory_info);
+  }
+
   // Send the CreateSharedWorker message.
   factory_ = std::move(factory);
   factory_->CreateSharedWorker(
       std::move(info), pause_on_start, devtools_worker_token,
       std::move(content_settings), std::move(service_worker_provider_info),
-      std::move(script_loader_factory), std::move(host),
-      std::move(worker_request_), std::move(interface_provider));
+      std::move(script_loader_factory), std::move(factory_bundle),
+      std::move(host), std::move(worker_request_),
+      std::move(interface_provider));
 
   // Monitor the lifetime of the worker.
   worker_.set_connection_error_handler(base::BindOnce(
       &SharedWorkerHost::OnWorkerConnectionLost, weak_factory_.GetWeakPtr()));
 }
 
+//  This is similar to
+//  RenderFrameHostImpl::CreateNetworkServiceDefaultFactoryAndObserve.
+void SharedWorkerHost::CreateNetworkFactory(
+    network::mojom::URLLoaderFactoryRequest request) {
+  network::mojom::URLLoaderFactoryParamsPtr params =
+      network::mojom::URLLoaderFactoryParams::New();
+  params->process_id = process_id_;
+  // TODO(lukasza): https://crbug.com/792546: Start using CORB.
+  params->is_corb_enabled = false;
+
+  service_->storage_partition()->GetNetworkContext()->CreateURLLoaderFactory(
+      std::move(request), std::move(params));
+
+  // TODO(falken): Detect connection error and send a IPC with a new network
+  // factory like UpdateSubresourceLoaderFactories does for frames.
+}
+
 void SharedWorkerHost::AllowFileSystem(
     const GURL& url,
     base::OnceCallback<void(bool)> callback) {

content/browser/shared_worker/shared_worker_host.h

@@ -36,6 +36,7 @@ namespace content {
 class SharedWorkerContentSettingsProxyImpl;
 class SharedWorkerInstance;
 class SharedWorkerServiceImpl;
+class URLLoaderFactoryBundleInfo;
 
 // The SharedWorkerHost is the interface that represents the browser side of
 // the browser <-> worker communication channel. This is owned by
@@ -66,7 +67,8 @@ class CONTENT_EXPORT SharedWorkerHost
       mojom::SharedWorkerFactoryPtr factory,
       mojom::ServiceWorkerProviderInfoForSharedWorkerPtr
           service_worker_provider_info,
-      network::mojom::URLLoaderFactoryAssociatedPtrInfo script_loader_factory);
+      network::mojom::URLLoaderFactoryAssociatedPtrInfo script_loader_factory,
+      std::unique_ptr<URLLoaderFactoryBundleInfo> factory_bundle);
 
   void AllowFileSystem(const GURL& url,
                        base::OnceCallback<void(bool)> callback);
@@ -137,6 +139,8 @@ class CONTENT_EXPORT SharedWorkerHost
   void GetInterface(const std::string& interface_name,
                     mojo::ScopedMessagePipeHandle interface_pipe) override;
 
+  void CreateNetworkFactory(network::mojom::URLLoaderFactoryRequest request);
+
   void AdvanceTo(Phase phase);
 
   mojo::Binding<mojom::SharedWorkerHost> binding_;

content/browser/shared_worker/shared_worker_host_unittest.cc

@@ -15,9 +15,11 @@
 #include "content/browser/shared_worker/shared_worker_instance.h"
 #include "content/browser/shared_worker/shared_worker_service_impl.h"
 #include "content/public/test/test_browser_thread_bundle.h"
+#include "content/public/test/test_storage_partition.h"
 #include "content/public/test/test_utils.h"
 #include "mojo/public/cpp/bindings/binding.h"
 #include "mojo/public/cpp/test_support/test_utils.h"
+#include "services/network/test/test_network_context.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/public/common/message_port/message_port_channel.h"
 #include "url/origin.h"
@@ -28,7 +30,10 @@ namespace content {
 
 class SharedWorkerHostTest : public testing::Test {
  public:
-  SharedWorkerHostTest() : service_(nullptr) {}
+  SharedWorkerHostTest()
+      : service_(&storage_partition_, nullptr /* service_worker_context */) {
+    storage_partition_.set_network_context(&network_context_);
+  }
 
   base::WeakPtr<SharedWorkerHost> CreateHost() {
     GURL url("http://www.example.com/w.js");
@@ -56,7 +61,8 @@ class SharedWorkerHostTest : public testing::Test {
   void StartWorker(SharedWorkerHost* host,
                    mojom::SharedWorkerFactoryPtr factory) {
     host->Start(std::move(factory), nullptr /* service_worker_provider_info */,
-                {} /* script_loader_factory_info */);
+                {} /* script_loader_factory_info */,
+                nullptr /* factory_bundle */);
   }
 
   MessagePortChannel AddClient(SharedWorkerHost* host,
@@ -71,6 +77,9 @@ class SharedWorkerHostTest : public testing::Test {
 
  protected:
   TestBrowserThreadBundle test_browser_thread_bundle_;
+  TestStoragePartition storage_partition_;
+  network::TestNetworkContext network_context_;
+
   SharedWorkerServiceImpl service_;
 
   DISALLOW_COPY_AND_ASSIGN(SharedWorkerHostTest);
@@ -181,7 +190,8 @@ TEST_F(SharedWorkerHostTest, TerminateAfterStarting) {
 
   // Start the worker.
   host->Start(std::move(factory), nullptr /* service_worker_provider_info */,
-              {} /* script_loader_factory_info */);
+              {} /* script_loader_factory_info */,
+              nullptr /* factory_bundle */);
 
   // Add a client.
   MockSharedWorkerClient client;

content/browser/shared_worker/shared_worker_service_impl.cc

@@ -16,6 +16,7 @@
 #include "content/browser/shared_worker/shared_worker_instance.h"
 #include "content/browser/shared_worker/shared_worker_script_loader_factory.h"
 #include "content/browser/storage_partition_impl.h"
+#include "content/browser/url_loader_factory_getter.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/common/service_worker/service_worker_provider.mojom.h"
 #include "content/common/service_worker/service_worker_utils.h"
@@ -39,13 +40,45 @@ bool IsShuttingDown(RenderProcessHost* host) {
          host->IsKeepAliveRefCountDisabled();
 }
 
+std::unique_ptr<URLLoaderFactoryBundleInfo> CreateFactoryBundle(
+    int process_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  ContentBrowserClient::NonNetworkURLLoaderFactoryMap factories;
+  GetContentClient()
+      ->browser()
+      ->RegisterNonNetworkSubresourceURLLoaderFactories(
+          process_id, MSG_ROUTING_NONE, &factories);
+
+  // TODO(falken): Add FileURLLoaderFactory if the requesting frame is a file
+  // resource.
+
+  auto factory_bundle = std::make_unique<URLLoaderFactoryBundleInfo>();
+  for (auto& pair : factories) {
+    const std::string& scheme = pair.first;
+    std::unique_ptr<network::mojom::URLLoaderFactory> factory =
+        std::move(pair.second);
+
+    network::mojom::URLLoaderFactoryPtr factory_ptr;
+    mojo::MakeStrongBinding(std::move(factory),
+                            mojo::MakeRequest(&factory_ptr));
+    factory_bundle->factories_info().emplace(scheme,
+                                             factory_ptr.PassInterface());
+  }
+
+  return factory_bundle;
+}
+
 void CreateScriptLoaderOnIO(
     scoped_refptr<URLLoaderFactoryGetter> loader_factory_getter,
-    std::unique_ptr<URLLoaderFactoryBundleInfo> factory_bundle_info,
+    std::unique_ptr<URLLoaderFactoryBundleInfo> factory_bundle_for_browser_info,
+    std::unique_ptr<URLLoaderFactoryBundleInfo>
+        factory_bundle_for_renderer_info,
     scoped_refptr<ServiceWorkerContextWrapper> context,
     int process_id,
     base::OnceCallback<void(mojom::ServiceWorkerProviderInfoForSharedWorkerPtr,
-                            network::mojom::URLLoaderFactoryAssociatedPtrInfo)>
+                            network::mojom::URLLoaderFactoryAssociatedPtrInfo,
+                            std::unique_ptr<URLLoaderFactoryBundleInfo>)>
         callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
@@ -54,10 +87,11 @@ void CreateScriptLoaderOnIO(
   base::WeakPtr<ServiceWorkerProviderHost> host =
       context->PreCreateHostForSharedWorker(process_id, &provider_info);
 
-  // Create the factory bundle for loading the script.
+  // Create the factory bundle for SharedWorkerScriptLoaderFactory to use to
+  // load the script.
   scoped_refptr<URLLoaderFactoryBundle> factory_bundle =
       base::MakeRefCounted<URLLoaderFactoryBundle>(
-          std::move(factory_bundle_info));
+          std::move(factory_bundle_for_browser_info));
 
   // Add the network factory to the bundle. The factory from
   // CloneNetworkFactory() doesn't support reconnection to the network service
@@ -76,24 +110,21 @@ void CreateScriptLoaderOnIO(
           std::move(factory_bundle)),
       mojo::MakeRequest(&script_loader_factory));
 
-  // TODO(falken): Also send the factory bundle to the renderer like
-  // CommitNavigation does, to be used for subresource requests from the shared
-  // worker (SharedWorkerScriptLoaderFactory is only used for the main resource
-  // request). However, the restartable network factory should be used in this
-  // case.
-
   // We continue in StartWorker.
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::BindOnce(std::move(callback), std::move(provider_info),
-                     std::move(script_loader_factory)));
+                     std::move(script_loader_factory),
+                     std::move(factory_bundle_for_renderer_info)));
 }
 
 }  // namespace
 
 SharedWorkerServiceImpl::SharedWorkerServiceImpl(
+    StoragePartition* storage_partition,
     scoped_refptr<ServiceWorkerContextWrapper> service_worker_context)
-    : service_worker_context_(std::move(service_worker_context)),
+    : storage_partition_(storage_partition),
+      service_worker_context_(std::move(service_worker_context)),
       weak_factory_(this) {}
 
 SharedWorkerServiceImpl::~SharedWorkerServiceImpl() {}
@@ -225,37 +256,28 @@ void SharedWorkerServiceImpl::CreateWorker(
   // Bounce to the IO thread to setup service worker support in case the request
   // for the worker script will need to be intercepted by service workers.
   if (ServiceWorkerUtils::IsServicificationEnabled()) {
-    // Set up the factory bundle for non-NetworkService URLs, e.g.,
-    // chrome-extension:// URLs.
-    ContentBrowserClient::NonNetworkURLLoaderFactoryMap factories;
-    GetContentClient()
-        ->browser()
-        ->RegisterNonNetworkSubresourceURLLoaderFactories(
-            process_id, MSG_ROUTING_NONE, &factories);
-
-    // TODO(falken): Add FileURLLoaderFactory if the requesting frame is a file
-    // resource.
-
-    auto factory_bundle = std::make_unique<URLLoaderFactoryBundleInfo>();
-    for (auto& pair : factories) {
-      const std::string& scheme = pair.first;
-      std::unique_ptr<network::mojom::URLLoaderFactory> factory =
-          std::move(pair.second);
-
-      network::mojom::URLLoaderFactoryPtr factory_ptr;
-      mojo::MakeStrongBinding(std::move(factory),
-                              mojo::MakeRequest(&factory_ptr));
-      factory_bundle->factories_info().emplace(scheme,
-                                               factory_ptr.PassInterface());
+    if (!service_worker_context_->storage_partition()) {
+      // The context is shutting down. Just drop the request.
+      return;
     }
 
+    // Set up the factory bundle for non-NetworkService URLs, e.g.,
+    // chrome-extension:// URLs. One factory bundle is consumed by the browser
+    // for SharedWorkerScriptLoaderFactory, and one is sent to the renderer.
+    std::unique_ptr<URLLoaderFactoryBundleInfo> factory_bundle_for_browser =
+        CreateFactoryBundle(process_id);
+    std::unique_ptr<URLLoaderFactoryBundleInfo> factory_bundle_for_renderer =
+        CreateFactoryBundle(process_id);
+
     BrowserThread::PostTask(
         BrowserThread::IO, FROM_HERE,
         base::BindOnce(
             &CreateScriptLoaderOnIO,
             service_worker_context_->storage_partition()
                 ->url_loader_factory_getter(),
-            std::move(factory_bundle), service_worker_context_, process_id,
+            std::move(factory_bundle_for_browser),
+            std::move(factory_bundle_for_renderer), service_worker_context_,
+            process_id,
             base::BindOnce(&SharedWorkerServiceImpl::StartWorker,
                            weak_factory_.GetWeakPtr(), std::move(instance),
                            weak_host, std::move(client), process_id, frame_id,
@@ -264,7 +286,7 @@ void SharedWorkerServiceImpl::CreateWorker(
   }
 
   StartWorker(std::move(instance), weak_host, std::move(client), process_id,
-              frame_id, message_port, nullptr, {});
+              frame_id, message_port, nullptr, {}, nullptr);
 }
 
 void SharedWorkerServiceImpl::StartWorker(
@@ -277,7 +299,8 @@ void SharedWorkerServiceImpl::StartWorker(
     mojom::ServiceWorkerProviderInfoForSharedWorkerPtr
         service_worker_provider_info,
     network::mojom::URLLoaderFactoryAssociatedPtrInfo
-        script_loader_factory_info) {
+        script_loader_factory_info,
+    std::unique_ptr<URLLoaderFactoryBundleInfo> factory_bundle) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   // The host may already be gone if something forcibly terminated the worker
@@ -304,7 +327,7 @@ void SharedWorkerServiceImpl::StartWorker(
   BindInterface(process_host, &factory);
 
   host->Start(std::move(factory), std::move(service_worker_provider_info),
-              std::move(script_loader_factory_info));
+              std::move(script_loader_factory_info), std::move(factory_bundle));
   host->AddClient(std::move(client), process_id, frame_id, message_port);
 }