[Chromoting] LogToServer correctly handles multiple simultaneous connections.

BUG=128753


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

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

3 files changed, 166 insertions, 32 deletions

diff --git a/remoting/host/log_to_server.cc b/remoting/host/log_to_server.cc
index 212143c..25e2f77 100644
--- a/remoting/host/log_to_server.cc
+++ b/remoting/host/log_to_server.cc
@@ -26,8 +26,7 @@ LogToServer::LogToServer(ChromotingHost* host,
                          SignalStrategy* signal_strategy)
     : host_(host),
       mode_(mode),
-      signal_strategy_(signal_strategy),
-      connection_type_set_(false) {
+      signal_strategy_(signal_strategy) {
   signal_strategy_->AddListener(this);
 
   // |host| may be NULL in tests.
@@ -41,7 +40,8 @@ LogToServer::~LogToServer() {
     host_->RemoveStatusObserver(this);
 }
 
-void LogToServer::LogSessionStateChange(bool connected) {
+void LogToServer::LogSessionStateChange(const std::string& jid,
+                                        bool connected) {
   DCHECK(CalledOnValidThread());
 
   scoped_ptr<ServerLogEntry> entry(
@@ -50,8 +50,8 @@ void LogToServer::LogSessionStateChange(bool connected) {
   entry->AddModeField(mode_);
 
   if (connected) {
-    DCHECK(connection_type_set_);
-    entry->AddConnectionTypeField(connection_type_);
+    DCHECK(connection_route_type_.count(jid) == 1);
+    entry->AddConnectionTypeField(connection_route_type_[jid]);
   }
   Log(*entry.get());
 }
@@ -69,13 +69,13 @@ void LogToServer::OnSignalStrategyStateChange(SignalStrategy::State state) {
 
 void LogToServer::OnClientConnected(const std::string& jid) {
   DCHECK(CalledOnValidThread());
-  LogSessionStateChange(true);
+  LogSessionStateChange(jid, true);
 }
 
 void LogToServer::OnClientDisconnected(const std::string& jid) {
   DCHECK(CalledOnValidThread());
-  LogSessionStateChange(false);
-  connection_type_set_ = false;
+  LogSessionStateChange(jid, false);
+  connection_route_type_.erase(jid);
 }
 
 void LogToServer::OnClientRouteChange(const std::string& jid,
@@ -84,8 +84,7 @@ void LogToServer::OnClientRouteChange(const std::string& jid,
   // Store connection type for the video channel. It is logged later
   // when client authentication is finished.
   if (channel_name == kVideoChannelName) {
-    connection_type_ = route.type;
-    connection_type_set_ = true;
+    connection_route_type_[jid] = route.type;
   }
 }
 
diff --git a/remoting/host/log_to_server.h b/remoting/host/log_to_server.h
index 520eeee..765239645 100644
--- a/remoting/host/log_to_server.h
+++ b/remoting/host/log_to_server.h
@@ -6,6 +6,8 @@
 #define REMOTING_HOST_LOG_TO_SERVER_H_
 
 #include <deque>
+#include <map>
+#include <string>
 
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
@@ -42,7 +44,7 @@ class LogToServer : public base::NonThreadSafe,
 
   // Logs a session state change. Currently, this is either
   // connection or disconnection.
-  void LogSessionStateChange(bool connected);
+  void LogSessionStateChange(const std::string& jid, bool connected);
 
   // SignalStrategy::Listener interface.
   virtual void OnSignalStrategyStateChange(
@@ -64,8 +66,10 @@ class LogToServer : public base::NonThreadSafe,
   ServerLogEntry::Mode mode_;
   SignalStrategy* signal_strategy_;
   scoped_ptr<IqSender> iq_sender_;
-  protocol::TransportRoute::RouteType connection_type_;
-  bool connection_type_set_;
+  // A map from client JID to the route type of that client's connection to
+  // this host.
+  std::map<std::string, protocol::TransportRoute::RouteType>
+      connection_route_type_;
   std::deque<ServerLogEntry> pending_entries_;
 
   DISALLOW_COPY_AND_ASSIGN(LogToServer);
diff --git a/remoting/host/log_to_server_unittest.cc b/remoting/host/log_to_server_unittest.cc
index 029bbec..54674fa 100644
--- a/remoting/host/log_to_server_unittest.cc
+++ b/remoting/host/log_to_server_unittest.cc
@@ -11,6 +11,8 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/libjingle/source/talk/xmllite/xmlelement.h"
 
+using buzz::XmlElement;
+using buzz::QName;
 using testing::_;
 using testing::DeleteArg;
 using testing::InSequence;
@@ -24,6 +26,96 @@ ACTION_P(QuitMainMessageLoop, message_loop) {
   message_loop->PostTask(FROM_HERE, MessageLoop::QuitClosure());
 }
 
+const char kJabberClientNamespace[] = "jabber:client";
+const char kChromotingNamespace[] = "google:remoting";
+const char kClientJid1[] = "client@domain.com/1234";
+const char kClientJid2[] = "client@domain.com/5678";
+const char kHostJid[] = "host@domain.com/1234";
+
+bool IsLogEntryForConnection(XmlElement* node, const char* connection_type) {
+  return (node->Name() == QName(kChromotingNamespace, "entry") &&
+          node->Attr(QName("", "event-name")) == "session-state" &&
+          node->Attr(QName("", "session-state")) == "connected" &&
+          node->Attr(QName("", "role")) == "host" &&
+          node->Attr(QName("", "mode")) == "me2me" &&
+          node->Attr(QName("", "connection-type")) == connection_type);
+}
+
+MATCHER_P(IsClientConnected, connection_type, "") {
+  if (arg->Name() != QName(kJabberClientNamespace, "iq")) {
+    return false;
+  }
+  buzz::XmlElement* log_stanza = arg->FirstChild()->AsElement();
+  if (log_stanza->Name() !=QName(kChromotingNamespace, "log")) {
+    return false;
+  }
+  if (log_stanza->NextChild()) {
+    return false;
+  }
+  buzz::XmlElement* log_entry = log_stanza->FirstChild()->AsElement();
+  if (!IsLogEntryForConnection(log_entry, connection_type)) {
+    return false;
+  }
+  if (log_entry->NextChild()) {
+    return false;
+  }
+  return true;
+}
+
+MATCHER_P2(IsTwoClientsConnected, connection_type1, connection_type2, "") {
+  if (arg->Name() != QName(kJabberClientNamespace, "iq")) {
+    return false;
+  }
+  buzz::XmlElement* log_stanza = arg->FirstChild()->AsElement();
+  if (log_stanza->Name() !=QName(kChromotingNamespace, "log")) {
+    return false;
+  }
+  if (log_stanza->NextChild()) {
+    return false;
+  }
+  buzz::XmlElement* log_entry = log_stanza->FirstChild()->AsElement();
+  if (!IsLogEntryForConnection(log_entry, connection_type1)) {
+    return false;
+  }
+  log_entry = log_entry->NextChild()->AsElement();
+  if (!IsLogEntryForConnection(log_entry, connection_type2)) {
+    return false;
+  }
+  if (log_entry->NextChild()) {
+    return false;
+  }
+  return true;
+}
+
+bool IsLogEntryForDisconnection(XmlElement* node) {
+  return (node->Name() == QName(kChromotingNamespace, "entry") &&
+          node->Attr(QName("", "event-name")) == "session-state" &&
+          node->Attr(QName("", "session-state")) == "closed" &&
+          node->Attr(QName("", "role")) == "host" &&
+          node->Attr(QName("", "mode")) == "me2me");
+}
+
+MATCHER(IsClientDisconnected, "") {
+  if (arg->Name() != QName(kJabberClientNamespace, "iq")) {
+    return false;
+  }
+  buzz::XmlElement* log_stanza = arg->FirstChild()->AsElement();
+  if (log_stanza->Name() !=QName(kChromotingNamespace, "log")) {
+    return false;
+  }
+  if (log_stanza->NextChild()) {
+    return false;
+  }
+  buzz::XmlElement* log_entry = log_stanza->FirstChild()->AsElement();
+  if (!IsLogEntryForDisconnection(log_entry)) {
+    return false;
+  }
+  if (log_entry->NextChild()) {
+    return false;
+  }
+  return true;
+}
+
 }  // namespace
 
 class LogToServerTest : public testing::Test {
@@ -48,10 +140,10 @@ TEST_F(LogToServerTest, SendNow) {
   {
     InSequence s;
     EXPECT_CALL(signal_strategy_, GetLocalJid())
-        .WillRepeatedly(Return("host@domain.com/1234"));
+        .WillRepeatedly(Return(kHostJid));
     EXPECT_CALL(signal_strategy_, AddListener(_));
     EXPECT_CALL(signal_strategy_, GetNextId());
-    EXPECT_CALL(signal_strategy_, SendStanzaPtr(_))
+    EXPECT_CALL(signal_strategy_, SendStanzaPtr(IsClientConnected("direct")))
         .WillOnce(DoAll(DeleteArg<0>(), Return(true)));
     EXPECT_CALL(signal_strategy_, RemoveListener(_))
         .WillOnce(QuitMainMessageLoop(&message_loop_))
@@ -60,9 +152,9 @@ TEST_F(LogToServerTest, SendNow) {
   log_to_server_->OnSignalStrategyStateChange(SignalStrategy::CONNECTED);
   protocol::TransportRoute route;
   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_->OnClientRouteChange(kClientJid1, "video", route);
+  log_to_server_->OnClientAuthenticated(kClientJid1);
+  log_to_server_->OnClientConnected(kClientJid1);
   log_to_server_->OnSignalStrategyStateChange(SignalStrategy::DISCONNECTED);
   message_loop_.Run();
 }
@@ -70,16 +162,16 @@ TEST_F(LogToServerTest, SendNow) {
 TEST_F(LogToServerTest, SendLater) {
   protocol::TransportRoute route;
   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_->OnClientRouteChange(kClientJid1, "video", route);
+  log_to_server_->OnClientAuthenticated(kClientJid1);
+  log_to_server_->OnClientConnected(kClientJid1);
   {
     InSequence s;
     EXPECT_CALL(signal_strategy_, GetLocalJid())
-        .WillRepeatedly(Return("host@domain.com/1234"));
+        .WillRepeatedly(Return(kHostJid));
     EXPECT_CALL(signal_strategy_, AddListener(_));
     EXPECT_CALL(signal_strategy_, GetNextId());
-    EXPECT_CALL(signal_strategy_, SendStanzaPtr(_))
+    EXPECT_CALL(signal_strategy_, SendStanzaPtr(IsClientConnected("direct")))
         .WillOnce(DoAll(DeleteArg<0>(), Return(true)));
     EXPECT_CALL(signal_strategy_, RemoveListener(_))
         .WillOnce(QuitMainMessageLoop(&message_loop_))
@@ -91,26 +183,65 @@ TEST_F(LogToServerTest, SendLater) {
 }
 
 TEST_F(LogToServerTest, SendTwoEntriesLater) {
-  protocol::TransportRoute route;
-  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_->OnClientAuthenticated("client@domain.com/6789");
-  log_to_server_->OnClientConnected("client@domain.com/6789");
+  protocol::TransportRoute route1;
+  route1.type = protocol::TransportRoute::DIRECT;
+  log_to_server_->OnClientRouteChange(kClientJid1, "video", route1);
+  log_to_server_->OnClientAuthenticated(kClientJid1);
+  log_to_server_->OnClientConnected(kClientJid1);
+  protocol::TransportRoute route2;
+  route2.type = protocol::TransportRoute::STUN;
+  log_to_server_->OnClientRouteChange(kClientJid2, "video", route2);
+  log_to_server_->OnClientAuthenticated(kClientJid2);
+  log_to_server_->OnClientConnected(kClientJid2);
+  {
+    InSequence s;
+    EXPECT_CALL(signal_strategy_, GetLocalJid())
+        .WillRepeatedly(Return(kHostJid));
+    EXPECT_CALL(signal_strategy_, AddListener(_));
+    EXPECT_CALL(signal_strategy_, GetNextId());
+    EXPECT_CALL(signal_strategy_, SendStanzaPtr(
+            IsTwoClientsConnected("direct", "stun")))
+        .WillOnce(DoAll(DeleteArg<0>(), Return(true)));
+    EXPECT_CALL(signal_strategy_, RemoveListener(_))
+        .WillOnce(QuitMainMessageLoop(&message_loop_))
+        .RetiresOnSaturation();
+  }
+  log_to_server_->OnSignalStrategyStateChange(SignalStrategy::CONNECTED);
+  log_to_server_->OnSignalStrategyStateChange(SignalStrategy::DISCONNECTED);
+  message_loop_.Run();
+}
+
+TEST_F(LogToServerTest, HandleRouteChangeInUnusualOrder) {
   {
     InSequence s;
     EXPECT_CALL(signal_strategy_, GetLocalJid())
-        .WillRepeatedly(Return("host@domain.com/1234"));
+        .WillRepeatedly(Return(kHostJid));
     EXPECT_CALL(signal_strategy_, AddListener(_));
     EXPECT_CALL(signal_strategy_, GetNextId());
-    EXPECT_CALL(signal_strategy_, SendStanzaPtr(_))
+    EXPECT_CALL(signal_strategy_, SendStanzaPtr(IsClientConnected("direct")))
+        .WillOnce(DoAll(DeleteArg<0>(), Return(true)));
+    EXPECT_CALL(signal_strategy_, GetNextId());
+    EXPECT_CALL(signal_strategy_, SendStanzaPtr(IsClientDisconnected()))
+        .WillOnce(DoAll(DeleteArg<0>(), Return(true)));
+    EXPECT_CALL(signal_strategy_, GetNextId());
+    EXPECT_CALL(signal_strategy_, SendStanzaPtr(IsClientConnected("stun")))
         .WillOnce(DoAll(DeleteArg<0>(), Return(true)));
     EXPECT_CALL(signal_strategy_, RemoveListener(_))
         .WillOnce(QuitMainMessageLoop(&message_loop_))
         .RetiresOnSaturation();
   }
   log_to_server_->OnSignalStrategyStateChange(SignalStrategy::CONNECTED);
+  protocol::TransportRoute route1;
+  route1.type = protocol::TransportRoute::DIRECT;
+  log_to_server_->OnClientRouteChange(kClientJid1, "video", route1);
+  log_to_server_->OnClientAuthenticated(kClientJid1);
+  log_to_server_->OnClientConnected(kClientJid1);
+  protocol::TransportRoute route2;
+  route2.type = protocol::TransportRoute::STUN;
+  log_to_server_->OnClientRouteChange(kClientJid2, "video", route2);
+  log_to_server_->OnClientDisconnected(kClientJid1);
+  log_to_server_->OnClientAuthenticated(kClientJid2);
+  log_to_server_->OnClientConnected(kClientJid2);
   log_to_server_->OnSignalStrategyStateChange(SignalStrategy::DISCONNECTED);
   message_loop_.Run();
 }