Revert "[ServiceWorker] Implement ServiceWorkerContainer.SetController"

This reverts commit 24e5c55f.

Revert "[ServiceWorker] Fix the warning for comparison of unsigned expression"

This reverts commit 4d47a13e.

Reverting due to crashes, see bug.

Bug: 774681, 755861
Change-Id: Ib90a143dec1ca9f378a487e330a98a1bb3d7d50c
TBR: xiaofeng.zhang, shimazu, tsepez, kinuko
Reviewed-on: https://chromium-review.googlesource.com/722840Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509559}

content/browser/service_worker/service_worker_provider_host.cc

@@ -573,13 +573,6 @@ ServiceWorkerProviderHost::PrepareForCrossSiteTransfer() {
   DCHECK_EQ(kDocumentMainThreadId, render_thread_id_);
   DCHECK_NE(SERVICE_WORKER_PROVIDER_UNKNOWN, info_.type);
 
-  // Clear the controller from the renderer-side provider, since no one knows
-  // what's going to happen until after cross-site transfer finishes.
-  if (controller_) {
-    SendSetControllerServiceWorker(nullptr,
-                                   false /* notify_controllerchange */);
-  }
-
   std::unique_ptr<ServiceWorkerProviderHost> provisional_host =
       base::WrapUnique(new ServiceWorkerProviderHost(
           process_id(),
@@ -592,6 +585,13 @@ ServiceWorkerProviderHost::PrepareForCrossSiteTransfer() {
 
   RemoveAllMatchingRegistrations();
 
+  // Clear the controller from the renderer-side provider, since no one knows
+  // what's going to happen until after cross-site transfer finishes.
+  if (controller_) {
+    SendSetControllerServiceWorker(nullptr,
+                                   false /* notify_controllerchange */);
+  }
+
   render_process_id_ = ChildProcessHost::kInvalidUniqueID;
   render_thread_id_ = kInvalidEmbeddedWorkerThreadId;
   dispatcher_host_ = nullptr;
@@ -898,20 +898,14 @@ void ServiceWorkerProviderHost::SendSetControllerServiceWorker(
     DCHECK_EQ(controller_.get(), version);
   }
 
-  std::vector<blink::mojom::WebFeature> used_features;
-  if (version) {
-    for (const uint32_t feature : version->used_features()) {
-      // TODO: version->used_features() should never have a feature outside the
-      // known feature range. But there is special case, see the details in
-      // crbug.com/758419.
-      if (feature <
-          static_cast<uint32_t>(blink::mojom::WebFeature::kNumberOfFeatures)) {
-        used_features.push_back(static_cast<blink::mojom::WebFeature>(feature));
-      }
-    }
-  }
-  container_->SetController(GetOrCreateServiceWorkerHandle(version),
-                            used_features, notify_controllerchange);
+  ServiceWorkerMsg_SetControllerServiceWorker_Params params;
+  params.thread_id = render_thread_id_;
+  params.provider_id = provider_id();
+  params.object_info = GetOrCreateServiceWorkerHandle(version);
+  params.should_notify_controllerchange = notify_controllerchange;
+  if (version)
+    params.used_features = version->used_features();
+  Send(new ServiceWorkerMsg_SetControllerServiceWorker(params));
 }
 
 void ServiceWorkerProviderHost::Register(

content/browser/service_worker/service_worker_provider_host_unittest.cc

@@ -36,6 +36,17 @@ namespace content {
 
 namespace {
 
+// Sets the document URL for |host| and associates it with |registration|.
+// A dumb version of
+// ServiceWorkerControlleeRequestHandler::PrepareForMainResource().
+void SimulateServiceWorkerControlleeRequestHandler(
+    ServiceWorkerProviderHost* host,
+    ServiceWorkerRegistration* registration) {
+  host->SetDocumentUrl(GURL("https://www.example.com/page"));
+  host->AssociateRegistration(registration,
+                              false /* notify_controllerchange */);
+}
+
 const char kServiceWorkerScheme[] = "i-can-use-service-worker";
 
 class ServiceWorkerTestContentClient : public TestContentClient {
@@ -461,27 +472,6 @@ TEST_F(ServiceWorkerProviderHostTest, RemoveProvider) {
   EXPECT_FALSE(context_->GetProviderHost(process_id, provider_id));
 }
 
-class MockServiceWorkerContainer : public mojom::ServiceWorkerContainer {
- public:
-  explicit MockServiceWorkerContainer(
-      mojom::ServiceWorkerContainerAssociatedRequest request)
-      : binding_(this, std::move(request)) {}
-
-  ~MockServiceWorkerContainer() override = default;
-
-  void SetController(const ServiceWorkerObjectInfo& controller,
-                     const std::vector<blink::mojom::WebFeature>& used_features,
-                     bool should_notify_controllerchange) override {
-    was_set_controller_called_ = true;
-  }
-
-  bool was_set_controller_called() const { return was_set_controller_called_; }
-
- private:
-  bool was_set_controller_called_ = false;
-  mojo::AssociatedBinding<mojom::ServiceWorkerContainer> binding_;
-};
-
 TEST_F(ServiceWorkerProviderHostTest, Controller) {
   base::CommandLine::ForCurrentProcess()->AppendSwitch(
       switches::kEnableBrowserSideNavigation);
@@ -495,31 +485,27 @@ TEST_F(ServiceWorkerProviderHostTest, Controller) {
                                      true /* is_parent_frame_secure */);
   remote_endpoints_.emplace_back();
   remote_endpoints_.back().BindWithProviderHostInfo(&info);
-  auto container = base::MakeUnique<MockServiceWorkerContainer>(
-      std::move(*remote_endpoints_.back().client_request()));
 
   // Create an active version and then start the navigation.
   scoped_refptr<ServiceWorkerVersion> version = new ServiceWorkerVersion(
       registration1_.get(), GURL("https://www.example.com/sw.js"),
       1 /* version_id */, helper_->context()->AsWeakPtr());
   registration1_->SetActiveVersion(version);
+  SimulateServiceWorkerControlleeRequestHandler(host.get(),
+                                                registration1_.get());
 
   // Finish the navigation.
-  host->SetDocumentUrl(GURL("https://www.example.com/page"));
   host->CompleteNavigationInitialized(
       helper_->mock_render_process_id(), std::move(info),
       helper_->GetDispatcherHostForProcess(helper_->mock_render_process_id())
           ->AsWeakPtr());
 
-  host->AssociateRegistration(registration1_.get(),
-                              false /* notify_controllerchange */);
-  base::RunLoop().RunUntilIdle();
-
   // The page should be controlled since there was an active version at the
   // time navigation started. The SetController IPC should have been sent.
   EXPECT_TRUE(host->active_version());
   EXPECT_EQ(host->active_version(), host->controller());
-  EXPECT_TRUE(container->was_set_controller_called());
+  EXPECT_TRUE(helper_->ipc_sink()->GetUniqueMessageMatching(
+      ServiceWorkerMsg_SetControllerServiceWorker::ID));
 }
 
 TEST_F(ServiceWorkerProviderHostTest, ActiveIsNotController) {
@@ -535,34 +521,31 @@ TEST_F(ServiceWorkerProviderHostTest, ActiveIsNotController) {
                                      true /* is_parent_frame_secure */);
   remote_endpoints_.emplace_back();
   remote_endpoints_.back().BindWithProviderHostInfo(&info);
-  auto container = base::MakeUnique<MockServiceWorkerContainer>(
-      std::move(*remote_endpoints_.back().client_request()));
 
-  // Create an installing version and then start the navigation.
+  // Associate it with an installing registration then start the navigation.
   scoped_refptr<ServiceWorkerVersion> version = new ServiceWorkerVersion(
       registration1_.get(), GURL("https://www.example.com/sw.js"),
       1 /* version_id */, helper_->context()->AsWeakPtr());
   registration1_->SetInstallingVersion(version);
+  SimulateServiceWorkerControlleeRequestHandler(host.get(),
+                                                registration1_.get());
+
+  // Promote the worker to active while navigation is still happening.
+  registration1_->SetActiveVersion(version);
 
   // Finish the navigation.
-  host->SetDocumentUrl(GURL("https://www.example.com/page"));
   host->CompleteNavigationInitialized(
       helper_->mock_render_process_id(), std::move(info),
       helper_->GetDispatcherHostForProcess(helper_->mock_render_process_id())
           ->AsWeakPtr());
 
-  host->AssociateRegistration(registration1_.get(),
-                              false /* notify_controllerchange */);
-  // Promote the worker to active while navigation is still happening.
-  registration1_->SetActiveVersion(version);
-  base::RunLoop().RunUntilIdle();
-
   // The page should not be controlled since there was no active version at the
   // time navigation started. Furthermore, no SetController IPC should have been
   // sent.
   EXPECT_TRUE(host->active_version());
   EXPECT_FALSE(host->controller());
-  EXPECT_FALSE(container->was_set_controller_called());
+  EXPECT_EQ(nullptr, helper_->ipc_sink()->GetFirstMessageMatching(
+                         ServiceWorkerMsg_SetControllerServiceWorker::ID));
 }
 
 TEST_F(ServiceWorkerProviderHostTest,

content/child/service_worker/service_worker_dispatcher.cc

@@ -90,6 +90,8 @@ void ServiceWorkerDispatcher::OnMessageReceived(const IPC::Message& msg) {
                         OnSetVersionAttributes)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_UpdateFound,
                         OnUpdateFound)
+    IPC_MESSAGE_HANDLER(ServiceWorkerMsg_SetControllerServiceWorker,
+                        OnSetControllerServiceWorker)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_MessageToDocument,
                         OnPostMessage)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_CountFeature, OnCountFeature)
@@ -171,18 +173,9 @@ void ServiceWorkerDispatcher::AddProviderClient(
 }
 
 void ServiceWorkerDispatcher::RemoveProviderClient(int provider_id) {
-  auto iter = provider_clients_.find(provider_id);
   // This could be possibly called multiple times to ensure termination.
-  if (iter != provider_clients_.end())
-    provider_clients_.erase(iter);
-}
-
-blink::WebServiceWorkerProviderClient*
-ServiceWorkerDispatcher::GetProviderClient(int provider_id) {
-  auto iter = provider_clients_.find(provider_id);
-  if (iter != provider_clients_.end())
-    return iter->second;
-  return nullptr;
+  if (base::ContainsKey(provider_clients_, provider_id))
+    provider_clients_.erase(provider_id);
 }
 
 ServiceWorkerDispatcher*
@@ -211,11 +204,6 @@ ServiceWorkerDispatcher* ServiceWorkerDispatcher::GetThreadSpecificInstance() {
       g_dispatcher_tls.Pointer()->Get());
 }
 
-std::unique_ptr<ServiceWorkerHandleReference> ServiceWorkerDispatcher::Adopt(
-    const ServiceWorkerObjectInfo& info) {
-  return ServiceWorkerHandleReference::Adopt(info, thread_safe_sender_.get());
-}
-
 void ServiceWorkerDispatcher::WillStopCurrentWorkerThread() {
   delete this;
 }
@@ -474,6 +462,43 @@ void ServiceWorkerDispatcher::OnUpdateFound(
     found->second->OnUpdateFound();
 }
 
+void ServiceWorkerDispatcher::OnSetControllerServiceWorker(
+    const ServiceWorkerMsg_SetControllerServiceWorker_Params& params) {
+  TRACE_EVENT2(
+      "ServiceWorker", "ServiceWorkerDispatcher::OnSetControllerServiceWorker",
+      "Thread ID", params.thread_id, "Provider ID", params.provider_id);
+
+  // Adopt the reference sent from the browser process and pass it to the
+  // provider context if it exists.
+  std::unique_ptr<ServiceWorkerHandleReference> handle_ref =
+      Adopt(params.object_info);
+  ProviderContextMap::iterator provider =
+      provider_contexts_.find(params.provider_id);
+  if (provider != provider_contexts_.end()) {
+    provider->second->SetController(std::move(handle_ref),
+                                    params.used_features);
+  }
+
+  ProviderClientMap::iterator found =
+      provider_clients_.find(params.provider_id);
+  if (found != provider_clients_.end()) {
+    // Sync the controllee's use counter with the service worker's one.
+    for (uint32_t feature : params.used_features)
+      found->second->CountFeature(feature);
+
+    // Get the existing worker object or create a new one with a new reference
+    // to populate the .controller field.
+    scoped_refptr<WebServiceWorkerImpl> worker =
+        GetOrCreateServiceWorker(ServiceWorkerHandleReference::Create(
+            params.object_info, thread_safe_sender_.get()));
+    found->second->SetController(WebServiceWorkerImpl::CreateHandle(worker),
+                                 params.should_notify_controllerchange);
+    // You must not access |found| after setController() because it may fire the
+    // controllerchange event that may remove the provider client, for example,
+    // by detaching an iframe.
+  }
+}
+
 void ServiceWorkerDispatcher::OnPostMessage(
     const ServiceWorkerMsg_MessageToDocument_Params& params) {
   // Make sure we're on the main document thread. (That must be the only
@@ -538,4 +563,9 @@ void ServiceWorkerDispatcher::RemoveServiceWorkerRegistration(
   registrations_.erase(registration_handle_id);
 }
 
+std::unique_ptr<ServiceWorkerHandleReference> ServiceWorkerDispatcher::Adopt(
+    const ServiceWorkerObjectInfo& info) {
+  return ServiceWorkerHandleReference::Adopt(info, thread_safe_sender_.get());
+}
+
 }  // namespace content

content/child/service_worker/service_worker_dispatcher.h

@@ -105,8 +105,6 @@ class CONTENT_EXPORT ServiceWorkerDispatcher : public WorkerThread::Observer {
                          blink::WebServiceWorkerProviderClient* client);
   void RemoveProviderClient(int provider_id);
 
-  blink::WebServiceWorkerProviderClient* GetProviderClient(int provider_id);
-
   // Returns the existing service worker or a newly created one with the given
   // handle reference. Returns nullptr if the given reference is invalid.
   scoped_refptr<WebServiceWorkerImpl> GetOrCreateServiceWorker(
@@ -138,11 +136,6 @@ class CONTENT_EXPORT ServiceWorkerDispatcher : public WorkerThread::Observer {
   // instance if thread-local instance doesn't exist.
   static ServiceWorkerDispatcher* GetThreadSpecificInstance();
 
-  // Assumes that the given object information retains an interprocess handle
-  // reference passed from the browser process, and adopts it.
-  std::unique_ptr<ServiceWorkerHandleReference> Adopt(
-      const ServiceWorkerObjectInfo& info);
-
   base::SingleThreadTaskRunner* main_thread_task_runner() {
     return main_thread_task_runner_.get();
   }
@@ -224,6 +217,11 @@ class CONTENT_EXPORT ServiceWorkerDispatcher : public WorkerThread::Observer {
   void RemoveServiceWorkerRegistration(
       int registration_handle_id);
 
+  // Assumes that the given object information retains an interprocess handle
+  // reference passed from the browser process, and adopts it.
+  std::unique_ptr<ServiceWorkerHandleReference> Adopt(
+      const ServiceWorkerObjectInfo& info);
+
   UnregistrationCallbackMap pending_unregistration_callbacks_;
   EnableNavigationPreloadCallbackMap enable_navigation_preload_callbacks_;
   GetNavigationPreloadStateCallbackMap get_navigation_preload_state_callbacks_;

content/child/service_worker/service_worker_dispatcher_unittest.cc

@@ -104,6 +104,20 @@ class ServiceWorkerDispatcherTest : public testing::Test {
     return ContainsKey(dispatcher_->registrations_, registration_handle_id);
   }
 
+  void OnSetControllerServiceWorker(int thread_id,
+                                    int provider_id,
+                                    const ServiceWorkerObjectInfo& info,
+                                    bool should_notify_controllerchange,
+                                    const std::set<uint32_t>& used_features) {
+    ServiceWorkerMsg_SetControllerServiceWorker_Params params;
+    params.thread_id = thread_id;
+    params.provider_id = provider_id;
+    params.object_info = info;
+    params.should_notify_controllerchange = should_notify_controllerchange;
+    params.used_features = used_features;
+    dispatcher_->OnSetControllerServiceWorker(params);
+  }
+
   void OnPostMessage(const ServiceWorkerMsg_MessageToDocument_Params& params) {
     dispatcher_->OnPostMessage(params);
   }
@@ -145,7 +159,11 @@ class MockWebServiceWorkerProviderClientImpl
   }
 
   void SetController(std::unique_ptr<blink::WebServiceWorker::Handle> handle,
-                     bool should_notify_controller_change) override {}
+                     bool shouldNotifyControllerChange) override {
+    // WebPassOwnPtr cannot be owned in Chromium, so drop the handle here.
+    // The destruction releases ServiceWorkerHandleReference.
+    is_set_controlled_called_ = true;
+  }
 
   void DispatchMessageEvent(
       std::unique_ptr<blink::WebServiceWorker::Handle> handle,
@@ -160,17 +178,136 @@ class MockWebServiceWorkerProviderClientImpl
     used_features_.insert(feature);
   }
 
+  bool is_set_controlled_called() const { return is_set_controlled_called_; }
+
   bool is_dispatch_message_event_called() const {
     return is_dispatch_message_event_called_;
   }
 
  private:
   const int provider_id_;
+  bool is_set_controlled_called_ = false;
   bool is_dispatch_message_event_called_ = false;
   ServiceWorkerDispatcher* dispatcher_;
   std::set<uint32_t> used_features_;
 };
 
+TEST_F(ServiceWorkerDispatcherTest, OnSetControllerServiceWorker) {
+  const int kProviderId = 10;
+  bool should_notify_controllerchange = true;
+
+  // Assume that these objects are passed from the browser process and own
+  // references to browser-side registration/worker representations.
+  blink::mojom::ServiceWorkerRegistrationObjectInfoPtr info;
+  ServiceWorkerVersionAttributes attrs;
+  CreateObjectInfoAndVersionAttributes(&info, &attrs);
+
+  // (1) In the case there are no SWProviderContext and WebSWProviderClient for
+  // the provider, the passed reference to the active worker should be adopted
+  // but immediately released because there is no provider context to own it.
+  OnSetControllerServiceWorker(kDocumentMainThreadId, kProviderId, attrs.active,
+                               should_notify_controllerchange,
+                               std::set<uint32_t>());
+  ASSERT_EQ(1UL, ipc_sink()->message_count());
+  EXPECT_EQ(ServiceWorkerHostMsg_DecrementServiceWorkerRefCount::ID,
+            ipc_sink()->GetMessageAt(0)->type());
+  ipc_sink()->ClearMessages();
+
+  // (2) In the case there is no WebSWProviderClient but SWProviderContext for
+  // the provider, the passed referecence should be adopted and owned by the
+  // provider context.
+  auto provider_context = base::MakeRefCounted<ServiceWorkerProviderContext>(
+      kProviderId, SERVICE_WORKER_PROVIDER_FOR_WINDOW,
+      nullptr /* provider_request */, nullptr /* host_ptr_info */, dispatcher(),
+      nullptr /* loader_factory_getter */);
+  ipc_sink()->ClearMessages();
+  OnSetControllerServiceWorker(kDocumentMainThreadId, kProviderId, attrs.active,
+                               should_notify_controllerchange,
+                               std::set<uint32_t>());
+  EXPECT_EQ(0UL, ipc_sink()->message_count());
+
+  // Destruction of the provider context should release references to the
+  // associated registration and the controller.
+  provider_context = nullptr;
+  ASSERT_EQ(1UL, ipc_sink()->message_count());
+  EXPECT_EQ(ServiceWorkerHostMsg_DecrementServiceWorkerRefCount::ID,
+            ipc_sink()->GetMessageAt(0)->type());
+  ipc_sink()->ClearMessages();
+
+  // (3) In the case there is no SWProviderContext but WebSWProviderClient for
+  // the provider, the new reference should be created and owned by the provider
+  // client (but the reference is immediately released due to limitation of the
+  // mock provider client. See the comment on setController() of the mock).
+  // In addition, the passed reference should be adopted but immediately
+  // released because there is no provider context to own it.
+  std::unique_ptr<MockWebServiceWorkerProviderClientImpl> provider_client(
+      new MockWebServiceWorkerProviderClientImpl(kProviderId, dispatcher()));
+  ASSERT_FALSE(provider_client->is_set_controlled_called());
+  OnSetControllerServiceWorker(kDocumentMainThreadId, kProviderId, attrs.active,
+                               should_notify_controllerchange,
+                               std::set<uint32_t>());
+  EXPECT_TRUE(provider_client->is_set_controlled_called());
+  ASSERT_EQ(3UL, ipc_sink()->message_count());
+  EXPECT_EQ(ServiceWorkerHostMsg_IncrementServiceWorkerRefCount::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());
+  provider_client.reset();
+  ipc_sink()->ClearMessages();
+
+  // (4) In the case there are both SWProviderContext and SWProviderClient for
+  // the provider, the passed referecence should be adopted and owned by the
+  // provider context. In addition, the new reference should be created for the
+  // provider client and immediately released due to limitation of the mock
+  // implementation.
+  provider_context = base::MakeRefCounted<ServiceWorkerProviderContext>(
+      kProviderId, SERVICE_WORKER_PROVIDER_FOR_WINDOW,
+      nullptr /* provider_request */, nullptr /* host_ptr_info */, dispatcher(),
+      nullptr /* loader_factory_getter */);
+  provider_client.reset(
+      new MockWebServiceWorkerProviderClientImpl(kProviderId, dispatcher()));
+  ASSERT_FALSE(provider_client->is_set_controlled_called());
+  ipc_sink()->ClearMessages();
+  OnSetControllerServiceWorker(kDocumentMainThreadId, kProviderId, attrs.active,
+                               should_notify_controllerchange,
+                               std::set<uint32_t>());
+  EXPECT_TRUE(provider_client->is_set_controlled_called());
+  ASSERT_EQ(2UL, ipc_sink()->message_count());
+  EXPECT_EQ(ServiceWorkerHostMsg_IncrementServiceWorkerRefCount::ID,
+            ipc_sink()->GetMessageAt(0)->type());
+  EXPECT_EQ(ServiceWorkerHostMsg_DecrementServiceWorkerRefCount::ID,
+            ipc_sink()->GetMessageAt(1)->type());
+}
+
+// Test that clearing the controller by sending a kInvalidServiceWorkerHandle
+// results in the provider context having a null controller.
+TEST_F(ServiceWorkerDispatcherTest, OnSetControllerServiceWorker_Null) {
+  const int kProviderId = 10;
+  bool should_notify_controllerchange = true;
+
+  blink::mojom::ServiceWorkerRegistrationObjectInfoPtr info;
+  ServiceWorkerVersionAttributes attrs;
+  CreateObjectInfoAndVersionAttributes(&info, &attrs);
+
+  std::unique_ptr<MockWebServiceWorkerProviderClientImpl> provider_client(
+      new MockWebServiceWorkerProviderClientImpl(kProviderId, dispatcher()));
+  auto provider_context = base::MakeRefCounted<ServiceWorkerProviderContext>(
+      kProviderId, SERVICE_WORKER_PROVIDER_FOR_WINDOW,
+      nullptr /* provider_request */, nullptr /* host_ptr_info */, dispatcher(),
+      nullptr /* loader_factory_getter */);
+
+  // Set the controller to kInvalidServiceWorkerHandle.
+  OnSetControllerServiceWorker(
+      kDocumentMainThreadId, kProviderId, ServiceWorkerObjectInfo(),
+      should_notify_controllerchange, std::set<uint32_t>());
+
+  // Check that it became null.
+  EXPECT_EQ(nullptr, provider_context->controller());
+  EXPECT_TRUE(provider_client->is_set_controlled_called());
+}
+
 TEST_F(ServiceWorkerDispatcherTest, OnPostMessage) {
   const int kProviderId = 10;
 

content/child/service_worker/service_worker_message_filter.cc

@@ -60,6 +60,8 @@ void ServiceWorkerMessageFilter::OnStaleMessageReceived(
   IPC_BEGIN_MESSAGE_MAP(ServiceWorkerMessageFilter, msg)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_SetVersionAttributes,
                         OnStaleSetVersionAttributes)
+    IPC_MESSAGE_HANDLER(ServiceWorkerMsg_SetControllerServiceWorker,
+                        OnStaleSetControllerServiceWorker)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_MessageToDocument,
                         OnStaleMessageToDocument)
   IPC_END_MESSAGE_MAP()
@@ -80,6 +82,12 @@ void ServiceWorkerMessageFilter::OnStaleSetVersionAttributes(
   // SetVersionAttributes message doesn't increment it.
 }
 
+void ServiceWorkerMessageFilter::OnStaleSetControllerServiceWorker(
+    const ServiceWorkerMsg_SetControllerServiceWorker_Params& params) {
+  SendServiceWorkerObjectDestroyed(thread_safe_sender(),
+                                   params.object_info.handle_id);
+}
+
 void ServiceWorkerMessageFilter::OnStaleMessageToDocument(
     const ServiceWorkerMsg_MessageToDocument_Params& params) {
   SendServiceWorkerObjectDestroyed(thread_safe_sender(),

content/child/service_worker/service_worker_message_filter.h

@@ -13,6 +13,7 @@
 #include "content/common/content_export.h"
 
 struct ServiceWorkerMsg_MessageToDocument_Params;
+struct ServiceWorkerMsg_SetControllerServiceWorker_Params;
 
 namespace content {
 
@@ -42,6 +43,8 @@ class CONTENT_EXPORT ServiceWorkerMessageFilter
       int registration_handle_id,
       int changed_mask,
       const ServiceWorkerVersionAttributes& attrs);
+  void OnStaleSetControllerServiceWorker(
+      const ServiceWorkerMsg_SetControllerServiceWorker_Params& params);
   void OnStaleMessageToDocument(
       const ServiceWorkerMsg_MessageToDocument_Params& params);
 

content/child/service_worker/service_worker_provider_context.cc

@@ -17,7 +17,6 @@
 #include "content/child/service_worker/service_worker_dispatcher.h"
 #include "content/child/service_worker/service_worker_handle_reference.h"
 #include "content/child/service_worker/service_worker_subresource_loader.h"
-#include "content/child/service_worker/web_service_worker_impl.h"
 #include "content/child/thread_safe_sender.h"
 #include "content/child/worker_thread_registry.h"
 #include "content/common/service_worker/service_worker_utils.h"
@@ -55,10 +54,6 @@ struct ServiceWorkerProviderContext::ControlleeState {
   // Tracks feature usage for UseCounter.
   std::set<uint32_t> used_features;
 
-  // Corresponds to a ServiceWorkerContainer. We notify it when
-  // ServiceWorkerContainer#controller should be changed.
-  base::WeakPtr<WebServiceWorkerProviderImpl> web_service_worker_provider;
-
   // Keeps ServiceWorkerWorkerClient pointers of dedicated or shared workers
   // which are associated with the ServiceWorkerProviderContext.
   // - If this ServiceWorkerProviderContext is for a Document, then
@@ -163,6 +158,50 @@ void ServiceWorkerProviderContext::TakeRegistrationForServiceWorkerGlobalScope(
     attrs->active = state->active->info();
 }
 
+void ServiceWorkerProviderContext::SetController(
+    std::unique_ptr<ServiceWorkerHandleReference> controller,
+    const std::set<uint32_t>& used_features) {
+  DCHECK(main_thread_task_runner_->RunsTasksInCurrentSequence());
+  ControlleeState* state = controllee_state_.get();
+  DCHECK(state);
+  DCHECK(!state->controller ||
+         state->controller->handle_id() != kInvalidServiceWorkerHandleId);
+
+  state->controller = std::move(controller);
+  state->used_features = used_features;
+
+  // Propagate the controller to workers related to this provider.
+  if (state->controller) {
+    for (const auto& worker : state->worker_clients) {
+      // This is a Mojo interface call to the (dedicated or shared) worker
+      // thread.
+      worker->SetControllerServiceWorker(state->controller->version_id());
+    }
+  }
+
+  // S13nServiceWorker
+  // Set up the URL loader factory for sending URL requests to the controller.
+  if (!ServiceWorkerUtils::IsServicificationEnabled() || !state->controller) {
+    state->controller_connector = nullptr;
+    state->subresource_loader_factory = nullptr;
+    return;
+  }
+  storage::mojom::BlobRegistryPtr blob_registry_ptr;
+  ChildThreadImpl::current()->GetConnector()->BindInterface(
+      mojom::kBrowserServiceName, mojo::MakeRequest(&blob_registry_ptr));
+  auto blob_registry = base::MakeRefCounted<
+      base::RefCountedData<storage::mojom::BlobRegistryPtr>>(
+      std::move(blob_registry_ptr));
+  state->controller_connector =
+      base::MakeRefCounted<ControllerServiceWorkerConnector>(
+          container_host_.get());
+  mojo::MakeStrongBinding(
+      base::MakeUnique<ServiceWorkerSubresourceLoaderFactory>(
+          state->controller_connector, state->default_loader_factory_getter,
+          state->controller->url().GetOrigin(), std::move(blob_registry)),
+      mojo::MakeRequest(&state->subresource_loader_factory));
+}
+
 ServiceWorkerHandleReference* ServiceWorkerProviderContext::controller() {
   DCHECK(main_thread_task_runner_->RunsTasksInCurrentSequence());
   DCHECK(controllee_state_);
@@ -194,12 +233,6 @@ const std::set<uint32_t>& ServiceWorkerProviderContext::used_features() const {
   return controllee_state_->used_features;
 }
 
-void ServiceWorkerProviderContext::SetWebServiceWorkerProvider(
-    base::WeakPtr<WebServiceWorkerProviderImpl> provider) {
-  DCHECK(controllee_state_);
-  controllee_state_->web_service_worker_provider = provider;
-}
-
 mojom::ServiceWorkerWorkerClientRequest
 ServiceWorkerProviderContext::CreateWorkerClientRequest() {
   DCHECK(main_thread_task_runner_->RunsTasksInCurrentSequence());
@@ -224,63 +257,6 @@ void ServiceWorkerProviderContext::UnregisterWorkerFetchContext(
       });
 }
 
-void ServiceWorkerProviderContext::SetController(
-    const ServiceWorkerObjectInfo& controller,
-    const std::vector<blink::mojom::WebFeature>& used_features,
-    bool should_notify_controllerchange) {
-  DCHECK(main_thread_task_runner_->RunsTasksInCurrentSequence());
-  ControlleeState* state = controllee_state_.get();
-  DCHECK(state);
-  DCHECK(!state->controller ||
-         state->controller->handle_id() != kInvalidServiceWorkerHandleId);
-  ServiceWorkerDispatcher* dispatcher =
-      ServiceWorkerDispatcher::GetThreadSpecificInstance();
-
-  state->controller = dispatcher->Adopt(controller);
-
-  // Propagate the controller to workers related to this provider.
-  if (state->controller) {
-    for (const auto& worker : state->worker_clients) {
-      // This is a Mojo interface call to the (dedicated or shared) worker
-      // thread.
-      worker->SetControllerServiceWorker(state->controller->version_id());
-    }
-  }
-  for (blink::mojom::WebFeature feature : used_features)
-    state->used_features.insert(static_cast<uint32_t>(feature));
-
-  // S13nServiceWorker
-  // Set up the URL loader factory for sending URL requests to the controller.
-  if (!ServiceWorkerUtils::IsServicificationEnabled() || !state->controller) {
-    state->controller_connector = nullptr;
-    state->subresource_loader_factory = nullptr;
-  } else {
-    storage::mojom::BlobRegistryPtr blob_registry_ptr;
-    ChildThreadImpl::current()->GetConnector()->BindInterface(
-        mojom::kBrowserServiceName, mojo::MakeRequest(&blob_registry_ptr));
-    auto blob_registry = base::MakeRefCounted<
-        base::RefCountedData<storage::mojom::BlobRegistryPtr>>();
-    blob_registry->data = std::move(blob_registry_ptr);
-    state->controller_connector =
-        base::MakeRefCounted<ControllerServiceWorkerConnector>(
-            container_host_.get());
-    mojo::MakeStrongBinding(
-        base::MakeUnique<ServiceWorkerSubresourceLoaderFactory>(
-            state->controller_connector, state->default_loader_factory_getter,
-            state->controller->url().GetOrigin(), std::move(blob_registry)),
-        mojo::MakeRequest(&state->subresource_loader_factory));
-  }
-
-  // The WebServiceWorkerProviderImpl might not exist yet because the document
-  // has not yet been created (as WebServiceWorkerImpl is created for a
-  // ServiceWorkerContainer). In that case, once it's created it will still get
-  // the controller from |this| via WebServiceWorkerProviderImpl::SetClient().
-  if (state->web_service_worker_provider) {
-    state->web_service_worker_provider->SetController(
-        controller, state->used_features, should_notify_controllerchange);
-  }
-}
-
 void ServiceWorkerProviderContext::OnNetworkProviderDestroyed() {
   container_host_.reset();
   if (controllee_state_ && controllee_state_->controller_connector)

content/child/service_worker/service_worker_provider_context.h

@@ -11,14 +11,12 @@
 #include "base/memory/ref_counted.h"
 #include "base/sequenced_task_runner_helpers.h"
 #include "content/child/service_worker/service_worker_dispatcher.h"
-#include "content/child/service_worker/web_service_worker_provider_impl.h"
 #include "content/common/content_export.h"
 #include "content/common/service_worker/service_worker_container.mojom.h"
 #include "content/common/service_worker/service_worker_provider.mojom.h"
 #include "content/common/service_worker/service_worker_types.h"
 #include "content/public/child/child_url_loader_factory_getter.h"
 #include "mojo/public/cpp/bindings/associated_binding.h"
-#include "third_party/WebKit/public/platform/modules/serviceworker/WebServiceWorkerProviderClient.h"
 #include "third_party/WebKit/public/platform/modules/serviceworker/service_worker_registration.mojom.h"
 
 namespace base {
@@ -104,6 +102,8 @@ class CONTENT_EXPORT ServiceWorkerProviderContext
 
   // For service worker clients. The controller for
   // ServiceWorkerContainer#controller.
+  void SetController(std::unique_ptr<ServiceWorkerHandleReference> controller,
+                     const std::set<uint32_t>& used_features);
   ServiceWorkerHandleReference* controller();
 
   // S13nServiceWorker:
@@ -115,14 +115,6 @@ class CONTENT_EXPORT ServiceWorkerProviderContext
   void CountFeature(uint32_t feature);
   const std::set<uint32_t>& used_features() const;
 
-  // For service worker clients. Sets a weak pointer back to the
-  // WebServiceWorkerProviderImpl (which corresponds to ServiceWorkerContainer
-  // in JavaScript) which has a strong reference to |this|. This allows us to
-  // notify the WebServiceWorkerProviderImpl when
-  // ServiceWorkerContainer#controller should be changed.
-  void SetWebServiceWorkerProvider(
-      base::WeakPtr<WebServiceWorkerProviderImpl> provider);
-
   // For service worker clients. Creates a ServiceWorkerWorkerClientRequest
   // which can be used to bind with a WorkerFetchContextImpl in a (dedicated or
   // shared) worker thread and receive SetControllerServiceWorker() method call
@@ -155,7 +147,6 @@ class CONTENT_EXPORT ServiceWorkerProviderContext
   friend class base::RefCountedThreadSafe<ServiceWorkerProviderContext,
                                           ServiceWorkerProviderContextDeleter>;
   friend struct ServiceWorkerProviderContextDeleter;
-
   struct ControlleeState;
   struct ControllerState;
 
@@ -166,11 +157,6 @@ class CONTENT_EXPORT ServiceWorkerProviderContext
   // shared) worker, when the connection to the worker is disconnected.
   void UnregisterWorkerFetchContext(mojom::ServiceWorkerWorkerClient*);
 
-  // Implementation of mojom::ServiceWorkerContainer.
-  void SetController(const ServiceWorkerObjectInfo& controller,
-                     const std::vector<blink::mojom::WebFeature>& used_features,
-                     bool should_notify_controllerchange) override;
-
   const ServiceWorkerProviderType provider_type_;
   const int provider_id_;
   scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner_;

content/child/service_worker/service_worker_provider_context_unittest.cc

@@ -21,7 +21,6 @@
 #include "ipc/ipc_test_sink.h"
 #include "mojo/public/cpp/bindings/associated_binding_set.h"
 #include "testing/gtest/include/gtest/gtest.h"
-#include "third_party/WebKit/public/platform/modules/serviceworker/WebServiceWorkerProviderClient.h"
 #include "third_party/WebKit/public/platform/modules/serviceworker/service_worker_error_type.mojom.h"
 #include "third_party/WebKit/public/platform/modules/serviceworker/service_worker_registration.mojom.h"
 
@@ -53,41 +52,6 @@ class MockServiceWorkerRegistrationObjectHost
       bindings_;
 };
 
-class MockWebServiceWorkerProviderClientImpl
-    : public blink::WebServiceWorkerProviderClient {
- public:
-  MockWebServiceWorkerProviderClientImpl() {}
-
-  ~MockWebServiceWorkerProviderClientImpl() override {}
-
-  void SetController(std::unique_ptr<blink::WebServiceWorker::Handle> handle,
-                     bool should_notify_controller_change) override {
-    was_set_controller_called_ = true;
-  }
-
-  void DispatchMessageEvent(
-      std::unique_ptr<blink::WebServiceWorker::Handle> handle,
-      const blink::WebString& message,
-      blink::WebVector<blink::MessagePortChannel> ports) override {
-    was_dispatch_message_event_called_ = true;
-  }
-
-  void CountFeature(uint32_t feature) override {
-    used_features_.insert(feature);
-  }
-
-  bool was_set_controller_called() const { return was_set_controller_called_; }
-
-  bool was_dispatch_message_event_called() const {
-    return was_dispatch_message_event_called_;
-  }
-
- private:
-  bool was_set_controller_called_ = false;
-  bool was_dispatch_message_event_called_ = false;
-  std::set<uint32_t> used_features_;
-};
-
 class ServiceWorkerTestSender : public ThreadSafeSender {
  public:
   explicit ServiceWorkerTestSender(IPC::TestSink* ipc_sink)
@@ -194,106 +158,4 @@ TEST_F(ServiceWorkerProviderContextTest, CreateForController) {
   EXPECT_EQ(0, remote_registration_object_host().GetBindingCount());
 }
 
-TEST_F(ServiceWorkerProviderContextTest, SetController) {
-  const int kProviderId = 10;
-
-  // Assume that these objects are passed from the browser process and own
-  // references to browser-side registration/worker representations.
-  blink::mojom::ServiceWorkerRegistrationObjectInfoPtr info;
-  ServiceWorkerVersionAttributes attrs;
-  CreateObjectInfoAndVersionAttributes(&info, &attrs);
-
-  {
-    // (1) In the case there is no WebSWProviderClient but SWProviderContext for
-    // the provider, the passed reference should be adopted and owned by the
-    // provider context.
-    mojom::ServiceWorkerContainerAssociatedPtr container_ptr;
-    mojom::ServiceWorkerContainerAssociatedRequest container_request =
-        mojo::MakeIsolatedRequest(&container_ptr);
-    auto provider_context = base::MakeRefCounted<ServiceWorkerProviderContext>(
-        kProviderId, SERVICE_WORKER_PROVIDER_FOR_WINDOW,
-        std::move(container_request), nullptr /* host_ptr_info */, dispatcher(),
-        nullptr /* loader_factory_getter */);
-
-    ipc_sink()->ClearMessages();
-    container_ptr->SetController(attrs.active,
-                                 std::vector<blink::mojom::WebFeature>(), true);
-    base::RunLoop().RunUntilIdle();
-    EXPECT_EQ(0UL, ipc_sink()->message_count());
-
-    // Destruction of the provider context should release references to the
-    // the controller.
-    provider_context = nullptr;
-    ASSERT_EQ(1UL, ipc_sink()->message_count());
-    EXPECT_EQ(ServiceWorkerHostMsg_DecrementServiceWorkerRefCount::ID,
-              ipc_sink()->GetMessageAt(0)->type());
-    ipc_sink()->ClearMessages();
-  }
-
-  {
-    // (2) In the case there are both SWProviderContext and SWProviderClient for
-    // the provider, the passed reference should be adopted and owned by the
-    // provider context. In addition, the new reference should be created for
-    // the provider client and immediately released due to limitation of the
-    // mock implementation.
-    mojom::ServiceWorkerContainerHostAssociatedPtrInfo host_ptr_info;
-    mojom::ServiceWorkerContainerHostAssociatedRequest host_request =
-        mojo::MakeRequest(&host_ptr_info);
-
-    mojom::ServiceWorkerContainerAssociatedPtr container_ptr;
-    mojom::ServiceWorkerContainerAssociatedRequest container_request =
-        mojo::MakeIsolatedRequest(&container_ptr);
-    auto provider_context = base::MakeRefCounted<ServiceWorkerProviderContext>(
-        kProviderId, SERVICE_WORKER_PROVIDER_FOR_WINDOW,
-        std::move(container_request), std::move(host_ptr_info), dispatcher(),
-        nullptr /* loader_factory_getter */);
-    auto provider_impl = base::MakeUnique<WebServiceWorkerProviderImpl>(
-        thread_safe_sender(), provider_context.get());
-    auto client = base::MakeUnique<MockWebServiceWorkerProviderClientImpl>();
-    provider_impl->SetClient(client.get());
-    ASSERT_FALSE(client->was_set_controller_called());
-
-    ipc_sink()->ClearMessages();
-    container_ptr->SetController(attrs.active,
-                                 std::vector<blink::mojom::WebFeature>(), true);
-    base::RunLoop().RunUntilIdle();
-
-    EXPECT_TRUE(client->was_set_controller_called());
-    ASSERT_EQ(2UL, ipc_sink()->message_count());
-    EXPECT_EQ(ServiceWorkerHostMsg_IncrementServiceWorkerRefCount::ID,
-              ipc_sink()->GetMessageAt(0)->type());
-    EXPECT_EQ(ServiceWorkerHostMsg_DecrementServiceWorkerRefCount::ID,
-              ipc_sink()->GetMessageAt(1)->type());
-  }
-}
-
-// Test that clearing the controller by sending a kInvalidServiceWorkerHandle
-// results in the provider context having a null controller.
-TEST_F(ServiceWorkerProviderContextTest, SetController_Null) {
-  const int kProviderId = 10;
-
-  mojom::ServiceWorkerContainerHostAssociatedPtrInfo host_ptr_info;
-  mojom::ServiceWorkerContainerHostAssociatedRequest host_request =
-      mojo::MakeRequest(&host_ptr_info);
-
-  mojom::ServiceWorkerContainerAssociatedPtr container_ptr;
-  mojom::ServiceWorkerContainerAssociatedRequest container_request =
-      mojo::MakeIsolatedRequest(&container_ptr);
-  auto provider_context = base::MakeRefCounted<ServiceWorkerProviderContext>(
-      kProviderId, SERVICE_WORKER_PROVIDER_FOR_WINDOW,
-      std::move(container_request), std::move(host_ptr_info), dispatcher(),
-      nullptr /* loader_factory_getter */);
-  auto provider_impl = base::MakeUnique<WebServiceWorkerProviderImpl>(
-      thread_safe_sender(), provider_context.get());
-  auto client = base::MakeUnique<MockWebServiceWorkerProviderClientImpl>();
-  provider_impl->SetClient(client.get());
-
-  container_ptr->SetController(ServiceWorkerObjectInfo(),
-                               std::vector<blink::mojom::WebFeature>(), true);
-  base::RunLoop().RunUntilIdle();
-
-  EXPECT_EQ(nullptr, provider_context->controller());
-  EXPECT_TRUE(client->was_set_controller_called());
-}
-
 }  // namespace content

content/child/service_worker/web_service_worker_provider_impl.cc

@@ -39,20 +39,8 @@ WebServiceWorkerProviderImpl::WebServiceWorkerProviderImpl(
       context_(context),
       weak_factory_(this) {
   DCHECK(context_);
-  switch (context_->provider_type()) {
-    case SERVICE_WORKER_PROVIDER_FOR_WINDOW:
-      DCHECK(context_->container_host());
-      context_->SetWebServiceWorkerProvider(weak_factory_.GetWeakPtr());
-      break;
-    case SERVICE_WORKER_PROVIDER_FOR_CONTROLLER:
-      // Do nothing.
-      break;
-    case SERVICE_WORKER_PROVIDER_FOR_WORKER:
-    case SERVICE_WORKER_PROVIDER_FOR_SHARED_WORKER:
-    case SERVICE_WORKER_PROVIDER_UNKNOWN:
-      NOTREACHED() << "Unimplemented type: " << context_->provider_type();
-      break;
-  }
+  DCHECK(context_->provider_type() != SERVICE_WORKER_PROVIDER_FOR_WINDOW ||
+         context_->container_host());
 }
 
 WebServiceWorkerProviderImpl::~WebServiceWorkerProviderImpl() {
@@ -76,8 +64,17 @@ void WebServiceWorkerProviderImpl::SetClient(
 
   if (!context_->controller())
     return;
-  SetController(context_->controller()->info(), context_->used_features(),
-                false /* notify_controllerchange */);
+  scoped_refptr<WebServiceWorkerImpl> controller =
+      GetDispatcher()->GetOrCreateServiceWorker(
+          ServiceWorkerHandleReference::Create(context_->controller()->info(),
+                                               thread_safe_sender_.get()));
+
+  // Sync the controllee's use counter with |context_|'s, which keeps
+  // track of the controller's use counter.
+  for (uint32_t feature : context_->used_features())
+    client->CountFeature(feature);
+  client->SetController(WebServiceWorkerImpl::CreateHandle(controller),
+                        false /* shouldNotifyControllerChange */);
 }
 
 void WebServiceWorkerProviderImpl::RegisterServiceWorker(
@@ -195,26 +192,6 @@ bool WebServiceWorkerProviderImpl::ValidateScopeAndScriptURL(
   return !has_error;
 }
 
-void WebServiceWorkerProviderImpl::SetController(
-    const ServiceWorkerObjectInfo& info,
-    const std::set<uint32_t>& features,
-    bool should_notify_controller_change) {
-  blink::WebServiceWorkerProviderClient* provider_client =
-      GetDispatcher()->GetProviderClient(context_->provider_id());
-  // The caller should only call this after SetClient() has been called.
-  DCHECK(provider_client);
-  scoped_refptr<WebServiceWorkerImpl> controller =
-      GetDispatcher()->GetOrCreateServiceWorker(
-          ServiceWorkerHandleReference::Create(info,
-                                               thread_safe_sender_.get()));
-
-  for (uint32_t feature : features)
-    provider_client->CountFeature(feature);
-
-  provider_client->SetController(WebServiceWorkerImpl::CreateHandle(controller),
-                                 should_notify_controller_change);
-}
-
 int WebServiceWorkerProviderImpl::provider_id() const {
   return context_->provider_id();
 }

content/child/service_worker/web_service_worker_provider_impl.h

@@ -11,7 +11,6 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
-#include "content/common/service_worker/service_worker_types.h"
 #include "third_party/WebKit/public/platform/modules/serviceworker/WebServiceWorkerProvider.h"
 #include "third_party/WebKit/public/platform/modules/serviceworker/service_worker_error_type.mojom.h"
 #include "third_party/WebKit/public/platform/modules/serviceworker/service_worker_registration.mojom.h"
@@ -31,8 +30,7 @@ struct ServiceWorkerVersionAttributes;
 
 // This class corresponds to one ServiceWorkerContainer interface in
 // JS context (i.e. navigator.serviceWorker).
-class CONTENT_EXPORT WebServiceWorkerProviderImpl
-    : public blink::WebServiceWorkerProvider {
+class WebServiceWorkerProviderImpl : public blink::WebServiceWorkerProvider {
  public:
   WebServiceWorkerProviderImpl(ThreadSafeSender* thread_safe_sender,
                                ServiceWorkerProviderContext* context);
@@ -57,12 +55,6 @@ class CONTENT_EXPORT WebServiceWorkerProviderImpl
                                  const blink::WebURL& script_url,
                                  blink::WebString* error_message) override;
 
-  // Sets the ServiceWorkerContainer#controller for this provider. It's not
-  // used when this WebServiceWorkerProvider is for a service worker context.
-  void SetController(const ServiceWorkerObjectInfo& info,
-                     const std::set<uint32_t>& features,
-                     bool should_notify_controller_change);
-
   int provider_id() const;
 
  private:

content/common/service_worker/service_worker_container.mojom

@@ -8,7 +8,6 @@ import "content/common/service_worker/controller_service_worker.mojom";
 import "content/common/service_worker/service_worker_types.mojom";
 import "third_party/WebKit/public/platform/modules/serviceworker/service_worker_error_type.mojom";
 import "third_party/WebKit/public/platform/modules/serviceworker/service_worker_registration.mojom";
-import "third_party/WebKit/public/platform/web_feature.mojom";
 import "url/mojo/url.mojom";
 
 // mojom::ServiceWorkerContainerHost is a browser-side interface. The renderer
@@ -80,17 +79,8 @@ interface ServiceWorkerContainerHost {
 // that can touch these objects should be a ServiceWorkerContainer, so it’s OK
 // to use this name.
 interface ServiceWorkerContainer {
-  // Corresponds to setting ServiceWorkerContainer#controller.
-  // If |controller| is invalid (its |handle_id| is invalid), then
-  // ServiceWorkerContainer#controller is cleared.
-  // If |controller| is valid, |used_features| is the set of features the
-  // controller has used, for UseCounter purposes.
-  // If |should_notify_controllerchange| is true, dispatch a 'controllerchange'
-  // event.
-  SetController(ServiceWorkerObjectInfo controller,
-                array<blink.mojom.WebFeature> used_features,
-                bool should_notify_controllerchange);
   // TODO(xiaofeng.zhang): implement them.
+  // SetController();
   // ServiceWorkerStateChanged();
   // ServiceWorkerRegistrationUpdateFound();
   // PostMessage();

content/common/service_worker/service_worker_messages.h

@@ -140,6 +140,17 @@ IPC_STRUCT_TRAITS_BEGIN(content::PushEventPayload)
   IPC_STRUCT_TRAITS_MEMBER(is_null)
 IPC_STRUCT_TRAITS_END()
 
+IPC_STRUCT_BEGIN(ServiceWorkerMsg_SetControllerServiceWorker_Params)
+  IPC_STRUCT_MEMBER(int, thread_id)
+  IPC_STRUCT_MEMBER(int, provider_id)
+  IPC_STRUCT_MEMBER(content::ServiceWorkerObjectInfo, object_info)
+  IPC_STRUCT_MEMBER(bool, should_notify_controllerchange)
+
+  // |used_features| is the set of features that the worker has used.
+  // The values must be from blink::UseCounter::Feature enum.
+  IPC_STRUCT_MEMBER(std::set<uint32_t>, used_features)
+IPC_STRUCT_END()
+
 //---------------------------------------------------------------------------
 // Messages sent from the child process to the browser.
 
@@ -292,6 +303,11 @@ IPC_MESSAGE_CONTROL2(ServiceWorkerMsg_UpdateFound,
                      int /* thread_id */,
                      int /* registration_handle_id */)
 
+// Tells the child process to set the controller ServiceWorker for the given
+// provider.
+IPC_MESSAGE_CONTROL1(ServiceWorkerMsg_SetControllerServiceWorker,
+                     ServiceWorkerMsg_SetControllerServiceWorker_Params)
+
 IPC_MESSAGE_CONTROL2(ServiceWorkerMsg_DidEnableNavigationPreload,
                      int /* thread_id */,
                      int /* request_id */)

content/common/service_worker/service_worker_types.mojom

@@ -24,7 +24,3 @@ enum ServiceWorkerProviderType {
 // Defined in service_worker_types.h.
 [Native]
 struct ServiceWorkerVersionAttributes;
-
-// Defined in service_worker_types.h.
-[Native]
-struct ServiceWorkerObjectInfo;

content/common/service_worker/service_worker_types.typemap

@@ -17,5 +17,4 @@ sources = [
 type_mappings = [
   "content.mojom.ServiceWorkerProviderType=::content::ServiceWorkerProviderType",
   "content.mojom.ServiceWorkerVersionAttributes=::content::ServiceWorkerVersionAttributes",
-  "content.mojom.ServiceWorkerObjectInfo=::content::ServiceWorkerObjectInfo",
 ]