[Chromoting] The daemon process now starts the networking process and passes the host configuration…

remoting/host/daemon_process.h

Index: remoting/host/daemon_process.h
diff --git a/remoting/host/daemon_process.h b/remoting/host/daemon_process.h
index ed5f4cba1fc4a37598baa350fe6ec8f124ff1885..1a4f5dc3d9e7b63b23ccb875e7803801acad1e9d 100644
--- a/remoting/host/daemon_process.h
+++ b/remoting/host/daemon_process.h
@@ -7,27 +7,30 @@
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
+#include "base/file_path.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/timer.h"
 #include "ipc/ipc_channel.h"
 #include "ipc/ipc_channel_proxy.h"
 #include "remoting/base/stoppable.h"
+#include "remoting/host/worker_process_delegate.h"
 
 namespace base {
 class SingleThreadTaskRunner;
 class Thread;
-}  // namespace base
 
-namespace IPC {
-class Message;
-} // namespace IPC
+namespace files {
+class FilePathWatcher;
+}  // namespace files
+}  // namespace base
 
 namespace remoting {
 
 // This class implements core of the daemon process. It manages the networking
 // process running at lower privileges and maintains the list of virtual
 // terminals.
-class DaemonProcess : public Stoppable, public IPC::Listener {
+class DaemonProcess : public Stoppable, public WorkerProcessDelegate {
  public:
   virtual ~DaemonProcess();
 
@@ -37,33 +40,81 @@ class DaemonProcess : public Stoppable, public IPC::Listener {
       scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
       const base::Closure& stopped_callback);
 
-  // IPC::Listener implementation.
+  // WorkerProcessDelegate implementation.
+  virtual void OnChannelConnected() OVERRIDE;
   virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
 
+  // Sends an IPC message to the worker process. The message will be dropped
+  // unless the network process is connected over the IPC channel.

[Wez, 2012/08/21 00:18:57]

Is this comment correct?

[alexeypa (please no reviews), 2012/08/21 17:16:36]

It appears correct to me.

[Wez, 2012/08/24 22:20:37]

Why is the message to the worker process dropped if the network process is not
connected?

[alexeypa (please no reviews), 2012/08/27 23:16:41]

We don't want to send a message that will become outdated when the child process
receives it. For instance:

1. A configuration update is sent with empty configuration.
2. The actual configuration is read and another message is sent.
2. The network process connects and receives two configuration update messages:
an empty and the real one.

The messages are noty ignored once the child process is connected and we get a
callback when it connects. Once the channel is broken the child process will be
restarted.

[Wez, 2012/08/30 20:36:04]

The point I'm making is that the wording sounds like the "worker" and "network"
processes are different, so it's not clear why the absence of one should affect
message-posting to the other.

[alexeypa (please no reviews), 2012/08/30 23:15:13]

This was a very obscure way of asking this question. Done. 

+  virtual void Send(IPC::Message* message) = 0;
+
  protected:
   DaemonProcess(scoped_refptr<base::SingleThreadTaskRunner> main_task_runner,
                 scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
                 const base::Closure& stopped_callback);
 
   // Reads the host configuration and launches the networking process.
-  void Init();
+  void Initialize();
 
   // Stoppable implementation.
   virtual void DoStop() OVERRIDE;
 
   // Launches the network process and establishes an IPC channel with it.
-  virtual bool LaunchNetworkProcess() = 0;
+  virtual void LaunchNetworkProcess() = 0;

[Wez, 2012/08/21 00:18:57]

nit: I think this should be public, according to the style guide, or better
still a method on a delegate interface, rather than having callers derived from
DaemonProcess.

[alexeypa (please no reviews), 2012/08/23 23:53:01]

The style guide said that inheritance should be public but it says nothing
whether inherited methods (including virtual once) should be public.
I could probably split DaemonProcess<Os> and DaemonProcess into independent
classes. I think it will complicate shutdown without bringing much value. It
does not seem we are going to take advantage from the separation in the nearest
future.

Factoring out the configuration watcher on the other hand should help to define
the shutdown sequence better.

+
+  scoped_refptr<base::SingleThreadTaskRunner> main_task_runner() {
+    return main_task_runner_;
+  }
+
+  scoped_refptr<base::SingleThreadTaskRunner> io_task_runner() {
+    return io_task_runner_;
+  }
 
  private:
+  // Called every time the host configuration file is updated.
+  void ConfigUpdated(const FilePath& path, bool error);
+
+  // Reads the configuration file and passes it to the |main_task_runner_|
+  // thread.
+  void ReloadConfig();
+
+  // Sends |config| to the network process.
+  void SendConfig(const std::string& config);
+
+  // Starts monitoring the configuration file updates.
+  void StartConfigWatcher();
+
+  // Stops monitoring the configuration file updates.
+  void StopConfigWatcher();
+
+  // Notifies the |main_task_runner_| thread that the configuration file watcher
+  // has been fully stopped.
+  void StopConfigWatcherCompleted();
+
+  // The configuration file contents.
+  std::string config_;
+
+  FilePath config_path_;
+
+  scoped_ptr<base::DelayTimer<DaemonProcess> > config_updated_timer_;
+
+  // Monitors the host configuration file changes.
+  scoped_ptr<base::files::FilePathWatcher> config_watcher_;
+
+  enum ConfigWatcherState {
+    kConfigWatcherRunning,
+    kConfigWatcherStopping,
+    kConfigWatcherStopped
+  };
+
+  ConfigWatcherState config_watcher_state_;
+
   // The main task runner. Typically it is the UI message loop.
   scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
 
   // Handles IPC and background I/O tasks.
   scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
 
-  // The IPC channel connecting the daemon process to the networking process.
-  scoped_ptr<IPC::ChannelProxy> network_process_channel_;
-
   DISALLOW_COPY_AND_ASSIGN(DaemonProcess);
 };