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..2f5e2e325d0d1538b448ac973a380a492dcc26ae 100644
--- a/content/browser/service_worker/embedded_worker_instance.cc
+++ b/content/browser/service_worker/embedded_worker_instance.cc
@@ -500,28 +500,61 @@ ServiceWorkerStatusCode EmbeddedWorkerInstance::Stop() {
   // Abort an inflight start task.
   inflight_start_task_.reset();
 
-  ServiceWorkerStatusCode status = SERVICE_WORKER_ERROR_IPC_FAILED;
   if (ServiceWorkerUtils::IsMojoForServiceWorkerEnabled()) {
-    status = SERVICE_WORKER_OK;
+    ServiceWorkerStatusCode status = SERVICE_WORKER_OK;
+    if (status_ == EmbeddedWorkerStatus::STARTING) {
+      // Handle like detach when connection hasn't been established yet.
+      switch (starting_phase()) {
+        case NOT_STARTING:
+          NOTREACHED();
+        case ALLOCATING_PROCESS:
+          status = SERVICE_WORKER_ERROR_PROCESS_NOT_FOUND;
+          OnDetached();
+          break;
+        case REGISTERING_TO_DEVTOOLS:
+          status = SERVICE_WORKER_ERROR_IPC_FAILED;
+          OnDetached();
+          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;
+        default:

[nhiroki, 2016/12/19 09:21:55]

Can you use "STARTING_PHASE_MAX_VALUE:" instead of "default"?

[shimazu, 2016/12/19 09:47:51]

Done.

+          NOTREACHED();
+      }
+    }
+    UMA_HISTOGRAM_ENUMERATION("ServiceWorker.SendStopWorker.Status", status,
+                              SERVICE_WORKER_ERROR_MAX_VALUE);
+    if (status != SERVICE_WORKER_OK)
+      return status;
     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();
+    return status;
   } 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.
-  if (status != SERVICE_WORKER_OK) {
-    OnDetached();
+    ServiceWorkerStatusCode 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.
+    if (status != SERVICE_WORKER_OK) {
+      OnDetached();
+      return status;
+    }
+    status_ = EmbeddedWorkerStatus::STOPPING;
+    for (auto& observer : listener_list_)
+      observer.OnStopping();
     return status;

[nhiroki, 2016/12/19 09:21:55]

nit: We could merge line 553-556 into 538-541 and then move them out of
if(IsMojoForServiceWorkerEnabled()).

[shimazu, 2016/12/19 09:47:51]

Done.

   }
-
-  status_ = EmbeddedWorkerStatus::STOPPING;
-  for (auto& observer : listener_list_)
-    observer.OnStopping();
-  return status;
 }
 
 void EmbeddedWorkerInstance::StopIfIdle() {