Add an unit test for ServiceWorkerContextClient

SWContextClient is getting complicated rather than just passing through the
events, especially after adding renderer-side timers. This CL is for adding unit
tests for SWContextClient.

Bug: 774374
Change-Id: Iaaf3944d0a11d812c9f9f3e2f9bfe2f334fcba19
Reviewed-on: https://chromium-review.googlesource.com/799518
Commit-Queue: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522355}

content/renderer/service_worker/embedded_worker_instance_client_impl.cc

@@ -9,6 +9,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "content/child/scoped_child_process_reference.h"
+#include "content/child/thread_safe_sender.h"
 #include "content/common/service_worker/embedded_worker_messages.h"
 #include "content/common/service_worker/service_worker_utils.h"
 #include "content/public/common/content_client.h"
@@ -76,7 +77,8 @@ void EmbeddedWorkerInstanceClientImpl::StartWorker(
       ServiceWorkerUtils::IsScriptStreamingEnabled() && installed_scripts_info,
       std::move(dispatcher_request), std::move(controller_request),
       std::move(service_worker_host), std::move(instance_host),
-      std::move(provider_info), std::move(temporal_self_));
+      std::move(provider_info), std::move(temporal_self_),
+      ChildThreadImpl::current()->thread_safe_sender(), io_thread_runner_);
   client->set_blink_initialized_time(blink_initialized_time_);
   client->set_start_worker_received_time(base::TimeTicks::Now());
   wrapper_ = StartWorkerContext(

content/renderer/service_worker/service_worker_context_client.cc

@@ -61,6 +61,7 @@
 #include "storage/common/blob_storage/blob_handle.h"
 #include "third_party/WebKit/common/blob/blob.mojom.h"
 #include "third_party/WebKit/common/blob/blob_registry.mojom.h"
+#include "third_party/WebKit/common/message_port/message_port_channel.h"
 #include "third_party/WebKit/common/service_worker/service_worker_error_type.mojom.h"
 #include "third_party/WebKit/common/service_worker/service_worker_event_status.mojom.h"
 #include "third_party/WebKit/common/service_worker/service_worker_object.mojom.h"
@@ -623,14 +624,16 @@ ServiceWorkerContextClient::ServiceWorkerContextClient(
     blink::mojom::ServiceWorkerHostAssociatedPtrInfo service_worker_host,
     mojom::EmbeddedWorkerInstanceHostAssociatedPtrInfo instance_host,
     mojom::ServiceWorkerProviderInfoForStartWorkerPtr provider_info,
-    std::unique_ptr<EmbeddedWorkerInstanceClientImpl> embedded_worker_client)
+    std::unique_ptr<EmbeddedWorkerInstanceClientImpl> embedded_worker_client,
+    scoped_refptr<ThreadSafeSender> sender,
+    scoped_refptr<base::SingleThreadTaskRunner> io_thread_task_runner)
     : embedded_worker_id_(embedded_worker_id),
       service_worker_version_id_(service_worker_version_id),
       service_worker_scope_(service_worker_scope),
       script_url_(script_url),
-      sender_(ChildThreadImpl::current()->thread_safe_sender()),
+      sender_(sender),
       main_thread_task_runner_(base::ThreadTaskRunnerHandle::Get()),
-      io_thread_task_runner_(ChildThreadImpl::current()->GetIOTaskRunner()),
+      io_thread_task_runner_(io_thread_task_runner),
       proxy_(nullptr),
       pending_dispatcher_request_(std::move(dispatcher_request)),
       pending_controller_request_(std::move(controller_request)),
@@ -643,7 +646,7 @@ ServiceWorkerContextClient::ServiceWorkerContextClient(
   // Create a content::ServiceWorkerNetworkProvider for this data source so
   // we can observe its requests.
   pending_network_provider_ = ServiceWorkerNetworkProvider::CreateForController(
-      std::move(provider_info));
+      std::move(provider_info), sender_);
   provider_context_ = pending_network_provider_->context();
 
   TRACE_EVENT_NESTABLE_ASYNC_BEGIN1("ServiceWorker",
@@ -1840,4 +1843,9 @@ ServiceWorkerContextClient::GetWeakPtr() {
   return context_->weak_factory.GetWeakPtr();
 }
 
+// static
+void ServiceWorkerContextClient::ResetThreadSpecificInstanceForTesting() {
+  g_worker_client_tls.Pointer()->Set(nullptr);
+}
+
 }  // namespace content

