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

remoting/host/client_session.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/client_session.h"
 
 #include <algorithm>
 
 #include "base/message_loop_proxy.h"
 #include "media/video/capture/screen/screen_capturer.h"
@@ skipping 106 matching lines @@
   if (client_resolution.IsValid())
     screen_controls_->SetScreenResolution(client_resolution);
 }
 
 void ClientSession::ControlVideo(const protocol::VideoControl& video_control) {
   DCHECK(CalledOnValidThread());
 
   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());
   }
 }
 
 void ClientSession::ControlAudio(const protocol::AudioControl& audio_control) {
   DCHECK(CalledOnValidThread());
 
   if (audio_control.has_enable()) {
     VLOG(1) << "Received AudioControl (enable="
             << audio_control.enable() << ")";
     if (audio_scheduler_)
@@ skipping 19 matching lines @@
   }
 
   client_capabilities_ = make_scoped_ptr(new std::string());
   if (capabilities.has_capabilities())
     *client_capabilities_ = capabilities.capabilities();
 
   VLOG(1) << "Client capabilities: " << *client_capabilities_;
 
   // 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);
 
   clipboard_echo_filter_.set_client_stub(connection_->client_stub());
   mouse_clamping_filter_.set_video_stub(connection_->video_stub());
 
   if (max_duration_ > base::TimeDelta()) {
     // TODO(simonmorris): Let Disconnect() tell the client that the
     // disconnection was caused by the session exceeding its maximum duration.
     max_duration_timer_.Start(FROM_HERE, max_duration_,
                               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_,
       desktop_environment_->CreateVideoCapturer(),
       video_encoder.Pass(),
       connection_->client_stub(),
       &mouse_clamping_filter_);
 
   // Create an AudioScheduler if audio is enabled, to pump audio samples.
   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);
 }
 
 void ClientSession::OnConnectionClosed(
     protocol::ConnectionToClient* connection,
     protocol::ErrorCode error) {
   DCHECK(CalledOnValidThread());
   DCHECK_EQ(connection_.get(), connection);
@@ skipping 121 matching lines @@
     return scoped_ptr<AudioEncoder>(new AudioEncoderSpeex());
   } else if (audio_config.codec == protocol::ChannelConfig::CODEC_OPUS) {
     return scoped_ptr<AudioEncoder>(new AudioEncoderOpus());
   }
 
   NOTIMPLEMENTED();
   return scoped_ptr<AudioEncoder>(NULL);
 }
 
 }  // namespace remoting