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

This CL changes the way screen/audio recorders and event executors are managed. New DesktopEnvironmentFactory class is now used by ChromotingHost's owner to specify the kind of desktop environment (or virtual terminal) to be used by the host. Screen/audio recorders and event executors now owned by the ClientSession instance, so there is a separate set of recorders and stubs exists for each authenticated client session. Clients sessions can now be torn dowsn in parallel with the host shuttting down.

This is the 4th attempt to land this change. This version includes:
- |ClientSession| objects are torn down asynchronously now.
- |ClientSession| objects are ref-counted to facilitate the asynchronous shutdown. They still have to be used and destroyed on the network thread.
- |ChromotingHost| now waits until all connections are torn down before deleting the session manager.
- The unit tests were fixed to run message loops until all asynchronous object have been destroyed.
- |ClientSessionTest.ClampMouseEvents| makes sure that the expectations are destroyed when leaving TEST_F scope so that ASAN is not getting confused.

BUG=134694
TEST=remoting_unittests

Review URL: https://chromiumcodereview.appspot.com/10916263

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@156398 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 37 insertions, 141 deletions

diff --git a/remoting/host/chromoting_host.cc b/remoting/host/chromoting_host.cc
index 8b71a1f..4c1503d 100644
--- a/remoting/host/chromoting_host.cc
+++ b/remoting/host/chromoting_host.cc
@@ -10,19 +10,11 @@
 #include "base/message_loop_proxy.h"
 #include "build/build_config.h"
 #include "remoting/base/constants.h"
-#include "remoting/codec/audio_encoder.h"
-#include "remoting/codec/audio_encoder_speex.h"
-#include "remoting/codec/audio_encoder_verbatim.h"
-#include "remoting/codec/video_encoder.h"
-#include "remoting/codec/video_encoder_row_based.h"
-#include "remoting/codec/video_encoder_vp8.h"
-#include "remoting/host/audio_capturer.h"
-#include "remoting/host/audio_scheduler.h"
 #include "remoting/host/chromoting_host_context.h"
 #include "remoting/host/desktop_environment.h"
+#include "remoting/host/desktop_environment_factory.h"
 #include "remoting/host/event_executor.h"
 #include "remoting/host/host_config.h"
-#include "remoting/host/screen_recorder.h"
 #include "remoting/protocol/connection_to_client.h"
 #include "remoting/protocol/client_stub.h"
 #include "remoting/protocol/host_stub.h"
