service worker: Remove "{Associate/Disassociate}Registration" IPC messages.

The renderer-side ServiceWorkerDispatcher had OnAssociateRegistration,
which was either called via IPC for service worker controllees, or
directly from ServiceWorkerNetworkProvider for "controllers" (service worker
execution contexts).

But service worker controllees don't actually need to keep track of the
associated registration. I think we needed this in old times when there were
things like navigator.serviceWorker.ready had to be populated with a registration
immediately <https://codereview.chromium.org/477593007>, now it's an async API
that asks the browser for the registration
<https://codereview.chromium.org/894973003>.

Similarly, we had OnDisassociateRegistration, but once OnAssociateRegistration
IPC is removed, all it did was set the controller to nullptr, so it can
be replaced with SetController. It's never called for service worker
execution contexts.

There's more simplificiation possible here, we can possibly remove the
whole ProviderContext interface as now there are no common functions between
controllees and controllers, but that's for follow-up work.

TBR=tsepez for removing messages from service_worker_messages.h

Change-Id: I8b24c831e850351b7da955f80006d1cf51039095
Reviewed-on: https://chromium-review.googlesource.com/599092Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492000}

content/browser/service_worker/service_worker_provider_host.cc

@@ -617,7 +617,6 @@ void ServiceWorkerProviderHost::AssociateRegistration(
   DCHECK(CanAssociateRegistration(registration));
   associated_registration_ = registration;
   AddMatchingRegistration(registration);
-  SendAssociateRegistrationMessage();
   SetControllerVersionAttribute(registration->active_version(),
                                 notify_controllerchange);
 }
