Always run PolicyWatcher on UI thread in It2Me host.

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/threading/platform_thread.h"
 #include "net/socket/client_socket_factory.h"
@@ skipping 338 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) {
-  // The policy watcher runs on the |ui_task_runner| on ChromeOS and the
-  // |network_task_runner| on other platforms.
+  // The policy watcher runs on the |ui_task_runner|.
   if (!host_context_->network_task_runner()->BelongsToCurrentThread()) {
     host_context_->network_task_runner()->PostTask(

[Łukasz Anforowicz, 2015/01/30 05:35:55]

After your changes, OnPolicyUpdate will always be called on the ui_task_runner,
and therefore we can remove this if/PostTask statement, right?
(i.e. this if statement can be replaced with a DCHECK, right?)

[Sergey Ulanov, 2015/01/30 19:33:45]

This callback is called on UI thread, but we still want to process it on the
network thread. This PostTask() jumps from UI to Network thread.

[Łukasz Anforowicz, 2015/01/30 19:50:59]

Acknowledged.

         FROM_HERE,
         base::Bind(&It2MeHost::OnPolicyUpdate, this, base::Passed(&policies)));
     return;
   }
 
   bool nat_policy;
   if (policies->GetBoolean(policy::key::kRemoteAccessHostFirewallTraversal,
                            &nat_policy)) {
     UpdateNatPolicy(nat_policy);
   }
@@ skipping 152 matching lines @@
   DCHECK(policy_service);
   DCHECK(!policy_service_) << "|policy_service| can only be set once.";
   policy_service_ = policy_service;
 }
 
 scoped_refptr<It2MeHost> It2MeHostFactory::CreateIt2MeHost(
     scoped_ptr<ChromotingHostContext> context,
     base::WeakPtr<It2MeHost::Observer> observer,
     const XmppSignalStrategy::XmppServerConfig& xmpp_server_config,
     const std::string& directory_bot_jid) {
+  DCHECK(context->ui_task_runner()->BelongsToCurrentThread());
+
   scoped_ptr<It2MeConfirmationDialogFactory> confirmation_dialog_factory(
       new It2MeConfirmationDialogFactory());
   scoped_ptr<policy_hack::PolicyWatcher> policy_watcher =
       policy_hack::PolicyWatcher::Create(policy_service_,
+                                         context->ui_task_runner(),
                                          context->network_task_runner());

[Łukasz Anforowicz, 2015/01/30 05:35:55]

I wonder if we should be using file_task_runner here (instead of
network_task_runner) - this thread will end up being used for reading policy
contents from json files / windows registry / mac preferences.

[Sergey Ulanov, 2015/01/30 19:33:45]

Yes, you are right. From the AsyncPolicyLoader name I assumed that it doesn't
block the "background thread", but it does. Fixed now.

   return new It2MeHost(context.Pass(), policy_watcher.Pass(),
                        confirmation_dialog_factory.Pass(),
                        observer, xmpp_server_config, directory_bot_jid);
 }
 
 }  // namespace remoting