Cleanups in ChromotingHost.

BUG=None
TEST=Unittests

Review URL: http://codereview.chromium.org/6907009

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

3 files changed, 45 insertions, 66 deletions

diff --git a/remoting/host/chromoting_host.cc b/remoting/host/chromoting_host.cc
index d604d52..ea55f6d 100644
--- a/remoting/host/chromoting_host.cc
+++ b/remoting/host/chromoting_host.cc
@@ -171,56 +171,11 @@ void ChromotingHost::Shutdown() {
   }
 }
 
-// This method is called when a client connects.
-void ChromotingHost::OnClientConnected(ConnectionToClient* connection) {
-  DCHECK_EQ(context_->main_message_loop(), MessageLoop::current());
-}
-
-void ChromotingHost::OnClientDisconnected(ConnectionToClient* connection) {
-  DCHECK_EQ(context_->main_message_loop(), MessageLoop::current());
-
-  // Find the client session corresponding to the given connection.
-  std::vector<scoped_refptr<ClientSession> >::iterator client;
-  for (client = clients_.begin(); client != clients_.end(); ++client) {
-    if (client->get()->connection() == connection)
-      break;
-  }
-  if (client == clients_.end())
-    return;
-
-  // Remove the connection from the session manager and stop the session.
-  // TODO(hclam): Stop only if the last connection disconnected.
-  if (recorder_.get()) {
-    recorder_->RemoveConnection(connection);
-    // The recorder only exists to serve the unique authenticated client.
-    // If that client has disconnected, then we can kill the recorder.
-    if (client->get()->authenticated()) {
-      recorder_->Stop(NULL);
-      recorder_ = NULL;
-    }
-  }
-
-  // Close the connection to connection just to be safe.
-  connection->Disconnect();
-
-  // Also remove reference to ConnectionToClient from this object.
-  clients_.erase(client);
-
-  if (!HasAuthenticatedClients())
-    EnableCurtainMode(false);
-}
-
 ////////////////////////////////////////////////////////////////////////////
 // protocol::ConnectionToClient::EventHandler implementations
 void ChromotingHost::OnConnectionOpened(ConnectionToClient* connection) {
   DCHECK_EQ(context_->network_message_loop(), MessageLoop::current());
-
-  // Completes the connection to the client.
   VLOG(1) << "Connection to client established.";
-  context_->main_message_loop()->PostTask(
-      FROM_HERE,
-      NewRunnableMethod(this, &ChromotingHost::OnClientConnected,
-                        make_scoped_refptr(connection)));
 }
 
 void ChromotingHost::OnConnectionClosed(ConnectionToClient* connection) {
@@ -347,14 +302,44 @@ void ChromotingHost::set_protocol_config(
   protocol_config_.reset(config);
 }
 
-void ChromotingHost::AddClient(ClientSession* client) {
-  clients_.push_back(client);
-}
-
 void ChromotingHost::OnServerClosed() {
   // Don't need to do anything here.
 }
 
+void ChromotingHost::OnClientDisconnected(ConnectionToClient* connection) {
+  DCHECK_EQ(context_->main_message_loop(), MessageLoop::current());
+
+  // Find the client session corresponding to the given connection.
+  std::vector<scoped_refptr<ClientSession> >::iterator client;
+  for (client = clients_.begin(); client != clients_.end(); ++client) {
+    if (client->get()->connection() == connection)
+      break;
+  }
+  if (client == clients_.end())
+    return;
+
+  // Remove the connection from the session manager and stop the session.
+  // TODO(hclam): Stop only if the last connection disconnected.
+  if (recorder_.get()) {
+    recorder_->RemoveConnection(connection);
+    // The recorder only exists to serve the unique authenticated client.
+    // If that client has disconnected, then we can kill the recorder.
+    if (client->get()->authenticated()) {
+      recorder_->Stop(NULL);
+      recorder_ = NULL;
+    }
+  }
+
+  // Close the connection to connection just to be safe.
+  connection->Disconnect();
+
+  // Also remove reference to ConnectionToClient from this object.
+  clients_.erase(client);
+
+  if (!HasAuthenticatedClients())
+    EnableCurtainMode(false);
+}
+
 // TODO(sergeyu): Move this to SessionManager?
 Encoder* ChromotingHost::CreateEncoder(const protocol::SessionConfig* config) {
   const protocol::ChannelConfig& video_config = config->video_config();
diff --git a/remoting/host/chromoting_host.h b/remoting/host/chromoting_host.h
index 02143fa..132e7b5 100644
--- a/remoting/host/chromoting_host.h
+++ b/remoting/host/chromoting_host.h
@@ -89,12 +89,6 @@ class ChromotingHost : public base::RefCountedThreadSafe<ChromotingHost>,
   // Asynchronously shutdown the host process.
   void Shutdown();
 
-  // This method is called if a client is connected to this object.
-  void OnClientConnected(protocol::ConnectionToClient* client);
-
-  // This method is called if a client is disconnected from the host.
-  void OnClientDisconnected(protocol::ConnectionToClient* client);
-
   ////////////////////////////////////////////////////////////////////////////
   // protocol::ConnectionToClient::EventHandler implementations
   virtual void OnConnectionOpened(protocol::ConnectionToClient* client);
@@ -121,11 +115,9 @@ class ChromotingHost : public base::RefCountedThreadSafe<ChromotingHost>,
   // |config| is transferred to the object. Must be called before Start().
   void set_protocol_config(protocol::CandidateSessionConfig* config);
 
-  // This setter is only used in unit test to simulate client connection.
-  void AddClient(ClientSession* client);
-
  private:
   friend class base::RefCountedThreadSafe<ChromotingHost>;
+  friend class ChromotingHostTest;
 
   ChromotingHost(ChromotingHostContext* context, MutableHostConfig* config,
                  DesktopEnvironment* environment);
@@ -140,6 +132,9 @@ class ChromotingHost : public base::RefCountedThreadSafe<ChromotingHost>,
   // Callback for protocol::SessionManager::Close().
   void OnServerClosed();
 
+  // This method is called if a client is disconnected from the host.
+  void OnClientDisconnected(protocol::ConnectionToClient* client);
+
   // Creates encoder for the specified configuration.
   Encoder* CreateEncoder(const protocol::SessionConfig* config);
 
diff --git a/remoting/host/chromoting_host_unittest.cc b/remoting/host/chromoting_host_unittest.cc
index 51093b5..ea8dbb4 100644
--- a/remoting/host/chromoting_host_unittest.cc
+++ b/remoting/host/chromoting_host_unittest.cc
@@ -160,14 +160,8 @@ class ChromotingHostTest : public testing::Test {
 
     context_.network_message_loop()->PostTask(
         FROM_HERE,
-        NewRunnableMethod(host_.get(),
-                          &ChromotingHost::AddClient,
-                          client));
-    context_.network_message_loop()->PostTask(
-        FROM_HERE,
-        NewRunnableMethod(host_.get(),
-                          &ChromotingHost::OnClientConnected,
-                          connection));
+        NewRunnableFunction(&ChromotingHostTest::AddClientToHost,
+                            host_, client));
     context_.network_message_loop()->PostTask(
         FROM_HERE,
         NewRunnableMethod(client.get(),
@@ -185,6 +179,11 @@ class ChromotingHostTest : public testing::Test {
                           connection_));
   }
 
+  static void AddClientToHost(scoped_refptr<ChromotingHost> host,
+                              scoped_refptr<ClientSession> session) {
+    host->clients_.push_back(session);
+  }
+
  protected:
   MessageLoop message_loop_;
   MockConnectionToClientEventHandler handler_;