Revert "shared worker: Give the renderer a factory bundle when Networ…

…kService is enabled."

This reverts commit 79573b3.

Reason for revert: Suspect for msan failures starting

https://ci.chromium.org/buildbot/chromium.memory/Linux%20ChromiumOS%20MSan%20Tests/7284

errors like


=1==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x26931324 in is_empty ./../../base/unguessable_token.h:68:45
    #1 0x26931324 in blink::IdentifiersFactory::IdFromToken(base::UnguessableToken const&) ./../../third_party/blink/renderer/core/inspector/identifiers_factory.cc:95:0
    #2 0x26b81609 in blink::InspectorNetworkAgent::InspectorNetworkAgent(blink::InspectedFrames*, blink::WorkerGlobalScope*, v8_inspector::V8InspectorSession*) ./../../third_party/blink/renderer/core/inspector/inspector_network_agent.cc:1809:23
...
 Uninitialized value was created by a heap allocation
    #0 0xac2699 in operator new(unsigned long) /b/build/slave/linux_upload_clang/build/src/third_party/llvm/compiler-rt/lib/msan/msan_new_delete.cc:45:35
    #1 0x2e6764af in blink::WebSharedWorker::Create(blink::WebSharedWorkerClient*) ./../../third_party/blink/renderer/core/exported/web_shared_worker_impl.cc:392:27
    #2 0x2ac0c8ea in content::EmbeddedSharedWorkerStub::EmbeddedSharedWorkerStub

Original change's description:
> 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}

TBR=falken@chromium.org,dcheng@chromium.org,kinuko@chromium.org,nhiroki@chromium.org

Change-Id: Iada07ec8bdddf612dbd441f702ede3dc8239ad21
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 839982
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/1073007
Reviewed-by: Trent Apted <tapted@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561728}

content/browser/shared_worker/mock_shared_worker.cc

@@ -4,7 +4,6 @@
 
 #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"
@@ -103,7 +102,6 @@ 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,7 +22,6 @@
 class GURL;
 
 namespace content {
-class URLLoaderFactoryBundleInfo;
 
 class MockSharedWorker : public mojom::SharedWorker {
  public:
@@ -72,7 +71,6 @@ 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,8 +14,6 @@
 #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"
@@ -123,8 +121,7 @@ void SharedWorkerHost::Start(
     mojom::SharedWorkerFactoryPtr factory,
     mojom::ServiceWorkerProviderInfoForSharedWorkerPtr
         service_worker_provider_info,
-    network::mojom::URLLoaderFactoryAssociatedPtrInfo script_loader_factory,
-    std::unique_ptr<URLLoaderFactoryBundleInfo> factory_bundle) {
+    network::mojom::URLLoaderFactoryAssociatedPtrInfo script_loader_factory) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   AdvanceTo(Phase::kStarted);
 
@@ -154,48 +151,19 @@ 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(factory_bundle),
-      std::move(host), std::move(worker_request_),
-      std::move(interface_provider));
+      std::move(script_loader_factory), 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,7 +36,6 @@ 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
@@ -67,8 +66,7 @@ class CONTENT_EXPORT SharedWorkerHost
       mojom::SharedWorkerFactoryPtr factory,
       mojom::ServiceWorkerProviderInfoForSharedWorkerPtr
           service_worker_provider_info,
-      network::mojom::URLLoaderFactoryAssociatedPtrInfo script_loader_factory,
-      std::unique_ptr<URLLoaderFactoryBundleInfo> factory_bundle);
+      network::mojom::URLLoaderFactoryAssociatedPtrInfo script_loader_factory);
 
   void AllowFileSystem(const GURL& url,
                        base::OnceCallback<void(bool)> callback);
@@ -139,8 +137,6 @@ 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,11 +15,9 @@
 #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"
@@ -30,10 +28,7 @@ namespace content {
 
 class SharedWorkerHostTest : public testing::Test {
  public:
-  SharedWorkerHostTest()
-      : service_(&storage_partition_, nullptr /* service_worker_context */) {
-    storage_partition_.set_network_context(&network_context_);
-  }
+  SharedWorkerHostTest() : service_(nullptr) {}
 
   base::WeakPtr<SharedWorkerHost> CreateHost() {
     GURL url("http://www.example.com/w.js");
@@ -61,8 +56,7 @@ 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 */,
-                nullptr /* factory_bundle */);
+                {} /* script_loader_factory_info */);
   }
 
   MessagePortChannel AddClient(SharedWorkerHost* host,
@@ -77,9 +71,6 @@ 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);
@@ -190,8 +181,7 @@ TEST_F(SharedWorkerHostTest, TerminateAfterStarting) {
 
   // Start the worker.
   host->Start(std::move(factory), nullptr /* service_worker_provider_info */,
-              {} /* script_loader_factory_info */,
-              nullptr /* factory_bundle */);
+              {} /* script_loader_factory_info */);
 
   // Add a client.
   MockSharedWorkerClient client;

