Bugfixes in JingleSession

remoting/protocol/jingle_session_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "remoting/protocol/jingle_session.h"
 
 #include <utility>
 #include <vector>
 
 #include "base/bind.h"
@@ skipping 50 matching lines @@
 
 class MockSessionEventHandler : public Session::EventHandler {
  public:
   MOCK_METHOD1(OnSessionStateChange, void(Session::State));
   MOCK_METHOD2(OnSessionRouteChange, void(const std::string& channel_name,
                                           const TransportRoute& route));
 };
 
 class FakeTransport : public Transport {
  public:
+  SendTransportInfoCallback send_transport_info_callback() {
+    return send_transport_info_callback_;
+  }
+
+  const std::vector<std::unique_ptr<buzz::XmlElement>>& received_messages() {
+    return received_messages_;
+  }
+
+  void set_on_message_callback(const base::Closure& on_message_callback) {
+    on_message_callback_ = on_message_callback;
+  }
+
+  // Transport interface.
   void Start(Authenticator* authenticator,
              SendTransportInfoCallback send_transport_info_callback) override {
     send_transport_info_callback_ = send_transport_info_callback;
   }
 
   bool ProcessTransportInfo(buzz::XmlElement* transport_info) override {
     received_messages_.push_back(
         base::MakeUnique<buzz::XmlElement>(*transport_info));
+    if (!on_message_callback_.is_null())
+      on_message_callback_.Run();
     return true;
   }
 
-  SendTransportInfoCallback send_transport_info_callback() {
-    return send_transport_info_callback_;
-  }
-
-  const std::vector<std::unique_ptr<buzz::XmlElement>>& received_messages() {
-    return received_messages_;
-  }
-
  private:
   SendTransportInfoCallback send_transport_info_callback_;
   std::vector<std::unique_ptr<buzz::XmlElement>> received_messages_;
+  base::Closure on_message_callback_;
 };
 
 std::unique_ptr<buzz::XmlElement> CreateTransportInfo(const std::string& id) {
   std::unique_ptr<buzz::XmlElement> result(
       buzz::XmlElement::ForStr("<transport xmlns='google:remoting:ice'/>"));
   result->AddAttr(buzz::QN_ID, id);
   return result;
 }
 
 }  // namespace
 
 class JingleSessionTest : public testing::Test {
  public:
   JingleSessionTest() {
     message_loop_.reset(new base::MessageLoopForIO());
     network_settings_ =
         NetworkSettings(NetworkSettings::NAT_TRAVERSAL_OUTGOING);
   }
 
   // Helper method that handles OnIncomingSession().
   void SetHostSession(Session* session) {
     DCHECK(session);
     host_session_.reset(session);
     host_session_->SetEventHandler(&host_session_event_handler_);
     host_session_->SetTransport(&host_transport_);
   }
 
-  void DeleteSession() {
-    host_session_.reset();
-  }
+  void DeleteHostSession() { host_session_.reset(); }
+
+  void DeleteClientSession() { client_session_.reset(); }
 
  protected:
   void TearDown() override {
     CloseSessions();
     CloseSessionManager();
     base::RunLoop().RunUntilIdle();
   }
 
   void CloseSessions() {
     host_session_.reset();
@@ skipping 26 matching lines @@
     CreateSessionManagers(auth_round_trips, 0, auth_action);
   }
 
   void CloseSessionManager() {
     host_server_.reset();
     client_server_.reset();
     host_signal_strategy_.reset();
     client_signal_strategy_.reset();
   }
 
-  void InitiateConnection(int auth_round_trips,
-                          FakeAuthenticator::Action auth_action,
-                          bool expect_fail) {
+  void SetHostExpectation(bool expect_fail) {
     EXPECT_CALL(host_server_listener_, OnIncomingSession(_, _))
-        .WillOnce(DoAll(
-            WithArg<0>(Invoke(this, &JingleSessionTest::SetHostSession)),
-            SetArgumentPointee<1>(protocol::SessionManager::ACCEPT)));
+        .WillOnce(
+            DoAll(WithArg<0>(Invoke(this, &JingleSessionTest::SetHostSession)),
+                  SetArgumentPointee<1>(protocol::SessionManager::ACCEPT)));
 
     {
       InSequence dummy;
 
       EXPECT_CALL(host_session_event_handler_,
                   OnSessionStateChange(Session::ACCEPTED))
           .Times(AtMost(1));
       EXPECT_CALL(host_session_event_handler_,
                   OnSessionStateChange(Session::AUTHENTICATING))
           .Times(AtMost(1));
       if (expect_fail) {
         EXPECT_CALL(host_session_event_handler_,
                     OnSessionStateChange(Session::FAILED))
             .Times(1);
       } else {
         EXPECT_CALL(host_session_event_handler_,
                     OnSessionStateChange(Session::AUTHENTICATED))
             .Times(1);
         // Expect that the connection will be closed eventually.
         EXPECT_CALL(host_session_event_handler_,
                     OnSessionStateChange(Session::CLOSED))
             .Times(AtMost(1));
       }
     }
+  }
 
-    {
-      InSequence dummy;
+  void SetClientExpectation(bool expect_fail) {
+    InSequence dummy;
 
+    EXPECT_CALL(client_session_event_handler_,
+                OnSessionStateChange(Session::ACCEPTED))
+        .Times(AtMost(1));
+    EXPECT_CALL(client_session_event_handler_,
+                OnSessionStateChange(Session::AUTHENTICATING))
+        .Times(AtMost(1));
+    if (expect_fail) {
       EXPECT_CALL(client_session_event_handler_,
-                  OnSessionStateChange(Session::ACCEPTED))
+                  OnSessionStateChange(Session::FAILED))
+          .Times(1);
+    } else {
+      EXPECT_CALL(client_session_event_handler_,
+                  OnSessionStateChange(Session::AUTHENTICATED))
+          .Times(1);
+      // Expect that the connection will be closed eventually.
+      EXPECT_CALL(client_session_event_handler_,
+                  OnSessionStateChange(Session::CLOSED))
           .Times(AtMost(1));
-      EXPECT_CALL(client_session_event_handler_,
-                  OnSessionStateChange(Session::AUTHENTICATING))
-          .Times(AtMost(1));
-      if (expect_fail) {
-        EXPECT_CALL(client_session_event_handler_,
-                    OnSessionStateChange(Session::FAILED))
-            .Times(1);
-      } else {
-        EXPECT_CALL(client_session_event_handler_,
-                    OnSessionStateChange(Session::AUTHENTICATED))
-            .Times(1);
-        // Expect that the connection will be closed eventually.
-        EXPECT_CALL(client_session_event_handler_,
-                    OnSessionStateChange(Session::CLOSED))
-            .Times(AtMost(1));
-      }
     }
+  }
 
-    std::unique_ptr<Authenticator> authenticator(new FakeAuthenticator(
-        FakeAuthenticator::CLIENT, auth_round_trips, auth_action, true));
-
+  void ConnectClient(std::unique_ptr<Authenticator> authenticator) {
     client_session_ =
         client_server_->Connect(kHostJid, std::move(authenticator));
     client_session_->SetEventHandler(&client_session_event_handler_);
     client_session_->SetTransport(&client_transport_);
+    base::RunLoop().RunUntilIdle();
+  }
 
-    base::RunLoop().RunUntilIdle();
+  void InitiateConnection(int auth_round_trips,
+                          FakeAuthenticator::Action auth_action,
+                          bool expect_fail) {
+    SetHostExpectation(expect_fail);
+    SetClientExpectation(expect_fail);
+    ConnectClient(base::MakeUnique<FakeAuthenticator>(
+        FakeAuthenticator::CLIENT, auth_round_trips, auth_action, true));
   }
 
   void ExpectRouteChange(const std::string& channel_name) {
     EXPECT_CALL(host_session_event_handler_,
                 OnSessionRouteChange(channel_name, _))
         .Times(AtLeast(1));
     EXPECT_CALL(client_session_event_handler_,
                 OnSessionRouteChange(channel_name, _))
         .Times(AtLeast(1));
   }
@@ skipping 63 matching lines @@
   ASSERT_EQ(kClientJid,
             jingle_element->Attr(buzz::QName(std::string(), "initiator")));
 }
 
 // Verify that we can connect two endpoints with multi-step authentication.
 TEST_F(JingleSessionTest, ConnectWithMultistep) {
   CreateSessionManagers(3, FakeAuthenticator::ACCEPT);
   InitiateConnection(3, FakeAuthenticator::ACCEPT, false);
 }
 
-TEST_F(JingleSessionTest, ConnectWithOutofOrderIqs) {
+TEST_F(JingleSessionTest, ConnectWithOutOfOrderIqs) {
   CreateSessionManagers(1, FakeAuthenticator::ACCEPT);
   InitiateConnection(1, FakeAuthenticator::ACCEPT, false);
   client_signal_strategy_->SimulatePackgeReordering();
+
   // Verify that out of order transport messages are received correctly.
   host_transport_.send_transport_info_callback().Run(CreateTransportInfo("1"));
   host_transport_.send_transport_info_callback().Run(CreateTransportInfo("2"));
   base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(client_transport_.received_messages()[0]->Attr(buzz::QN_ID), "1");
-  EXPECT_EQ(client_transport_.received_messages()[1]->Attr(buzz::QN_ID), "2");
+
+  ASSERT_EQ(client_transport_.received_messages().size(), 2U);
+  EXPECT_EQ("1", client_transport_.received_messages()[0]->Attr(buzz::QN_ID));
+  EXPECT_EQ("2", client_transport_.received_messages()[1]->Attr(buzz::QN_ID));
+}
+
+// Verify that out-of-order messages are handled correctly when the session is
+// torn down after the first message.
+TEST_F(JingleSessionTest, ConnectWithOutOfOrderIqsDestroyOnFirstMessage) {
+  CreateSessionManagers(1, FakeAuthenticator::ACCEPT);
+  InitiateConnection(1, FakeAuthenticator::ACCEPT, false);
+  client_signal_strategy_->SimulatePackgeReordering();
+
+  // Verify that out of order transport messages are received correctly.
+  host_transport_.send_transport_info_callback().Run(CreateTransportInfo("1"));
+  host_transport_.send_transport_info_callback().Run(CreateTransportInfo("2"));
+
+  // Destroy the session as soon as the first message is received.
+  client_transport_.set_on_message_callback(base::Bind(
+      &JingleSessionTest::DeleteClientSession, base::Unretained(this)));
+
+  base::RunLoop().RunUntilIdle();
+
+  ASSERT_EQ(client_transport_.received_messages().size(), 1U);
+  EXPECT_EQ("1", client_transport_.received_messages()[0]->Attr(buzz::QN_ID));
 }
 
 // Verify that connection is terminated when single-step auth fails.
 TEST_F(JingleSessionTest, ConnectWithBadAuth) {
   CreateSessionManagers(1, FakeAuthenticator::REJECT);
   InitiateConnection(1, FakeAuthenticator::ACCEPT, true);
 }
 
 // Verify that connection is terminated when multi-step auth fails.
 TEST_F(JingleSessionTest, ConnectWithBadMultistepAuth) {
@@ skipping 60 matching lines @@
   EXPECT_CALL(host_server_listener_, OnIncomingSession(_, _))
       .WillOnce(DoAll(
           WithArg<0>(Invoke(this, &JingleSessionTest::SetHostSession)),
           SetArgumentPointee<1>(protocol::SessionManager::ACCEPT)));
 
   EXPECT_CALL(host_session_event_handler_,
       OnSessionStateChange(Session::ACCEPTED))
       .Times(AtMost(1));
 
   EXPECT_CALL(host_session_event_handler_,
-      OnSessionStateChange(Session::AUTHENTICATING))
-      .WillOnce(InvokeWithoutArgs(this, &JingleSessionTest::DeleteSession));
+              OnSessionStateChange(Session::AUTHENTICATING))
+      .WillOnce(InvokeWithoutArgs(this, &JingleSessionTest::DeleteHostSession));
 
   std::unique_ptr<Authenticator> authenticator(new FakeAuthenticator(
       FakeAuthenticator::CLIENT, 3, FakeAuthenticator::ACCEPT, true));
 
   client_session_ = client_server_->Connect(kHostJid, std::move(authenticator));
 
   base::RunLoop().RunUntilIdle();
 }
 
 TEST_F(JingleSessionTest, DeleteSessionOnAuth) {
   // Same as the previous test, but set messages_till_started to 2 in
   // CreateSessionManagers so that the session will goes into the
   // AUTHENTICATING state after two message exchanges.
   CreateSessionManagers(3, 2, FakeAuthenticator::ACCEPT);
 
   EXPECT_CALL(host_server_listener_, OnIncomingSession(_, _))
-      .WillOnce(DoAll(
-          WithArg<0>(Invoke(this, &JingleSessionTest::SetHostSession)),
-          SetArgumentPointee<1>(protocol::SessionManager::ACCEPT)));
+      .WillOnce(
+          DoAll(WithArg<0>(Invoke(this, &JingleSessionTest::SetHostSession)),
+                SetArgumentPointee<1>(protocol::SessionManager::ACCEPT)));
 
   EXPECT_CALL(host_session_event_handler_,
       OnSessionStateChange(Session::ACCEPTED))
       .Times(AtMost(1));
 
   EXPECT_CALL(host_session_event_handler_,
-      OnSessionStateChange(Session::AUTHENTICATING))
-      .WillOnce(InvokeWithoutArgs(this, &JingleSessionTest::DeleteSession));
+              OnSessionStateChange(Session::AUTHENTICATING))
+      .WillOnce(InvokeWithoutArgs(this, &JingleSessionTest::DeleteHostSession));
 
   std::unique_ptr<Authenticator> authenticator(new FakeAuthenticator(
       FakeAuthenticator::CLIENT, 3, FakeAuthenticator::ACCEPT, true));
 
   client_session_ = client_server_->Connect(kHostJid, std::move(authenticator));
   base::RunLoop().RunUntilIdle();
 }
 
 // Verify that we can connect with multistep authentication.
 TEST_F(JingleSessionTest, TestMultistepAuth) {
   CreateSessionManagers(3, FakeAuthenticator::ACCEPT);
   ASSERT_NO_FATAL_FAILURE(
       InitiateConnection(3, FakeAuthenticator::ACCEPT, false));
 }
 
