Remote assistance on Chrome OS Part IV - It2MeHost

remoting/host/remoting_me2me_host.cc

Index: remoting/host/remoting_me2me_host.cc
diff --git a/remoting/host/remoting_me2me_host.cc b/remoting/host/remoting_me2me_host.cc
index 6aa6b1e27eefdfead7a33b6ab89920cf4f6ba5cc..0a6466cab39a0b8a5fa246c20a42959c92369916 100644
--- a/remoting/host/remoting_me2me_host.cc
+++ b/remoting/host/remoting_me2me_host.cc
@@ -19,7 +19,6 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
-#include "base/synchronization/waitable_event.h"
 #include "base/threading/thread.h"
 #include "build/build_config.h"
 #include "crypto/nss_util.h"
@@ -260,6 +259,8 @@ class HostProcess
 
   void ShutdownOnNetworkThread();
 
+  void PostPolicyWatcherShutdown();
+
   // Crashes the process in response to a daemon's request. The daemon passes
   // the location of the code that detected the fatal error resulted in this
   // request.
@@ -376,15 +377,6 @@ HostProcess::~HostProcess() {
   DCHECK(!config_watcher_);
   DCHECK(!daemon_channel_);
   DCHECK(!desktop_environment_factory_);
-
-  // We might be getting deleted on one of the threads the |host_context| owns,
-  // so we need to post it back to the caller thread to safely join & delete the
-  // threads it contains.  This will go away when we move to AutoThread.
-  // |context_release()| will null |context_| before the method is invoked, so
-  // we need to pull out the task-runner on which to call DeleteSoon first.
-  scoped_refptr<base::SingleThreadTaskRunner> task_runner =
-      context_->ui_task_runner();
-  task_runner->DeleteSoon(FROM_HERE, context_.release());

[Jamie, 2014/10/14 01:18:42]

Why has this been deleted?

[kelvinp, 2014/10/15 23:03:10]

Good catch

 }
 
 bool HostProcess::InitWithCommandLine(const base::CommandLine* cmd_line) {
@@ -524,8 +516,8 @@ void HostProcess::OnConfigUpdated(
     // already loaded so PolicyWatcher has to be started here. Separate policy
     // loading from policy verifications and move |policy_watcher_|
     // initialization to StartOnNetworkThread().
-    policy_watcher_.reset(
-        policy_hack::PolicyWatcher::Create(context_->file_task_runner()));
+    policy_watcher_.reset(policy_hack::PolicyWatcher::Create(
+        context_.get(), context_->file_task_runner()));
     policy_watcher_->StartWatching(
         base::Bind(&HostProcess::OnPolicyUpdate, base::Unretained(this)));
   } else {
@@ -1411,24 +1403,25 @@ void HostProcess::ShutdownOnNetworkThread() {
     state_ = HOST_STOPPED;
 
     if (policy_watcher_.get()) {
-      base::WaitableEvent done_event(true, false);
-      policy_watcher_->StopWatching(&done_event);
-      done_event.Wait();
-      policy_watcher_.reset();
+      policy_watcher_->StopWatching(
+          base::Bind(&HostProcess::PostPolicyWatcherShutdown, this));
+    } else {
+      PostPolicyWatcherShutdown();
     }
-
-    config_watcher_.reset();
-
-    // Complete the rest of shutdown on the main thread.
-    context_->ui_task_runner()->PostTask(
-        FROM_HERE,
-        base::Bind(&HostProcess::ShutdownOnUiThread, this));
   } else {
     // This method is only called in STOPPING_TO_RESTART and STOPPING states.
     NOTREACHED();
   }
 }
 
+void HostProcess::PostPolicyWatcherShutdown() {

[rmsousa, 2014/10/14 00:18:25]

nit: OnPolicyWatcherShutdown.
"Post" is an ambiguous term - we typically use it with its verb meaning (i.e. to
post something to a task), so to use it with its adverb meaning (i.e. this will
happen after the policy watcher is shutdown) is confusing.

[kelvinp, 2014/10/15 23:03:10]

Done.

+  policy_watcher_.reset();
+
+  // Complete the rest of shutdown on the main thread.
+  context_->ui_task_runner()->PostTask(
+      FROM_HERE, base::Bind(&HostProcess::ShutdownOnUiThread, this));
+}
+
 void HostProcess::OnCrash(const std::string& function_name,
                           const std::string& file_name,
                           const int& line_number) {