@@ -67,13 +59,13 @@ const net::BackoffEntry::Policy kDefaultBackoffPolicy = {
 ChromotingHost::ChromotingHost(
     ChromotingHostContext* context,
     SignalStrategy* signal_strategy,
-    DesktopEnvironment* environment,
+    DesktopEnvironmentFactory* desktop_environment_factory,
     scoped_ptr<protocol::SessionManager> session_manager)
     : context_(context),
-      desktop_environment_(environment),
+      desktop_environment_factory_(desktop_environment_factory),
       session_manager_(session_manager.Pass()),
       signal_strategy_(signal_strategy),
-      stopping_recorders_(0),
+      clients_count_(0),
       state_(kInitial),
       protocol_config_(protocol::CandidateSessionConfig::CreateDefault()),
       login_backoff_(&kDefaultBackoffPolicy),
@@ -81,10 +73,9 @@ ChromotingHost::ChromotingHost(
       reject_authenticating_client_(false) {
   DCHECK(context_);
   DCHECK(signal_strategy);
-  DCHECK(desktop_environment_);
   DCHECK(context_->network_task_runner()->BelongsToCurrentThread());
 
-  if (!AudioCapturer::IsSupported()) {
+  if (!desktop_environment_factory_->SupportsAudioCapture()) {
     // Disable audio by replacing our list of supported audio configurations
     // with the NONE config.
     protocol_config_->mutable_audio_configs()->clear();
@@ -142,18 +133,15 @@ void ChromotingHost::Shutdown(const base::Closure& shutdown_task) {
         shutdown_tasks_.push_back(shutdown_task);
       state_ = kStopping;
 
-      // Disconnect all of the clients, implicitly stopping the ScreenRecorder.
+      // Disconnect all of the clients.
       while (!clients_.empty()) {
         clients_.front()->Disconnect();
       }
-      DCHECK(!recorder_.get());
-      DCHECK(!audio_scheduler_.get());
 
-      // Destroy session manager.
-      session_manager_.reset();
-
-      if (!stopping_recorders_)
+      // Run the remaining shutdown tasks.
+      if (state_ == kStopping && !clients_count_)
         ShutdownFinish();
+
       break;
   }
 }
@@ -197,15 +185,13 @@ void ChromotingHost::OnSessionAuthenticated(ClientSession* client) {
   ClientList clients_copy(clients_);
   for (ClientList::const_iterator other_client = clients_copy.begin();
        other_client != clients_copy.end(); ++other_client) {
-    if ((*other_client) != client) {
+    if (other_client->get() != client) {
       (*other_client)->Disconnect();
     }
   }
 
   // Disconnects above must have destroyed all other clients and |recorder_|.
   DCHECK_EQ(clients_.size(), 1U);
-  DCHECK(!recorder_.get());
-  DCHECK(!audio_scheduler_.get());
 
   // Notify observers that there is at least one authenticated client.
   const std::string& jid = client->client_jid();
@@ -225,32 +211,7 @@ void ChromotingHost::OnSessionAuthenticated(ClientSession* client) {
 void ChromotingHost::OnSessionChannelsConnected(ClientSession* client) {
   DCHECK(context_->network_task_runner()->BelongsToCurrentThread());
 
-  // Then we create a ScreenRecorder passing the message loops that
-  // it should run on.
-  VideoEncoder* video_encoder =
-      CreateVideoEncoder(client->connection()->session()->config());
-
-  recorder_ = new ScreenRecorder(context_->capture_task_runner(),
-                                 context_->encode_task_runner(),
-                                 context_->network_task_runner(),
-                                 desktop_environment_->capturer(),
-                                 video_encoder);
-  if (client->connection()->session()->config().is_audio_enabled()) {
-    scoped_ptr<AudioEncoder> audio_encoder =
-        CreateAudioEncoder(client->connection()->session()->config());
-    audio_scheduler_ = new AudioScheduler(
-        context_->audio_task_runner(),
-        context_->network_task_runner(),
-        desktop_environment_->audio_capturer(),
-        audio_encoder.Pass(),
-        client->connection()->audio_stub());
-  }
-
-  // Immediately add the connection and start the session.
-  recorder_->AddConnection(client->connection());
-  recorder_->Start();
-  desktop_environment_->OnSessionStarted(client->CreateClipboardProxy());
-
+  // Notify observers.
   FOR_EACH_OBSERVER(HostStatusObserver, status_observers_,
                     OnClientConnected(client->client_jid()));
 }
@@ -266,42 +227,26 @@ void ChromotingHost::OnSessionAuthenticationFailed(ClientSession* client) {
 void ChromotingHost::OnSessionClosed(ClientSession* client) {
   DCHECK(context_->network_task_runner()->BelongsToCurrentThread());
 
-  scoped_ptr<ClientSession> client_destroyer(client);
-
-  ClientList::iterator it = std::find(clients_.begin(), clients_.end(), client);
-  CHECK(it != clients_.end());
-  clients_.erase(it);
-
-  if (recorder_.get()) {
-    recorder_->RemoveConnection(client->connection());
-  }
-
-  if (audio_scheduler_.get()) {
-    audio_scheduler_->OnClientDisconnected();
-    StopAudioScheduler();
+  ClientList::iterator it = clients_.begin();
+  for (; it != clients_.end(); ++it) {
+    if (it->get() == client) {
+      break;
+    }
   }
+  CHECK(it != clients_.end());
 
   if (client->is_authenticated()) {
     FOR_EACH_OBSERVER(HostStatusObserver, status_observers_,
                       OnClientDisconnected(client->client_jid()));
-
-    // TODO(sergeyu): This teardown logic belongs to ClientSession
-    // class. It should start/stop screen recorder or tell the host
-    // when to do it.
-    if (recorder_.get()) {
-      // Currently we don't allow more than one simultaneous connection,
-      // so we need to shutdown recorder when a client disconnects.
-      StopScreenRecorder();
-    }
-    desktop_environment_->OnSessionFinished();
   }
+
+  client->Stop(base::Bind(&ChromotingHost::OnClientStopped, this));
+  clients_.erase(it);
 }
 
 void ChromotingHost::OnSessionSequenceNumber(ClientSession* session,
                                              int64 sequence_number) {
   DCHECK(context_->network_task_runner()->BelongsToCurrentThread());
-  if (recorder_.get())
-    recorder_->UpdateSequenceNumber(sequence_number);
 }
 
 void ChromotingHost::OnSessionRouteChange(
@@ -355,17 +300,23 @@ void ChromotingHost::OnIncomingSession(
 
   LOG(INFO) << "Client connected: " << session->jid();
 
+  // Create the desktop integration implementation for the client to use.
+  scoped_ptr<DesktopEnvironment> desktop_environment =
+      desktop_environment_factory_->Create(context_);
+
   // Create a client object.
   scoped_ptr<protocol::ConnectionToClient> connection(
       new protocol::ConnectionToClient(session));
-  ClientSession* client = new ClientSession(
+  scoped_refptr<ClientSession> client = new ClientSession(
       this,
+      context_->capture_task_runner(),
+      context_->encode_task_runner(),
+      context_->network_task_runner(),
       connection.Pass(),
-      desktop_environment_->event_executor(),
-      desktop_environment_->event_executor(),
-      desktop_environment_->capturer(),
+      desktop_environment.Pass(),
       max_session_duration_);
   clients_.push_back(client);
+  clients_count_++;
 }
 
 void ChromotingHost::set_protocol_config(
@@ -424,75 +375,20 @@ void ChromotingHost::SetUiStrings(const UiStrings& ui_strings) {
   ui_strings_ = ui_strings;
 }
 
-// TODO(sergeyu): Move this to SessionManager?
-// static
-VideoEncoder* ChromotingHost::CreateVideoEncoder(
-    const protocol::SessionConfig& config) {
-  const protocol::ChannelConfig& video_config = config.video_config();
-
-  if (video_config.codec == protocol::ChannelConfig::CODEC_VERBATIM) {
-    return VideoEncoderRowBased::CreateVerbatimEncoder();
-  } else if (video_config.codec == protocol::ChannelConfig::CODEC_ZIP) {
-    return VideoEncoderRowBased::CreateZlibEncoder();
-  } else if (video_config.codec == protocol::ChannelConfig::CODEC_VP8) {
-    return new remoting::VideoEncoderVp8();
-  }
-
-  return NULL;
-}
-
-// static
-scoped_ptr<AudioEncoder> ChromotingHost::CreateAudioEncoder(
-    const protocol::SessionConfig& config) {
-  const protocol::ChannelConfig& audio_config = config.audio_config();
-
-  if (audio_config.codec == protocol::ChannelConfig::CODEC_VERBATIM) {
-    return scoped_ptr<AudioEncoder>(new AudioEncoderVerbatim());
-  } else if (audio_config.codec == protocol::ChannelConfig::CODEC_SPEEX) {
-    return scoped_ptr<AudioEncoder>(new AudioEncoderSpeex());
-  }
-
-  NOTIMPLEMENTED();
-  return scoped_ptr<AudioEncoder>(NULL);
-}
-
-void ChromotingHost::StopScreenRecorder() {
+void ChromotingHost::OnClientStopped() {
   DCHECK(context_->network_task_runner()->BelongsToCurrentThread());
-  DCHECK(recorder_.get());
 
-  ++stopping_recorders_;
-  scoped_refptr<ScreenRecorder> recorder = recorder_;
-  recorder_ = NULL;
-  recorder->Stop(base::Bind(&ChromotingHost::OnRecorderStopped, this));
-}
-
-void ChromotingHost::StopAudioScheduler() {
-  DCHECK(context_->network_task_runner()->BelongsToCurrentThread());
-  DCHECK(audio_scheduler_.get());
-
-  ++stopping_recorders_;
-  scoped_refptr<AudioScheduler> recorder = audio_scheduler_;
-  audio_scheduler_ = NULL;
-  recorder->Stop(base::Bind(&ChromotingHost::OnRecorderStopped, this));
-}
-
-void ChromotingHost::OnRecorderStopped() {
-  if (!context_->network_task_runner()->BelongsToCurrentThread()) {
-    context_->network_task_runner()->PostTask(
-        FROM_HERE, base::Bind(&ChromotingHost::OnRecorderStopped, this));
-    return;
-  }
-
-  --stopping_recorders_;
-  DCHECK_GE(stopping_recorders_, 0);
-
-  if (!stopping_recorders_ && state_ == kStopping)
+  --clients_count_;
+  if (state_ == kStopping && !clients_count_)
     ShutdownFinish();
 }
 
 void ChromotingHost::ShutdownFinish() {
   DCHECK(context_->network_task_runner()->BelongsToCurrentThread());
-  DCHECK(!stopping_recorders_);
+  DCHECK_EQ(state_, kStopping);
+
+  // Destroy session manager.
+  session_manager_.reset();
 
   state_ = kStopped;