Create the desktop environment before any of the channel were connected.

remoting/host/client_session.cc

Index: remoting/host/client_session.cc
diff --git a/remoting/host/client_session.cc b/remoting/host/client_session.cc
index 24cad356852183deea19934d2a9afb99f86af3fa..04b531a9b8fd7f6bcdd0db4b54bcae9f9ab66ab7 100644
--- a/remoting/host/client_session.cc
+++ b/remoting/host/client_session.cc
@@ -124,8 +124,7 @@ void ClientSession::ControlVideo(const protocol::VideoControl& video_control) {
   if (video_control.has_enable()) {
     VLOG(1) << "Received VideoControl (enable="
             << video_control.enable() << ")";
-    if (video_scheduler_)
-      video_scheduler_->Pause(!video_control.enable());
+    video_scheduler_->Pause(!video_control.enable());
   }
 }
 
@@ -165,17 +164,19 @@ void ClientSession::SetCapabilities(
 
   // Calculate the set of capabilities enabled by both client and host and
   // pass it to the desktop environment if it is available.
-  if (desktop_environment_) {
-    desktop_environment_->SetCapabilities(
-        IntersectCapabilities(*client_capabilities_, host_capabilities_));
-  }
+  desktop_environment_->SetCapabilities(
+      IntersectCapabilities(*client_capabilities_, host_capabilities_));
 }
 
 void ClientSession::OnConnectionAuthenticated(
     protocol::ConnectionToClient* connection) {
   DCHECK(CalledOnValidThread());
   DCHECK_EQ(connection_.get(), connection);
+  DCHECK(!audio_scheduler_);
   DCHECK(!desktop_environment_);
+  DCHECK(!input_injector_);
+  DCHECK(!screen_controls_);
+  DCHECK(!video_scheduler_);
 
   auth_input_filter_.set_enabled(true);
   auth_clipboard_filter_.set_enabled(true);
@@ -190,65 +191,41 @@ void ClientSession::OnConnectionAuthenticated(
                               this, &ClientSession::DisconnectSession);
   }
 
