Disconnect active connections when another client is authenticated.

Previosly active connections were disconnected only when a new connection has
connected all channels. This means that in some cases we had multiple simultaneous
connections. Only one active connection was allowed, but there could
be multiple clients trying to establish connection simultaneously. This CL
tightens this restriction: now there can't be no more than one authenticated
connection.
Also modified HostStatusObserver to make it possible to distinguish between
authentication and connection events.

BUG=128689

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

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

5 files changed, 38 insertions, 38 deletions

diff --git a/remoting/host/chromoting_host.cc b/remoting/host/chromoting_host.cc
index 1c90477..ae5a02b 100644
--- a/remoting/host/chromoting_host.cc
+++ b/remoting/host/chromoting_host.cc
@@ -183,10 +183,6 @@ void ChromotingHost::OnSessionAuthenticated(ClientSession* client) {
   DCHECK(context_->network_message_loop()->BelongsToCurrentThread());
 
   login_backoff_.Reset();
-}
-
-void ChromotingHost::OnSessionChannelsConnected(ClientSession* client) {
-  DCHECK(context_->network_message_loop()->BelongsToCurrentThread());
 
   // Disconnect all other clients.
   // Iterate over a copy of the list of clients, to avoid mutating the list
@@ -203,6 +199,24 @@ void ChromotingHost::OnSessionChannelsConnected(ClientSession* client) {
   DCHECK_EQ(clients_.size(), 1U);
   DCHECK(!recorder_.get());
 
+  // Notify observers that there is at least one authenticated client.
+  const std::string& jid = client->client_jid();
+
+  reject_authenticating_client_ = false;
+
+  authenticating_client_ = true;
+  FOR_EACH_OBSERVER(HostStatusObserver, status_observers_,
+                    OnClientAuthenticated(jid));
+  authenticating_client_ = false;
+
+  if (reject_authenticating_client_) {
+    client->Disconnect();
+  }
+}
+
+void ChromotingHost::OnSessionChannelsConnected(ClientSession* client) {
+  DCHECK(context_->network_message_loop()->BelongsToCurrentThread());
+
   // Then we create a ScreenRecorder passing the message loops that
   // it should run on.
   Encoder* encoder = CreateEncoder(client->connection()->session()->config());
@@ -217,20 +231,6 @@ void ChromotingHost::OnSessionChannelsConnected(ClientSession* client) {
   recorder_->AddConnection(client->connection());
   recorder_->Start();
   desktop_environment_->OnSessionStarted();
-
-  // Notify observers that there is at least one authenticated client.
-  const std::string& jid = client->client_jid();
-
-  reject_authenticating_client_ = false;
-
-  authenticating_client_ = true;
-  FOR_EACH_OBSERVER(HostStatusObserver, status_observers_,
-                    OnClientAuthenticated(jid));
-  authenticating_client_ = false;
-
-  if (reject_authenticating_client_) {
-    client->Disconnect();
-  }
 }
 
 void ChromotingHost::OnSessionAuthenticationFailed(ClientSession* client) {
diff --git a/remoting/host/host_status_observer.h b/remoting/host/host_status_observer.h
index 6a2a2c9..482b1fb 100644
--- a/remoting/host/host_status_observer.h
+++ b/remoting/host/host_status_observer.h
@@ -19,29 +19,33 @@ struct TransportRoute;
 };
 
 // Interface for host status observer. All methods are invoked on the
-// network thread.
+// network thread. Observers must not tear-down ChromotingHost state
+// on receipt of these callbacks; they are purely informational.
 class HostStatusObserver {
  public:
   HostStatusObserver() { }
   virtual ~HostStatusObserver() { }
 
   // Called when an unauthorized user attempts to connect to the host.
-  virtual void OnAccessDenied(const std::string& jid) = 0;
+  virtual void OnAccessDenied(const std::string& jid) {}
 
-  // Called when a client authenticates, or disconnects. Observers
-  // must not tear-down ChromotingHost state on receipt of this
-  // callback; it is purely informational.
-  virtual void OnClientAuthenticated(const std::string& jid) = 0;
-  virtual void OnClientDisconnected(const std::string& jid) = 0;
+  // A new client is authenticated.
+  virtual void OnClientAuthenticated(const std::string& jid) {}
+
+  // All channels for an autheticated client are connected.
+  virtual void OnClientConnected(const std::string& jid) {}
+
+  // An authenticated client is disconnected.
+  virtual void OnClientDisconnected(const std::string& jid) {}
 
   // Called on notification of a route change event, when a channel is
   // connected.
   virtual void OnClientRouteChange(const std::string& jid,
                                    const std::string& channel_name,
-                                   const protocol::TransportRoute& route) { }
+                                   const protocol::TransportRoute& route) {}
 
   // Called when the host shuts down.
-  virtual void OnShutdown() = 0;
+  virtual void OnShutdown() {}
 };
 
 }  // namespace remoting
