M63: Revert "[ServiceWorker] Implement ServiceWorkerContainer.SetController"

This reverts commit 24e5c55.

Bug: 774681
Change-Id: I2624c46835a32d60a7361077b3bbe20939faa307
TBR: kinuko
Reviewed-on: https://chromium-review.googlesource.com/728879
Reviewed-by: Matt Falkenhagen <[email protected]>
Cr-Commit-Position: refs/branch-heads/3239@{#85}
Cr-Branched-From: adb61db-refs/heads/master@{#508578}

Author: mfalken

content/browser/service_worker/service_worker_provider_host.cc

@@ -559,13 +559,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(),
@@ -578,6 +571,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;
@@ -881,20 +881,14 @@ void ServiceWorkerProviderHost::SendSetControllerServiceWorker(
     DCHECK_EQ(controller_.get(), version);
   }
 
-  std::vector<blink::mojom::WebFeature> used_features;
-  if (version) {
-    for (const auto& 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 >= 0 &&
-          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

@@ -35,6 +35,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 {
@@ -361,27 +372,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);
@@ -395,31 +385,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) {
@@ -435,34 +421,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));
 }
 
 }  // namespace content

content/child/service_worker/service_worker_dispatcher.cc

@@ -93,6 +93,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)
@@ -184,18 +186,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*
@@ -224,11 +217,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;
 }
@@ -522,6 +510,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
@@ -586,4 +611,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

@@ -112,8 +112,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(
@@ -141,11 +139,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();
   }
@@ -234,6 +227,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);
+
   UpdateCallbackMap pending_update_callbacks_;
   UnregistrationCallbackMap pending_unregistration_callbacks_;
   EnableNavigationPreloadCallbackMap enable_navigation_preload_callbacks_;