[Chromoting] Request the daemon to open a terminal once a connection has been accepted.

remoting/host/ipc_desktop_environment.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_IPC_DESKTOP_ENVIRONMENT_H_
+#define REMOTING_HOST_IPC_DESKTOP_ENVIRONMENT_H_
+
+#include "base/basictypes.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "remoting/host/desktop_environment.h"
+
+namespace remoting {
+
+class ChromotingHost;
+class DesktopSessionConnector;
+
+// A variant of desktop environment integrating with the desktop by means of
+// a helper process and talking to that process via IPC.
+class IpcDesktopEnvironment : public DesktopEnvironment {
+ public:
+  // Passes relevant task runners, |desktop_session_connector| and |host|.

[Wez, 2012/10/17 17:51:57]

nit: Lose this line.

[alexeypa (please no reviews), 2012/10/17 21:35:53]

Done.

+  // |desktop_session_connector| is used to bind the IpcDesktopEnvironment and

[Wez, 2012/10/17 17:51:57]

nit: and the -> to a

[alexeypa (please no reviews), 2012/10/17 21:35:53]

Done.

+  // the desktop session so that the former would then receive calls every time
+  // a new IPC channel to the desktop process is allocated.

[Wez, 2012/10/17 17:51:57]

nit: "... so that the former... " -> "... to be notified with a new IPC channel
every time the desktop process is changed."

[alexeypa (please no reviews), 2012/10/17 21:35:53]

Done.

+  // |desktop_session_connector| must outlive |this|.
+  // |host| specifis the host instance that indirectly owns |this|.

[Wez, 2012/10/17 17:51:57]

typo: specifies

[Wez, 2012/10/17 17:51:57]

nit: Explain what |host| is used for and restrictions, e.g. |host| must refer to
the ChromotingHost for which this instance was created. It is used to disconnect
the client, e.g. if the desktop session is logged-out.

[alexeypa (please no reviews), 2012/10/17 21:35:53]

Done.

[alexeypa (please no reviews), 2012/10/17 21:35:53]

Done.

+  IpcDesktopEnvironment(scoped_ptr<AudioCapturer> audio_capturer,
+                        scoped_ptr<EventExecutor> event_executor,
+                        scoped_ptr<VideoFrameCapturer> video_capturer,

[Wez, 2012/10/17 17:51:57]

nit: Does the factory really need to pass these, or could it pass the task
runners and let the desktop environment itself create them?

[alexeypa (please no reviews), 2012/10/17 21:35:53]

Done.

+                        DesktopSessionConnector* desktop_session_connector,
+                        scoped_refptr<ChromotingHost> host);
+  virtual ~IpcDesktopEnvironment();
+
+  virtual void Start(
+      scoped_ptr<protocol::ClipboardStub> client_clipboard) OVERRIDE;
+
+  // Disconnects the client that is using |desktop_environment|.
+  void DisconnectClient();

[Wez, 2012/10/17 17:51:57]

Won't this be triggered by an IPC from the daemon, in which case should it be a
private API within IpcDesktopEnvironment?

[alexeypa (please no reviews), 2012/10/17 21:35:53]

IpcDesktopEnvironmentFactory calls it, so it needs to be public.

+
+ private:
+  // True if |this| has been connected to a desktop session.
+  bool connected_;

[Wez, 2012/10/17 17:51:57]

nit: Put this after the two fields that come from the ctor

[alexeypa (please no reviews), 2012/10/17 21:35:53]

Done.

+
+  DesktopSessionConnector* desktop_session_connector_;
+
+  // The host instance |this| belongs to.
+  scoped_refptr<ChromotingHost> host_;
+
+  DISALLOW_COPY_AND_ASSIGN(IpcDesktopEnvironment);
+};
+
+}  // namespace remoting
+
+#endif  // REMOTING_HOST_IPC_DESKTOP_ENVIRONMENT_H_