Prevent DCHECK on shutdown of EmbeddedApplicationRunner

content/common/mojo/embedded_application_runner.cc

 // Copyright 2016 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/common/mojo/embedded_application_runner.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/macros.h"
@@ skipping 34 matching lines @@
 
     DCHECK(application_task_runner_);
     application_task_runner_->PostTask(
         FROM_HERE,
         base::Bind(&Instance::BindServiceRequestOnApplicationThread, this,
                    base::Passed(&request)));
   }
 
   void ShutDown() {
     DCHECK(runner_thread_checker_.CalledOnValidThread());
+    // Any extant ServiceContexts must be destroyed on the correct thread.

[Ken Rockot(use gerrit already), 2016/07/27 18:23:05]

If application_task_runner_ is null, the app has already been torn down and
there's nothing to do.
So I would change this to


if (!application_task_runner_)
  return;
if (application_task_runner_->BelongsToCurrentThread())
  Quit();
else
  // the PostTask you're adding here

[kmackay, 2016/07/27 18:51:59]

Done.

+    if (application_task_runner_) {
+      application_task_runner_->PostTask(FROM_HERE,
+                                         base::Bind(&Instance::Quit, this));
+    }
     if (thread_) {

[Ken Rockot(use gerrit already), 2016/07/27 18:23:05]

And you can just delete this block now

[kmackay, 2016/07/27 18:51:59]

I wasn't sure if we wanted to make sure that the application thread was dead
before completing the destruction of EmbeddedApplicationRunner. I guess it must
be safe though since it needs to work when use_own_thread_ is false.

       thread_.reset();
       application_task_runner_ = nullptr;
     }
   }
 
  private:
   void BindServiceRequestOnApplicationThread(
       shell::mojom::ServiceRequest request) {
     DCHECK(application_task_runner_->BelongsToCurrentThread());
 
@@ skipping 29 matching lines @@
         break;
       }
     }
   }
 
   void Quit() {
     DCHECK(application_task_runner_->BelongsToCurrentThread());
 
     shell_connections_.clear();
     service_.reset();
     quit_task_runner_->PostTask(

[Ken Rockot(use gerrit already), 2016/07/27 18:23:05]

Since you're here, mind also changing this to invoke QuitOnRunnerThread
synchronously when quit_task_runner_->BelongsToCurrentThread() is true?

[kmackay, 2016/07/27 18:51:59]

Done.

         FROM_HERE, base::Bind(&Instance::QuitOnRunnerThread, this));
   }
 
   void QuitOnRunnerThread() {
     DCHECK(runner_thread_checker_.CalledOnValidThread());
-    ShutDown();
+    if (thread_) {
+      thread_.reset();
+      application_task_runner_ = nullptr;
+    }
     quit_closure_.Run();
   }
 
   const std::string name_;
   const MojoApplicationInfo::ApplicationFactory factory_callback_;
   const bool use_own_thread_;
   const base::Closure quit_closure_;
   const scoped_refptr<base::SingleThreadTaskRunner> quit_task_runner_;
 
   // Thread checker used to ensure certain operations happen only on the
@@ skipping 35 matching lines @@
     const base::Closure& quit_closure) {
   quit_closure_ = quit_closure;
 }
 
 void EmbeddedApplicationRunner::OnQuit() {
   if (!quit_closure_.is_null())
     quit_closure_.Run();
 }
 
 }  // namespace content