Adding ability to stream windows and inject events to them

remoting/host/remoting_me2me_host.cc

Index: remoting/host/remoting_me2me_host.cc
diff --git a/remoting/host/remoting_me2me_host.cc b/remoting/host/remoting_me2me_host.cc
index cde2684a86c5be003ad21b0f60d3165e091040a1..76f985c546e4cac58714828d6b967eb63660b3fa 100644
--- a/remoting/host/remoting_me2me_host.cc
+++ b/remoting/host/remoting_me2me_host.cc
@@ -61,6 +61,7 @@
 #include "remoting/host/json_host_config.h"
 #include "remoting/host/logging.h"
 #include "remoting/host/me2me_desktop_environment.h"
+#include "remoting/host/me2me_single_window_desktop_environment.h"
 #include "remoting/host/pairing_registry_delegate.h"
 #include "remoting/host/policy_hack/policy_watcher.h"
 #include "remoting/host/session_manager_factory.h"
@@ -127,6 +128,8 @@ const char kEnableVp9SwitchName[] = "enable-vp9";
 // from stdin.
 const char kStdinConfigPath[] = "-";
 
+extern const char kWindowIdSwitchName[] = "window-id";

[Lambros, 2014/08/05 22:47:24]

nit: remove 'extern'

[ronakvora do not use, 2014/08/06 20:56:18]

Ah, I had this for when I thought I was going to use this directly in
input_injector_mac.mm to redirect the flow of the code.

+
 }  // namespace
 
 namespace remoting {
@@ -302,6 +305,12 @@ class HostProcess
   ThirdPartyAuthConfig third_party_auth_config_;
   bool enable_gnubby_auth_;
 
+  // Boolean to change flow, where ncessary, if we're
+  // capturing a window instead of the entire desktop.
+  bool enable_window_capture_;
+  // Used to specify which window to stream, if enabled.

[Lambros, 2014/08/05 22:47:24]

nit: Blank line before this comment.

[ronakvora do not use, 2014/08/06 20:56:18]

Done.

+  webrtc::WindowId window_id_;
+
   scoped_ptr<OAuthTokenGetter> oauth_token_getter_;
   scoped_ptr<XmppSignalStrategy> signal_strategy_;
   scoped_ptr<SignalingConnector> signaling_connector_;
@@ -425,6 +434,7 @@ bool HostProcess::InitWithCommandLine(const base::CommandLine* cmd_line) {
     LOG(ERROR) << "Can't find host config at " << host_config_path_.value();
     return false;
   }
+

[Lambros, 2014/08/05 22:47:25]

optional: remove this blank line

[ronakvora do not use, 2014/08/06 20:56:19]

Done.

 #endif  // !defined(REMOTING_MULTI_PROCESS)
 
   // Ignore certificate requests - the host currently has no client certificate
@@ -445,7 +455,24 @@ bool HostProcess::InitWithCommandLine(const base::CommandLine* cmd_line) {
   directory_bot_jid_ = service_urls->directory_bot_jid();
 
   signal_parent_ = cmd_line->HasSwitch(kSignalParentSwitchName);
-
+  enable_window_capture_ = cmd_line->HasSwitch(kWindowIdSwitchName);

[Wez, 2014/08/06 04:10:16]

nit: Blank line before this, to keep the window-capture option block of code
visually separated from signal_parent

[ronakvora do not use, 2014/08/06 20:56:19]

Done.

+  if (enable_window_capture_) {
+    // CGWindowId is a uint32_t

[Wez, 2014/08/06 04:10:16]

This comment doesn't seem relevant?

[ronakvora do not use, 2014/08/06 20:56:19]

i'm not sure how window ids are represented on other platforms. Will that matter
in the future? What if someone decides to make it a short since that's how
windows or linux gets their window ids? 

[Wez, 2014/08/07 23:14:32]

This is cross-platform code; it shouldn't have a comment that assumes it's being
built on a particular platform - you could leave a comment to clarify that
unit32_t is large enough to hold window-ids on all of our currently-supported
platforms, if you like.

[ronakvora do not use, 2014/08/08 17:07:13]

Done.

+    uint32_t window_id;
+    if (base::StringToUint(
+            cmd_line->GetSwitchValueASCII(kWindowIdSwitchName),
+            &window_id)) {
+      // window_id_ is of type long int, so we should not lose

[Wez, 2014/08/06 04:10:16]

Doesn't its type depend upon the platform, since it's the WindowId define?

[ronakvora do not use, 2014/08/06 20:56:18]

I'm not sure what the best way would be to get the window_id_ in correctly for
all platforms. Should I have a #if defined(...) for each platform?

[Wez, 2014/08/07 23:14:32]

What's important is the effect of truncation in precision - in this case it'll
mean we get a bogus window Id. If we were going to use it as an index into a
buffer that would be a different situation.

[ronakvora do not use, 2014/08/08 17:07:13]

Got it.

+      // any precision here.
+      window_id_ = static_cast<webrtc::WindowId>(window_id);
+    } else {
+      // Set enable_window_capture_ to false because we failed to get

[Lambros, 2014/08/05 22:47:25]

optional: I would prefer to avoid 'we' in comments and logging. It can be
confusing for non-native English speakers. Maybe say 'The window id from the
command line was not found. Log an error and stream the entire desktop
instead.'?

[ronakvora do not use, 2014/08/06 20:56:18]

Done.

+      // the window id from the command line. Continue streaming
+      // the entire desktop instead.
+      LOG(ERROR) << "We wanted to stream a window but could not find its id.";

[Lambros, 2014/08/05 22:47:25]

Suggestion:
LOG(ERROR) << "Window " << window_id_
           << " not found, streaming entire desktop instead."

[ronakvora do not use, 2014/08/06 20:56:18]

Done.

+      enable_window_capture_ = false;
+    }
+  }
   return true;
 }
 
@@ -676,11 +703,21 @@ void HostProcess::StartOnUiThread() {
           daemon_channel_.get());
   desktop_session_connector_ = desktop_environment_factory;
 #else  // !defined(OS_WIN)
-  DesktopEnvironmentFactory* desktop_environment_factory =
+  DesktopEnvironmentFactory* desktop_environment_factory;
+  if (enable_window_capture_) {
+    desktop_environment_factory =
+      new SingleWindowDesktopEnvironmentFactory(
+          context_->network_task_runner(),
+          context_->input_task_runner(),
+          context_->ui_task_runner(),
+          window_id_);
+  } else {
+    desktop_environment_factory =
       new Me2MeDesktopEnvironmentFactory(
           context_->network_task_runner(),
           context_->input_task_runner(),
           context_->ui_task_runner());
+  }
 #endif  // !defined(OS_WIN)
 
   desktop_environment_factory_.reset(desktop_environment_factory);