diff --git a/remoting/host/log_to_server.cc b/remoting/host/log_to_server.cc
index d296270..212143c 100644
--- a/remoting/host/log_to_server.cc
+++ b/remoting/host/log_to_server.cc
@@ -67,7 +67,7 @@ void LogToServer::OnSignalStrategyStateChange(SignalStrategy::State state) {
   }
 }
 
-void LogToServer::OnClientAuthenticated(const std::string& jid) {
+void LogToServer::OnClientConnected(const std::string& jid) {
   DCHECK(CalledOnValidThread());
   LogSessionStateChange(true);
 }
@@ -78,9 +78,6 @@ void LogToServer::OnClientDisconnected(const std::string& jid) {
   connection_type_set_ = false;
 }
 
-void LogToServer::OnAccessDenied(const std::string& jid) {
-}
-
 void LogToServer::OnClientRouteChange(const std::string& jid,
                                       const std::string& channel_name,
                                       const protocol::TransportRoute& route) {
@@ -92,9 +89,6 @@ void LogToServer::OnClientRouteChange(const std::string& jid,
   }
 }
 
-void LogToServer::OnShutdown() {
-}
-
 void LogToServer::Log(const ServerLogEntry& entry) {
   pending_entries_.push_back(entry);
   SendPendingEntries();
diff --git a/remoting/host/log_to_server.h b/remoting/host/log_to_server.h
index f17b3af..520eeee 100644
--- a/remoting/host/log_to_server.h
+++ b/remoting/host/log_to_server.h
@@ -49,14 +49,12 @@ class LogToServer : public base::NonThreadSafe,
       SignalStrategy::State state) OVERRIDE;
 
   // HostStatusObserver interface.
-  virtual void OnClientAuthenticated(const std::string& jid) OVERRIDE;
+  virtual void OnClientConnected(const std::string& jid) OVERRIDE;
   virtual void OnClientDisconnected(const std::string& jid) OVERRIDE;
-  virtual void OnAccessDenied(const std::string& jid) OVERRIDE;
   virtual void OnClientRouteChange(
       const std::string& jid,
       const std::string& channel_name,
       const protocol::TransportRoute& route) OVERRIDE;
-  virtual void OnShutdown() OVERRIDE;
 
  private:
   void Log(const ServerLogEntry& entry);
diff --git a/remoting/host/log_to_server_unittest.cc b/remoting/host/log_to_server_unittest.cc
index c443157..029bbec 100644
--- a/remoting/host/log_to_server_unittest.cc
+++ b/remoting/host/log_to_server_unittest.cc
@@ -62,6 +62,7 @@ TEST_F(LogToServerTest, SendNow) {
   route.type = protocol::TransportRoute::DIRECT;
   log_to_server_->OnClientRouteChange("client@domain.com/5678", "video", route);
   log_to_server_->OnClientAuthenticated("client@domain.com/5678");
+  log_to_server_->OnClientConnected("client@domain.com/5678");
   log_to_server_->OnSignalStrategyStateChange(SignalStrategy::DISCONNECTED);
   message_loop_.Run();
 }
@@ -71,6 +72,7 @@ TEST_F(LogToServerTest, SendLater) {
   route.type = protocol::TransportRoute::DIRECT;
   log_to_server_->OnClientRouteChange("client@domain.com/5678", "video", route);
   log_to_server_->OnClientAuthenticated("client@domain.com/5678");
+  log_to_server_->OnClientConnected("client@domain.com/5678");
   {
     InSequence s;
     EXPECT_CALL(signal_strategy_, GetLocalJid())
@@ -93,7 +95,9 @@ TEST_F(LogToServerTest, SendTwoEntriesLater) {
   route.type = protocol::TransportRoute::DIRECT;
   log_to_server_->OnClientRouteChange("client@domain.com/5678", "video", route);
   log_to_server_->OnClientAuthenticated("client@domain.com/5678");
-  log_to_server_->OnClientAuthenticated("client2@domain.com/6789");
+  log_to_server_->OnClientConnected("client@domain.com/5678");
+  log_to_server_->OnClientAuthenticated("client@domain.com/6789");
+  log_to_server_->OnClientConnected("client@domain.com/6789");
   {
     InSequence s;
     EXPECT_CALL(signal_strategy_, GetLocalJid())