content/renderer/service_worker/service_worker_context_client.h

@@ -75,8 +75,9 @@ class WebWorkerFetchContext;
 //
 // Unless otherwise noted (here or in base class documentation), all methods are
 // called on the worker thread.
-class ServiceWorkerContextClient : public blink::WebServiceWorkerContextClient,
-                                   public mojom::ServiceWorkerEventDispatcher {
+class CONTENT_EXPORT ServiceWorkerContextClient
+    : public blink::WebServiceWorkerContextClient,
+      public mojom::ServiceWorkerEventDispatcher {
  public:
   // Returns a thread-specific client instance.  This does NOT create a
   // new instance.
@@ -96,7 +97,9 @@ class ServiceWorkerContextClient : public blink::WebServiceWorkerContextClient,
       blink::mojom::ServiceWorkerHostAssociatedPtrInfo service_worker_host,
       mojom::EmbeddedWorkerInstanceHostAssociatedPtrInfo instance_host,
       mojom::ServiceWorkerProviderInfoForStartWorkerPtr provider_info,
-      std::unique_ptr<EmbeddedWorkerInstanceClientImpl> embedded_worker_client);
+      std::unique_ptr<EmbeddedWorkerInstanceClientImpl> embedded_worker_client,
+      scoped_refptr<ThreadSafeSender> sender,
+      scoped_refptr<base::SingleThreadTaskRunner> io_thread_task_runner);
   ~ServiceWorkerContextClient() override;
 
   // Called on the main thread.
@@ -253,6 +256,7 @@ class ServiceWorkerContextClient : public blink::WebServiceWorkerContextClient,
   struct WorkerContextData;
   class NavigationPreloadRequest;
   friend class ControllerServiceWorkerImpl;
+  friend class ServiceWorkerContextClientTest;
 
   // Get routing_id for sending message to the ServiceWorkerVersion
   // in the browser process.
@@ -381,6 +385,8 @@ class ServiceWorkerContextClient : public blink::WebServiceWorkerContextClient,
 
   base::WeakPtr<ServiceWorkerContextClient> GetWeakPtr();
 
+  static void ResetThreadSpecificInstanceForTesting();
+
   const int embedded_worker_id_;
   const int64_t service_worker_version_id_;
   const GURL service_worker_scope_;

content/renderer/service_worker/service_worker_dispatcher.cc

@@ -43,14 +43,18 @@ void* const kHasBeenDeleted = reinterpret_cast<void*>(0x1);
 }  // namespace
 
 ServiceWorkerDispatcher::ServiceWorkerDispatcher(
-    ThreadSafeSender* thread_safe_sender,
-    base::SingleThreadTaskRunner* main_thread_task_runner)
-    : thread_safe_sender_(thread_safe_sender),
-      main_thread_task_runner_(main_thread_task_runner) {
+    scoped_refptr<ThreadSafeSender> thread_safe_sender,
+    scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner)
+    : thread_safe_sender_(std::move(thread_safe_sender)),
+      main_thread_task_runner_(std::move(main_thread_task_runner)) {
   g_dispatcher_tls.Pointer()->Set(static_cast<void*>(this));
 }
 
 ServiceWorkerDispatcher::~ServiceWorkerDispatcher() {
+  if (allow_reinstantiation_) {
+    g_dispatcher_tls.Pointer()->Set(nullptr);
+    return;
+  }
   g_dispatcher_tls.Pointer()->Set(kHasBeenDeleted);
 }
 
