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

remoting/host/daemon_process.h

 // Copyright (c) 2012 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.
 
 #ifndef REMOTING_HOST_DAEMON_PROCESS_H_
 #define REMOTING_HOST_DAEMON_PROCESS_H_
 
 #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 files {
+class FilePathWatcher;
+}  // namespace files
 }  // namespace base
 
-namespace IPC {
-class Message;
-} // namespace IPC
-
 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();
 
   // Creates a platform-specific implementation of the daemon process object.
   static scoped_ptr<DaemonProcess> Create(
       scoped_refptr<base::SingleThreadTaskRunner> main_task_runner,
       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);
 };
 
 }  // namespace remoting
 
 #endif  // REMOTING_HOST_DAEMON_PROCESS_H_