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

remoting/protocol/jingle_session_unittest.cc

Index: remoting/protocol/jingle_session_unittest.cc
diff --git a/remoting/protocol/jingle_session_unittest.cc b/remoting/protocol/jingle_session_unittest.cc
index d8e5fa0570e95ed1bf914083d40abc62ae41fc5c..2e8f5432f3d1432cf1081a37753d31726c184297 100644
--- a/remoting/protocol/jingle_session_unittest.cc
+++ b/remoting/protocol/jingle_session_unittest.cc
@@ -10,6 +10,7 @@
 #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"
@@ -27,6 +28,7 @@
 #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;
@@ -46,8 +48,8 @@ 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:
@@ -65,12 +67,34 @@ class MockSessionEventHandler : public Session::EventHandler {
 
 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 {
@@ -160,10 +184,13 @@ class JingleSessionTest : public testing::Test {
                     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_,
@@ -185,12 +212,19 @@ class JingleSessionTest : public testing::Test {
         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))
@@ -290,6 +324,12 @@ TEST_F(JingleSessionTest, ConnectWithMultistep) {
   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);