service worker: Simplify ServiceWorkerVersion destructor.

It was calling EmbeddedWorkerInstance::Stop() but it's not needed since
it owns EWI and when EWI gets destructed the Mojo connection breaks.
EmbeddedWorkerInstanceClient handles the Mojo connection failure the
same as receiving a Stop message in EmbeddedWorkerInstanceClient::OnError.

Change-Id: I86f850245feee0dec492e88d3e6f5110e3479e6c
Reviewed-on: https://chromium-review.googlesource.com/c/1457796Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#629891}

content/browser/service_worker/embedded_worker_instance.cc

@@ -645,9 +645,6 @@ class EmbeddedWorkerInstance::StartTask {
 
 EmbeddedWorkerInstance::~EmbeddedWorkerInstance() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  DCHECK(status_ == EmbeddedWorkerStatus::STOPPING ||
-         status_ == EmbeddedWorkerStatus::STOPPED)
-      << static_cast<int>(status_);
   devtools_proxy_.reset();
   if (registry_->GetWorker(embedded_worker_id_))
     registry_->RemoveWorker(process_id(), embedded_worker_id_);

content/browser/service_worker/service_worker_version.cc

@@ -256,6 +256,8 @@ ServiceWorkerVersion::ServiceWorkerVersion(
 }
 
 ServiceWorkerVersion::~ServiceWorkerVersion() {
+  // TODO(falken): Investigate whether this can be removed. The destructor used
+  // to be more complicated and could result in various methods being called.
   in_dtor_ = true;
 
   // Record UMA if the worker was trying to start. One way we get here is if the
@@ -271,10 +273,6 @@ ServiceWorkerVersion::~ServiceWorkerVersion() {
   if (context_)
     context_->RemoveLiveVersion(version_id_);
 
-  if (running_status() == EmbeddedWorkerStatus::STARTING ||
-      running_status() == EmbeddedWorkerStatus::RUNNING) {
-    embedded_worker_->Stop();
-  }
   embedded_worker_->RemoveObserver(this);
 }
 

content/browser/service_worker/service_worker_version_unittest.cc

@@ -688,21 +688,6 @@ TEST_F(ServiceWorkerVersionTest, SetDevToolsAttached) {
   EXPECT_EQ(EmbeddedWorkerStatus::RUNNING, version_->running_status());
 }
 
-TEST_F(ServiceWorkerVersionTest, StoppingBeforeDestruct) {
-  RunningStateListener listener;
-  version_->AddObserver(&listener);
-  StartWorker(version_.get(), ServiceWorkerMetrics::EventType::UNKNOWN);
-  EXPECT_EQ(EmbeddedWorkerStatus::RUNNING, listener.last_status);
-
-  // Destruct |version_| by releasing all references, including the provider
-  // host's.
-  helper_->context()->RemoveProviderHost(
-      version_->provider_host()->process_id(),
-      version_->provider_host()->provider_id());
-  version_ = nullptr;
-  EXPECT_EQ(EmbeddedWorkerStatus::STOPPING, listener.last_status);
-}
-
 // Test that update isn't triggered for a non-stale worker.
 TEST_F(ServiceWorkerVersionTest, StaleUpdate_FreshWorker) {
   version_->SetStatus(ServiceWorkerVersion::ACTIVATED);