content/browser/shared_worker/shared_worker_service_impl.cc

@@ -16,7 +16,6 @@
 #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"
@@ -40,45 +39,13 @@ 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_for_browser_info,
-    std::unique_ptr<URLLoaderFactoryBundleInfo>
-        factory_bundle_for_renderer_info,
+    std::unique_ptr<URLLoaderFactoryBundleInfo> factory_bundle_info,
     scoped_refptr<ServiceWorkerContextWrapper> context,
     int process_id,
     base::OnceCallback<void(mojom::ServiceWorkerProviderInfoForSharedWorkerPtr,
-                            network::mojom::URLLoaderFactoryAssociatedPtrInfo,
-                            std::unique_ptr<URLLoaderFactoryBundleInfo>)>
+                            network::mojom::URLLoaderFactoryAssociatedPtrInfo)>
         callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
@@ -87,11 +54,10 @@ void CreateScriptLoaderOnIO(
   base::WeakPtr<ServiceWorkerProviderHost> host =
       context->PreCreateHostForSharedWorker(process_id, &provider_info);
 
-  // Create the factory bundle for SharedWorkerScriptLoaderFactory to use to
-  // load the script.
+  // Create the factory bundle for loading the script.
   scoped_refptr<URLLoaderFactoryBundle> factory_bundle =
       base::MakeRefCounted<URLLoaderFactoryBundle>(
-          std::move(factory_bundle_for_browser_info));
+          std::move(factory_bundle_info));
 
   // Add the network factory to the bundle. The factory from
   // CloneNetworkFactory() doesn't support reconnection to the network service
@@ -110,21 +76,24 @@ 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(factory_bundle_for_renderer_info)));
+                     std::move(script_loader_factory)));
 }
 
 }  // namespace
 
 SharedWorkerServiceImpl::SharedWorkerServiceImpl(
-    StoragePartition* storage_partition,
     scoped_refptr<ServiceWorkerContextWrapper> service_worker_context)
-    : storage_partition_(storage_partition),
-      service_worker_context_(std::move(service_worker_context)),
+    : service_worker_context_(std::move(service_worker_context)),
       weak_factory_(this) {}
 
 SharedWorkerServiceImpl::~SharedWorkerServiceImpl() {}
@@ -256,28 +225,37 @@ 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()) {
-    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);
+    // 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());
+    }
 
     BrowserThread::PostTask(
         BrowserThread::IO, FROM_HERE,
         base::BindOnce(
             &CreateScriptLoaderOnIO,
             service_worker_context_->storage_partition()
                 ->url_loader_factory_getter(),
-            std::move(factory_bundle_for_browser),
-            std::move(factory_bundle_for_renderer), service_worker_context_,
-            process_id,
+            std::move(factory_bundle), service_worker_context_, process_id,
             base::BindOnce(&SharedWorkerServiceImpl::StartWorker,
                            weak_factory_.GetWeakPtr(), std::move(instance),
                            weak_host, std::move(client), process_id, frame_id,
@@ -286,7 +264,7 @@ void SharedWorkerServiceImpl::CreateWorker(
   }
 
   StartWorker(std::move(instance), weak_host, std::move(client), process_id,
-              frame_id, message_port, nullptr, {}, nullptr);
+              frame_id, message_port, nullptr, {});
 }
 
 void SharedWorkerServiceImpl::StartWorker(
@@ -299,8 +277,7 @@ void SharedWorkerServiceImpl::StartWorker(
     mojom::ServiceWorkerProviderInfoForSharedWorkerPtr
         service_worker_provider_info,
     network::mojom::URLLoaderFactoryAssociatedPtrInfo
-        script_loader_factory_info,
-    std::unique_ptr<URLLoaderFactoryBundleInfo> factory_bundle) {
+        script_loader_factory_info) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   // The host may already be gone if something forcibly terminated the worker
@@ -327,7 +304,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(factory_bundle));
+              std::move(script_loader_factory_info));
   host->AddClient(std::move(client), process_id, frame_id, message_port);
 }