Process incoming IQs in the same order that they were sent.

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 "base/bind.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/test/test_timeouts.h"
+#include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "net/socket/socket.h"
 #include "net/socket/stream_socket.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "remoting/base/constants.h"
 #include "remoting/protocol/authenticator.h"
 #include "remoting/protocol/channel_authenticator.h"
 #include "remoting/protocol/chromium_port_allocator_factory.h"
 #include "remoting/protocol/connection_tester.h"
 #include "remoting/protocol/fake_authenticator.h"
 #include "remoting/protocol/jingle_session_manager.h"
 #include "remoting/protocol/network_settings.h"
 #include "remoting/protocol/transport.h"
 #include "remoting/protocol/transport_context.h"
 #include "remoting/signaling/fake_signal_strategy.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/webrtc/libjingle/xmpp/constants.h"
 
 using testing::_;
 using testing::AtLeast;
 using testing::AtMost;
 using testing::DeleteArg;
 using testing::DoAll;
 using testing::InSequence;
 using testing::Invoke;
 using testing::InvokeWithoutArgs;
 using testing::Return;
 using testing::SaveArg;
 using testing::SetArgumentPointee;
 using testing::WithArg;
 
 namespace remoting {
 namespace protocol {
 
 namespace {
 
-const char kHostJid[] = "host1@gmail.com/123";
-const char kClientJid[] = "host2@gmail.com/321";
+const char kHostJid[] = "host@gmail.com/123";
+const char kClientJid[] = "client@gmail.com/321";
 
 class MockSessionManagerListener {
  public:
   MOCK_METHOD2(OnIncomingSession,
                void(Session*,
                     SessionManager::IncomingSessionResponse*));
 };
 
 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 MockTransport : public Transport {

[Sergey Ulanov, 2016/10/19 21:23:39]

Maybe rename this to FakeTransport and stop using GMock for this class. Instead
the class could just store the messages it receives:

 bool ProcessTransportInfo(buzz::XmlElement* transport_info) {
   received_message_.push_back(
      base::MakeUnique<buzz::XmlElement>(transport_info));
 }
 std::vector<std::unique_ptr<buzz::XmlElement>> received_messages() {
   return received_messages_;
 }
private:
 std::vector<std::unique_ptr<buzz::XmlElement>> received_messages_;


Then in the test:

  EXPECT_EQ(transport_->received_messages()[0]->Attr(buzz::QN_ID),
            "1");

[kelvinp, 2016/10/21 00:26:03]

Done.

  public:
-  MOCK_METHOD2(Start,
+  void Start(Authenticator* authenticator,
+             SendTransportInfoCallback send_transport_info_callback) {
+    StartInternal(authenticator, send_transport_info_callback);
+    auto task_runner = base::ThreadTaskRunnerHandle::Get();
+    task_runner->PostTask(FROM_HERE,
+                          base::Bind(send_transport_info_callback,
+                                     base::Passed(CreateTransportInfo("1"))));

[Sergey Ulanov, 2016/10/19 21:23:39]

I think it's better to move this inside the test itself.
Just store send_transport_info_callback here, and then add function for the
tests to access it: 
SendTransportInfoCallback send_transport_info_callback() {
  return send_transport_info_callback_;
}

In the tests:
 transport->send_transport_info_callback().Run(CreateTransportInfo("0"));


This would allow to send transport messages only for ConnectWithOutofOrderIqs
test, where we need them. They are not useful in other tests.

[kelvinp, 2016/10/21 00:26:03]

Done.

+  }
+
+  MOCK_METHOD2(StartInternal,
                void(Authenticator* authenticator,
                     SendTransportInfoCallback send_transport_info_callback));
   MOCK_METHOD1(ProcessTransportInfo, bool(buzz::XmlElement* transport_info));
+
+ private:
+  std::unique_ptr<buzz::XmlElement> CreateTransportInfo(std::string id) {

[Sergey Ulanov, 2016/10/19 21:23:39]

const reference for the parameter.

[kelvinp, 2016/10/21 00:26:03]

Done.

+    std::unique_ptr<buzz::XmlElement> result(
+        buzz::XmlElement::ForStr("<transport xmlns='google:remoting:ice'/>"));
+    result->AddAttr(buzz::QN_ID, id);
+    return result;
+  }
 };
 
+MATCHER_P(WithId, expected, "") {
+  std::string actual = arg->Attr(buzz::QN_ID);
+  return actual == expected;
+}
+
 }  // namespace
 
 class JingleSessionTest : public testing::Test {
  public:
   JingleSessionTest() {
     message_loop_.reset(new base::MessageLoopForIO());
     network_settings_ =
         NetworkSettings(NetworkSettings::NAT_TRAVERSAL_OUTGOING);
   }
 
@@ skipping 69 matching lines @@
                   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_transport_, Start(_, _)).Times(1);
+        EXPECT_CALL(host_transport_, StartInternal(_, _)).Times(1);
         EXPECT_CALL(host_session_event_handler_,
                     OnSessionStateChange(Session::AUTHENTICATED))
             .Times(1);
+        EXPECT_CALL(host_transport_, ProcessTransportInfo(WithId("1")))
+            .Times(1)
+            .WillRepeatedly(Return(true));
 
         // Expect that the connection will be closed eventually.
         EXPECT_CALL(host_session_event_handler_,
                     OnSessionStateChange(Session::CLOSED))
             .Times(AtMost(1));
       }
     }
 
     {
       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::FAILED))
             .Times(1);
-      } else {
-        EXPECT_CALL(client_transport_, Start(_, _)).Times(1);
+      }
+    }
+
+    if (!expect_fail) {
+      EXPECT_CALL(client_transport_, StartInternal(_, _)).Times(1);
+      EXPECT_CALL(client_transport_, ProcessTransportInfo(WithId("1")))
+          .Times(1)
+          .WillRepeatedly(Return(true));
+      {
+        InSequence dummy;
         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));
 
@@ skipping 79 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) {
+  CreateSessionManagers(3, FakeAuthenticator::ACCEPT);
+  client_signal_strategy_->SimulatePackgeReordering();
+  InitiateConnection(3, FakeAuthenticator::ACCEPT, false);
+}
+
 // 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) {
   CreateSessionManagers(3, FakeAuthenticator::REJECT);
   InitiateConnection(3, FakeAuthenticator::ACCEPT, true);
@@ skipping 104 matching lines @@
 
 // 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));
 }
 
 }  // namespace protocol
 }  // namespace remoting