[ServiceWorker] Implement ServiceWorkerObjectHost.Terminate

This CL mojofies the legacy IPC ServiceWorkerHostMsg_TerminateWorker.

BUG=772713

Change-Id: If91640688b0c7e7d08b7d375a7776d7c19c17781
Reviewed-on: https://chromium-review.googlesource.com/897582
Commit-Queue: Han Leon <leon.han@intel.com>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536033}

content/browser/service_worker/service_worker_dispatcher_host.cc

@@ -134,7 +134,6 @@ bool ServiceWorkerDispatcherHost::OnMessageReceived(
     IPC_MESSAGE_HANDLER(ServiceWorkerHostMsg_PostMessageToWorker,
                         OnPostMessageToWorker)
     IPC_MESSAGE_HANDLER(EmbeddedWorkerHostMsg_CountFeature, OnCountFeature)
-    IPC_MESSAGE_HANDLER(ServiceWorkerHostMsg_TerminateWorker, OnTerminateWorker)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
 
@@ -454,14 +453,4 @@ ServiceWorkerContextCore* ServiceWorkerDispatcherHost::GetContext() {
   return context_wrapper_->context();
 }
 
-void ServiceWorkerDispatcherHost::OnTerminateWorker(int handle_id) {
-  ServiceWorkerHandle* handle = handles_.Lookup(handle_id);
-  if (!handle) {
-    bad_message::ReceivedBadMessage(this,
-                                    bad_message::SWDH_TERMINATE_BAD_HANDLE);
-    return;
-  }
-  handle->version()->StopWorker(base::BindOnce(&base::DoNothing));
-}
-
 }  // namespace content

content/browser/service_worker/service_worker_dispatcher_host.h

@@ -144,8 +144,6 @@ class CONTENT_EXPORT ServiceWorkerDispatcherHost
           message,
       const url::Origin& source_origin);
 
-  void OnTerminateWorker(int handle_id);
-
   void DispatchExtendableMessageEvent(
       scoped_refptr<ServiceWorkerVersion> worker,
       blink::TransferableMessage message,

content/browser/service_worker/service_worker_handle.cc

@@ -86,6 +86,10 @@ void ServiceWorkerHandle::RegisterIntoDispatcherHost(
   dispatcher_host_->RegisterServiceWorkerHandle(base::WrapUnique(this));
 }
 
+void ServiceWorkerHandle::TerminateForTesting() {
+  version_->StopWorker(base::BindOnce(&base::DoNothing));
+}
+
 base::WeakPtr<ServiceWorkerHandle> ServiceWorkerHandle::AsWeakPtr() {
   return weak_ptr_factory_.GetWeakPtr();
 }

content/browser/service_worker/service_worker_handle.h

@@ -80,6 +80,9 @@ class CONTENT_EXPORT ServiceWorkerHandle
                       base::WeakPtr<ServiceWorkerProviderHost> provider_host,
                       ServiceWorkerVersion* version);
 
+  // Implements blink::mojom::ServiceWorkerObjectHost.
+  void TerminateForTesting() override;
+
   base::WeakPtr<ServiceWorkerHandle> AsWeakPtr();
 
   void OnConnectionError();

content/common/service_worker/service_worker_messages.h

@@ -99,11 +99,6 @@ IPC_MESSAGE_CONTROL4(
         base::RefCountedData<blink::TransferableMessage>> /* message */,
     url::Origin /* source_origin */)
 