+// Verify that incoming transport-info messages are handled correctly while in
+// AUTHENTICATING state.
+TEST_F(JingleSessionTest, TransportInfoDuringAuthentication) {
+  CreateSessionManagers(2, FakeAuthenticator::ACCEPT);
+
+  SetHostExpectation(false);
+  {
+    InSequence dummy;
+
+    EXPECT_CALL(client_session_event_handler_,
+                OnSessionStateChange(Session::ACCEPTED))
+        .Times(AtMost(1));
+    EXPECT_CALL(client_session_event_handler_,
+                OnSessionStateChange(Session::AUTHENTICATING))
+        .Times(AtMost(1));
+  }
+
+  // Create connection and pause it before authentication is finished.
+  FakeAuthenticator* authenticator = new FakeAuthenticator(
+      FakeAuthenticator::CLIENT, 2, FakeAuthenticator::ACCEPT, true);
+  authenticator->set_pause_message_index(4);
+  ConnectClient(base::WrapUnique(authenticator));
+
+  // Send 2 transport messages.
+  host_transport_.send_transport_info_callback().Run(CreateTransportInfo("1"));
+  host_transport_.send_transport_info_callback().Run(CreateTransportInfo("2"));
+
+  base::RunLoop().RunUntilIdle();
+
+  // The transport-info messages should not be received here because
+  // authentication hasn't finished.
+  EXPECT_TRUE(client_transport_.received_messages().empty());
+
+  // Destroy the session as soon as the first message is received.
+  client_transport_.set_on_message_callback(base::Bind(
+      &JingleSessionTest::DeleteClientSession, base::Unretained(this)));
+
+  // Resume authentication.
+  authenticator->Resume();
+  base::RunLoop().RunUntilIdle();
+
+  // Verify that transport-info that the first transport-info message was
+  // received.
+  ASSERT_EQ(client_transport_.received_messages().size(), 1U);
+  EXPECT_EQ("1", client_transport_.received_messages()[0]->Attr(buzz::QN_ID));
+}
+
 }  // namespace protocol
 }  // namespace remoting