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(status, callback);
     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) {
+    // 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);
-    ReleaseProcess();
+    // |this| may be destroyed by the callback.
     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(status, callback);
     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(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) {
     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