ServiceWorker: Stop don't send a message before connection established

content/browser/service_worker/embedded_worker_instance.cc

Index: content/browser/service_worker/embedded_worker_instance.cc
diff --git a/content/browser/service_worker/embedded_worker_instance.cc b/content/browser/service_worker/embedded_worker_instance.cc
index 17246a1f1bc6c450541f893130461e1b9bffd0ef..02ade117fe62fe4227734fa3680f37663882f7f1 100644
--- a/content/browser/service_worker/embedded_worker_instance.cc
+++ b/content/browser/service_worker/embedded_worker_instance.cc
@@ -500,24 +500,53 @@ ServiceWorkerStatusCode EmbeddedWorkerInstance::Stop() {
   // Abort an inflight start task.
   inflight_start_task_.reset();
 
-  ServiceWorkerStatusCode status = SERVICE_WORKER_ERROR_IPC_FAILED;
+  ServiceWorkerStatusCode status = SERVICE_WORKER_OK;
   if (ServiceWorkerUtils::IsMojoForServiceWorkerEnabled()) {
-    status = SERVICE_WORKER_OK;
-    client_->StopWorker(base::Bind(&EmbeddedWorkerRegistry::OnWorkerStopped,
-                                   base::Unretained(registry_.get()),
-                                   process_id(), embedded_worker_id()));
+    if (status_ == EmbeddedWorkerStatus::STARTING) {
+      // Abort sending StopWorker message when the connection hasn't been
+      // established yet.
+      switch (starting_phase()) {
+        case NOT_STARTING:
+          NOTREACHED();
+        case ALLOCATING_PROCESS:
+          status = SERVICE_WORKER_ERROR_PROCESS_NOT_FOUND;
+          break;
+        case REGISTERING_TO_DEVTOOLS:
+          status = SERVICE_WORKER_ERROR_IPC_FAILED;

[falken, 2016/12/20 04:41:31]

Hm I don't think we really need to log these as errors. The point of
SendStopWorker status was to track when sending an IPC fails. But this
Mojo-based code is really just avoiding sending a Stop before a Start was sent.
IPC_FAILED in particular feels misleading.

Basically I think SendStopWorker shouldn't be logged in the mojo case, WDYT?

[shimazu, 2016/12/20 07:55:01]

Removing the uma seems good. Removed.

+          break;
+        case SENT_START_WORKER:
+        case SCRIPT_DOWNLOADING:
+        case SCRIPT_READ_STARTED:
+        case SCRIPT_READ_FINISHED:
+        case SCRIPT_LOADED:
+        case THREAD_STARTED:
+        case SCRIPT_EVALUATED:
+          status = SERVICE_WORKER_OK;
+          break;
+        case STARTING_PHASE_MAX_VALUE:
+          NOTREACHED();
+          status = SERVICE_WORKER_ERROR_ABORT;
+          break;
+      }
+    }
   } else {
     status = registry_->StopWorker(process_id(), embedded_worker_id_);
   }
+
   UMA_HISTOGRAM_ENUMERATION("ServiceWorker.SendStopWorker.Status", status,
                             SERVICE_WORKER_ERROR_MAX_VALUE);
-  // StopWorker could fail if we were starting up and don't have a process yet,
-  // or we can no longer communicate with the process. So just detach.
+  // StopWorker could fail if we were starting up and don't have a process
+  // yet, or we can no longer communicate with the process. So just detach.
   if (status != SERVICE_WORKER_OK) {
     OnDetached();
     return status;
   }
 
+  if (ServiceWorkerUtils::IsMojoForServiceWorkerEnabled()) {
+    client_->StopWorker(base::Bind(&EmbeddedWorkerRegistry::OnWorkerStopped,
+                                   base::Unretained(registry_.get()),
+                                   process_id(), embedded_worker_id()));
+  }
   status_ = EmbeddedWorkerStatus::STOPPING;
   for (auto& observer : listener_list_)
     observer.OnStopping();