Chromoting service now launches the host process in the session the is currently attached to the ph…

remoting/host/wts_session_process_launcher_win.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_WTS_SESSION_PROCESS_LAUNCHER_WIN_H_
 #define REMOTING_HOST_WTS_SESSION_PROCESS_LAUNCHER_WIN_H_
 
 #include <windows.h>
 
 #include "base/basictypes.h"
+#include "base/file_path.h"
+#include "base/memory/scoped_ptr.h"

[Wez, 2012/02/28 00:22:07]

nit: This include goes after compiler_specific.h

[alexeypa (please no reviews), 2012/02/28 01:14:04]

Done.

 #include "base/compiler_specific.h"
+#include "base/process.h"
+#include "base/time.h"
+#include "base/timer.h"
+#include "base/win/object_watcher.h"
 
 #include "remoting/host/wts_console_observer_win.h"
 
+namespace base {
+namespace win {
+
+class ScopedHandle;
+
+} // namespace win
+} // namespace base
+
 namespace remoting {
 
 class WtsConsoleMonitor;
 
-class WtsSessionProcessLauncher : public WtsConsoleObserver {
+class WtsSessionProcessLauncher
+    : public base::win::ObjectWatcher::Delegate,
+      public WtsConsoleObserver {
  public:
   // Constructs a WtsSessionProcessLauncher object. |monitor| must outlive this
-  // class.
-  WtsSessionProcessLauncher(WtsConsoleMonitor* monitor);
+  // class. |host_binary| is the host executable name to be launched in

[Wez, 2012/02/28 00:22:07]

nit: ... is the name of the executable to be launched ...

[alexeypa (please no reviews), 2012/02/28 01:14:04]

Done.

+  // the session(s) attached to the console.

[Wez, 2012/02/28 00:22:07]

nit: the console session.

There can't be >1 console session, i.e. no plurals, AFAIK.

[alexeypa (please no reviews), 2012/02/28 01:14:04]

Done.

+  WtsSessionProcessLauncher(WtsConsoleMonitor* monitor,
+                            const FilePath& host_binary);
+
   virtual ~WtsSessionProcessLauncher();
 
+  // base::win::ObjectWatcher::Delegate implementation
+  virtual void OnObjectSignaled(HANDLE object) OVERRIDE;
+
   // WtsConsoleObserver implementation
   virtual void OnSessionAttached(uint32 session_id) OVERRIDE;
   virtual void OnSessionDetached() OVERRIDE;
 
  private:
+  // Attempts to launch the host process in the currently attached session.
+  // Schedules next launch attempt if creation of the process fails for any
+  // reason.
+  void LaunchProcess();
+
+  // Name of the host executable.
+  FilePath host_binary_;
+
+  // Time of the last launch attempt.
+  base::Time launch_time_;
+
+  // Current backoff delay.
+  base::TimeDelta launch_backoff_;
+
+  // Number of times we tried to launch the process.
+  int launch_attempts_;

[alexeypa (please no reviews), 2012/02/28 01:14:04]

launch_attempts_ is not used BTW. I removed it.

+
+  // This pointer is used to unsubscribe from session attach and detach events.
   WtsConsoleMonitor* monitor_;
 
+  // Impersonation token that has the SE_TCB_NAME privilege enabled.
+  scoped_ptr<base::win::ScopedHandle> privileged_token_;
+
+  // A handle of the process injected into the watched session.

[Wez, 2012/02/28 00:22:07]

nit: A -> The

[alexeypa (please no reviews), 2012/02/28 01:14:04]

Done.

+  base::Process process_;
+
+  // Monitors the launched process calling a callback when it terminates.
+  base::win::ObjectWatcher process_watcher_;
+
+  // A token to be used to launch a process in a different session.

[Wez, 2012/02/28 00:22:07]

nit: A -> The

This is the token with the console session-Id set in it, ready for launch,
right?

[alexeypa (please no reviews), 2012/02/28 01:14:04]

Done.
Correct.

+  scoped_ptr<base::win::ScopedHandle> session_token_;
+
+  // Defines the states the process launcher can be.

[Wez, 2012/02/28 00:22:07]

nit: ... can be in.

[alexeypa (please no reviews), 2012/02/28 01:14:04]

Done.

+  enum State {
+    StateDetached,
+    StateStarting,
+    StateAttached,
+  };
+
+  // Current state of the process launcher.
+  State state_;
+
+  // This time is used to delay next attempt to launch the host.

[Wez, 2012/02/28 00:22:07]

nit: suggest "Timer used to schedule the next attempt to launch the process."

[alexeypa (please no reviews), 2012/02/28 01:14:04]

Done.

+  base::OneShotTimer<WtsSessionProcessLauncher> timer_;

[Wez, 2012/02/28 00:22:07]

Why is this timer down here when the |launch_time_| and |launch_backoff_| appear
earlier in the file?

[alexeypa (please no reviews), 2012/02/28 01:14:04]

The members are sorted alphabetically. It makes sense to group related members
together.

[Wez, 2012/02/28 22:55:36]

I'd group them logically and sort alphabetically only to provide ordering within
groupings, ideally.

[alexeypa (please no reviews), 2012/02/29 04:17:56]

Make sense. Coincidentally they are grouped and sorted already.

+
   DISALLOW_COPY_AND_ASSIGN(WtsSessionProcessLauncher);
 };
 
 }  // namespace remoting
 
 #endif  // REMOTING_HOST_WTS_SESSION_PROCESS_LAUNCHER_WIN_H_