ChromotingHost can have multiple connections, but only one

authenticated connection. When a connection is
authenticated, the host disconnects all other connections.

The result is that if a client has disconnected without the
host noticing, another client can connect immediately,
without having to wait for the older connection to time out.

The new ClientSession class encapsulates a
ConnectionToClient and per-client state. It has taken the
HostStub implementation away from DesktopEnvironment.

BUG=70013
TEST=extra unit test; also see repro steps in BUG

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

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

1 files changed, 97 insertions, 27 deletions

diff --git a/remoting/host/chromoting_host_unittest.cc b/remoting/host/chromoting_host_unittest.cc
index 23295ae..5338c08 100644
--- a/remoting/host/chromoting_host_unittest.cc
+++ b/remoting/host/chromoting_host_unittest.cc
@@ -6,12 +6,12 @@
 #include "remoting/host/capturer_fake.h"
 #include "remoting/host/chromoting_host.h"
 #include "remoting/host/chromoting_host_context.h"
-#include "remoting/host/desktop_environment_fake.h"
 #include "remoting/host/host_mock_objects.h"
 #include "remoting/host/in_memory_host_config.h"
 #include "remoting/proto/video.pb.h"
 #include "remoting/protocol/protocol_mock_objects.h"
 #include "remoting/protocol/session_config.h"
+#include "testing/gmock_mutant.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -27,6 +27,7 @@ using ::remoting::protocol::SessionConfig;
 
 using testing::_;
 using testing::AnyNumber;
+using testing::CreateFunctor;
 using testing::DeleteArg;
 using testing::DoAll;
 using testing::InSequence;
@@ -41,20 +42,6 @@ void PostQuitTask(MessageLoop* message_loop) {
   message_loop->PostTask(FROM_HERE, new MessageLoop::QuitTask());
 }
 
-void BeginSessionRequest(protocol::HostStub* host_stub) {
-  LocalLoginCredentials* credentials =
-      new LocalLoginCredentials();
-  credentials->set_type(protocol::PASSWORD);
-  credentials->set_username("hello");
-
-  const std::string password = "world!";
-  credentials->set_credential(password.data(), password.length());
-
-  host_stub->BeginSessionRequest(
-      credentials,
-      new DeleteTask<LocalLoginCredentials>(credentials));
-}
-
 // Run the task and delete it afterwards. This action is used to deal with
 // done callbacks.
 ACTION(RunDoneTask) {
@@ -66,7 +53,7 @@ ACTION_P(QuitMainMessageLoop, message_loop) {
   PostQuitTask(message_loop);
 }
 
-}  // namepace
+}  // namespace
 
 class ChromotingHostTest : public testing::Test {
  public:
@@ -90,57 +77,91 @@ class ChromotingHostTest : public testing::Test {
 
     Capturer* capturer = new CapturerFake(context_.main_message_loop());
     host_stub_ = new MockHostStub();
+    host_stub2_ = new MockHostStub();
     input_stub_ = new MockInputStub();
+    input_stub2_ = new MockInputStub();
     DesktopEnvironment* desktop =
-        new DesktopEnvironmentFake(capturer, input_stub_);
+        new DesktopEnvironment(capturer, input_stub_);
     host_ = ChromotingHost::Create(&context_, config_, desktop);
     connection_ = new MockConnectionToClient(
         &message_loop_, &handler_, host_stub_, input_stub_);
+    connection2_ = new MockConnectionToClient(
+        &message_loop_, &handler_, host_stub2_, input_stub2_);
     session_ = new MockSession();
+    session2_ = new MockSession();
     session_config_.reset(SessionConfig::CreateDefault());
+    session_config2_.reset(SessionConfig::CreateDefault());
 
     ON_CALL(video_stub_, ProcessVideoPacket(_, _))
         .WillByDefault(
             DoAll(DeleteArg<0>(), DeleteArg<1>()));
+    ON_CALL(video_stub2_, ProcessVideoPacket(_, _))
+        .WillByDefault(
+            DoAll(DeleteArg<0>(), DeleteArg<1>()));
     ON_CALL(*connection_.get(), video_stub())
         .WillByDefault(Return(&video_stub_));
     ON_CALL(*connection_.get(), client_stub())
         .WillByDefault(Return(&client_stub_));
     ON_CALL(*connection_.get(), session())
         .WillByDefault(Return(session_));
+    ON_CALL(*connection2_.get(), video_stub())
+        .WillByDefault(Return(&video_stub2_));
+    ON_CALL(*connection2_.get(), client_stub())
+        .WillByDefault(Return(&client_stub2_));
+    ON_CALL(*connection2_.get(), session())
+        .WillByDefault(Return(session2_));
     ON_CALL(*session_.get(), config())
         .WillByDefault(Return(session_config_.get()));
+    ON_CALL(*session2_.get(), config())
+        .WillByDefault(Return(session_config2_.get()));
     EXPECT_CALL(*connection_.get(), video_stub())
         .Times(AnyNumber());
     EXPECT_CALL(*connection_.get(), client_stub())
         .Times(AnyNumber());
     EXPECT_CALL(*connection_.get(), session())
         .Times(AnyNumber());
+    EXPECT_CALL(*connection2_.get(), video_stub())
+        .Times(AnyNumber());
+    EXPECT_CALL(*connection2_.get(), client_stub())
+        .Times(AnyNumber());
+    EXPECT_CALL(*connection2_.get(), session())
+        .Times(AnyNumber());
     EXPECT_CALL(*session_.get(), config())
         .Times(AnyNumber());
+    EXPECT_CALL(*session2_.get(), config())
+        .Times(AnyNumber());
+
   }
 
   virtual void TearDown() {
   }
 
-  // Helper metjod to pretend a client is connected to ChromotingHost.
-  void SimulateClientConnection() {
+  // Helper method to pretend a client is connected to ChromotingHost.
+  void SimulateClientConnection(int connection_index) {
+    scoped_refptr<MockConnectionToClient> connection =
+        (connection_index == 0) ? connection_ : connection2_;
+    scoped_refptr<ClientSession> client = new ClientSession(host_.get(),
+                                                            connection);
+    connection->set_host_stub(client.get());
+
     context_.network_message_loop()->PostTask(
         FROM_HERE,
         NewRunnableMethod(host_.get(),
-                          &ChromotingHost::set_connection,
-                          connection_));
+                          &ChromotingHost::AddClient,
+                          client));
     context_.network_message_loop()->PostTask(
         FROM_HERE,
         NewRunnableMethod(host_.get(),
                           &ChromotingHost::OnClientConnected,
-                          connection_));
+                          connection));
     context_.network_message_loop()->PostTask(
         FROM_HERE,
-        NewRunnableFunction(&BeginSessionRequest, host_->host_stub()));
+        NewRunnableMethod(host_.get(),
+                          &ChromotingHost::LocalLoginSucceeded,
+                          connection));
   }
 
