Reland: shared worker: Self-terminate when network service crashes.

This treats the network service crashing like the worker's process
crashing, and prevents clients getting stuck with a broken worker.

Originally landed at: https://chromium-review.googlesource.com/c/1260903
It was reverted because the test flaked because the host was sometimes
not yet created. The diff in the reland is to wait for the host
to be created.

Bug: 848256
Change-Id: Ic656f0bf5a320526cf6f7715399f8f71db05619b
Reviewed-on: https://chromium-review.googlesource.com/c/1264139Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597063}

content/browser/network_service_restart_browsertest.cc

@@ -811,12 +811,8 @@ IN_PROC_BROWSER_TEST_F(NetworkServiceRestartBrowserTest, ServiceWorkerFetch) {
   service_worker_context->RemoveObserver(&observer);
 }
 
-// Make sure fetch from shared worker context works after crash.
-//
-// Disabled since shared workers don't support recovery from a NS crash:
-// https://crbug.com/848256.
-IN_PROC_BROWSER_TEST_F(NetworkServiceRestartBrowserTest,
-                       DISABLED_SharedWorkerFetch) {
+// Make sure shared workers terminate after crash.
+IN_PROC_BROWSER_TEST_F(NetworkServiceRestartBrowserTest, SharedWorker) {
   StoragePartitionImpl* partition = static_cast<StoragePartitionImpl*>(
       BrowserContext::GetDefaultStoragePartition(browser_context()));
 
@@ -827,19 +823,23 @@ IN_PROC_BROWSER_TEST_F(NetworkServiceRestartBrowserTest,
   // Navigate to the page and prepare a shared worker.
   EXPECT_TRUE(NavigateToURL(shell(), page_url));
 
-  // Fetch from the shared worker.
+  // Fetch from the shared worker to ensure it has started.
   const std::string script =
       "fetch_from_shared_worker('" + fetch_url.spec() + "');";
   EXPECT_EQ("Echo", EvalJs(shell(), script));
 
-  // Crash the NetworkService process. Existing interfaces should receive error
-  // notifications at some point.
+  // There should be one worker host. We will later wait for it to terminate.
+  SharedWorkerServiceImpl* service = partition->GetSharedWorkerService();
+  EXPECT_EQ(1u, service->worker_hosts_.size());
+  base::RunLoop loop;
+  service->SetWorkerTerminationCallbackForTesting(loop.QuitClosure());
+
+  // Crash the NetworkService process.
   SimulateNetworkServiceCrash();
-  // Flush the interface to make sure the error notification was received.
-  partition->FlushNetworkInterfaceForTesting();
 
-  // Fetch from the shared worker again.
-  EXPECT_EQ("Echo", EvalJs(shell(), script));
+  // Wait for the worker to detect the crash and self-terminate.
+  loop.Run();
+  EXPECT_TRUE(service->worker_hosts_.empty());
 }
 
 // Make sure the entry in |NetworkService::GetTotalNetworkUsages()| was cleared

content/browser/shared_worker/shared_worker_host.cc

@@ -292,7 +292,9 @@ void SharedWorkerHost::Start(
 }
 
 //  This is similar to
-//  RenderFrameHostImpl::CreateNetworkServiceDefaultFactoryAndObserve.
+//  RenderFrameHostImpl::CreateNetworkServiceDefaultFactoryAndObserve, but this
+//  host doesn't observe network service crashes. Instead, the renderer detects
+//  the connection error and terminates the worker.
 void SharedWorkerHost::CreateNetworkFactory(
     network::mojom::URLLoaderFactoryRequest request) {
   network::mojom::URLLoaderFactoryParamsPtr params =
@@ -303,9 +305,6 @@ void SharedWorkerHost::CreateNetworkFactory(
 
   service_->storage_partition()->GetNetworkContext()->CreateURLLoaderFactory(
       std::move(request), std::move(params));
-
-  // TODO(crbug.com/848256): Detect connection error and send a IPC with a new
-  // network factory like UpdateSubresourceLoaderFactories does for frames.
 }
 
 void SharedWorkerHost::AllowFileSystem(

content/browser/shared_worker/shared_worker_service_impl.cc

@@ -297,6 +297,12 @@ void SharedWorkerServiceImpl::TerminateAllWorkersForTesting(
   }
 }
 
+void SharedWorkerServiceImpl::SetWorkerTerminationCallbackForTesting(
+    base::OnceClosure callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  terminate_all_workers_callback_ = std::move(callback);
+}
+
 void SharedWorkerServiceImpl::ConnectToWorker(
     int process_id,
     int frame_id,
@@ -363,7 +369,7 @@ void SharedWorkerServiceImpl::DestroyHost(SharedWorkerHost* host) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   worker_hosts_.erase(worker_hosts_.find(host));
 
-  // Complete the call to TerminateAllWorkersForTesting if no more workers.
+  // Run the termination callback if no more workers.
   if (worker_hosts_.empty() && terminate_all_workers_callback_)
     std::move(terminate_all_workers_callback_).Run();
 }

content/browser/shared_worker/shared_worker_service_impl.h

@@ -54,6 +54,7 @@ class CONTENT_EXPORT SharedWorkerServiceImpl : public SharedWorkerService {
                        const url::Origin& constructor_origin) override;
 
   void TerminateAllWorkersForTesting(base::OnceClosure callback);
+  void SetWorkerTerminationCallbackForTesting(base::OnceClosure callback);
 
   // Creates the worker if necessary or connects to an already existing worker.
   void ConnectToWorker(
@@ -72,6 +73,7 @@ class CONTENT_EXPORT SharedWorkerServiceImpl : public SharedWorkerService {
  private:
   friend class SharedWorkerServiceImplTest;
   friend class SharedWorkerHostTest;
+  FRIEND_TEST_ALL_PREFIXES(NetworkServiceRestartBrowserTest, SharedWorker);
 
   static void AddAdditionalRequestHeaders(
       network::ResourceRequest* resource_request,

content/renderer/shared_worker/embedded_shared_worker_stub.cc

@@ -15,6 +15,7 @@
 #include "content/public/common/appcache_info.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/content_features.h"
+#include "content/public/common/content_switches.h"
 #include "content/public/common/origin_util.h"
 #include "content/public/common/renderer_preferences.h"
 #include "content/public/renderer/content_renderer_client.h"
@@ -202,6 +203,14 @@ class WebServiceWorkerNetworkProviderForSharedWorker
   std::unique_ptr<NavigationResponseOverrideParameters> response_override_;
 };
 
+// "ForSharedWorker" is to avoid collisions in Jumbo builds.
+bool IsOutOfProcessNetworkServiceForSharedWorker() {
+  return base::FeatureList::IsEnabled(network::features::kNetworkService) &&
+         !base::FeatureList::IsEnabled(features::kNetworkServiceInProcess) &&
+         !base::CommandLine::ForCurrentProcess()->HasSwitch(
+             switches::kSingleProcess);
+}
+
 }  // namespace
 
 EmbeddedSharedWorkerStub::EmbeddedSharedWorkerStub(
@@ -217,7 +226,7 @@ EmbeddedSharedWorkerStub::EmbeddedSharedWorkerStub(
     network::mojom::URLLoaderFactoryAssociatedPtrInfo
         main_script_loader_factory,
     blink::mojom::SharedWorkerMainScriptLoadParamsPtr main_script_load_params,
-    std::unique_ptr<URLLoaderFactoryBundleInfo> subresource_loader_factories,
+    std::unique_ptr<URLLoaderFactoryBundleInfo> factory_bundle,
     mojom::ControllerServiceWorkerInfoPtr controller_info,
     mojom::SharedWorkerHostPtr host,
     mojom::SharedWorkerRequest request,
@@ -272,10 +281,30 @@ EmbeddedSharedWorkerStub::EmbeddedSharedWorkerStub(
         base::nullopt /* subresource_overrides */);
   }
 
-  if (subresource_loader_factories) {
+  // |factory_bundle| is provided in the
+  // ServiceWorkerServicification or NetworkService case.
+  DCHECK(factory_bundle ||
+         !blink::ServiceWorkerUtils::IsServicificationEnabled());
+  if (factory_bundle) {
+    // If the network service crashes, then self-destruct so clients don't get
+    // stuck with a worker with a broken loader. Self-destruction is effectively
+    // the same as the worker's process crashing.
+    // The default factory might not be to the network service if a feature like
+    // AppCache set itself to the default, but treat a connection error as fatal
+    // anyway so clients don't get stuck.
+    if (IsOutOfProcessNetworkServiceForSharedWorker()) {
+      default_factory_connection_error_handler_holder_.Bind(
+          std::move(factory_bundle->default_factory_info()));
+      default_factory_connection_error_handler_holder_->Clone(
+          mojo::MakeRequest(&factory_bundle->default_factory_info()));
+      default_factory_connection_error_handler_holder_
+          .set_connection_error_handler(base::BindOnce(
+              &EmbeddedSharedWorkerStub::Terminate, base::Unretained(this)));
+    }
+
     subresource_loader_factories_->Update(
         std::make_unique<ChildURLLoaderFactoryBundleInfo>(
-            std::move(subresource_loader_factories),
+            std::move(factory_bundle),
             nullptr /* prefetch_loader_factory_info */),
         base::nullopt /* subresource_overrides */);
   }
@@ -466,7 +495,7 @@ void EmbeddedSharedWorkerStub::Connect(int connection_request_id,
 }
 
 void EmbeddedSharedWorkerStub::Terminate() {
-  // After this we wouldn't get any IPC for this stub.
+  // After this we should ignore any IPC for this stub.
   running_ = false;
   impl_->TerminateWorkerContext();
 }

content/renderer/shared_worker/embedded_shared_worker_stub.h

@@ -148,6 +148,12 @@ class EmbeddedSharedWorkerStub : public blink::WebSharedWorkerClient,
   // taking a resource pre-requested by the browser process.
   std::unique_ptr<NavigationResponseOverrideParameters> response_override_;
 
+  // Out-of-process NetworkService:
+  // Detects disconnection from the default factory of the loader factory bundle
+  // used by this worker (typically the network service).
+  network::mojom::URLLoaderFactoryPtr
+      default_factory_connection_error_handler_holder_;
+
   DISALLOW_COPY_AND_ASSIGN(EmbeddedSharedWorkerStub);
 };