[Chromoting] Refactoring DesktopEnvironment and moving screen/audio recorders to ClientSession.

remoting/host/win/session_desktop_environment_factory.cc

+// 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.
+
+#include "remoting/host/win/session_desktop_environment_factory.h"
+
+#include "remoting/host/audio_capturer.h"
+#include "remoting/host/chromoting_host_context.h"
+#include "remoting/host/desktop_environment.h"
+#include "remoting/host/event_executor.h"
+#include "remoting/host/video_frame_capturer.h"
+
+#if defined(OS_WIN)
+#include "remoting/host/win/session_event_executor.h"
+#endif  // defined(OS_WIN)
+
+namespace remoting {
+
+SessionDesktopEnvironmentFactory::SessionDesktopEnvironmentFactory() {
+}
+
+SessionDesktopEnvironmentFactory::~SessionDesktopEnvironmentFactory() {
+}
+
+scoped_ptr<DesktopEnvironment> SessionDesktopEnvironmentFactory::Create(
+    ChromotingHostContext* context) {
+#if defined(OS_WIN)

[Wez, 2012/08/31 18:18:02]

This only gets build on Windows, so I don't think you need these checks any
more?

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

Done.

+  scoped_ptr<AudioCapturer> audio_capturer = AudioCapturer::Create();
+  scoped_ptr<EventExecutor> event_executor = EventExecutor::Create(
+      context->desktop_task_runner(),
+      context->ui_task_runner());
+  event_executor.reset(new SessionEventExecutorWin(
+      context->desktop_task_runner(),
+      event_executor.Pass()));
+  scoped_ptr<VideoFrameCapturer> video_capturer(VideoFrameCapturer::Create());
+  return scoped_ptr<DesktopEnvironment>(new DesktopEnvironment(
+      audio_capturer.Pass(),
+      event_executor.Pass(),
+      video_capturer.Pass()));

[simonmorris, 2012/09/04 18:47:21]

This duplicates code, and it's confusing to have cross-platform code in
remoting/host/win.

Could this class have a DesktopEnvironmentFactory member, and delegate Create()
to it, but then construct another DesktopEnvironment using the members of the
Create()'d one, but adding the SessionEventExecutorWin wrapper to the
event_executor member?

That would also simplify the call site. You'd construct an ordinary
DesktopEnvironmentFactory, and on Windows, replace it by the Session-wrapped
version of itself.

[alexeypa (please no reviews), 2012/09/05 22:53:29]

It is Windows-only code. #if defined(OS_WIN) should've not been here. I removed
it in response to Wez's comment.
That would make sense if all kinds of |DesktopEnvironment|'s supported chaining.
It seems that it is not going to be the case for IPC stubs that will live in the
networking process. So I'd like to have a little duplicated code but not let
this feature leak to the interface.
This is how it is done now.

[Wez, 2012/09/06 00:13:22]

Chaining should work for the IPC stubs depending on the behaviour being added by
the chaining implementation; let's discuss off-CL.

+#else  // !defined(OS_WIN)
+  return DesktopEnvironmentFactory::Create(context);
+#endif  // !defined(OS_WIN)
+}
+
+}  // namespace remoting