Remote assistance on Chrome OS Part IV - It2MeHost

remoting/host/it2me/it2me_host.cc

Index: remoting/host/it2me/it2me_host.cc
diff --git a/remoting/host/it2me/it2me_host.cc b/remoting/host/it2me/it2me_host.cc
index b9a17f8c9431b159fddbcd15a586eb438b315995..c0990c711d0055ee97e2c326d1822e2978df1543 100644
--- a/remoting/host/it2me/it2me_host.cc
+++ b/remoting/host/it2me/it2me_host.cc
@@ -6,7 +6,6 @@
 
 #include "base/bind.h"
 #include "base/strings/string_util.h"
-#include "base/synchronization/waitable_event.h"
 #include "base/threading/platform_thread.h"
 #include "net/socket/client_socket_factory.h"
 #include "remoting/base/auto_thread.h"
@@ -36,18 +35,19 @@ const int kMaxLoginAttempts = 5;
 }  // namespace
 
 It2MeHost::It2MeHost(
-    ChromotingHostContext* host_context,
-    scoped_refptr<base::SingleThreadTaskRunner> task_runner,
+    scoped_ptr<ChromotingHostContext> host_context,
+    scoped_ptr<policy_hack::PolicyWatcher> policy_watcher,
     base::WeakPtr<It2MeHost::Observer> observer,
     const XmppSignalStrategy::XmppServerConfig& xmpp_server_config,
     const std::string& directory_bot_jid)
-  : host_context_(host_context),
-    task_runner_(task_runner),
+  : host_context_(host_context.Pass()),
+    task_runner_(host_context_->ui_task_runner()),
     observer_(observer),
     xmpp_server_config_(xmpp_server_config),
     directory_bot_jid_(directory_bot_jid),
     state_(kDisconnected),
     failed_login_attempts_(0),
+    policy_watcher_(policy_watcher.Pass()),
     nat_traversal_enabled_(false),
     policy_received_(false) {
   DCHECK(task_runner_->BelongsToCurrentThread());
@@ -67,10 +67,7 @@ void It2MeHost::Connect() {
       host_context_->ui_task_runner()));
 
   // Start monitoring configured policies.
-  policy_watcher_.reset(
-      policy_hack::PolicyWatcher::Create(host_context_->network_task_runner()));
-  policy_watcher_->StartWatching(
-      base::Bind(&It2MeHost::OnPolicyUpdate, this));
+  policy_watcher_->StartWatching(base::Bind(&It2MeHost::OnPolicyUpdate, this));
 
   // Switch to the network thread to start the actual connection.
   host_context_->network_task_runner()->PostTask(
@@ -257,13 +254,16 @@ void It2MeHost::ShutdownOnUiThread() {
 
   // Stop listening for policy updates.
   if (policy_watcher_.get()) {
-    base::WaitableEvent policy_watcher_stopped_(true, false);
-    policy_watcher_->StopWatching(&policy_watcher_stopped_);
-    policy_watcher_stopped_.Wait();
-    policy_watcher_.reset();
+    policy_watcher_->StopWatching(
+        base::Bind(&It2MeHost::OnPolicyWatcherShutdown, this));
+    return;
   }
 }
 
+void It2MeHost::OnPolicyWatcherShutdown() {
+  policy_watcher_.reset();
+}
+
 void It2MeHost::OnAccessDenied(const std::string& jid) {
   DCHECK(host_context_->network_task_runner()->BelongsToCurrentThread());
 
@@ -311,7 +311,12 @@ void It2MeHost::OnClientDisconnected(const std::string& jid) {
 }
 
 void It2MeHost::OnPolicyUpdate(scoped_ptr<base::DictionaryValue> policies) {
-  DCHECK(host_context_->network_task_runner()->BelongsToCurrentThread());
+  if (!host_context_->network_task_runner()->BelongsToCurrentThread()) {

[Wez, 2014/10/24 00:28:48]

Previous code assumed/required that this be called on network thread; why has
this changed; looks like that's the thread passed in below, still?

[kelvinp, 2014/10/24 21:39:42]

Because on ChromeOS, the policy_watcher runs in the ui_task_runner of the
browser process.

[Wez, 2014/10/24 23:29:26]

Right, and on other platforms it runs on some other thread... so what changed?
Or are you saying we run it on network thread on other platforms..? Or on other
platforms are we hopping back to ther caller (network) thread to dispatch the
callbacks?

[kelvinp, 2014/10/29 01:22:51]

The policy watcher supports being called from any thread as under the hood, it
re-posts the call to its task_runner.

On other platforms, we always run the policy watcher on the network_task_runner,
and the policy_watcher and it calls us back on the network_task_runner as well.

On chrome OS, we run the policy watcher on the UI thread and it calls us back on
the ui_task_runner.  Therefore, we need to call ourselves again in the
network_task_runner.

+    host_context_->network_task_runner()->PostTask(
+        FROM_HERE,
+        base::Bind(&It2MeHost::OnPolicyUpdate, this, base::Passed(&policies)));
+    return;
+  }
 
   bool nat_policy;
   if (policies->GetBoolean(policy_hack::PolicyWatcher::kNatPolicyName,
@@ -459,18 +464,22 @@ void It2MeHost::OnReceivedSupportID(
   SetState(kReceivedAccessCode);
 }
 
-It2MeHostFactory::It2MeHostFactory() {}
+It2MeHostFactory::It2MeHostFactory(policy::PolicyService* policy_service)
+    : policy_service_(policy_service) {
+}
 
 It2MeHostFactory::~It2MeHostFactory() {}
 
 scoped_refptr<It2MeHost> It2MeHostFactory::CreateIt2MeHost(
-    ChromotingHostContext* context,
-    scoped_refptr<base::SingleThreadTaskRunner> task_runner,
+    scoped_ptr<ChromotingHostContext> context,
     base::WeakPtr<It2MeHost::Observer> observer,
     const XmppSignalStrategy::XmppServerConfig& xmpp_server_config,
     const std::string& directory_bot_jid) {
-  return new It2MeHost(
-      context, task_runner, observer, xmpp_server_config, directory_bot_jid);
+  scoped_ptr<policy_hack::PolicyWatcher> policy_watcher =
+      policy_hack::PolicyWatcher::Create(policy_service_,
+                                         context->network_task_runner());
+  return new It2MeHost(context.Pass(), policy_watcher.Pass(), observer,
+                       xmpp_server_config, directory_bot_jid);
 }
 
 }  // namespace remoting