-// Tells the browser to terminate a service worker. Used in layout tests to
-// verify behavior when a service worker isn't running.
-IPC_MESSAGE_CONTROL1(ServiceWorkerHostMsg_TerminateWorker,
-                     int /* handle_id */)
-
 // Sends MessageEvent to a client (renderer->browser).
 IPC_MESSAGE_ROUTED2(
     ServiceWorkerHostMsg_PostMessageToClient,

content/renderer/service_worker/service_worker_context_client.cc

@@ -894,6 +894,10 @@ void ServiceWorkerContextClient::WorkerContextStarted(
   // same thread before the worker context goes away in
   // willDestroyWorkerContext.
   context_.reset(new WorkerContextData(this));
+  // Create ServiceWorkerDispatcher first for this worker thread to be used
+  // later by TakeRegistrationForServiceWorkerGlobalScope() etc.
+  ServiceWorkerDispatcher::GetOrCreateThreadSpecificInstance(sender_.get())
+      ->SetIOThreadTaskRunner(io_thread_task_runner_);
 
   DCHECK(pending_dispatcher_request_.is_pending());
   DCHECK(pending_controller_request_.is_pending());
@@ -913,9 +917,6 @@ void ServiceWorkerContextClient::WorkerContextStarted(
       blink::mojom::ThreadSafeServiceWorkerHostAssociatedPtr::Create(
           std::move(pending_service_worker_host_), io_thread_task_runner_);
   // Set ServiceWorkerGlobalScope#registration.
-  // TakeRegistrationForServiceWorkerGlobalScope() expects the dispatcher to be
-  // already created, so create it first.
-  ServiceWorkerDispatcher::GetOrCreateThreadSpecificInstance(sender_.get());
   proxy_->SetRegistration(WebServiceWorkerRegistrationImpl::CreateHandle(
       provider_context_->TakeRegistrationForServiceWorkerGlobalScope(
           io_thread_task_runner_)));
@@ -1589,11 +1590,10 @@ void ServiceWorkerContextClient::DispatchExtendableMessageEvent(
              blink::mojom::kInvalidServiceWorkerHandleId &&
          event->source_info_for_service_worker->version_id !=
              blink::mojom::kInvalidServiceWorkerVersionId);
-  ServiceWorkerDispatcher* dispatcher =
-      ServiceWorkerDispatcher::GetOrCreateThreadSpecificInstance(sender_.get());
   scoped_refptr<WebServiceWorkerImpl> worker =
-      dispatcher->GetOrCreateServiceWorker(
-          std::move(event->source_info_for_service_worker));
+      ServiceWorkerDispatcher::GetThreadSpecificInstance()
+          ->GetOrCreateServiceWorker(
+              std::move(event->source_info_for_service_worker));
   proxy_->DispatchExtendableMessageEvent(
       request_id, std::move(event->message), event->source_origin,
       WebServiceWorkerImpl::CreateHandle(worker));

content/renderer/service_worker/service_worker_dispatcher.cc

@@ -116,8 +116,25 @@ ServiceWorkerDispatcher::GetOrCreateServiceWorker(
   if (found != service_workers_.end())
     return found->second;
 
-  // WebServiceWorkerImpl constructor calls AddServiceWorker.
-  return new WebServiceWorkerImpl(std::move(info), thread_safe_sender_.get());
+  if (WorkerThread::GetCurrentId()) {
+    // Because we do not support navigator.serviceWorker in
+    // WorkerNavigator (see https://crbug.com/371690), both dedicated worker and
+    // shared worker context can never have a ServiceWorker object, but service
+    // worker execution context is different as it can have some ServiceWorker
+    // objects via the ServiceWorkerGlobalScope#registration object.
+    // So, if we're on a worker thread here we know it's definitely a service
+    // worker thread.
+    DCHECK(io_thread_task_runner_);
+    return WebServiceWorkerImpl::CreateForServiceWorkerGlobalScope(
+        std::move(info), thread_safe_sender_.get(), io_thread_task_runner_);
+  }
+  return WebServiceWorkerImpl::CreateForServiceWorkerClient(
+      std::move(info), thread_safe_sender_.get());
+}
+
+void ServiceWorkerDispatcher::SetIOThreadTaskRunner(
+    scoped_refptr<base::SingleThreadTaskRunner> io_thread_task_runner) {
+  io_thread_task_runner_ = std::move(io_thread_task_runner);
 }
 
 void ServiceWorkerDispatcher::OnServiceWorkerStateChanged(

content/renderer/service_worker/service_worker_dispatcher.h

@@ -27,6 +27,10 @@
 #include "third_party/WebKit/public/platform/modules/serviceworker/WebServiceWorkerProvider.h"
 #include "third_party/WebKit/public/platform/modules/serviceworker/WebServiceWorkerRegistration.h"
 
+namespace base {
+class SingleThreadTaskRunner;
+}
+
 namespace IPC {
 class Message;
 }
@@ -52,6 +56,15 @@ class CONTENT_EXPORT ServiceWorkerDispatcher : public WorkerThread::Observer {
   scoped_refptr<WebServiceWorkerImpl> GetOrCreateServiceWorker(
       blink::mojom::ServiceWorkerObjectInfoPtr info);
 
+  // Sets the IO thread task runner. This is only called for a
+  // ServiceWorkerDispatcher instance on a service worker thread when the thread
+  // has just started, and the provided IO thread task runner will be used only
+  // for creating WebServiceWorkerImpl later.
+  // TODO(leonhsl): Remove this function once we addressed the TODO in
+  // WebServiceWorkerImpl about the legacy IPC channel-associated interface.
+  void SetIOThreadTaskRunner(
+      scoped_refptr<base::SingleThreadTaskRunner> io_thread_task_runner);
+
   static ServiceWorkerDispatcher* GetOrCreateThreadSpecificInstance(
       scoped_refptr<ThreadSafeSender> thread_safe_sender);
 
@@ -86,6 +99,7 @@ class CONTENT_EXPORT ServiceWorkerDispatcher : public WorkerThread::Observer {
   WorkerObjectMap service_workers_;
 
   scoped_refptr<ThreadSafeSender> thread_safe_sender_;
+  scoped_refptr<base::SingleThreadTaskRunner> io_thread_task_runner_;
 
   DISALLOW_COPY_AND_ASSIGN(ServiceWorkerDispatcher);
 };

content/renderer/service_worker/service_worker_dispatcher_unittest.cc

@@ -37,6 +37,9 @@ class MockServiceWorkerObjectHost
   int GetBindingCount() const { return bindings_.size(); }
 
  private:
+  // Implements blink::mojom::ServiceWorkerObjectHost.
+  void TerminateForTesting() override { NOTREACHED(); }
+
   int32_t handle_id_;
   int64_t version_id_;
   mojo::AssociatedBindingSet<blink::mojom::ServiceWorkerObjectHost> bindings_;

content/renderer/service_worker/service_worker_provider_context_unittest.cc

@@ -60,6 +60,9 @@ class MockServiceWorkerObjectHost
   int GetBindingCount() const { return bindings_.size(); }
 
  private:
+  // Implements blink::mojom::ServiceWorkerObjectHost.
+  void TerminateForTesting() override { NOTREACHED(); }
+
   int32_t handle_id_;
   int64_t version_id_;
   mojo::AssociatedBindingSet<blink::mojom::ServiceWorkerObjectHost> bindings_;

content/renderer/service_worker/web_service_worker_impl.cc

@@ -26,9 +26,8 @@ namespace {
 
 class ServiceWorkerHandleImpl : public blink::WebServiceWorker::Handle {
  public:
-  explicit ServiceWorkerHandleImpl(
-      const scoped_refptr<WebServiceWorkerImpl>& worker)
-      : worker_(worker) {}
+  explicit ServiceWorkerHandleImpl(scoped_refptr<WebServiceWorkerImpl> worker)
+      : worker_(std::move(worker)) {}
   ~ServiceWorkerHandleImpl() override {}
 
   blink::WebServiceWorker* ServiceWorker() override { return worker_.get(); }
@@ -41,18 +40,29 @@ class ServiceWorkerHandleImpl : public blink::WebServiceWorker::Handle {
 
 }  // namespace
 
-WebServiceWorkerImpl::WebServiceWorkerImpl(
+// static
+scoped_refptr<WebServiceWorkerImpl>
+WebServiceWorkerImpl::CreateForServiceWorkerGlobalScope(
     blink::mojom::ServiceWorkerObjectInfoPtr info,
-    ThreadSafeSender* thread_safe_sender)
-    : info_(std::move(info)),
-      state_(info_->state),
-      thread_safe_sender_(thread_safe_sender),
-      proxy_(nullptr) {
-  DCHECK_NE(blink::mojom::kInvalidServiceWorkerHandleId, info_->handle_id);
-  ServiceWorkerDispatcher* dispatcher =
-      ServiceWorkerDispatcher::GetThreadSpecificInstance();
-  DCHECK(dispatcher);
-  dispatcher->AddServiceWorker(info_->handle_id, this);
+    ThreadSafeSender* thread_safe_sender,
+    scoped_refptr<base::SingleThreadTaskRunner> io_task_runner) {
+  scoped_refptr<WebServiceWorkerImpl> impl =
+      new WebServiceWorkerImpl(std::move(info), thread_safe_sender);
+  impl->host_for_global_scope_ =
+      blink::mojom::ThreadSafeServiceWorkerObjectHostAssociatedPtr::Create(
+          std::move(impl->info_->host_ptr_info), io_task_runner);
+  return impl;
+}
+
+// static
+scoped_refptr<WebServiceWorkerImpl>
+WebServiceWorkerImpl::CreateForServiceWorkerClient(
+    blink::mojom::ServiceWorkerObjectInfoPtr info,
+    ThreadSafeSender* thread_safe_sender) {
+  scoped_refptr<WebServiceWorkerImpl> impl =
+      new WebServiceWorkerImpl(std::move(info), thread_safe_sender);
+  impl->host_for_client_.Bind(std::move(impl->info_->host_ptr_info));
+  return impl;
 }
 
 void WebServiceWorkerImpl::OnStateChanged(
@@ -92,18 +102,30 @@ void WebServiceWorkerImpl::PostMessageToWorker(
       url::Origin(source_origin)));
 }
 
-void WebServiceWorkerImpl::Terminate() {
-  thread_safe_sender_->Send(
-      new ServiceWorkerHostMsg_TerminateWorker(info_->handle_id));
+void WebServiceWorkerImpl::TerminateForTesting() {
+  GetObjectHost()->TerminateForTesting();
 }
 
 // static
 std::unique_ptr<blink::WebServiceWorker::Handle>
-WebServiceWorkerImpl::CreateHandle(
-    const scoped_refptr<WebServiceWorkerImpl>& worker) {
+WebServiceWorkerImpl::CreateHandle(scoped_refptr<WebServiceWorkerImpl> worker) {
   if (!worker)
     return nullptr;
-  return std::make_unique<ServiceWorkerHandleImpl>(worker);
+  return std::make_unique<ServiceWorkerHandleImpl>(std::move(worker));
+}
+
+WebServiceWorkerImpl::WebServiceWorkerImpl(
+    blink::mojom::ServiceWorkerObjectInfoPtr info,
+    ThreadSafeSender* thread_safe_sender)
+    : info_(std::move(info)),
+      state_(info_->state),
+      thread_safe_sender_(thread_safe_sender),
+      proxy_(nullptr) {
+  DCHECK_NE(blink::mojom::kInvalidServiceWorkerHandleId, info_->handle_id);
+  ServiceWorkerDispatcher* dispatcher =
+      ServiceWorkerDispatcher::GetThreadSpecificInstance();
+  DCHECK(dispatcher);
+  dispatcher->AddServiceWorker(info_->handle_id, this);
 }
 
 WebServiceWorkerImpl::~WebServiceWorkerImpl() {
@@ -113,4 +135,13 @@ WebServiceWorkerImpl::~WebServiceWorkerImpl() {
     dispatcher->RemoveServiceWorker(info_->handle_id);
 }
 
+blink::mojom::ServiceWorkerObjectHost* WebServiceWorkerImpl::GetObjectHost() {
+  if (host_for_client_)
+    return host_for_client_.get();
+  if (host_for_global_scope_)
+    return host_for_global_scope_->get();
+  NOTREACHED();
+  return nullptr;
+}
+
 }  // namespace content

content/renderer/service_worker/web_service_worker_impl.h

@@ -17,6 +17,10 @@
 #include "third_party/WebKit/public/platform/modules/serviceworker/WebServiceWorker.h"
 #include "third_party/WebKit/public/web/WebFrame.h"
 
+namespace base {
+class SingleThreadTaskRunner;
+}
+
 namespace blink {
 class WebServiceWorkerProxy;
 }
@@ -29,16 +33,24 @@ class ThreadSafeSender;
 // is held by ServiceWorker object in Blink's C++ layer via
 // WebServiceWorker::Handle.
 //
-// Each instance holds one Mojo connection for interface
-// blink::mojom::ServiceWorkerObjectHost inside |info_|, so the corresponding
-// ServiceWorkerHandle doesn't go away in the browser process while the
-// ServiceWorker object is alive.
+// Each instance holds one Mojo connection (|host_for_global_scope_| or
+// |host_for_client_|) for interface blink::mojom::ServiceWorkerObjectHost, so
+// the corresponding ServiceWorkerHandle doesn't go away in the browser process
+// while the ServiceWorker object is alive.
 class CONTENT_EXPORT WebServiceWorkerImpl
     : public blink::WebServiceWorker,
       public base::RefCounted<WebServiceWorkerImpl> {
  public:
-  WebServiceWorkerImpl(blink::mojom::ServiceWorkerObjectInfoPtr info,
-                       ThreadSafeSender* thread_safe_sender);
+  // |io_task_runner| is used to bind |host_for_global_scope_| for service
+  // worker execution context, as ServiceWorkerObjectHost is Channel-associated
+  // interface and needs to be bound on either the main or IO thread.
+  static scoped_refptr<WebServiceWorkerImpl> CreateForServiceWorkerGlobalScope(
+      blink::mojom::ServiceWorkerObjectInfoPtr info,
+      ThreadSafeSender* thread_safe_sender,
+      scoped_refptr<base::SingleThreadTaskRunner> io_task_runner);
+  static scoped_refptr<WebServiceWorkerImpl> CreateForServiceWorkerClient(
+      blink::mojom::ServiceWorkerObjectInfoPtr info,
+      ThreadSafeSender* thread_safe_sender);
 
   void OnStateChanged(blink::mojom::ServiceWorkerState new_state);
 
@@ -51,17 +63,37 @@ class CONTENT_EXPORT WebServiceWorkerImpl
       blink::WebServiceWorkerProvider* provider,
       blink::TransferableMessage message,
       const blink::WebSecurityOrigin& source_origin) override;
-  void Terminate() override;
+  void TerminateForTesting() override;
 
   // Creates WebServiceWorker::Handle object that owns a reference to the given
   // WebServiceWorkerImpl object.
   static std::unique_ptr<blink::WebServiceWorker::Handle> CreateHandle(
-      const scoped_refptr<WebServiceWorkerImpl>& worker);
+      scoped_refptr<WebServiceWorkerImpl> worker);
 
  private:
   friend class base::RefCounted<WebServiceWorkerImpl>;
+  WebServiceWorkerImpl(blink::mojom::ServiceWorkerObjectInfoPtr info,
+                       ThreadSafeSender* thread_safe_sender);
   ~WebServiceWorkerImpl() override;
 
+  blink::mojom::ServiceWorkerObjectHost* GetObjectHost();
+
+  // Either |host_for_global_scope_| or |host_for_client_| is non-null.
+  //
+  // |host_for_global_scope_| is for service worker execution contexts. It is
+  // used on the worker thread but bound on the IO thread, because it's a
+  // channel-associated interface which can be bound only on the main or IO
+  // thread.
+  // TODO(leonhsl): Once we can detach this interface out from the legacy IPC
+  // channel-associated interfaces world, we should bind it always on the worker
+  // thread on which |this| lives.
+  // Although it is a scoped_refptr, the only one owner is |this|.
+  scoped_refptr<blink::mojom::ThreadSafeServiceWorkerObjectHostAssociatedPtr>
+      host_for_global_scope_;
+  // |host_for_client_| is for service worker clients (document).
+  // It is bound and used on the main thread.
+  blink::mojom::ServiceWorkerObjectHostAssociatedPtr host_for_client_;
+
   blink::mojom::ServiceWorkerObjectInfoPtr info_;
   blink::mojom::ServiceWorkerState state_;
   scoped_refptr<ThreadSafeSender> thread_safe_sender_;

third_party/WebKit/Source/modules/serviceworkers/ServiceWorker.cpp

@@ -83,7 +83,7 @@ void ServiceWorker::postMessage(ScriptState* script_state,
 }
 
 void ServiceWorker::InternalsTerminate() {
-  handle_->ServiceWorker()->Terminate();
+  handle_->ServiceWorker()->TerminateForTesting();
 }
 
 void ServiceWorker::DispatchStateChangeEvent() {

third_party/WebKit/common/service_worker/service_worker_object.mojom

@@ -40,6 +40,7 @@ interface ServiceWorkerObjectHost {
   // PostMessage(string message, url.mojom.Origin source_origin,
   //     array<handle<message_pipe>> sent_message_ports);
 
-  // Replace ServiceWorkerHostMsg_TerminateWorker.
-  // Terminate();
+  // Tells the browser process to terminate the service worker. Used in layout
+  // tests to verify behavior when a service worker isn't running.
+  TerminateForTesting();
 };

third_party/WebKit/public/platform/modules/serviceworker/WebServiceWorker.h

@@ -76,7 +76,7 @@ class WebServiceWorker {
                                    TransferableMessage,
                                    const WebSecurityOrigin&) = 0;
 
-  virtual void Terminate() {}
+  virtual void TerminateForTesting() {}
 };
 }