-  // The session may be destroyed as the result result of this call, so it must
-  // be the last in this method.
-  event_handler_->OnSessionAuthenticated(this);
-}
-
-void ClientSession::OnConnectionChannelsConnected(
-    protocol::ConnectionToClient* connection) {
-  DCHECK(CalledOnValidThread());
-  DCHECK_EQ(connection_.get(), connection);
-  DCHECK(!audio_scheduler_);
-  DCHECK(!input_injector_);
-  DCHECK(!screen_controls_);
-  DCHECK(!video_scheduler_);
+  // Disconnect the session if the connection was rejected by the host.
+  if (!event_handler_->OnSessionAuthenticated(this)) {
+    DisconnectSession();
+    return;
+  }
 
   // Create the desktop environment.
   desktop_environment_ =
       desktop_environment_factory_->Create(control_factory_.GetWeakPtr());
   host_capabilities_ = desktop_environment_->GetCapabilities();
 
-  // Negotiate capabilities with the client.
-  if (connection_->session()->config().SupportsCapabilities()) {
-    VLOG(1) << "Host capabilities: " << host_capabilities_;
-
-    protocol::Capabilities capabilities;
-    capabilities.set_capabilities(host_capabilities_);
-    connection_->client_stub()->SetCapabilities(capabilities);
-
-    // |client_capabilities_| could have been received before all channels were
-    // connected. Process them now.
-    if (client_capabilities_) {
-      desktop_environment_->SetCapabilities(
-          IntersectCapabilities(*client_capabilities_, host_capabilities_));
-    }
-  } else {
+  // Ignore protocol::Capabilities messages from the client if it does not
+  // support any capabilities.
+  if (!connection_->session()->config().SupportsCapabilities()) {
     VLOG(1) << "The client does not support any capabilities.";
 
     client_capabilities_ = make_scoped_ptr(new std::string());
-    desktop_environment_->SetCapabilities(*client_capabilities_);
   }
 
   // Create the object that controls the screen resolution.
   screen_controls_ = desktop_environment_->CreateScreenControls();
 
-  // Create and start the event executor.
+  // Create the event executor.
   input_injector_ = desktop_environment_->CreateInputInjector();

[Wez, 2013/04/30 13:48:39]

nit: You don't really need to do this at this point, since the input pipeline
specifically ignores input until SetDisableInputs(false) is called.

[alexeypa (please no reviews), 2013/04/30 18:16:20]

Yes, but it allows the host to create the event executor in parallel with
channel creation. 

-  input_injector_->Start(CreateClipboardProxy());
 
   // Connect the host clipboard and input stubs.
   host_input_filter_.set_input_stub(input_injector_.get());
   clipboard_echo_filter_.set_host_stub(input_injector_.get());
 
-  SetDisableInputs(false);
-
   // Create a VideoEncoder based on the session's video channel configuration.
   scoped_ptr<VideoEncoder> video_encoder =
       CreateVideoEncoder(connection_->session()->config());
 
   // Create a VideoScheduler to pump frames from the capturer to the client.
-  video_scheduler_ = VideoScheduler::Create(
+  video_scheduler_ = new VideoScheduler(
       video_capture_task_runner_,
       video_encode_task_runner_,
       network_task_runner_,
@@ -261,13 +238,49 @@ void ClientSession::OnConnectionChannelsConnected(
   if (connection_->session()->config().is_audio_enabled()) {
     scoped_ptr<AudioEncoder> audio_encoder =
         CreateAudioEncoder(connection_->session()->config());
-    audio_scheduler_ = AudioScheduler::Create(
+    audio_scheduler_ = new AudioScheduler(
         audio_task_runner_,
         network_task_runner_,
         desktop_environment_->CreateAudioCapturer(),
         audio_encoder.Pass(),
         connection_->audio_stub());
   }
+}
+
+void ClientSession::OnConnectionChannelsConnected(
+    protocol::ConnectionToClient* connection) {
+  DCHECK(CalledOnValidThread());
+  DCHECK_EQ(connection_.get(), connection);
+  DCHECK(desktop_environment_);
+  DCHECK(input_injector_);
+  DCHECK(video_scheduler_);

[Sergey Ulanov, 2013/04/30 17:19:26]

nit: Many of these dchecks are not necessary - scoped_ptr<> dchecks in
operator-> anyway.

[alexeypa (please no reviews), 2013/04/30 18:16:20]

Done.

+
+  // Negotiate capabilities with the client.
+  if (connection_->session()->config().SupportsCapabilities()) {
+    VLOG(1) << "Host capabilities: " << host_capabilities_;
+
+    protocol::Capabilities capabilities;
+    capabilities.set_capabilities(host_capabilities_);
+    connection_->client_stub()->SetCapabilities(capabilities);
+  }
+
+  // |client_capabilities_| could have been received before all channels were
+  // connected. Process them now.
+  if (client_capabilities_) {
+    desktop_environment_->SetCapabilities(

[Sergey Ulanov, 2013/04/30 17:19:26]

Do you need this here? ClientSession::SetCapabilities() always calls
desktop_environment_->SetCapabilities(), so if client_capabilities_ is set, it
has already been called.

[alexeypa (please no reviews), 2013/04/30 18:16:20]

Done.

+        IntersectCapabilities(*client_capabilities_, host_capabilities_));
+  }
+
+  // Start the event executor.
+  input_injector_->Start(CreateClipboardProxy());
+  SetDisableInputs(false);
+
+  // Start capturing the screen.
+  video_scheduler_->Start();
+
+  // Start recording audio.
+  if (connection_->session()->config().is_audio_enabled())
+    audio_scheduler_->Start();
 
   // Notify the event handler that all our channels are now connected.
   event_handler_->OnSessionChannelsConnected(this);