[Chromoting] Allow tasks to be posted to the plugin thread while a plugin instance is being shutdown.

This CL reliminates a restriction on posting tasks to the plugin thread making asynchronous shutdown logic more uniform and as result - simpler. The CL affects both the client and host plugins. 

BUG=150783


Review URL: https://chromiumcodereview.appspot.com/10998002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@159581 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 35 insertions, 42 deletions

diff --git a/remoting/host/plugin/host_script_object.cc b/remoting/host/plugin/host_script_object.cc
index 21b79be..2d441f3 100644
--- a/remoting/host/plugin/host_script_object.cc
+++ b/remoting/host/plugin/host_script_object.cc
@@ -95,8 +95,6 @@ HostNPScriptObject::HostNPScriptObject(
           new PluginThreadTaskRunner(plugin_thread_delegate)),
       desktop_environment_factory_(new DesktopEnvironmentFactory()),
       failed_login_attempts_(0),
-      disconnected_event_(true, false),
-      stopped_event_(true, false),
       nat_traversal_enabled_(false),
       policy_received_(false),
       daemon_controller_(DaemonController::Create()),
@@ -109,12 +107,6 @@ HostNPScriptObject::~HostNPScriptObject() {
 
   HostLogHandler::UnregisterLoggingScriptObject(this);
 
-  // Stop the message loop. Any attempt to post a task to
-  // |context_.ui_task_runner()| will result in a CHECK() after this point.
-  // TODO(alexeypa): Enable posting messages to |plugin_task_runner_| during
-  // shutdown to avoid this hack.
-  plugin_task_runner_->Detach();
-
   // Stop listening for policy updates.
   if (policy_watcher_.get()) {
     base::WaitableEvent policy_watcher_stopped_(true, false);
@@ -124,35 +116,17 @@ HostNPScriptObject::~HostNPScriptObject() {
   }
 
   if (host_context_.get()) {
-    // Disconnect synchronously. We cannot disconnect asynchronously
-    // here because |host_context_| needs to be stopped on the plugin
-    // thread, but the plugin thread may not exist after the instance
-    // is destroyed.
-    disconnected_event_.Reset();
     DisconnectInternal();
-
-    // |disconnected_event_| is signalled when the host is completely stopped.
-    disconnected_event_.Wait();
-
-    // UI needs to be shut down on the UI thread before we destroy the
-    // host context (because it depends on the context object), but
-    // only after the host has been shut down (becase the UI object is
-    // registered as status observer for the host, and we can't
-    // unregister it from this thread).
-    it2me_host_user_interface_.reset();
-
-    // Release the context's TaskRunner references for the threads, so they can
-    // exit when no objects need them.
-    host_context_->ReleaseTaskRunners();
-
-    // |stopped_event_| is signalled when the last reference to the plugin
-    // thread is dropped.
-    stopped_event_.Wait();
-
-    // Stop all threads.
-    host_context_.reset();
+  } else {
+    plugin_task_runner_->Quit();
   }
 
+  // Stop the message loop and run the remaining tasks. The loop will exit
+  // once the wrapping AutoThreadTaskRunner is destroyed.
+  plugin_task_runner_->DetachAndRunShutdownLoop();
+
+  // Stop all threads.
+  host_context_.reset();
   worker_thread_.Stop();
 }
 
@@ -160,10 +134,12 @@ bool HostNPScriptObject::Init() {
   DCHECK(plugin_task_runner_->BelongsToCurrentThread());
   VLOG(2) << "Init";
 
-  host_context_.reset(new ChromotingHostContext(new AutoThreadTaskRunner(
-      plugin_task_runner_,
-      base::Bind(&base::WaitableEvent::Signal,
-                 base::Unretained(&stopped_event_)))));
+  scoped_refptr<AutoThreadTaskRunner> auto_plugin_task_runner =
+      new AutoThreadTaskRunner(plugin_task_runner_,
+                               base::Bind(&PluginThreadTaskRunner::Quit,
+                                          plugin_task_runner_));
+  host_context_.reset(new ChromotingHostContext(auto_plugin_task_runner));
+  auto_plugin_task_runner = NULL;
   if (!host_context_->Start()) {
     host_context_.reset();
     return false;
@@ -879,6 +855,9 @@ bool HostNPScriptObject::StopDaemon(const NPVariant* args,
 }
 
 void HostNPScriptObject::DisconnectInternal() {
+  if (!host_context_->network_task_runner())
+    return;
+
   if (!host_context_->network_task_runner()->BelongsToCurrentThread()) {
     host_context_->network_task_runner()->PostTask(
         FROM_HERE, base::Bind(&HostNPScriptObject::DisconnectInternal,
@@ -888,13 +867,13 @@ void HostNPScriptObject::DisconnectInternal() {
 
   switch (state_) {
     case kDisconnected:
-      disconnected_event_.Signal();
+      OnShutdownFinished();
       return;
 
     case kStarting:
       SetState(kDisconnecting);
       SetState(kDisconnected);
-      disconnected_event_.Signal();
+      OnShutdownFinished();
       return;
 
     case kDisconnecting:
@@ -921,9 +900,23 @@ void HostNPScriptObject::DisconnectInternal() {
 }
 
 void HostNPScriptObject::OnShutdownFinished() {
-  DCHECK(host_context_->network_task_runner()->BelongsToCurrentThread());
+  if (!host_context_->ui_task_runner()->BelongsToCurrentThread()) {
+    host_context_->ui_task_runner()->PostTask(
+        FROM_HERE, base::Bind(&HostNPScriptObject::OnShutdownFinished,
+                              base::Unretained(this)));
+    return;
+  }
+
+  // UI needs to be shut down on the UI thread before we destroy the
+  // host context (because it depends on the context object), but
+  // only after the host has been shut down (becase the UI object is
+  // registered as status observer for the host, and we can't
+  // unregister it from this thread).
+  it2me_host_user_interface_.reset();
 
-  disconnected_event_.Signal();
+  // Release the context's TaskRunner references for the threads, so they can
+  // exit when no objects need them.
+  host_context_->ReleaseTaskRunners();
 }
 
 void HostNPScriptObject::OnPolicyUpdate(