Fix crash during EmbeddedWorkerInstance startup sequence failures

content/browser/service_worker/embedded_worker_instance.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/service_worker/embedded_worker_instance.h"
 
 #include <algorithm>
 #include <string>
 #include <utility>
 
@@ skipping 137 matching lines @@
     context_->process_manager()->ReleaseWorkerProcess(embedded_worker_id_);
   registry_->RemoveWorker(process_id_, embedded_worker_id_);
 }
 
 void EmbeddedWorkerInstance::Start(int64 service_worker_version_id,
                                    const GURL& scope,
                                    const GURL& script_url,
                                    const StatusCallback& callback) {
   if (!context_) {
     callback.Run(SERVICE_WORKER_ERROR_ABORT);
+    // |this| may be destroyed by the callback.
     return;
   }
   DCHECK(status_ == STOPPED);
   start_timing_ = base::TimeTicks::Now();
   status_ = STARTING;
   starting_phase_ = ALLOCATING_PROCESS;
   network_accessed_for_script_ = false;
   service_registry_.reset(new ServiceRegistryImpl());
   FOR_EACH_OBSERVER(Listener, listener_list_, OnStarting());
   scoped_ptr<EmbeddedWorkerMsg_StartWorker_Params> params(
@@ skipping 109 matching lines @@
     const StatusCallback& callback,
     int process_id,
     bool is_new_process,
     ServiceWorkerStatusCode status) {
   DCHECK_EQ(process_id_, -1);
   TRACE_EVENT_ASYNC_END1("ServiceWorker",
                          "EmbeddedWorkerInstance::ProcessAllocate",
                          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(callback, status);
     return;
   }
   const int64 service_worker_version_id = params->service_worker_version_id;
   process_id_ = process_id;
   GURL script_url(params->script_url);
 
   // Register this worker to DevToolsManager on UI thread, then continue to
   // call SendStartWorker on IO thread.
   starting_phase_ = REGISTERING_TO_DEVTOOLS;
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(RegisterToWorkerDevToolsManagerOnUI, process_id_,
                  context_.get(), context_, service_worker_version_id,
                  script_url,
                  base::Bind(&EmbeddedWorkerInstance::SendStartWorker,
                             weak_factory_.GetWeakPtr(), base::Passed(&params),
                             callback, is_new_process)));
 }
 
 void EmbeddedWorkerInstance::SendStartWorker(
     scoped_ptr<EmbeddedWorkerMsg_StartWorker_Params> params,
     const StatusCallback& callback,
     bool is_new_process,
     int worker_devtools_agent_route_id,
     bool wait_for_debugger) {
   // We may have been detached or stopped at some point during the start up
   // 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);
-    ReleaseProcess();
+    OnStartFailed(callback, SERVICE_WORKER_ERROR_ABORT);
     return;
   }
 
   if (worker_devtools_agent_route_id != MSG_ROUTING_NONE) {
     DCHECK(!devtools_proxy_);
     devtools_proxy_.reset(new DevToolsProxy(process_id_,
                                             worker_devtools_agent_route_id));
   }
   params->worker_devtools_agent_route_id = worker_devtools_agent_route_id;
   params->wait_for_debugger = wait_for_debugger;
@@ skipping 14 matching lines @@
                           is_new_process);
     // Reset |start_timing_| to measure the time excluding the process
     // allocation time.
     start_timing_ = base::TimeTicks::Now();
   }
 
   starting_phase_ = SENT_START_WORKER;
   ServiceWorkerStatusCode status =
       registry_->SendStartWorker(params.Pass(), process_id_);
   if (status != SERVICE_WORKER_OK) {
-    callback.Run(status);
+    OnStartFailed(callback, status);

[nhiroki, 2015/09/17 06:14:13]

I guess this could fix issue 531345. Before this patch, EmbeddedWorkerInstance
could be stick on STARTING state and subsequent StartWorker requests are queued
up until the timeout timer is fired (5 minutes). The timeout timer stops the
EWInstance[1] and records TimeoutPhase as SENT_START_WORKER.

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/se...

     return;
   }
   DCHECK(start_callback_.is_null());
   start_callback_ = callback;
 }
 
 void EmbeddedWorkerInstance::OnReadyForInspection() {
   if (devtools_proxy_)
     devtools_proxy_->NotifyWorkerReadyForInspection();
 }
@@ skipping 35 matching lines @@
 void EmbeddedWorkerInstance::OnScriptEvaluated(bool success) {
   starting_phase_ = SCRIPT_EVALUATED;
   if (start_callback_.is_null()) {
     DVLOG(1) << "Received unexpected OnScriptEvaluated message.";
     return;
   }
   if (success && !start_timing_.is_null()) {
     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() {
   // Stop is requested before OnStarted is sent back from the worker.
   if (status_ == STOPPING)
     return;
   DCHECK(status_ == STARTING);
   status_ = RUNNING;
   FOR_EACH_OBSERVER(Listener, listener_list_, OnStarted());
 }
@@ skipping 71 matching lines @@
   devtools_proxy_.reset();
   if (context_ && process_id_ != -1)
     context_->process_manager()->ReleaseWorkerProcess(embedded_worker_id_);
   status_ = STOPPED;
   process_id_ = -1;
   thread_id_ = -1;
   service_registry_.reset();
   start_callback_.Reset();
 }
 
+void EmbeddedWorkerInstance::OnStartFailed(const StatusCallback& callback,
+                                           ServiceWorkerStatusCode status) {
+  Status old_status = status_;
+  ReleaseProcess();
+  base::WeakPtr<EmbeddedWorkerInstance> weak_this = weak_factory_.GetWeakPtr();
+  callback.Run(status);
+  if (weak_this && old_status != STOPPED)
+    FOR_EACH_OBSERVER(Listener, weak_this->listener_list_,
+                      OnStopped(old_status));

[kinuko, 2015/09/13 08:41:11]

nit: would be better to update the comment for Listener::OnStopped about when it
could be called

+}
+
 // static
 std::string EmbeddedWorkerInstance::StatusToString(Status status) {
   switch (status) {
     case STOPPED:
       return "STOPPED";
     case STARTING:
       return "STARTING";
     case RUNNING:
       return "RUNNING";
     case STOPPING:
@@ skipping 21 matching lines @@
     case SCRIPT_EVALUATED:
       return "Script evaluated";
     case STARTING_PHASE_MAX_VALUE:
       NOTREACHED();
   }
   NOTREACHED() << phase;
   return std::string();
 }
 
 }  // namespace content