service worker: Remove more of EmbeddedWorkerRegistry.

Now that the implementation is mojofied, there is no need for this class
to keep track of running workers.

Bug: 931084
Change-Id: Ie8b21af1561f7939453e8f95793da4152076ec06
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1504653
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#637989}

content/browser/service_worker/embedded_worker_instance.cc

@@ -687,7 +687,7 @@ EmbeddedWorkerInstance::~EmbeddedWorkerInstance() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   devtools_proxy_.reset();
   if (registry_->GetWorker(embedded_worker_id_))
-    registry_->RemoveWorker(process_id(), embedded_worker_id_);
+    registry_->RemoveWorker(embedded_worker_id_);
   ReleaseProcess();
 }
 
@@ -846,7 +846,6 @@ void EmbeddedWorkerInstance::SendStartWorker(
   params->provider_info->cache_storage = std::move(cache_storage);
 
   client_->StartWorker(std::move(params));
-  registry_->BindWorkerToProcess(process_id(), embedded_worker_id());
 
   starting_phase_ = is_script_streaming ? SCRIPT_STREAMING : SENT_START_WORKER;
   for (auto& observer : listener_list_)
@@ -944,8 +943,6 @@ void EmbeddedWorkerInstance::OnStarted(
   if (!devtools_attached_)
     lifetime_tracker_ = std::make_unique<ScopedLifetimeTracker>();
 
-  if (!registry_->OnWorkerStarted(process_id(), embedded_worker_id_))
-    return;
   // Stop was requested before OnStarted was sent back from the worker. Just
   // pretend startup didn't happen, so observers don't try to use the running
   // worker as it will stop soon.
@@ -981,8 +978,6 @@ void EmbeddedWorkerInstance::OnStarted(
 }
 
 void EmbeddedWorkerInstance::OnStopped() {
-  registry_->OnWorkerStopped(process_id(), embedded_worker_id_);
-
   EmbeddedWorkerStatus old_status = status_;
   ReleaseProcess();
   for (auto& observer : listener_list_)
@@ -993,7 +988,6 @@ void EmbeddedWorkerInstance::Detach() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   if (status() == EmbeddedWorkerStatus::STOPPED)
     return;
-  registry_->DetachWorker(process_id(), embedded_worker_id());
 
   EmbeddedWorkerStatus old_status = status_;
   ReleaseProcess();

content/browser/service_worker/embedded_worker_instance_unittest.cc

@@ -343,43 +343,6 @@ TEST_F(EmbeddedWorkerInstanceTest, StopWhenDevToolsAttached) {
   EXPECT_EQ(EmbeddedWorkerStatus::STOPPED, worker->status());
 }
 
-// Test that the removal of a worker from the registry doesn't remove
-// other workers in the same process.
-TEST_F(EmbeddedWorkerInstanceTest, RemoveWorkerInSharedProcess) {
-  const GURL scope("http://example.com/");
-  const GURL url("http://example.com/worker.js");
-
-  RegistrationAndVersionPair pair1 = PrepareRegistrationAndVersion(scope, url);
-  std::unique_ptr<EmbeddedWorkerInstance> worker1 =
-      embedded_worker_registry()->CreateWorker(pair1.second.get());
-  RegistrationAndVersionPair pair2 = PrepareRegistrationAndVersion(scope, url);
-  std::unique_ptr<EmbeddedWorkerInstance> worker2 =
-      embedded_worker_registry()->CreateWorker(pair2.second.get());
-
-  int process_id = helper_->mock_render_process_id();
-
-  // Start workers.
-  StartWorker(worker1.get(), CreateStartParams(pair1.second));
-  StartWorker(worker2.get(), CreateStartParams(pair2.second));
-
-  // The two workers share the same process.
-  EXPECT_EQ(worker1->process_id(), worker2->process_id());
-
-  // Destroy worker1. It removes itself from the registry.
-  int worker1_id = worker1->embedded_worker_id();
-  worker1->Stop();
-  worker1.reset();
-
-  // Only worker1 should be removed from the registry's process_map.
-  EmbeddedWorkerRegistry* registry =
-      helper_->context()->embedded_worker_registry();
-  EXPECT_EQ(0UL, registry->worker_process_map_[process_id].count(worker1_id));
-  EXPECT_EQ(1UL, registry->worker_process_map_[process_id].count(
-                     worker2->embedded_worker_id()));
-
-  worker2->Stop();
-}
-
 TEST_F(EmbeddedWorkerInstanceTest, DetachDuringProcessAllocation) {
   const GURL scope("http://example.com/");
   const GURL url("http://example.com/worker.js");
@@ -583,22 +546,13 @@ TEST_F(EmbeddedWorkerInstanceTest, Detach) {
   RegistrationAndVersionPair pair = PrepareRegistrationAndVersion(scope, url);
   std::unique_ptr<EmbeddedWorkerInstance> worker =
       embedded_worker_registry()->CreateWorker(pair.second.get());
-  worker->AddObserver(this);
 
   // Start the worker.
-  base::RunLoop run_loop;
   StartWorker(worker.get(), CreateStartParams(pair.second));
 
   // Detach.
-  int process_id = worker->process_id();
   worker->Detach();
   EXPECT_EQ(EmbeddedWorkerStatus::STOPPED, worker->status());
-
-  // Send the registry a message from the detached worker. Nothing should
-  // happen.
-  embedded_worker_registry()->OnWorkerStarted(process_id,
-                                              worker->embedded_worker_id());
-  EXPECT_EQ(EmbeddedWorkerStatus::STOPPED, worker->status());
 }
 
 // Test for when sending the start IPC failed.

content/browser/service_worker/embedded_worker_registry.cc

@@ -4,17 +4,8 @@
 
 #include "content/browser/service_worker/embedded_worker_registry.h"
 
-#include "base/bind_helpers.h"
-#include "base/metrics/histogram_macros.h"
-#include "base/stl_util.h"
-#include "content/browser/renderer_host/render_widget_helper.h"
 #include "content/browser/service_worker/embedded_worker_instance.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
-#include "content/browser/service_worker/service_worker_context_wrapper.h"
-#include "content/browser/service_worker/service_worker_dispatcher_host.h"
-#include "content/public/browser/browser_thread.h"
-#include "ipc/ipc_message.h"
-#include "ipc/ipc_sender.h"
 
 namespace content {
 
@@ -43,21 +34,6 @@ std::unique_ptr<EmbeddedWorkerInstance> EmbeddedWorkerRegistry::CreateWorker(
   return worker;
 }
 
-bool EmbeddedWorkerRegistry::OnWorkerStarted(int process_id,
-                                             int embedded_worker_id) {
-  if (!base::ContainsKey(worker_process_map_, process_id) ||
-      !base::ContainsKey(worker_process_map_[process_id], embedded_worker_id)) {
-    return false;
-  }
-
-  return true;
-}
-
-void EmbeddedWorkerRegistry::OnWorkerStopped(int process_id,
-                                             int embedded_worker_id) {
-  worker_process_map_[process_id].erase(embedded_worker_id);
-}
-
 EmbeddedWorkerInstance* EmbeddedWorkerRegistry::GetWorker(
     int embedded_worker_id) {
   auto found = worker_map_.find(embedded_worker_id);
@@ -76,32 +52,9 @@ EmbeddedWorkerRegistry::EmbeddedWorkerRegistry(
 
 EmbeddedWorkerRegistry::~EmbeddedWorkerRegistry() = default;
 
-void EmbeddedWorkerRegistry::BindWorkerToProcess(int process_id,
-                                                 int embedded_worker_id) {
-  DCHECK(GetWorker(embedded_worker_id));
-  DCHECK_EQ(GetWorker(embedded_worker_id)->process_id(), process_id);
-  DCHECK(
-      !base::ContainsKey(worker_process_map_, process_id) ||
-      !base::ContainsKey(worker_process_map_[process_id], embedded_worker_id));
-
-  worker_process_map_[process_id].insert(embedded_worker_id);
-}
-
-void EmbeddedWorkerRegistry::RemoveWorker(int process_id,
-                                          int embedded_worker_id) {
+void EmbeddedWorkerRegistry::RemoveWorker(int embedded_worker_id) {
   DCHECK(base::ContainsKey(worker_map_, embedded_worker_id));
-  DetachWorker(process_id, embedded_worker_id);
   worker_map_.erase(embedded_worker_id);
 }
 
-void EmbeddedWorkerRegistry::DetachWorker(int process_id,
-                                          int embedded_worker_id) {
-  DCHECK(base::ContainsKey(worker_map_, embedded_worker_id));
-  if (!base::ContainsKey(worker_process_map_, process_id))
-    return;
-  worker_process_map_[process_id].erase(embedded_worker_id);
-  if (worker_process_map_[process_id].empty())
-    worker_process_map_.erase(process_id);
-}
-
 }  // namespace content

content/browser/service_worker/embedded_worker_registry.h

@@ -7,16 +7,12 @@
 
 #include <map>
 #include <memory>
-#include <set>
-#include <vector>
 
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
-#include "base/strings/string16.h"
 #include "content/common/content_export.h"
-#include "third_party/blink/public/common/service_worker/service_worker_status_code.h"
 
 namespace content {
 
@@ -24,9 +20,6 @@ class EmbeddedWorkerInstance;
 class ServiceWorkerContextCore;
 class ServiceWorkerVersion;
 
-// Acts as a thin stub between MessageFilter and each EmbeddedWorkerInstance,
-// which sends/receives messages to/from each EmbeddedWorker in child process.
-//
 // Hangs off ServiceWorkerContextCore (its reference is also held by each
 // EmbeddedWorkerInstance).  Operated only on IO thread.
 class CONTENT_EXPORT EmbeddedWorkerRegistry
@@ -46,17 +39,11 @@ class CONTENT_EXPORT EmbeddedWorkerRegistry
   std::unique_ptr<EmbeddedWorkerInstance> CreateWorker(
       ServiceWorkerVersion* owner_version);
 
-  // Called by EmbeddedWorkerInstance when it starts or stops. This registry
-  // keeps track of running workers.
-  bool OnWorkerStarted(int process_id, int embedded_worker_id);
-  void OnWorkerStopped(int process_id, int embedded_worker_id);
-
   // Returns an embedded worker instance for given |embedded_worker_id|.
   EmbeddedWorkerInstance* GetWorker(int embedded_worker_id);
 
  private:
   friend class base::RefCounted<EmbeddedWorkerRegistry>;
-  friend class MojoEmbeddedWorkerInstanceTest;
   friend class EmbeddedWorkerInstance;
   friend class EmbeddedWorkerInstanceTest;
   FRIEND_TEST_ALL_PREFIXES(EmbeddedWorkerInstanceTest,
@@ -69,29 +56,13 @@ class CONTENT_EXPORT EmbeddedWorkerRegistry
       int initial_embedded_worker_id);
   ~EmbeddedWorkerRegistry();
 
-  // Called when EmbeddedWorkerInstance is ready for IPC. This function
-  // prepares a route to the child worker thread.
-  // TODO(shimazu): Remove this function once mojofication is completed.
-  void BindWorkerToProcess(int process_id, int embedded_worker_id);
-
   // RemoveWorker is called when EmbeddedWorkerInstance is destructed.
-  // |process_id| could be invalid (i.e. ChildProcessHost::kInvalidUniqueID)
-  // if it's not running.
-  void RemoveWorker(int process_id, int embedded_worker_id);
-
-  // Removes the bookkeeping that binds the worker to the process.  This is
-  // called instead of WorkerStopped() in cases when the worker could not be
-  // cleanly stopped, e.g., because connection with the renderer was lost.
-  void DetachWorker(int process_id, int embedded_worker_id);
+  void RemoveWorker(int embedded_worker_id);
 
   base::WeakPtr<ServiceWorkerContextCore> context_;
 
   WorkerInstanceMap worker_map_;
 
-  // Map from process_id to embedded_worker_id.
-  // This map only contains starting and running workers.
-  std::map<int, std::set<int> > worker_process_map_;
-
   int next_embedded_worker_id_;
   const int initial_embedded_worker_id_;