Implement Turn/Turn first logic for connection selection.

webrtc/p2p/base/p2ptransportchannel_unittest.cc

 /*
  *  Copyright 2009 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 63 matching lines @@
 // The addresses for the public relay server.
 static const SocketAddress kRelayUdpIntAddr("99.99.99.2", 5000);
 static const SocketAddress kRelayUdpExtAddr("99.99.99.3", 5001);
 static const SocketAddress kRelayTcpIntAddr("99.99.99.2", 5002);
 static const SocketAddress kRelayTcpExtAddr("99.99.99.3", 5003);
 static const SocketAddress kRelaySslTcpIntAddr("99.99.99.2", 5004);
 static const SocketAddress kRelaySslTcpExtAddr("99.99.99.3", 5005);
 // The addresses for the public turn server.
 static const SocketAddress kTurnUdpIntAddr("99.99.99.4",
                                            cricket::STUN_SERVER_PORT);
+static const SocketAddress kTurnTcpIntAddr("99.99.99.4",
+                                           cricket::STUN_SERVER_PORT + 1);
 static const SocketAddress kTurnUdpExtAddr("99.99.99.5", 0);
 static const cricket::RelayCredentials kRelayCredentials("test", "test");
 
 // Based on ICE_UFRAG_LENGTH
 static const char* kIceUfrag[4] = {"TESTICEUFRAG0000", "TESTICEUFRAG0001",
                                    "TESTICEUFRAG0002", "TESTICEUFRAG0003"};
 // Based on ICE_PWD_LENGTH
 static const char* kIcePwd[4] = {"TESTICEPWD00000000000000",
                                  "TESTICEPWD00000000000001",
                                  "TESTICEPWD00000000000002",
@@ skipping 1664 matching lines @@
         vss_(new rtc::VirtualSocketServer(pss_.get())),
         ss_scope_(vss_.get()) {}
 
  protected:
   void PrepareChannel(cricket::P2PTransportChannel* ch) {
     ch->SetIceRole(cricket::ICEROLE_CONTROLLING);
     ch->SetIceCredentials(kIceUfrag[0], kIcePwd[0]);
     ch->SetRemoteIceCredentials(kIceUfrag[1], kIcePwd[1]);
   }
 
-  cricket::Candidate CreateCandidate(const std::string& ip,
-                                     int port,
-                                     int priority,
-                                     const std::string& ufrag = "") {
+  cricket::Candidate CreateLocalCandidate(const std::string& ip,

[pthatcher1, 2016/01/27 19:59:59]

We ought to call this CreateHostCandidate.  LOCAL_PORT_TYPE should be renamed to
CANDIDATE_TYPE_HOST.  But that's for another day.

[guoweis_webrtc, 2016/02/29 18:03:01]

Done.

+                                          int port,
+                                          int priority,
+                                          const std::string& ufrag = "") {
     cricket::Candidate c;
     c.set_address(rtc::SocketAddress(ip, port));
     c.set_component(1);
     c.set_protocol(cricket::UDP_PROTOCOL_NAME);
     c.set_priority(priority);
     c.set_username(ufrag);
+    c.set_type(cricket::LOCAL_PORT_TYPE);
     return c;
   }
 
   cricket::Connection* WaitForConnectionTo(cricket::P2PTransportChannel* ch,
                                            const std::string& ip,
                                            int port_num) {
     EXPECT_TRUE_WAIT(GetConnectionTo(ch, ip, port_num) != nullptr, 3000);
     return GetConnectionTo(ch, ip, port_num);
   }
 
@@ skipping 19 matching lines @@
   rtc::scoped_ptr<rtc::VirtualSocketServer> vss_;
   rtc::SocketServerScope ss_scope_;
 };
 
 TEST_F(P2PTransportChannelPingTest, TestTriggeredChecks) {
   cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   cricket::P2PTransportChannel ch("trigger checks", 1, nullptr, &pa);
   PrepareChannel(&ch);
   ch.Connect();
   ch.MaybeStartGathering();
-  ch.AddRemoteCandidate(CreateCandidate("1.1.1.1", 1, 1));
-  ch.AddRemoteCandidate(CreateCandidate("2.2.2.2", 2, 2));
+  ch.AddRemoteCandidate(CreateLocalCandidate("1.1.1.1", 1, 1));
+  ch.AddRemoteCandidate(CreateLocalCandidate("2.2.2.2", 2, 2));
 
   cricket::Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
   ASSERT_TRUE(conn1 != nullptr);
   ASSERT_TRUE(conn2 != nullptr);
 
   // Before a triggered check, the first connection to ping is the
   // highest priority one.
   EXPECT_EQ(conn2, ch.FindNextPingableConnection());
 
   // Receiving a ping causes a triggered check which should make conn1
   // be pinged first instead of conn2, even though conn2 has a higher
   // priority.
   conn1->ReceivedPing();
   EXPECT_EQ(conn1, ch.FindNextPingableConnection());
 }
 
 TEST_F(P2PTransportChannelPingTest, TestNoTriggeredChecksWhenWritable) {
   cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   cricket::P2PTransportChannel ch("trigger checks", 1, nullptr, &pa);
   PrepareChannel(&ch);
   ch.Connect();
   ch.MaybeStartGathering();
-  ch.AddRemoteCandidate(CreateCandidate("1.1.1.1", 1, 1));
-  ch.AddRemoteCandidate(CreateCandidate("2.2.2.2", 2, 2));
+  ch.AddRemoteCandidate(CreateLocalCandidate("1.1.1.1", 1, 1));
+  ch.AddRemoteCandidate(CreateLocalCandidate("2.2.2.2", 2, 2));
 
   cricket::Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
   ASSERT_TRUE(conn1 != nullptr);
   ASSERT_TRUE(conn2 != nullptr);
 
   EXPECT_EQ(conn2, ch.FindNextPingableConnection());
+  EXPECT_EQ(conn1, ch.FindNextPingableConnection());
   conn1->ReceivedPingResponse();
   ASSERT_TRUE(conn1->writable());
   conn1->ReceivedPing();
 
   // Ping received, but the connection is already writable, so no
   // "triggered check" and conn2 is pinged before conn1 because it has
   // a higher priority.
   EXPECT_EQ(conn2, ch.FindNextPingableConnection());
 }
 
 // Test adding remote candidates with different ufrags. If a remote candidate
 // is added with an old ufrag, it will be discarded. If it is added with a
 // ufrag that was not seen before, it will be used to create connections
 // although the ICE pwd in the remote candidate will be set when the ICE
 // credentials arrive. If a remote candidate is added with the current ICE
 // ufrag, its pwd and generation will be set properly.
 TEST_F(P2PTransportChannelPingTest, TestAddRemoteCandidateWithVariousUfrags) {
   cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   cricket::P2PTransportChannel ch("add candidate", 1, nullptr, &pa);
   PrepareChannel(&ch);
   ch.Connect();
   ch.MaybeStartGathering();
   // Add a candidate with a future ufrag.
-  ch.AddRemoteCandidate(CreateCandidate("1.1.1.1", 1, 1, kIceUfrag[2]));
+  ch.AddRemoteCandidate(CreateLocalCandidate("1.1.1.1", 1, 1, kIceUfrag[2]));
   cricket::Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   ASSERT_TRUE(conn1 != nullptr);
   const cricket::Candidate& candidate = conn1->remote_candidate();
   EXPECT_EQ(kIceUfrag[2], candidate.username());
   EXPECT_TRUE(candidate.password().empty());
   EXPECT_TRUE(ch.FindNextPingableConnection() == nullptr);
 
   // Set the remote credentials with the "future" ufrag.
   // This should set the ICE pwd in the remote candidate of |conn1|, making
   // it pingable.
   ch.SetRemoteIceCredentials(kIceUfrag[2], kIcePwd[2]);
   EXPECT_EQ(kIceUfrag[2], candidate.username());
   EXPECT_EQ(kIcePwd[2], candidate.password());
   EXPECT_EQ(conn1, ch.FindNextPingableConnection());
 
   // Add a candidate with an old ufrag. No connection will be created.
-  ch.AddRemoteCandidate(CreateCandidate("2.2.2.2", 2, 2, kIceUfrag[1]));
+  ch.AddRemoteCandidate(CreateLocalCandidate("2.2.2.2", 2, 2, kIceUfrag[1]));
   rtc::Thread::Current()->ProcessMessages(500);
   EXPECT_TRUE(GetConnectionTo(&ch, "2.2.2.2", 2) == nullptr);
 
   // Add a candidate with the current ufrag, its pwd and generation will be
   // assigned, even if the generation is not set.
-  ch.AddRemoteCandidate(CreateCandidate("3.3.3.3", 3, 0, kIceUfrag[2]));
+  ch.AddRemoteCandidate(CreateLocalCandidate("3.3.3.3", 3, 0, kIceUfrag[2]));
   cricket::Connection* conn3 = nullptr;
   ASSERT_TRUE_WAIT((conn3 = GetConnectionTo(&ch, "3.3.3.3", 3)) != nullptr,
                    3000);
   const cricket::Candidate& new_candidate = conn3->remote_candidate();
   EXPECT_EQ(kIcePwd[2], new_candidate.password());
   EXPECT_EQ(1U, new_candidate.generation());
 
   // Check that the pwd of all remote candidates are properly assigned.
   for (const cricket::RemoteCandidate& candidate : ch.remote_candidates()) {
     EXPECT_TRUE(candidate.username() == kIceUfrag[1] ||
                 candidate.username() == kIceUfrag[2]);
     if (candidate.username() == kIceUfrag[1]) {
       EXPECT_EQ(kIcePwd[1], candidate.password());
     } else if (candidate.username() == kIceUfrag[2]) {
       EXPECT_EQ(kIcePwd[2], candidate.password());
     }
   }
 }
 
 TEST_F(P2PTransportChannelPingTest, ConnectionResurrection) {
   cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   cricket::P2PTransportChannel ch("connection resurrection", 1, nullptr, &pa);
   PrepareChannel(&ch);
   ch.Connect();
   ch.MaybeStartGathering();
 
   // Create conn1 and keep track of original candidate priority.
-  ch.AddRemoteCandidate(CreateCandidate("1.1.1.1", 1, 1));
+  ch.AddRemoteCandidate(CreateLocalCandidate("1.1.1.1", 1, 1));
   cricket::Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   ASSERT_TRUE(conn1 != nullptr);
   uint32_t remote_priority = conn1->remote_candidate().priority();
 
   // Create a higher priority candidate and make the connection
   // receiving/writable. This will prune conn1.
-  ch.AddRemoteCandidate(CreateCandidate("2.2.2.2", 2, 2));
+  ch.AddRemoteCandidate(CreateLocalCandidate("2.2.2.2", 2, 2));
   cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
   ASSERT_TRUE(conn2 != nullptr);
   conn2->ReceivedPing();
   conn2->ReceivedPingResponse();
 
   // Wait for conn1 to be pruned.
   EXPECT_TRUE_WAIT(conn1->pruned(), 3000);
   // Destroy the connection to test SignalUnknownAddress.
   conn1->Destroy();
   EXPECT_TRUE_WAIT(GetConnectionTo(&ch, "1.1.1.1", 1) == nullptr, 1000);
@@ skipping 30 matching lines @@
   PrepareChannel(&ch);
   // Default receiving timeout and checking receiving delay should not be too
   // small.
   EXPECT_LE(1000, ch.receiving_timeout());
   EXPECT_LE(200, ch.check_receiving_delay());
   ch.SetIceConfig(CreateIceConfig(500, false));
   EXPECT_EQ(500, ch.receiving_timeout());
   EXPECT_EQ(50, ch.check_receiving_delay());
   ch.Connect();
   ch.MaybeStartGathering();
-  ch.AddRemoteCandidate(CreateCandidate("1.1.1.1", 1, 1));
+  ch.AddRemoteCandidate(CreateLocalCandidate("1.1.1.1", 1, 1));
   cricket::Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   ASSERT_TRUE(conn1 != nullptr);
 
   conn1->ReceivedPing();
   conn1->OnReadPacket("ABC", 3, rtc::CreatePacketTime(0));
   EXPECT_TRUE_WAIT(ch.best_connection() != nullptr, 1000);
   EXPECT_TRUE_WAIT(ch.receiving(), 1000);
   EXPECT_TRUE_WAIT(!ch.receiving(), 1000);
 }
 
 // The controlled side will select a connection as the "best connection" based
 // on priority until the controlling side nominates a connection, at which
 // point the controlled side will select that connection as the
 // "best connection".
 TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) {
   cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   cricket::P2PTransportChannel ch("receiving state change", 1, nullptr, &pa);
   PrepareChannel(&ch);
   ch.SetIceRole(cricket::ICEROLE_CONTROLLED);
   ch.Connect();
   ch.MaybeStartGathering();
-  ch.AddRemoteCandidate(CreateCandidate("1.1.1.1", 1, 1));
+  ch.AddRemoteCandidate(CreateLocalCandidate("1.1.1.1", 1, 1));
   cricket::Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   ASSERT_TRUE(conn1 != nullptr);
   EXPECT_EQ(conn1, ch.best_connection());
 
   // When a higher priority candidate comes in, the new connection is chosen
   // as the best connection.
-  ch.AddRemoteCandidate(CreateCandidate("2.2.2.2", 2, 10));
+  ch.AddRemoteCandidate(CreateLocalCandidate("2.2.2.2", 2, 10));
   cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
   ASSERT_TRUE(conn2 != nullptr);
   EXPECT_EQ(conn2, ch.best_connection());
 
   // If a stun request with use-candidate attribute arrives, the receiving
   // connection will be set as the best connection, even though
   // its priority is lower.
-  ch.AddRemoteCandidate(CreateCandidate("3.3.3.3", 3, 1));
+  ch.AddRemoteCandidate(CreateLocalCandidate("3.3.3.3", 3, 1));
   cricket::Connection* conn3 = WaitForConnectionTo(&ch, "3.3.3.3", 3);
   ASSERT_TRUE(conn3 != nullptr);
   // Because it has a lower priority, the best connection is still conn2.
   EXPECT_EQ(conn2, ch.best_connection());
   conn3->ReceivedPingResponse();  // Become writable.
   // But if it is nominated via use_candidate, it is chosen as the best
   // connection.
   conn3->set_nominated(true);
   conn3->SignalNominated(conn3);
   EXPECT_EQ(conn3, ch.best_connection());
 
   // Even if another higher priority candidate arrives,
   // it will not be set as the best connection because the best connection
   // is nominated by the controlling side.
-  ch.AddRemoteCandidate(CreateCandidate("4.4.4.4", 4, 100));
+  ch.AddRemoteCandidate(CreateLocalCandidate("4.4.4.4", 4, 100));
   cricket::Connection* conn4 = WaitForConnectionTo(&ch, "4.4.4.4", 4);
   ASSERT_TRUE(conn4 != nullptr);
   EXPECT_EQ(conn3, ch.best_connection());
   // But if it is nominated via use_candidate and writable, it will be set as
   // the best connection.
   conn4->set_nominated(true);
   conn4->SignalNominated(conn4);
   // Not switched yet because conn4 is not writable.
   EXPECT_EQ(conn3, ch.best_connection());
   // The best connection switches after conn4 becomes writable.
@@ skipping 26 matching lines @@
                              cricket::PROTO_UDP, &request, kIceUfrag[1], false);
   cricket::Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   ASSERT_TRUE(conn1 != nullptr);
   EXPECT_TRUE(port->sent_binding_response());
   EXPECT_EQ(conn1, ch.best_connection());
   conn1->ReceivedPingResponse();
   EXPECT_EQ(conn1, ch.best_connection());
   port->set_sent_binding_response(false);
 
   // Another connection is nominated via use_candidate.
-  ch.AddRemoteCandidate(CreateCandidate("2.2.2.2", 2, 1));
+  ch.AddRemoteCandidate(CreateLocalCandidate("2.2.2.2", 2, 1));
   cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
   ASSERT_TRUE(conn2 != nullptr);
   // Because it has a lower priority, the best connection is still conn1.
   EXPECT_EQ(conn1, ch.best_connection());
   // When it is nominated via use_candidate and writable, it is chosen as the
   // best connection.
   conn2->ReceivedPingResponse();  // Become writable.
   conn2->set_nominated(true);
   conn2->SignalNominated(conn2);
   EXPECT_EQ(conn2, ch.best_connection());
@@ skipping 41 matching lines @@
 // based on media received until the controlling side nominates a connection,
 // at which point the controlled side will select that connection as
 // the "best connection".
 TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBasedOnMediaReceived) {
   cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   cricket::P2PTransportChannel ch("receiving state change", 1, nullptr, &pa);
   PrepareChannel(&ch);
   ch.SetIceRole(cricket::ICEROLE_CONTROLLED);
   ch.Connect();
   ch.MaybeStartGathering();
-  ch.AddRemoteCandidate(CreateCandidate("1.1.1.1", 1, 10));
+  ch.AddRemoteCandidate(CreateLocalCandidate("1.1.1.1", 1, 10));
   cricket::Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   ASSERT_TRUE(conn1 != nullptr);
   EXPECT_EQ(conn1, ch.best_connection());
 
   // If a data packet is received on conn2, the best connection should
   // switch to conn2 because the controlled side must mirror the media path
   // chosen by the controlling side.
-  ch.AddRemoteCandidate(CreateCandidate("2.2.2.2", 2, 1));
+  ch.AddRemoteCandidate(CreateLocalCandidate("2.2.2.2", 2, 1));
   cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
   ASSERT_TRUE(conn2 != nullptr);
   conn2->ReceivedPing();  // Start receiving.
   // Do not switch because it is not writable.
   conn2->OnReadPacket("ABC", 3, rtc::CreatePacketTime(0));
   EXPECT_EQ(conn1, ch.best_connection());
 
   conn2->ReceivedPingResponse();  // Become writable.
   // Switch because it is writable.
   conn2->OnReadPacket("DEF", 3, rtc::CreatePacketTime(0));
@@ skipping 29 matching lines @@
 
 // When the current best connection is strong, lower-priority connections will
 // be pruned. Otherwise, lower-priority connections are kept.
 TEST_F(P2PTransportChannelPingTest, TestDontPruneWhenWeak) {
   cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   cricket::P2PTransportChannel ch("test channel", 1, nullptr, &pa);
   PrepareChannel(&ch);
   ch.SetIceRole(cricket::ICEROLE_CONTROLLED);
   ch.Connect();
   ch.MaybeStartGathering();
-  ch.AddRemoteCandidate(CreateCandidate("1.1.1.1", 1, 1));
+  ch.AddRemoteCandidate(CreateLocalCandidate("1.1.1.1", 1, 1));
   cricket::Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   ASSERT_TRUE(conn1 != nullptr);
   EXPECT_EQ(conn1, ch.best_connection());
   conn1->ReceivedPingResponse();  // Becomes writable and receiving
 
   // When a higher-priority, nominated candidate comes in, the connections with
   // lower-priority are pruned.
-  ch.AddRemoteCandidate(CreateCandidate("2.2.2.2", 2, 10));
+  ch.AddRemoteCandidate(CreateLocalCandidate("2.2.2.2", 2, 10));
   cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
   ASSERT_TRUE(conn2 != nullptr);
   conn2->ReceivedPingResponse();  // Becomes writable and receiving
   conn2->set_nominated(true);
   conn2->SignalNominated(conn2);
   EXPECT_TRUE_WAIT(conn1->pruned(), 3000);
 
   ch.SetIceConfig(CreateIceConfig(500, false));
   // Wait until conn2 becomes not receiving.
   EXPECT_TRUE_WAIT(!conn2->receiving(), 3000);
 
-  ch.AddRemoteCandidate(CreateCandidate("3.3.3.3", 3, 1));
+  ch.AddRemoteCandidate(CreateLocalCandidate("3.3.3.3", 3, 1));
   cricket::Connection* conn3 = WaitForConnectionTo(&ch, "3.3.3.3", 3);
   ASSERT_TRUE(conn3 != nullptr);
   // The best connection should still be conn2. Even through conn3 has lower
   // priority and is not receiving/writable, it is not pruned because the best
   // connection is not receiving.
   WAIT(conn3->pruned(), 1000);
   EXPECT_FALSE(conn3->pruned());
 }
 
 // Test that GetState returns the state correctly.
 TEST_F(P2PTransportChannelPingTest, TestGetState) {
   cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   cricket::P2PTransportChannel ch("test channel", 1, nullptr, &pa);
   PrepareChannel(&ch);
   ch.Connect();
   ch.MaybeStartGathering();
   EXPECT_EQ(cricket::TransportChannelState::STATE_INIT, ch.GetState());
-  ch.AddRemoteCandidate(CreateCandidate("1.1.1.1", 1, 100));
-  ch.AddRemoteCandidate(CreateCandidate("2.2.2.2", 2, 1));
+  ch.AddRemoteCandidate(CreateLocalCandidate("1.1.1.1", 1, 100));
+  ch.AddRemoteCandidate(CreateLocalCandidate("2.2.2.2", 2, 1));
   cricket::Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
   ASSERT_TRUE(conn1 != nullptr);
   ASSERT_TRUE(conn2 != nullptr);
   // Now there are two connections, so the transport channel is connecting.
   EXPECT_EQ(cricket::TransportChannelState::STATE_CONNECTING, ch.GetState());
   // |conn1| becomes writable and receiving; it then should prune |conn2|.
   conn1->ReceivedPingResponse();
   EXPECT_TRUE_WAIT(conn2->pruned(), 1000);
   EXPECT_EQ(cricket::TransportChannelState::STATE_COMPLETED, ch.GetState());
   conn1->Prune();  // All connections are pruned.
   // Need to wait until the channel state is updated.
   EXPECT_EQ_WAIT(cricket::TransportChannelState::STATE_FAILED, ch.GetState(),
                  1000);
 }
 
 // Test that when a low-priority connection is pruned, it is not deleted
 // right away, and it can become active and be pruned again.
 TEST_F(P2PTransportChannelPingTest, TestConnectionPrunedAgain) {
   cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   cricket::P2PTransportChannel ch("test channel", 1, nullptr, &pa);
   PrepareChannel(&ch);
   ch.SetIceConfig(CreateIceConfig(1000, false));
   ch.Connect();
   ch.MaybeStartGathering();
-  ch.AddRemoteCandidate(CreateCandidate("1.1.1.1", 1, 100));
+  ch.AddRemoteCandidate(CreateLocalCandidate("1.1.1.1", 1, 100));
   cricket::Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   ASSERT_TRUE(conn1 != nullptr);
   EXPECT_EQ(conn1, ch.best_connection());
   conn1->ReceivedPingResponse();  // Becomes writable and receiving
 
   // Add a low-priority connection |conn2|, which will be pruned, but it will
   // not be deleted right away. Once the current best connection becomes not
   // receiving, |conn2| will start to ping and upon receiving the ping response,
   // it will become the best connection.
-  ch.AddRemoteCandidate(CreateCandidate("2.2.2.2", 2, 1));
+  ch.AddRemoteCandidate(CreateLocalCandidate("2.2.2.2", 2, 1));
   cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
   ASSERT_TRUE(conn2 != nullptr);
   EXPECT_TRUE_WAIT(!conn2->active(), 1000);
   // |conn2| should not send a ping yet.
   EXPECT_EQ(cricket::Connection::STATE_WAITING, conn2->state());
   EXPECT_EQ(cricket::TransportChannelState::STATE_COMPLETED, ch.GetState());
   // Wait for |conn1| becoming not receiving.
   EXPECT_TRUE_WAIT(!conn1->receiving(), 3000);
   // Make sure conn2 is not deleted.
   conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
@@ skipping 12 matching lines @@
 
 // Test that if all connections in a channel has timed out on writing, they
 // will all be deleted. We use Prune to simulate write_time_out.
 TEST_F(P2PTransportChannelPingTest, TestDeleteConnectionsIfAllWriteTimedout) {
   cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   cricket::P2PTransportChannel ch("test channel", 1, nullptr, &pa);
   PrepareChannel(&ch);
   ch.Connect();
   ch.MaybeStartGathering();
   // Have one connection only but later becomes write-time-out.
-  ch.AddRemoteCandidate(CreateCandidate("1.1.1.1", 1, 100));
+  ch.AddRemoteCandidate(CreateLocalCandidate("1.1.1.1", 1, 100));
   cricket::Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   ASSERT_TRUE(conn1 != nullptr);
   conn1->ReceivedPing();  // Becomes receiving
   conn1->Prune();
   EXPECT_TRUE_WAIT(ch.connections().empty(), 1000);
 
   // Have two connections but both become write-time-out later.
-  ch.AddRemoteCandidate(CreateCandidate("2.2.2.2", 2, 1));
+  ch.AddRemoteCandidate(CreateLocalCandidate("2.2.2.2", 2, 1));
   cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
   ASSERT_TRUE(conn2 != nullptr);
   conn2->ReceivedPing();  // Becomes receiving
-  ch.AddRemoteCandidate(CreateCandidate("3.3.3.3", 3, 2));
+  ch.AddRemoteCandidate(CreateLocalCandidate("3.3.3.3", 3, 2));
   cricket::Connection* conn3 = WaitForConnectionTo(&ch, "3.3.3.3", 3);
   ASSERT_TRUE(conn3 != nullptr);
   conn3->ReceivedPing();  // Becomes receiving
   // Now prune both conn2 and conn3; they will be deleted soon.
   conn2->Prune();
   conn3->Prune();
   EXPECT_TRUE_WAIT(ch.connections().empty(), 1000);
 }
 
 // Test that after a port allocator session is started, it will be stopped
 // when a new connection becomes writable and receiving. Also test that this
 // holds even if the transport channel did not lose the writability.
 TEST_F(P2PTransportChannelPingTest, TestStopPortAllocatorSessions) {
   cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   cricket::P2PTransportChannel ch("test channel", 1, nullptr, &pa);
   PrepareChannel(&ch);
   ch.SetIceConfig(CreateIceConfig(2000, false));
   ch.Connect();
   ch.MaybeStartGathering();
-  ch.AddRemoteCandidate(CreateCandidate("1.1.1.1", 1, 100));
+  ch.AddRemoteCandidate(CreateLocalCandidate("1.1.1.1", 1, 100));
   cricket::Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   ASSERT_TRUE(conn1 != nullptr);
   conn1->ReceivedPingResponse();  // Becomes writable and receiving
   EXPECT_TRUE(!ch.allocator_session()->IsGettingPorts());
 
   // Restart gathering even if the transport channel is still writable.
   // It should stop getting ports after a new connection becomes strongly
   // connected.
   ch.SetIceCredentials(kIceUfrag[1], kIcePwd[1]);
   ch.MaybeStartGathering();
-  ch.AddRemoteCandidate(CreateCandidate("2.2.2.2", 2, 100));
+  ch.AddRemoteCandidate(CreateLocalCandidate("2.2.2.2", 2, 100));
   cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
   ASSERT_TRUE(conn2 != nullptr);
   conn2->ReceivedPingResponse();  // Becomes writable and receiving
   EXPECT_TRUE(!ch.allocator_session()->IsGettingPorts());
 }
+
+class P2PTransportChannelTurnTurnFirstTest

[pthatcher1, 2016/01/27 19:59:59]

Can you call this P2PTransportChannelMostLikelyToWorkFirstTest?

[guoweis_webrtc, 2016/02/29 18:03:00]

Done.

+    : public P2PTransportChannelPingTest {
+ public:
+  P2PTransportChannelTurnTurnFirstTest()
+      : turn_server_(rtc::Thread::Current(), kTurnUdpIntAddr, kTurnUdpExtAddr) {
+    network_manager_.AddInterface(kPublicAddrs[0]);
+    allocator_.reset(new cricket::BasicPortAllocator(
+        &network_manager_, ServerAddresses(), rtc::SocketAddress(),
+        rtc::SocketAddress(), rtc::SocketAddress()));
+    allocator_->set_flags(allocator_->flags() |
+                          cricket::PORTALLOCATOR_DISABLE_STUN |
+                          cricket::PORTALLOCATOR_DISABLE_TCP);
+    cricket::RelayServerConfig config(cricket::RELAY_TURN);
+    config.credentials = kRelayCredentials;
+    config.ports.push_back(
+        cricket::ProtocolAddress(kTurnUdpIntAddr, cricket::PROTO_UDP, false));
+    allocator_->AddTurnServer(config);
+    allocator_->set_step_delay(kMinimumStepDelay);
+  }
+
+  cricket::P2PTransportChannel& StartChannel() {

[pthatcher1, 2016/01/27 19:59:59]

Can we call this StartTransportChannel()?

And it's not clear from the name that it's prioritizing most likely to work
candidate pairs.  Would it make sense to add that to the name, or to pass in a
variable?

[guoweis_webrtc, 2016/02/29 18:03:01]

Done.

+    channel_.reset(
+        new cricket::P2PTransportChannel("checks", 1, nullptr, allocator()));
+    cricket::IceConfig config = channel_->GetIceConfig();
+    config.ping_most_likely_candidate_pair_first = true;
+    channel_->SetIceConfig(config);
+    PrepareChannel(channel_.get());
+    channel_->Connect();
+    channel_->MaybeStartGathering();
+    return *channel_.get();
+  }
+
+  cricket::BasicPortAllocator* allocator() { return allocator_.get(); }
+  cricket::TestTurnServer* turn_server() { return &turn_server_; }
+
+  // This verifies the next pingable connection has the expected candidates'
+  // types and, for relay local candidate, the expected relay protocol and ping
+  // it.
+  void VerifyNextPingableConnection(
+      const std::string& local_candidate_type,
+      const std::string& remote_candidate_type,
+      const std::string& relay_protocol_type = cricket::UDP_PROTOCOL_NAME) {
+    cricket::Connection* conn = channel_->FindNextPingableConnection();
+    EXPECT_EQ(conn->local_candidate().type(), local_candidate_type);
+    if (conn->local_candidate().type() == cricket::RELAY_PORT_TYPE) {
+      EXPECT_EQ(conn->local_candidate().relay_protocol(), relay_protocol_type);
+    }
+    EXPECT_EQ(conn->remote_candidate().type(), remote_candidate_type);
+  }
+
+  cricket::Candidate CreateRelayCandidate(const std::string& ip,
+                                          int port,
+                                          int priority,
+                                          const std::string& ufrag = "") {
+    cricket::Candidate c = CreateLocalCandidate(ip, port, priority, ufrag);
+    c.set_type(cricket::RELAY_PORT_TYPE);
+    return c;
+  }
+
+ private:
+  rtc::scoped_ptr<cricket::BasicPortAllocator> allocator_;
+  rtc::FakeNetworkManager network_manager_;
+  cricket::TestTurnServer turn_server_;
+  rtc::scoped_ptr<cricket::P2PTransportChannel> channel_;
+};
+
+// Test that Relay/Relay connection will be pinged when no other connections

[pthatcher1, 2016/01/27 19:59:59]

Relay/Relay connection => Relay/Relay connections

Here and in other comments like this.

[pthatcher1, 2016/01/27 19:59:59]

Can you say "pinged first" instead of just "pinged", like you do in the other
comments?

[guoweis_webrtc, 2016/02/29 18:03:01]

Done.

[guoweis_webrtc, 2016/02/29 18:03:01]

Done.

+// have been pinged yet, unless we need to ping a trigger check or we have a
+// best connection.
+TEST_F(P2PTransportChannelTurnTurnFirstTest,
+       TestTurnTurnFirstWhenNothingPingedYet) {

[pthatcher1, 2016/01/27 19:59:59]

The comment says Relay/Relay, but the variable name says Turn/Turn.  Can we pick
one?  I think RelayRelay would be slightly better.  Here and in other places.

[guoweis_webrtc, 2016/02/29 18:03:01]

Done.

+  cricket::P2PTransportChannel& ch = StartChannel();
+  EXPECT_TRUE_WAIT(ch.ports().size() == 2, 5000);
+  EXPECT_EQ(ch.ports()[0]->Type(), cricket::LOCAL_PORT_TYPE);
+  EXPECT_EQ(ch.ports()[1]->Type(), cricket::RELAY_PORT_TYPE);
+
+  ch.AddRemoteCandidate(CreateRelayCandidate("1.1.1.1", 1, 1));
+  ch.AddRemoteCandidate(CreateLocalCandidate("2.2.2.2", 2, 2));
+
+  EXPECT_TRUE_WAIT(ch.connections().size() == 4, 5000);
+
+  // Relay/Relay should be the first pingable connection.
+  cricket::Connection* conn = ch.FindNextPingableConnection();
+  EXPECT_EQ(conn->local_candidate().type(), cricket::RELAY_PORT_TYPE);
+  EXPECT_EQ(conn->remote_candidate().type(), cricket::RELAY_PORT_TYPE);
+
+  // Unless that we have a trigger check waiting to be pinged.
+  cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
+  EXPECT_EQ(conn2->local_candidate().type(), cricket::LOCAL_PORT_TYPE);
+  EXPECT_EQ(conn2->remote_candidate().type(), cricket::LOCAL_PORT_TYPE);
+  conn2->ReceivedPing();
+  EXPECT_EQ(conn2, ch.FindNextPingableConnection());
+
+  // Make conn3 the best connection.
+  cricket::Connection* conn3 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
+  EXPECT_EQ(conn3->local_candidate().type(), cricket::LOCAL_PORT_TYPE);
+  EXPECT_EQ(conn3->remote_candidate().type(), cricket::RELAY_PORT_TYPE);
+  conn3->ReceivedPingResponse();
+  ASSERT_TRUE(conn3->writable());
+  conn3->ReceivedPing();
+
+  // Verify that conn3 will be the "best connection" since it is readable and
+  // writable. After |MAX_CURRENT_STRONG_DELAY|, it should be the next pingable
+  // connection.
+  EXPECT_TRUE_WAIT(conn3 == ch.best_connection(), 5000);
+  WAIT(false, cricket::MAX_CURRENT_STRONG_DELAY + 100);

[pthatcher1, 2016/01/27 19:59:59]

We have to wait 1 second for each of these tests?  I'm not a fan of that.  I'd
prefer instead to put the delay into IceConfig and make it shorter for the unit
tests (50ms?).  Then the default could be the constant.  

[guoweis_webrtc, 2016/02/29 18:03:01]

Done.

+  conn3->ReceivedPingResponse();
+  ASSERT_TRUE(conn3->writable());
+  EXPECT_EQ(conn3, ch.FindNextPingableConnection());
+}
+
+// Test that Relay/Relay connection will be pinged first when everything has
+// been pinged even if the Relay/Relay connection wasn't the first to be pinged
+// in the first round.
+TEST_F(P2PTransportChannelTurnTurnFirstTest,
+       TestTurnTurnFirstWhenEverythingPinged) {
+  cricket::P2PTransportChannel& ch = StartChannel();
+  EXPECT_TRUE_WAIT(ch.ports().size() == 2, 5000);
+  EXPECT_EQ(ch.ports()[0]->Type(), cricket::LOCAL_PORT_TYPE);
+  EXPECT_EQ(ch.ports()[1]->Type(), cricket::RELAY_PORT_TYPE);
+
+  ch.AddRemoteCandidate(CreateLocalCandidate("1.1.1.1", 1, 1));
+  EXPECT_TRUE_WAIT(ch.connections().size() == 2, 5000);
+
+  // Initially, only have Local/Local and Local/Relay.
+  VerifyNextPingableConnection(cricket::LOCAL_PORT_TYPE,
+                               cricket::LOCAL_PORT_TYPE);
+  VerifyNextPingableConnection(cricket::RELAY_PORT_TYPE,
+                               cricket::LOCAL_PORT_TYPE);
+
+  // Remote Relay candidate arrives.
+  ch.AddRemoteCandidate(CreateRelayCandidate("2.2.2.2", 2, 2));
+  EXPECT_TRUE_WAIT(ch.connections().size() == 4, 5000);
+
+  // Relay/Relay should be the first since it hasn't been pinged before.
+  VerifyNextPingableConnection(cricket::RELAY_PORT_TYPE,
+                               cricket::RELAY_PORT_TYPE);
+
+  // Local/Relay is the final one.
+  VerifyNextPingableConnection(cricket::LOCAL_PORT_TYPE,
+                               cricket::RELAY_PORT_TYPE);
+
+  // Now, every connection has been pinged once. The next one should be
+  // Relay/Relay.
+  VerifyNextPingableConnection(cricket::RELAY_PORT_TYPE,
+                               cricket::RELAY_PORT_TYPE);
+}
+
+// Test that when we receive a new remote candidate, they will be tried first
+// before we re-ping Relay/Relay connection.

[pthatcher1, 2016/01/27 19:59:59]

connection => connections again

[guoweis_webrtc, 2016/02/29 18:03:01]

Done.

+TEST_F(P2PTransportChannelTurnTurnFirstTest,
+       TestNoStarvationOnNonRelayConnection) {
+  cricket::P2PTransportChannel& ch = StartChannel();
+  EXPECT_TRUE_WAIT(ch.ports().size() == 2, 5000);
+  EXPECT_EQ(ch.ports()[0]->Type(), cricket::LOCAL_PORT_TYPE);
+  EXPECT_EQ(ch.ports()[1]->Type(), cricket::RELAY_PORT_TYPE);
+
+  ch.AddRemoteCandidate(CreateRelayCandidate("1.1.1.1", 1, 1));
+  EXPECT_TRUE_WAIT(ch.connections().size() == 2, 5000);
+
+  // Initially, only have Relay/Relay and Local/Relay. Ping Relay/Relay first.
+  VerifyNextPingableConnection(cricket::RELAY_PORT_TYPE,
+                               cricket::RELAY_PORT_TYPE);
+
+  // Next, ping Local/Relay.
+  VerifyNextPingableConnection(cricket::LOCAL_PORT_TYPE,
+                               cricket::RELAY_PORT_TYPE);
+
+  // Remote Local candidate arrives.
+  ch.AddRemoteCandidate(CreateLocalCandidate("2.2.2.2", 2, 2));
+  EXPECT_TRUE_WAIT(ch.connections().size() == 4, 5000);
+
+  // Local/Local should be the first since it hasn't been pinged before.
+  VerifyNextPingableConnection(cricket::LOCAL_PORT_TYPE,
+                               cricket::LOCAL_PORT_TYPE);
+
+  // Relay/Local is the final one.
+  VerifyNextPingableConnection(cricket::RELAY_PORT_TYPE,
+                               cricket::LOCAL_PORT_TYPE);
+
+  // Now, every connection has been pinged once. The next one should be
+  // Relay/Relay.
+  VerifyNextPingableConnection(cricket::RELAY_PORT_TYPE,
+                               cricket::RELAY_PORT_TYPE);
+}
+
+// Test the ping sequence is UDP Relay/Relay followed by TCP Relay/Relay,
+// followed by the rest.
+TEST_F(P2PTransportChannelTurnTurnFirstTest, TestTcpTurn) {
+  // Add a Tcp Turn server.
+  turn_server()->AddInternalSocket(kTurnTcpIntAddr, cricket::PROTO_TCP);
+  cricket::RelayServerConfig config(cricket::RELAY_TURN);
+  config.credentials = kRelayCredentials;
+  config.ports.push_back(
+      cricket::ProtocolAddress(kTurnTcpIntAddr, cricket::PROTO_TCP, false));
+  allocator()->AddTurnServer(config);
+
+  cricket::P2PTransportChannel& ch = StartChannel();
+  EXPECT_TRUE_WAIT(ch.ports().size() == 3, 5000);
+  EXPECT_EQ(ch.ports()[0]->Type(), cricket::LOCAL_PORT_TYPE);
+  EXPECT_EQ(ch.ports()[1]->Type(), cricket::RELAY_PORT_TYPE);
+  EXPECT_EQ(ch.ports()[2]->Type(), cricket::RELAY_PORT_TYPE);
+
+  // Remote Relay candidate arrives.
+  ch.AddRemoteCandidate(CreateRelayCandidate("1.1.1.1", 1, 1));
+  EXPECT_TRUE_WAIT(ch.connections().size() == 3, 5000);
+
+  // UDP Relay/Relay should be pinged first.
+  VerifyNextPingableConnection(cricket::RELAY_PORT_TYPE,
+                               cricket::RELAY_PORT_TYPE);
+
+  // TCP Relay/Relay is the next.
+  VerifyNextPingableConnection(cricket::RELAY_PORT_TYPE,
+                               cricket::RELAY_PORT_TYPE,
+                               cricket::TCP_PROTOCOL_NAME);
+
+  // Finally, Local/Relay will be pinged.
+  VerifyNextPingableConnection(cricket::LOCAL_PORT_TYPE,
+                               cricket::RELAY_PORT_TYPE);
+}