-  // Helper method to remove a client connection from ChromotongHost.
+  // Helper method to remove a client connection from ChromotingHost.
   void RemoveClientConnection() {
     context_.network_message_loop()->PostTask(
         FROM_HERE,
@@ -162,6 +183,13 @@ class ChromotingHostTest : public testing::Test {
   MockClientStub client_stub_;
   MockHostStub* host_stub_;
   MockInputStub* input_stub_;
+  scoped_refptr<MockConnectionToClient> connection2_;
+  scoped_refptr<MockSession> session2_;
+  scoped_ptr<SessionConfig> session_config2_;
+  MockVideoStub video_stub2_;
+  MockClientStub client_stub2_;
+  MockHostStub* host_stub2_;
+  MockInputStub* input_stub2_;
 };
 
 TEST_F(ChromotingHostTest, StartAndShutdown) {
@@ -192,7 +220,7 @@ TEST_F(ChromotingHostTest, Connect) {
   EXPECT_CALL(*connection_.get(), Disconnect())
       .RetiresOnSaturation();
 
-  SimulateClientConnection();
+  SimulateClientConnection(0);
   message_loop_.Run();
 }
 
@@ -222,7 +250,7 @@ TEST_F(ChromotingHostTest, Reconnect) {
       .WillOnce(QuitMainMessageLoop(&message_loop_))
       .RetiresOnSaturation();
 
-  SimulateClientConnection();
+  SimulateClientConnection(0);
   message_loop_.Run();
 
   // Connect the client again.
@@ -239,7 +267,49 @@ TEST_F(ChromotingHostTest, Reconnect) {
   EXPECT_CALL(*connection_.get(), Disconnect())
       .RetiresOnSaturation();
 
-  SimulateClientConnection();
+  SimulateClientConnection(0);
+  message_loop_.Run();
+}
+
+TEST_F(ChromotingHostTest, ConnectTwice) {
+  host_->Start(NewRunnableFunction(&PostQuitTask, &message_loop_));
+
+  EXPECT_CALL(client_stub_, BeginSessionResponse(_, _))
+      .Times(1)
+      .WillRepeatedly(RunDoneTask());
+
+  EXPECT_CALL(client_stub2_, BeginSessionResponse(_, _))
+      .Times(1)
+      .WillRepeatedly(RunDoneTask());
+
+  // When a video packet is received we connect the second mock
+  // connection.
+  {
+    InSequence s;
+    EXPECT_CALL(video_stub_, ProcessVideoPacket(_, _))
+        .WillOnce(DoAll(
+            InvokeWithoutArgs(
+                CreateFunctor(
+                    this, &ChromotingHostTest::SimulateClientConnection, 1)),
+            RunDoneTask()))
+        .RetiresOnSaturation();
+    EXPECT_CALL(video_stub_, ProcessVideoPacket(_, _))
+        .Times(AnyNumber());
+    EXPECT_CALL(video_stub2_, ProcessVideoPacket(_, _))
+        .WillOnce(DoAll(
+            InvokeWithoutArgs(host_.get(), &ChromotingHost::Shutdown),
+            RunDoneTask()))
+        .RetiresOnSaturation();
+    EXPECT_CALL(video_stub2_, ProcessVideoPacket(_, _))
+        .Times(AnyNumber());
+  }
+
+  EXPECT_CALL(*connection_.get(), Disconnect())
+      .RetiresOnSaturation();
+  EXPECT_CALL(*connection2_.get(), Disconnect())
+      .RetiresOnSaturation();
+
+  SimulateClientConnection(0);
   message_loop_.Run();
 }