Fix crash during EmbeddedWorkerInstance startup sequence failures

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 68e473dcebc15d57c5ee825054bcb4a78ba2ea23..0046b21ad86c6a7bee4f2821feaf696f86b291de 100644
--- a/content/browser/service_worker/embedded_worker_instance.cc
+++ b/content/browser/service_worker/embedded_worker_instance.cc
@@ -155,6 +155,7 @@ void EmbeddedWorkerInstance::Start(int64 service_worker_version_id,
                                    const StatusCallback& callback) {
   if (!context_) {
     callback.Run(SERVICE_WORKER_ERROR_ABORT);
+    // |this| may be destroyed by the callback.
     return;
   }
   DCHECK(status_ == STOPPED);
@@ -284,11 +285,7 @@ void EmbeddedWorkerInstance::ProcessAllocated(
                          params.get(),
                          "Status", status);
   if (status != SERVICE_WORKER_OK) {
-    Status old_status = status_;
-    status_ = STOPPED;
-    service_registry_.reset();
-    callback.Run(status);
-    FOR_EACH_OBSERVER(Listener, listener_list_, OnStopped(old_status));
+    OnStartFailed(status, callback);
     return;
   }
   const int64 service_worker_version_id = params->service_worker_version_id;
@@ -318,8 +315,11 @@ void EmbeddedWorkerInstance::SendStartWorker(
   // process, making process_id_ and other state invalid. If that happened,
   // abort instead of trying to send the IPC.
   if (status_ != STARTING) {
-    callback.Run(SERVICE_WORKER_ERROR_ABORT);
+    // Don't call OnStartFailed() since it calls OnStopped() which may have
+    // already been called.

[nhiroki, 2015/09/11 04:33:02]

The following code might work?

// Here
  OnStartFailed(ABORT, callback);
  return;

// In OnStartFailed
  Status old_status = status_;
  if (status_ != STOPPED)
    ReleaseProcess();  // Sets |status_| to STOPPED. 
  base::WeakPtr<EmbeddedWorkerInstance> weak_this = weak_factory_.GetWeakPtr();
  callback.Run(status);
  if (weak_this && old_status_ != STOPPED)
    FOR_EACH_OBSERVER(Listener, listener_list_, OnStopped(STARTING));

[falken, 2015/09/11 09:48:16]

I like this, done, with the minor exception of calling ReleaseProcess even if
STOPPED, it should be a no-op.

     ReleaseProcess();
+    callback.Run(SERVICE_WORKER_ERROR_ABORT);
+    // |this| may be destroyed by the callback.
     return;
   }
 
@@ -354,7 +354,7 @@ void EmbeddedWorkerInstance::SendStartWorker(
   ServiceWorkerStatusCode status =
       registry_->SendStartWorker(params.Pass(), process_id_);
   if (status != SERVICE_WORKER_OK) {
-    callback.Run(status);
+    OnStartFailed(status, callback);
     return;
   }
   DCHECK(start_callback_.is_null());
@@ -410,9 +410,11 @@ void EmbeddedWorkerInstance::OnScriptEvaluated(bool success) {
     UMA_HISTOGRAM_TIMES("EmbeddedWorkerInstance.ScriptEvaluate",
                         base::TimeTicks::Now() - start_timing_);
   }
-  start_callback_.Run(success ? SERVICE_WORKER_OK
-                              : SERVICE_WORKER_ERROR_SCRIPT_EVALUATE_FAILED);
+  StatusCallback callback = start_callback_;
   start_callback_.Reset();
+  callback.Run(success ? SERVICE_WORKER_OK
+                       : SERVICE_WORKER_ERROR_SCRIPT_EVALUATE_FAILED);
+  // |this| may be destroyed by the callback.
 }
 
 void EmbeddedWorkerInstance::OnStarted() {
@@ -504,6 +506,15 @@ void EmbeddedWorkerInstance::ReleaseProcess() {
   start_callback_.Reset();
 }
 
+void EmbeddedWorkerInstance::OnStartFailed(ServiceWorkerStatusCode status,
+                                           const StatusCallback& callback) {
+  ReleaseProcess();
+  base::WeakPtr<EmbeddedWorkerInstance> weak_this = weak_factory_.GetWeakPtr();
+  callback.Run(status);
+  if (weak_this)
+    FOR_EACH_OBSERVER(Listener, listener_list_, OnStopped(STARTING));
+}
+
 // static
 std::string EmbeddedWorkerInstance::StatusToString(Status status) {
   switch (status) {