@@ -70,8 +74,8 @@ void ServiceWorkerDispatcher::OnMessageReceived(const IPC::Message& msg) {
 
 ServiceWorkerDispatcher*
 ServiceWorkerDispatcher::GetOrCreateThreadSpecificInstance(
-    ThreadSafeSender* thread_safe_sender,
-    base::SingleThreadTaskRunner* main_thread_task_runner) {
+    scoped_refptr<ThreadSafeSender> thread_safe_sender,
+    scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner) {
   if (g_dispatcher_tls.Pointer()->Get() == kHasBeenDeleted) {
     NOTREACHED() << "Re-instantiating TLS ServiceWorkerDispatcher.";
     g_dispatcher_tls.Pointer()->Set(nullptr);
@@ -80,8 +84,8 @@ ServiceWorkerDispatcher::GetOrCreateThreadSpecificInstance(
     return static_cast<ServiceWorkerDispatcher*>(
         g_dispatcher_tls.Pointer()->Get());
 
-  ServiceWorkerDispatcher* dispatcher =
-      new ServiceWorkerDispatcher(thread_safe_sender, main_thread_task_runner);
+  ServiceWorkerDispatcher* dispatcher = new ServiceWorkerDispatcher(
+      std::move(thread_safe_sender), std::move(main_thread_task_runner));
   if (WorkerThread::GetCurrentId())
     WorkerThread::AddObserver(dispatcher);
   return dispatcher;
@@ -94,6 +98,10 @@ ServiceWorkerDispatcher* ServiceWorkerDispatcher::GetThreadSpecificInstance() {
       g_dispatcher_tls.Pointer()->Get());
 }
 
+void ServiceWorkerDispatcher::AllowReinstantiationForTesting() {
+  allow_reinstantiation_ = true;
+}
+
 void ServiceWorkerDispatcher::WillStopCurrentWorkerThread() {
   delete this;
 }

content/renderer/service_worker/service_worker_dispatcher.h

@@ -47,8 +47,8 @@ class WebServiceWorkerImpl;
 class CONTENT_EXPORT ServiceWorkerDispatcher : public WorkerThread::Observer {
  public:
   ServiceWorkerDispatcher(
-      ThreadSafeSender* thread_safe_sender,
-      base::SingleThreadTaskRunner* main_thread_task_runner);
+      scoped_refptr<ThreadSafeSender> thread_safe_sender,
+      scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner);
   ~ServiceWorkerDispatcher() override;
 
   void OnMessageReceived(const IPC::Message& msg);
@@ -59,8 +59,8 @@ class CONTENT_EXPORT ServiceWorkerDispatcher : public WorkerThread::Observer {
       std::unique_ptr<ServiceWorkerHandleReference> handle_ref);
 
   static ServiceWorkerDispatcher* GetOrCreateThreadSpecificInstance(
-      ThreadSafeSender* thread_safe_sender,
-      base::SingleThreadTaskRunner* main_thread_task_runner);
+      scoped_refptr<ThreadSafeSender> thread_safe_sender,
+      scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner);
 
   // Unlike GetOrCreateThreadSpecificInstance() this doesn't create a new
   // instance if thread-local instance doesn't exist.
@@ -77,9 +77,12 @@ class CONTENT_EXPORT ServiceWorkerDispatcher : public WorkerThread::Observer {
  private:
   using WorkerObjectMap = std::map<int, WebServiceWorkerImpl*>;
 
+  friend class ServiceWorkerContextClientTest;
   friend class ServiceWorkerDispatcherTest;
   friend class WebServiceWorkerImpl;
 
+  void AllowReinstantiationForTesting();
+
   // WorkerThread::Observer implementation.
   void WillStopCurrentWorkerThread() override;
 
@@ -91,6 +94,10 @@ class CONTENT_EXPORT ServiceWorkerDispatcher : public WorkerThread::Observer {
   void AddServiceWorker(int handle_id, WebServiceWorkerImpl* worker);
   void RemoveServiceWorker(int handle_id);
 
+  // True if another dispatcher is allowed to be created on the same thread
+  // after this instance is destructed.
+  bool allow_reinstantiation_ = false;
+
   WorkerObjectMap service_workers_;
 
   scoped_refptr<ThreadSafeSender> thread_safe_sender_;

content/renderer/service_worker/service_worker_network_provider.cc

@@ -219,8 +219,10 @@ ServiceWorkerNetworkProvider::CreateForSharedWorker(int route_id) {
 // static
 std::unique_ptr<ServiceWorkerNetworkProvider>
 ServiceWorkerNetworkProvider::CreateForController(
-    mojom::ServiceWorkerProviderInfoForStartWorkerPtr info) {
-  return base::WrapUnique(new ServiceWorkerNetworkProvider(std::move(info)));
+    mojom::ServiceWorkerProviderInfoForStartWorkerPtr info,
+    scoped_refptr<ThreadSafeSender> sender) {
+  return base::WrapUnique(
+      new ServiceWorkerNetworkProvider(std::move(info), std::move(sender)));
 }
 
 // static
@@ -301,12 +303,12 @@ ServiceWorkerNetworkProvider::ServiceWorkerNetworkProvider(
 
 // Constructor for service worker execution contexts.
 ServiceWorkerNetworkProvider::ServiceWorkerNetworkProvider(
-    mojom::ServiceWorkerProviderInfoForStartWorkerPtr info) {
+    mojom::ServiceWorkerProviderInfoForStartWorkerPtr info,
+    scoped_refptr<ThreadSafeSender> sender) {
   // Initialize the provider context with info for
   // ServiceWorkerGlobalScope#registration.
-  ThreadSafeSender* sender = ChildThreadImpl::current()->thread_safe_sender();
   ServiceWorkerDispatcher::GetOrCreateThreadSpecificInstance(
-      sender, base::ThreadTaskRunnerHandle::Get().get());
+      sender, base::ThreadTaskRunnerHandle::Get());
   // TODO(kinuko): Split ServiceWorkerProviderContext ctor for
   // controller and controllee.
   context_ = base::MakeRefCounted<ServiceWorkerProviderContext>(

content/renderer/service_worker/service_worker_network_provider.h

@@ -33,6 +33,7 @@ class URLLoaderFactory;
 struct RequestNavigationParams;
 class ServiceWorkerProviderContext;
 class ChildURLLoaderFactoryGetter;
+class ThreadSafeSender;
 
 // ServiceWorkerNetworkProvider enables the browser process to recognize
 // resource requests from Blink that should be handled by service worker
@@ -79,7 +80,8 @@ class CONTENT_EXPORT ServiceWorkerNetworkProvider {
   // Creates a ServiceWorkerNetworkProvider for a "controller" (i.e.
   // a service worker execution context).
   static std::unique_ptr<ServiceWorkerNetworkProvider> CreateForController(
-      mojom::ServiceWorkerProviderInfoForStartWorkerPtr info);
+      mojom::ServiceWorkerProviderInfoForStartWorkerPtr info,
+      scoped_refptr<ThreadSafeSender> sender);
 
   // Valid only for WebServiceWorkerNetworkProvider created by
   // CreateForNavigation.
@@ -122,7 +124,8 @@ class CONTENT_EXPORT ServiceWorkerNetworkProvider {
 
   // This is for controllers, used in CreateForController.
   explicit ServiceWorkerNetworkProvider(
-      mojom::ServiceWorkerProviderInfoForStartWorkerPtr info);
+      mojom::ServiceWorkerProviderInfoForStartWorkerPtr info,
+      scoped_refptr<ThreadSafeSender> sender);
 
   scoped_refptr<ServiceWorkerProviderContext> context_;
   mojom::ServiceWorkerDispatcherHostAssociatedPtr dispatcher_host_;

content/test/BUILD.gn

@@ -1573,6 +1573,7 @@ test("content_unittests") {
     "../renderer/render_thread_impl_unittest.cc",
     "../renderer/render_widget_unittest.cc",
     "../renderer/scheduler/resource_dispatch_throttler_unittest.cc",
+    "../renderer/service_worker/service_worker_context_client_unittest.cc",
     "../renderer/service_worker/service_worker_dispatcher_unittest.cc",
     "../renderer/service_worker/service_worker_provider_context_unittest.cc",
     "../renderer/service_worker/service_worker_subresource_loader_unittest.cc",