@@ -626,16 +625,8 @@ void ServiceWorkerProviderHost::DisassociateRegistration() {
   queued_events_.clear();
   if (!associated_registration_.get())
     return;
-  associated_registration_ = NULL;
-  SetControllerVersionAttribute(NULL, false /* notify_controllerchange */);
-
-  if (!dispatcher_host_)
-    return;
-
-  // Disassociation message should be sent only for controllees.
-  DCHECK(IsProviderForClient());
-  Send(new ServiceWorkerMsg_DisassociateRegistration(
-      render_thread_id_, provider_id()));
+  associated_registration_ = nullptr;
+  SetControllerVersionAttribute(nullptr, false /* notify_controllerchange */);
 }
 
 void ServiceWorkerProviderHost::AddMatchingRegistration(
@@ -833,8 +824,12 @@ ServiceWorkerProviderHost::PrepareForCrossSiteTransfer() {
 
   if (associated_registration_.get()) {
     if (dispatcher_host_) {
-      Send(new ServiceWorkerMsg_DisassociateRegistration(
-          render_thread_id_, provider_id()));
+      ServiceWorkerMsg_SetControllerServiceWorker_Params params;
+      params.thread_id = render_thread_id_;
+      params.provider_id = provider_id();
+      params.object_info = ServiceWorkerObjectInfo();
+      params.should_notify_controllerchange = false;
+      Send(new ServiceWorkerMsg_SetControllerServiceWorker(params));
     }
   }
 
@@ -1044,28 +1039,6 @@ void ServiceWorkerProviderHost::SetReadyToSendMessagesToWorker(
   queued_events_.clear();
 }
 
-void ServiceWorkerProviderHost::SendAssociateRegistrationMessage() {
-  if (!dispatcher_host_)
-    return;
-
-  ServiceWorkerRegistrationHandle* handle =
-      dispatcher_host_->GetOrCreateRegistrationHandle(
-          AsWeakPtr(), associated_registration_.get());
-
-  ServiceWorkerVersionAttributes attrs;
-  attrs.installing = GetOrCreateServiceWorkerHandle(
-      associated_registration_->installing_version());
-  attrs.waiting = GetOrCreateServiceWorkerHandle(
-      associated_registration_->waiting_version());
-  attrs.active = GetOrCreateServiceWorkerHandle(
-      associated_registration_->active_version());
-
-  // Association message should be sent only for controllees.
-  DCHECK(IsProviderForClient());
-  dispatcher_host_->Send(new ServiceWorkerMsg_AssociateRegistration(
-      render_thread_id_, provider_id(), handle->GetObjectInfo(), attrs));
-}
-
 void ServiceWorkerProviderHost::SyncMatchingRegistrations() {
   DCHECK(context_);
   RemoveAllMatchingRegistrations();
@@ -1134,7 +1107,6 @@ void ServiceWorkerProviderHost::Send(IPC::Message* message) const {
 
 void ServiceWorkerProviderHost::NotifyControllerToAssociatedProvider() {
   if (associated_registration_.get()) {
-    SendAssociateRegistrationMessage();
     if (dispatcher_host_ && associated_registration_->active_version()) {
       ServiceWorkerMsg_SetControllerServiceWorker_Params params;
       params.thread_id = render_thread_id_;

content/child/service_worker/service_worker_dispatcher.cc

@@ -68,10 +68,6 @@ void ServiceWorkerDispatcher::OnMessageReceived(const IPC::Message& msg) {
   // handler in ServiceWorkerMessageFilter to release references passed from
   // the browser process in case we fail to post task to the thread.
   IPC_BEGIN_MESSAGE_MAP(ServiceWorkerDispatcher, msg)
-    IPC_MESSAGE_HANDLER(ServiceWorkerMsg_AssociateRegistration,
-                        OnAssociateRegistration)
-    IPC_MESSAGE_HANDLER(ServiceWorkerMsg_DisassociateRegistration,
-                        OnDisassociateRegistration)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_ServiceWorkerRegistered, OnRegistered)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_ServiceWorkerUpdated, OnUpdated)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_ServiceWorkerUnregistered,
@@ -390,8 +386,7 @@ ServiceWorkerDispatcher::GetOrAdoptRegistration(
   return registration;
 }
 
-void ServiceWorkerDispatcher::OnAssociateRegistration(
-    int thread_id,
+void ServiceWorkerDispatcher::OnAssociateRegistrationForController(
     int provider_id,
     const ServiceWorkerRegistrationObjectInfo& info,
     const ServiceWorkerVersionAttributes& attrs) {
@@ -411,15 +406,6 @@ void ServiceWorkerDispatcher::OnAssociateRegistration(
   }
 }
 
-void ServiceWorkerDispatcher::OnDisassociateRegistration(
-    int thread_id,
-    int provider_id) {
-  ProviderContextMap::iterator provider = provider_contexts_.find(provider_id);
-  if (provider == provider_contexts_.end())
-    return;
-  provider->second->OnDisassociateRegistration();
-}
-
 void ServiceWorkerDispatcher::OnRegistered(
     int thread_id,
     int request_id,

content/child/service_worker/service_worker_dispatcher.h

@@ -165,10 +165,12 @@ class CONTENT_EXPORT ServiceWorkerDispatcher : public WorkerThread::Observer {
       const ServiceWorkerRegistrationObjectInfo& info,
       const ServiceWorkerVersionAttributes& attrs);
 
-  void OnAssociateRegistration(int thread_id,
-                               int provider_id,
-                               const ServiceWorkerRegistrationObjectInfo& info,
-                               const ServiceWorkerVersionAttributes& attrs);
+  // TODO(falken): Change the caller to just call the appropriate function on
+  // ServiceWorkerProviderContext directly.
+  void OnAssociateRegistrationForController(
+      int provider_id,
+      const ServiceWorkerRegistrationObjectInfo& info,
+      const ServiceWorkerVersionAttributes& attrs);
 
   static ServiceWorkerDispatcher* GetOrCreateThreadSpecificInstance(
       ThreadSafeSender* thread_safe_sender,
@@ -217,8 +219,6 @@ class CONTENT_EXPORT ServiceWorkerDispatcher : public WorkerThread::Observer {
   // WorkerThread::Observer implementation.
   void WillStopCurrentWorkerThread() override;
 
-  void OnDisassociateRegistration(int thread_id,
-                                  int provider_id);
   void OnRegistered(int thread_id,
                     int request_id,
                     const ServiceWorkerRegistrationObjectInfo& info,

content/child/service_worker/service_worker_dispatcher_unittest.cc

@@ -73,15 +73,11 @@ class ServiceWorkerDispatcherTest : public testing::Test {
     return ContainsKey(dispatcher_->registrations_, registration_handle_id);
   }
 
-  void OnAssociateRegistration(int thread_id,
-                               int provider_id,
-                               const ServiceWorkerRegistrationObjectInfo& info,
-                               const ServiceWorkerVersionAttributes& attrs) {
-    dispatcher_->OnAssociateRegistration(thread_id, provider_id, info, attrs);
-  }
-
-  void OnDisassociateRegistration(int thread_id, int provider_id) {
-    dispatcher_->OnDisassociateRegistration(thread_id, provider_id);
+  void OnAssociateRegistrationForController(
+      int provider_id,
+      const ServiceWorkerRegistrationObjectInfo& info,
+      const ServiceWorkerVersionAttributes& attrs) {
+    dispatcher_->OnAssociateRegistrationForController(provider_id, info, attrs);
   }
 
   void OnSetControllerServiceWorker(int thread_id,
@@ -177,7 +173,7 @@ TEST_F(ServiceWorkerDispatcherTest, OnAssociateRegistration_NoProviderContext) {
   // The passed references should be adopted but immediately released because
   // there is no provider context to own the references.
   const int kProviderId = 10;
-  OnAssociateRegistration(kDocumentMainThreadId, kProviderId, info, attrs);
+  OnAssociateRegistrationForController(kProviderId, info, attrs);
   ASSERT_EQ(4UL, ipc_sink()->message_count());
   EXPECT_EQ(ServiceWorkerHostMsg_DecrementServiceWorkerRefCount::ID,
             ipc_sink()->GetMessageAt(0)->type());
@@ -206,7 +202,7 @@ TEST_F(ServiceWorkerDispatcherTest,
           thread_safe_sender()));
 
   // The passed references should be adopted and owned by the provider context.
-  OnAssociateRegistration(kDocumentMainThreadId, kProviderId, info, attrs);
+  OnAssociateRegistrationForController(kProviderId, info, attrs);
   EXPECT_EQ(0UL, ipc_sink()->message_count());
 
   // Destruction of the provider context should release references to the
@@ -223,42 +219,6 @@ TEST_F(ServiceWorkerDispatcherTest,
             ipc_sink()->GetMessageAt(3)->type());
 }
 
-TEST_F(ServiceWorkerDispatcherTest,
-       OnAssociateRegistration_ProviderContextForControllee) {
-  // Assume that these objects are passed from the browser process and own
-  // references to browser-side registration/worker representations.
-  ServiceWorkerRegistrationObjectInfo info;
-  ServiceWorkerVersionAttributes attrs;
-  CreateObjectInfoAndVersionAttributes(&info, &attrs);
-
-  // Set up ServiceWorkerProviderContext for a document context.
-  const int kProviderId = 10;
-  scoped_refptr<ServiceWorkerProviderContext> provider_context(
-      new ServiceWorkerProviderContext(
-          kProviderId, SERVICE_WORKER_PROVIDER_FOR_WINDOW,
-          mojom::ServiceWorkerProviderAssociatedRequest(),
-          thread_safe_sender()));
-
-  // The passed references should be adopted and only the registration reference
-  // should be owned by the provider context.
-  OnAssociateRegistration(kDocumentMainThreadId, kProviderId, info, attrs);
-  ASSERT_EQ(3UL, ipc_sink()->message_count());
-  EXPECT_EQ(ServiceWorkerHostMsg_DecrementServiceWorkerRefCount::ID,
-            ipc_sink()->GetMessageAt(0)->type());
-  EXPECT_EQ(ServiceWorkerHostMsg_DecrementServiceWorkerRefCount::ID,
-            ipc_sink()->GetMessageAt(1)->type());
-  EXPECT_EQ(ServiceWorkerHostMsg_DecrementServiceWorkerRefCount::ID,
-            ipc_sink()->GetMessageAt(2)->type());
-  ipc_sink()->ClearMessages();
-
-  // Disassociating the provider context from the registration should release
-  // the reference.
-  OnDisassociateRegistration(kDocumentMainThreadId, kProviderId);
-  ASSERT_EQ(1UL, ipc_sink()->message_count());
-  EXPECT_EQ(ServiceWorkerHostMsg_DecrementRegistrationRefCount::ID,
-            ipc_sink()->GetMessageAt(0)->type());
-}
-
 TEST_F(ServiceWorkerDispatcherTest, OnSetControllerServiceWorker) {
   const int kProviderId = 10;
   bool should_notify_controllerchange = true;
@@ -288,7 +248,6 @@ TEST_F(ServiceWorkerDispatcherTest, OnSetControllerServiceWorker) {
           kProviderId, SERVICE_WORKER_PROVIDER_FOR_WINDOW,
           mojom::ServiceWorkerProviderAssociatedRequest(),
           thread_safe_sender()));
-  OnAssociateRegistration(kDocumentMainThreadId, kProviderId, info, attrs);
   ipc_sink()->ClearMessages();
   OnSetControllerServiceWorker(kDocumentMainThreadId, kProviderId, attrs.active,
                                should_notify_controllerchange,
@@ -298,11 +257,9 @@ TEST_F(ServiceWorkerDispatcherTest, OnSetControllerServiceWorker) {
   // Destruction of the provider context should release references to the
   // associated registration and the controller.
   provider_context = nullptr;
-  ASSERT_EQ(2UL, ipc_sink()->message_count());
+  ASSERT_EQ(1UL, ipc_sink()->message_count());
   EXPECT_EQ(ServiceWorkerHostMsg_DecrementServiceWorkerRefCount::ID,
             ipc_sink()->GetMessageAt(0)->type());
-  EXPECT_EQ(ServiceWorkerHostMsg_DecrementRegistrationRefCount::ID,
-            ipc_sink()->GetMessageAt(1)->type());
   ipc_sink()->ClearMessages();
 
   // (3) In the case there is no SWProviderContext but WebSWProviderClient for
@@ -336,7 +293,6 @@ TEST_F(ServiceWorkerDispatcherTest, OnSetControllerServiceWorker) {
   provider_context = new ServiceWorkerProviderContext(
       kProviderId, SERVICE_WORKER_PROVIDER_FOR_WINDOW,
       mojom::ServiceWorkerProviderAssociatedRequest(), thread_safe_sender());
-  OnAssociateRegistration(kDocumentMainThreadId, kProviderId, info, attrs);
   provider_client.reset(
       new MockWebServiceWorkerProviderClientImpl(kProviderId, dispatcher()));
   ASSERT_FALSE(provider_client->is_set_controlled_called());
@@ -370,8 +326,6 @@ TEST_F(ServiceWorkerDispatcherTest, OnSetControllerServiceWorker_Null) {
           mojom::ServiceWorkerProviderAssociatedRequest(),
           thread_safe_sender()));
 
-  OnAssociateRegistration(kDocumentMainThreadId, kProviderId, info, attrs);
-
   // Set the controller to kInvalidServiceWorkerHandle.
   OnSetControllerServiceWorker(
       kDocumentMainThreadId, kProviderId, ServiceWorkerObjectInfo(),

content/child/service_worker/service_worker_message_filter.cc

@@ -67,8 +67,6 @@ void ServiceWorkerMessageFilter::OnStaleMessageReceived(
   // Specifically handle some messages in case we failed to post task
   // to the thread (meaning that the context on the thread is now gone).
   IPC_BEGIN_MESSAGE_MAP(ServiceWorkerMessageFilter, msg)
-    IPC_MESSAGE_HANDLER(ServiceWorkerMsg_AssociateRegistration,
-                        OnStaleAssociateRegistration)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_ServiceWorkerRegistered,
                         OnStaleGetRegistration)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_DidGetRegistration,
@@ -86,20 +84,6 @@ void ServiceWorkerMessageFilter::OnStaleMessageReceived(
   IPC_END_MESSAGE_MAP()
 }
 
-void ServiceWorkerMessageFilter::OnStaleAssociateRegistration(
-    int thread_id,
-    int provider_id,
-    const ServiceWorkerRegistrationObjectInfo& info,
-    const ServiceWorkerVersionAttributes& attrs) {
-  SendServiceWorkerObjectDestroyed(thread_safe_sender(),
-                                   attrs.installing.handle_id);
-  SendServiceWorkerObjectDestroyed(thread_safe_sender(),
-                                   attrs.waiting.handle_id);
-  SendServiceWorkerObjectDestroyed(thread_safe_sender(),
-                                   attrs.active.handle_id);
-  SendRegistrationObjectDestroyed(thread_safe_sender(), info.handle_id);
-}
-
 void ServiceWorkerMessageFilter::OnStaleGetRegistration(
     int thread_id,
     int request_id,

content/child/service_worker/service_worker_message_filter.h

@@ -39,11 +39,6 @@ class CONTENT_EXPORT ServiceWorkerMessageFilter
   void OnStaleMessageReceived(const IPC::Message& msg) override;
 
   // Message handlers for stale messages.
-  void OnStaleAssociateRegistration(
-      int thread_id,
-      int provider_id,
-      const ServiceWorkerRegistrationObjectInfo& info,
-      const ServiceWorkerVersionAttributes& attrs);
   void OnStaleGetRegistration(int thread_id,
                               int request_id,
                               const ServiceWorkerRegistrationObjectInfo& info,

content/child/service_worker/service_worker_network_provider.cc

@@ -266,9 +266,8 @@ ServiceWorkerNetworkProvider::ServiceWorkerNetworkProvider(
           ChildThreadImpl::current()->thread_safe_sender(),
           base::ThreadTaskRunnerHandle::Get().get());
   // TODO(shimazu): Set registration/attributes directly to |context_|.
-  dispatcher->OnAssociateRegistration(-1 /* unused thread_id */,
-                                      info->provider_id, info->registration,
-                                      info->attributes);
+  dispatcher->OnAssociateRegistrationForController(
+      info->provider_id, info->registration, info->attributes);
   provider_host_.Bind(std::move(info->host_ptr_info));
 }
 

content/child/service_worker/service_worker_provider_context.cc

@@ -25,7 +25,6 @@ class ServiceWorkerProviderContext::Delegate {
       std::unique_ptr<ServiceWorkerHandleReference> installing,
       std::unique_ptr<ServiceWorkerHandleReference> waiting,
       std::unique_ptr<ServiceWorkerHandleReference> active) = 0;
-  virtual void DisassociateRegistration() = 0;
   virtual void GetAssociatedRegistration(
       ServiceWorkerRegistrationObjectInfo* info,
       ServiceWorkerVersionAttributes* attrs) = 0;
@@ -36,8 +35,7 @@ class ServiceWorkerProviderContext::Delegate {
 };
 
 // Delegate class for ServiceWorker client (Document, SharedWorker, etc) to
-// keep the associated registration and the controller until
-// ServiceWorkerContainer is initialized.
+// keep the controller until ServiceWorkerContainer is initialized.
 class ServiceWorkerProviderContext::ControlleeDelegate
     : public ServiceWorkerProviderContext::Delegate {
  public:
@@ -49,24 +47,20 @@ class ServiceWorkerProviderContext::ControlleeDelegate
       std::unique_ptr<ServiceWorkerHandleReference> /* installing */,
       std::unique_ptr<ServiceWorkerHandleReference> /* waiting */,
       std::unique_ptr<ServiceWorkerHandleReference> /* active */) override {
-    DCHECK(!registration_);
-    registration_ = std::move(registration);
-  }
-
-  void DisassociateRegistration() override {
-    controller_.reset();
-    registration_.reset();
+    NOTREACHED();
   }
 
   void SetController(
       std::unique_ptr<ServiceWorkerHandleReference> controller) override {
-    DCHECK(registration_);
     DCHECK(!controller ||
            controller->handle_id() != kInvalidServiceWorkerHandleId);
     controller_ = std::move(controller);
   }
 
-  bool HasAssociatedRegistration() override { return !!registration_; }
+  bool HasAssociatedRegistration() override {
+    NOTREACHED();
+    return false;
+  }
 
   void GetAssociatedRegistration(
       ServiceWorkerRegistrationObjectInfo* /* info */,
@@ -79,7 +73,6 @@ class ServiceWorkerProviderContext::ControlleeDelegate
   }
 
  private:
-  std::unique_ptr<ServiceWorkerRegistrationHandleReference> registration_;
   std::unique_ptr<ServiceWorkerHandleReference> controller_;
 
   DISALLOW_COPY_AND_ASSIGN(ControlleeDelegate);
@@ -105,11 +98,6 @@ class ServiceWorkerProviderContext::ControllerDelegate
     active_ = std::move(active);
   }
 
-  void DisassociateRegistration() override {
-    // ServiceWorkerGlobalScope is never disassociated.
-    NOTREACHED();
-  }
-
   void SetController(
       std::unique_ptr<ServiceWorkerHandleReference> /* controller */) override {
     NOTREACHED();
@@ -183,11 +171,6 @@ void ServiceWorkerProviderContext::OnAssociateRegistration(
                                    std::move(active));
 }
 
-void ServiceWorkerProviderContext::OnDisassociateRegistration() {
-  DCHECK(main_thread_task_runner_->RunsTasksInCurrentSequence());
-  delegate_->DisassociateRegistration();
-}
-
 void ServiceWorkerProviderContext::OnSetControllerServiceWorker(
     std::unique_ptr<ServiceWorkerHandleReference> controller,
     const std::set<uint32_t>& used_features,

content/child/service_worker/service_worker_provider_context.h

@@ -43,13 +43,13 @@ class ThreadSafeSender;
 // to the corresponding ServiceWorkerNetworkProvider.
 //
 // The role of this class varies for controllees and controllers:
-//  - For controllees, this is used for keeping the associated registration and
-//    the controller alive to create controllee's ServiceWorkerContainer. The
-//    references to them are kept until OnDisassociateRegistration() is called
-//    or OnSetControllerServiceWorker() is called with an invalid worker info.
+//  - For controllees, this is used for keeping the controller alive to create
+//    controllee's ServiceWorkerContainer#controller.
+//    The reference to the controller is kept until
+//    OnSetControllerServiceWorker() is called with an invalid worker info.
 //  - For controllers, this is used for keeping the associated registration and
-//    its versions alive to create controller's ServiceWorkerGlobalScope. The
-//    references to them are kept until OnDisassociateRegistration() is called.
+//    its versions alive to create the controller's
+//    ServiceWorkerGlobalScope#registration.
 //
 // These operations are actually done in delegate classes owned by this class:
 // ControlleeDelegate and ControllerDelegate.
@@ -75,7 +75,6 @@ class CONTENT_EXPORT ServiceWorkerProviderContext
       std::unique_ptr<ServiceWorkerHandleReference> installing,
       std::unique_ptr<ServiceWorkerHandleReference> waiting,
       std::unique_ptr<ServiceWorkerHandleReference> active);
-  void OnDisassociateRegistration();
   void OnSetControllerServiceWorker(
       std::unique_ptr<ServiceWorkerHandleReference> controller,
       const std::set<uint32_t>& used_features,

content/common/service_worker/service_worker_messages.h

@@ -330,17 +330,6 @@ IPC_MESSAGE_ROUTED1(ServiceWorkerHostMsg_ClaimClients,
 // extract it and dispatch the message to the correct ServiceWorkerDispatcher
 // on the correct thread.
 
-// Informs the child process that the given provider gets associated or
-// disassociated with the registration.
-IPC_MESSAGE_CONTROL4(ServiceWorkerMsg_AssociateRegistration,
-                     int /* thread_id */,
-                     int /* provider_id */,
-                     content::ServiceWorkerRegistrationObjectInfo,
-                     content::ServiceWorkerVersionAttributes)
-IPC_MESSAGE_CONTROL2(ServiceWorkerMsg_DisassociateRegistration,
-                     int /* thread_id */,
-                     int /* provider_id */)
-
 // Response to ServiceWorkerHostMsg_RegisterServiceWorker.
 IPC_MESSAGE_CONTROL4(ServiceWorkerMsg_ServiceWorkerRegistered,
                      int /* thread_id */,