Remote assistance on Chrome OS Part IV - It2MeHost

remoting/host/it2me/it2me_host.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 "remoting/host/it2me/it2me_host.h"
 
 #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"
 #include "remoting/base/logging.h"
 #include "remoting/base/rsa_key_pair.h"
 #include "remoting/host/chromoting_host.h"
 #include "remoting/host/chromoting_host_context.h"
 #include "remoting/host/host_event_logger.h"
 #include "remoting/host/host_secret.h"
 #include "remoting/host/host_status_logger.h"
 #include "remoting/host/it2me_desktop_environment.h"
 #include "remoting/host/policy_hack/policy_watcher.h"
 #include "remoting/host/register_support_host_request.h"
 #include "remoting/host/session_manager_factory.h"
 #include "remoting/protocol/it2me_host_authenticator_factory.h"
 #include "remoting/protocol/network_settings.h"
 #include "remoting/signaling/server_log_entry.h"
 
 namespace remoting {
 
 namespace {
 
 // This is used for tagging system event logs.
 const char kApplicationName[] = "chromoting";
 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());
 }
 
 void It2MeHost::Connect() {
   if (!host_context_->ui_task_runner()->BelongsToCurrentThread()) {
     DCHECK(task_runner_->BelongsToCurrentThread());
     host_context_->ui_task_runner()->PostTask(
         FROM_HERE, base::Bind(&It2MeHost::Connect, this));
     return;
   }
 
   desktop_environment_factory_.reset(new It2MeDesktopEnvironmentFactory(
       host_context_->network_task_runner(),
       host_context_->input_task_runner(),
       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(
       FROM_HERE, base::Bind(&It2MeHost::ReadPolicyAndConnect, this));
 }
 
 void It2MeHost::Disconnect() {
   if (!host_context_->network_task_runner()->BelongsToCurrentThread()) {
     DCHECK(task_runner_->BelongsToCurrentThread());
     host_context_->network_task_runner()->PostTask(
@@ skipping 166 matching lines @@
 }
 
 void It2MeHost::ShutdownOnUiThread() {
   DCHECK(host_context_->ui_task_runner()->BelongsToCurrentThread());
 
   // Destroy the DesktopEnvironmentFactory, to free thread references.
   desktop_environment_factory_.reset();
 
   // 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());
 
   ++failed_login_attempts_;
   if (failed_login_attempts_ == kMaxLoginAttempts) {
     Disconnect();
   }
 }
 
 void It2MeHost::OnClientAuthenticated(const std::string& jid) {
@@ skipping 27 matching lines @@
   SetState(kConnected);
 }
 
 void It2MeHost::OnClientDisconnected(const std::string& jid) {
   DCHECK(host_context_->network_task_runner()->BelongsToCurrentThread());
 
   Disconnect();
 }
 
 void It2MeHost::OnPolicyUpdate(scoped_ptr<base::DictionaryValue> policies) {
-  DCHECK(host_context_->network_task_runner()->BelongsToCurrentThread());
+  // The policy watcher runs on the |ui_task_runner| on ChromeOS and the
+  // |network_task_runner| on other platforms.

[Wez, 2014/10/29 18:27:51]

That's just an implementation detail of the CrOS policy watcher though -
wouldn't it be cleaner to fix that to post the update on the |task_runner| that
is passed to it?

[kelvinp, 2014/10/29 22:20:17]

Unfortunately, We can't.  In ChromeOS, |task_runner| is the |ui_task_runner| and
we would always like to handle the policy update on the |network_task_runner|

[Wez, 2014/10/30 01:08:59]

No it isn't - it's still passed the network task runner (see line 488), but then
internally it picks up the UI task runner.

+  if (!host_context_->network_task_runner()->BelongsToCurrentThread()) {
+    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,
                            &nat_policy)) {
     UpdateNatPolicy(nat_policy);
   }
   std::string host_domain;
   if (policies->GetString(policy_hack::PolicyWatcher::kHostDomainPolicyName,
                           &host_domain)) {
     UpdateHostDomainPolicy(host_domain);
@@ skipping 127 matching lines @@
   host_->SetAuthenticatorFactory(factory.Pass());
 
   // Pass the Access Code to the script object before changing state.
   task_runner_->PostTask(
       FROM_HERE, base::Bind(&It2MeHost::Observer::OnStoreAccessCode,
                             observer_, access_code, lifetime));
 
   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