Remote assistance on Chrome OS Part IV - It2MeHost

remoting/host/it2me/it2me_host.h

Index: remoting/host/it2me/it2me_host.h
diff --git a/remoting/host/it2me/it2me_host.h b/remoting/host/it2me/it2me_host.h
index c24f2d10d4e6f136f47027888180a9cd35afc276..58aba0b5c692113aec4c05d0bb0f272f077930ed 100644
--- a/remoting/host/it2me/it2me_host.h
+++ b/remoting/host/it2me/it2me_host.h
@@ -123,6 +123,10 @@ class It2MeHost : public base::RefCountedThreadSafe<It2MeHost>,
   // the UI thread.
   void ShutdownOnUiThread();
 
+  // Called when |policy_watcher_| has stopped listening for changes and it is
+  // safe to delete the |policy_watcher_| object.

[Wez, 2014/10/17 17:58:01]

nit: ... safe to delete it.

[kelvinp, 2014/10/20 00:21:17]

Done.

+  void OnPolicyWatcherShutdown();
+
   // Called when initial policies are read, and when they change.
   void OnPolicyUpdate(scoped_ptr<base::DictionaryValue> policies);
 
@@ -135,7 +139,10 @@ class It2MeHost : public base::RefCountedThreadSafe<It2MeHost>,
   // The creator of the It2MeHost object owns the the host context and is
   // responsible for keeping it alive throughout the liftime of the host.
   ChromotingHostContext* host_context_;

[Wez, 2014/10/17 17:58:01]

That doesn't make sense; the It2Me object is ref-counted, so the caller has no
control over its lifetime.

Can you just have the It2MeHost own the context? You create a new one for each
host instance anyway.

[kelvinp, 2014/10/20 00:21:17]

We can but this would mean that it2meHost would need a context factory as a new
context is created each time.

+
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;

[Wez, 2014/10/17 17:58:01]

Add a comment, or better still rename this to ui_task_runner, to clarify the
distinction between it and the network task runner, below.

[kelvinp, 2014/10/20 00:21:17]

I have removed network_task_runner.

+  scoped_refptr<base::SingleThreadTaskRunner> network_task_runner_;

[Wez, 2014/10/17 17:58:01]

Why have you pulled this out of the host context?

[kelvinp, 2014/10/20 00:21:18]

Because It2MeHost outlives the host context and we still need the
network_task_runner() on shutdown.  

+
   base::WeakPtr<It2MeHost::Observer> observer_;
   XmppSignalStrategy::XmppServerConfig xmpp_server_config_;
   std::string directory_bot_jid_;