Do not switch best connection on the controlled side too frequently

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 511 matching lines @@
     connect_time = rtc::TimeMillis() - connect_start;
     if (connect_time < expected.connect_wait) {
       LOG(LS_INFO) << "Connect time: " << connect_time << " ms";
     } else {
       LOG(LS_INFO) << "Connect time: " << "TIMEOUT ("
                    << expected.connect_wait << " ms)";
     }
 
     // Allow a few turns of the crank for the best connections to emerge.
     // This may take up to 2 seconds.
-    if (ep1_ch1()->best_connection() &&
-        ep2_ch1()->best_connection()) {
+    if (ep1_ch1()->selected_connection() && ep2_ch1()->selected_connection()) {
       int64_t converge_start = rtc::TimeMillis();
       int64_t converge_time;
       int64_t converge_wait = 2000;
       // Verifying local and remote channel best connection information. This is
       // done only for the RFC 5245 as controlled agent will use USE-CANDIDATE
       // from controlling (ep1) agent. We can easily predict from EP1 result
       // matrix.
       EXPECT_TRUE_WAIT_MARGIN(
           CheckCandidate1(expected) && CheckCandidate2(expected), converge_wait,
           converge_wait);
@@ skipping 89 matching lines @@
     // has higher tiebreaker value, channel1 should receive SignalRoleConflict.
     EXPECT_TRUE_WAIT(GetRoleConflict(0), 1000);
     EXPECT_FALSE(GetRoleConflict(1));
 
     EXPECT_TRUE_WAIT(ep1_ch1()->receiving() &&
                      ep1_ch1()->writable() &&
                      ep2_ch1()->receiving() &&
                      ep2_ch1()->writable(),
                      1000);
 
-    EXPECT_TRUE(ep1_ch1()->best_connection() &&
-                ep2_ch1()->best_connection());
+    EXPECT_TRUE(ep1_ch1()->selected_connection() &&
+                ep2_ch1()->selected_connection());
 
     TestSendRecv(1);
   }
 
   // We pass the candidates directly to the other side.
   void OnCandidateGathered(cricket::TransportChannelImpl* ch,
                            const cricket::Candidate& c) {
     if (force_relay_ && c.type() != cricket::RELAY_PORT_TYPE)
       return;
 
@@ skipping 90 matching lines @@
                const char* data, size_t len) {
     rtc::PacketOptions options;
     return channel->SendPacket(data, len, options, 0);
   }
   bool CheckDataOnChannel(cricket::TransportChannel* channel,
                           const char* data, int len) {
     return GetChannelData(channel)->CheckData(data, len);
   }
   static const cricket::Candidate* LocalCandidate(
       cricket::P2PTransportChannel* ch) {
-    return (ch && ch->best_connection()) ?
-        &ch->best_connection()->local_candidate() : NULL;
+    return (ch && ch->selected_connection())
+               ? &ch->selected_connection()->local_candidate()
+               : NULL;
   }
   static const cricket::Candidate* RemoteCandidate(
       cricket::P2PTransportChannel* ch) {
-    return (ch && ch->best_connection()) ?
-        &ch->best_connection()->remote_candidate() : NULL;
+    return (ch && ch->selected_connection())
+               ? &ch->selected_connection()->remote_candidate()
+               : NULL;
   }
   Endpoint* GetEndpoint(cricket::TransportChannel* ch) {
     if (ep1_.HasChannel(ch)) {
       return &ep1_;
     } else if (ep2_.HasChannel(ch)) {
       return &ep2_;
     } else {
       return NULL;
     }
   }
@@ skipping 335 matching lines @@
   // Emulate no remote credentials coming in.
   set_clear_remote_candidates_ufrag_pwd(false);
   CreateChannels(1);
   // Only have remote credentials come in for ep2, not ep1.
   ep2_ch1()->SetRemoteIceCredentials(kIceUfrag[0], kIcePwd[0]);
 
   // Pause sending ep2's candidates to ep1 until ep1 receives the peer reflexive
   // candidate.
   PauseCandidates(1);
 
   // The caller should have the best connection connected to the peer reflexive

[Taylor Brandstetter, 2016/06/21 18:33:26]

s/best connection/selected connnection/g

[honghaiz3, 2016/06/22 08:03:16]

Done.

   // candidate.
-  const cricket::Connection* best_connection = NULL;
-  WAIT((best_connection = ep1_ch1()->best_connection()) != NULL, 2000);
-  EXPECT_EQ("prflx", ep1_ch1()->best_connection()->remote_candidate().type());
+  const cricket::Connection* selected_connection = NULL;
+  WAIT((selected_connection = ep1_ch1()->selected_connection()) != NULL, 2000);
+  EXPECT_EQ("prflx",
+            ep1_ch1()->selected_connection()->remote_candidate().type());
 
   // Because we don't have a remote pwd, we don't ping yet.
   EXPECT_EQ(kIceUfrag[1],
-            ep1_ch1()->best_connection()->remote_candidate().username());
-  EXPECT_EQ("", ep1_ch1()->best_connection()->remote_candidate().password());
+            ep1_ch1()->selected_connection()->remote_candidate().username());
+  EXPECT_EQ("",
+            ep1_ch1()->selected_connection()->remote_candidate().password());
   // Because we don't have ICE credentials yet, we don't know the generation.
-  EXPECT_EQ(0u, ep1_ch1()->best_connection()->remote_candidate().generation());
+  EXPECT_EQ(0u,
+            ep1_ch1()->selected_connection()->remote_candidate().generation());
   EXPECT_TRUE(nullptr == ep1_ch1()->FindNextPingableConnection());
 
   // Add two sets of remote ICE credentials, so that the ones used by the
   // candidate will be generation 1 instead of 0.
   ep1_ch1()->SetRemoteIceCredentials(kIceUfrag[3], kIcePwd[3]);
   ep1_ch1()->SetRemoteIceCredentials(kIceUfrag[1], kIcePwd[1]);
   // After setting the remote ICE credentials, the password and generation
   // of the peer reflexive candidate should be updated.
   EXPECT_EQ(kIcePwd[1],
-            ep1_ch1()->best_connection()->remote_candidate().password());
-  EXPECT_EQ(1u, ep1_ch1()->best_connection()->remote_candidate().generation());
+            ep1_ch1()->selected_connection()->remote_candidate().password());
+  EXPECT_EQ(1u,
+            ep1_ch1()->selected_connection()->remote_candidate().generation());
   EXPECT_TRUE(nullptr != ep1_ch1()->FindNextPingableConnection());
 
   ResumeCandidates(1);
 
-  WAIT(ep2_ch1()->best_connection() != NULL, 2000);
+  WAIT(ep2_ch1()->selected_connection() != NULL, 2000);
   // Verify ep1's best connection is updated to use the 'local' candidate.
-  EXPECT_EQ_WAIT(
-      "local",
-      ep1_ch1()->best_connection()->remote_candidate().type(),
-      2000);
-  EXPECT_EQ(best_connection, ep1_ch1()->best_connection());
+  EXPECT_EQ_WAIT("local",
+                 ep1_ch1()->selected_connection()->remote_candidate().type(),
+                 2000);
+  EXPECT_EQ(selected_connection, ep1_ch1()->selected_connection());
   DestroyChannels();
 }
 
 // Test that we properly create a connection on a STUN ping from unknown address
 // when the signaling is slow and the end points are behind NAT.
 TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignalingWithNAT) {
   ConfigureEndpoints(OPEN, NAT_SYMMETRIC, kDefaultPortAllocatorFlags,
                      kDefaultPortAllocatorFlags);
   // Emulate no remote credentials coming in.
   set_clear_remote_candidates_ufrag_pwd(false);
   CreateChannels(1);
   // Only have remote credentials come in for ep2, not ep1.
   ep2_ch1()->SetRemoteIceCredentials(kIceUfrag[0], kIcePwd[0]);
   // Pause sending ep2's candidates to ep1 until ep1 receives the peer reflexive
   // candidate.
   PauseCandidates(1);
 
   // The caller should have the best connection connected to the peer reflexive
   // candidate.
-  WAIT(ep1_ch1()->best_connection() != NULL, 2000);
-  EXPECT_EQ("prflx", ep1_ch1()->best_connection()->remote_candidate().type());
+  WAIT(ep1_ch1()->selected_connection() != NULL, 2000);
+  EXPECT_EQ("prflx",
+            ep1_ch1()->selected_connection()->remote_candidate().type());
 
   // Because we don't have a remote pwd, we don't ping yet.
   EXPECT_EQ(kIceUfrag[1],
-            ep1_ch1()->best_connection()->remote_candidate().username());
-  EXPECT_EQ("", ep1_ch1()->best_connection()->remote_candidate().password());
+            ep1_ch1()->selected_connection()->remote_candidate().username());
+  EXPECT_EQ("",
+            ep1_ch1()->selected_connection()->remote_candidate().password());
   // Because we don't have ICE credentials yet, we don't know the generation.
-  EXPECT_EQ(0u, ep1_ch1()->best_connection()->remote_candidate().generation());
+  EXPECT_EQ(0u,
+            ep1_ch1()->selected_connection()->remote_candidate().generation());
   EXPECT_TRUE(nullptr == ep1_ch1()->FindNextPingableConnection());
 
   // Add two sets of remote ICE credentials, so that the ones used by the
   // candidate will be generation 1 instead of 0.
   ep1_ch1()->SetRemoteIceCredentials(kIceUfrag[3], kIcePwd[3]);
   ep1_ch1()->SetRemoteIceCredentials(kIceUfrag[1], kIcePwd[1]);
   // After setting the remote ICE credentials, the password and generation
   // of the peer reflexive candidate should be updated.
   EXPECT_EQ(kIcePwd[1],
-            ep1_ch1()->best_connection()->remote_candidate().password());
-  EXPECT_EQ(1u, ep1_ch1()->best_connection()->remote_candidate().generation());
+            ep1_ch1()->selected_connection()->remote_candidate().password());
+  EXPECT_EQ(1u,
+            ep1_ch1()->selected_connection()->remote_candidate().generation());
 
   ResumeCandidates(1);
 
-  const cricket::Connection* best_connection = NULL;
-  WAIT((best_connection = ep2_ch1()->best_connection()) != NULL, 2000);
+  const cricket::Connection* selected_connection = NULL;
+  WAIT((selected_connection = ep2_ch1()->selected_connection()) != NULL, 2000);
 
   // Wait to verify the connection is not culled.
   WAIT(ep1_ch1()->writable(), 2000);
-  EXPECT_EQ(ep2_ch1()->best_connection(), best_connection);
-  EXPECT_EQ("prflx", ep1_ch1()->best_connection()->remote_candidate().type());
+  EXPECT_EQ(ep2_ch1()->selected_connection(), selected_connection);
+  EXPECT_EQ("prflx",
+            ep1_ch1()->selected_connection()->remote_candidate().type());
   DestroyChannels();
 }
 
 // Test that if remote candidates don't have ufrag and pwd, we still work.
 TEST_F(P2PTransportChannelTest, RemoteCandidatesWithoutUfragPwd) {
   set_clear_remote_candidates_ufrag_pwd(true);
   ConfigureEndpoints(OPEN, OPEN, kDefaultPortAllocatorFlags,
                      kDefaultPortAllocatorFlags);
   CreateChannels(1);
-  const cricket::Connection* best_connection = NULL;
+  const cricket::Connection* selected_connection = NULL;
   // Wait until the callee's connections are created.
-  WAIT((best_connection = ep2_ch1()->best_connection()) != NULL, 1000);
+  WAIT((selected_connection = ep2_ch1()->selected_connection()) != NULL, 1000);
   // Wait to see if they get culled; they shouldn't.
-  WAIT(ep2_ch1()->best_connection() != best_connection, 1000);
-  EXPECT_TRUE(ep2_ch1()->best_connection() == best_connection);
+  WAIT(ep2_ch1()->selected_connection() != selected_connection, 1000);
+  EXPECT_TRUE(ep2_ch1()->selected_connection() == selected_connection);
   DestroyChannels();
 }
 
 // Test that a host behind NAT cannot be reached when incoming_only
 // is set to true.
 TEST_F(P2PTransportChannelTest, IncomingOnlyBlocked) {
   ConfigureEndpoints(NAT_FULL_CONE, OPEN, kDefaultPortAllocatorFlags,
                      kDefaultPortAllocatorFlags);
 
   SetAllocatorFlags(0, kOnlyLocalPorts);
@@ skipping 55 matching lines @@
   VerifySavedTcpCandidates(0, cricket::TCPTYPE_PASSIVE_STR);
   VerifySavedTcpCandidates(1, cricket::TCPTYPE_ACTIVE_STR);
 
   // Resume candidates.
   ResumeCandidates(0);
   ResumeCandidates(1);
 
   EXPECT_TRUE_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() &&
                    ep2_ch1()->receiving() && ep2_ch1()->writable(),
                    1000);
-  EXPECT_TRUE(
-      ep1_ch1()->best_connection() && ep2_ch1()->best_connection() &&
-      LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) &&
-      RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1]));
+  EXPECT_TRUE(ep1_ch1()->selected_connection() &&
+              ep2_ch1()->selected_connection() &&
+              LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) &&
+              RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1]));
 
   TestSendRecv(1);
   DestroyChannels();
 }
 
 TEST_F(P2PTransportChannelTest, TestIceRoleConflict) {
   AddAddress(0, kPublicAddrs[0]);
   AddAddress(1, kPublicAddrs[1]);
   TestSignalRoleConflict();
 }
@@ skipping 29 matching lines @@
     // original value.
     EXPECT_EQ(kTiebreaker1, ports_before[i]->IceTiebreaker());
   }
 
   EXPECT_TRUE_WAIT(ep1_ch1()->receiving() &&
                    ep1_ch1()->writable() &&
                    ep2_ch1()->receiving() &&
                    ep2_ch1()->writable(),
                    1000);
 
-  EXPECT_TRUE(ep1_ch1()->best_connection() &&
-              ep2_ch1()->best_connection());
+  EXPECT_TRUE(ep1_ch1()->selected_connection() &&
+              ep2_ch1()->selected_connection());
 
   TestSendRecv(1);
   DestroyChannels();
 }
 
 // Verify that we can set DSCP value and retrieve properly from P2PTC.
 TEST_F(P2PTransportChannelTest, TestDefaultDscpValue) {
   AddAddress(0, kPublicAddrs[0]);
   AddAddress(1, kPublicAddrs[1]);
 
@@ skipping 33 matching lines @@
   // Enable IPv6
   SetAllocatorFlags(0, cricket::PORTALLOCATOR_ENABLE_IPV6);
   SetAllocatorFlags(1, cricket::PORTALLOCATOR_ENABLE_IPV6);
 
   CreateChannels(1);
 
   EXPECT_TRUE_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() &&
                    ep2_ch1()->receiving() && ep2_ch1()->writable(),
                    1000);
   EXPECT_TRUE(
-      ep1_ch1()->best_connection() && ep2_ch1()->best_connection() &&
+      ep1_ch1()->selected_connection() && ep2_ch1()->selected_connection() &&
       LocalCandidate(ep1_ch1())->address().EqualIPs(kIPv6PublicAddrs[0]) &&
       RemoteCandidate(ep1_ch1())->address().EqualIPs(kIPv6PublicAddrs[1]));
 
   TestSendRecv(1);
   DestroyChannels();
 }
 
 // Testing forceful TURN connections.
 TEST_F(P2PTransportChannelTest, TestForceTurn) {
   ConfigureEndpoints(
       NAT_PORT_RESTRICTED, NAT_SYMMETRIC,
       kDefaultPortAllocatorFlags | cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET,
       kDefaultPortAllocatorFlags | cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET);
   set_force_relay(true);
 
   SetAllocationStepDelay(0, kMinimumStepDelay);
   SetAllocationStepDelay(1, kMinimumStepDelay);
 
   CreateChannels(1);
 
   EXPECT_TRUE_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() &&
                        ep2_ch1()->receiving() && ep2_ch1()->writable(),
                    2000);
 
-  EXPECT_TRUE(ep1_ch1()->best_connection() &&
-              ep2_ch1()->best_connection());
+  EXPECT_TRUE(ep1_ch1()->selected_connection() &&
+              ep2_ch1()->selected_connection());
 
   EXPECT_EQ("relay", RemoteCandidate(ep1_ch1())->type());
   EXPECT_EQ("relay", LocalCandidate(ep1_ch1())->type());
   EXPECT_EQ("relay", RemoteCandidate(ep2_ch1())->type());
   EXPECT_EQ("relay", LocalCandidate(ep2_ch1())->type());
 
   TestSendRecv(1);
   DestroyChannels();
 }
 
@@ skipping 55 matching lines @@
                               ep1_ch1()->receiving() && ep1_ch1()->writable() &&
                               ep2_ch1()->receiving() && ep2_ch1()->writable(),
                           1000, 1000);
   TestSendRecv(1);
   // Make sure the P2PTransportChannels are actually using ports from the
   // pooled sessions.
   auto pooled_ports_1 = pooled_session_1->ReadyPorts();
   auto pooled_ports_2 = pooled_session_2->ReadyPorts();
   EXPECT_NE(pooled_ports_1.end(),
             std::find(pooled_ports_1.begin(), pooled_ports_1.end(),
-                      ep1_ch1()->best_connection()->port()));
+                      ep1_ch1()->selected_connection()->port()));
   EXPECT_NE(pooled_ports_2.end(),
             std::find(pooled_ports_2.begin(), pooled_ports_2.end(),
-                      ep2_ch1()->best_connection()->port()));
+                      ep2_ch1()->selected_connection()->port()));
 }
 
 // Test that a connection succeeds when the P2PTransportChannel uses a pooled
 // PortAllocatorSession that already finished gathering candidates.
 TEST_F(P2PTransportChannelTest, TestUsingPooledSessionAfterDoneGathering) {
   ConfigureEndpoints(OPEN, OPEN, kDefaultPortAllocatorFlags,
                      kDefaultPortAllocatorFlags);
   // First create a pooled session for each endpoint.
   auto& allocator_1 = GetEndpoint(0)->allocator_;
   auto& allocator_2 = GetEndpoint(1)->allocator_;
@@ skipping 19 matching lines @@
                               ep1_ch1()->receiving() && ep1_ch1()->writable() &&
                               ep2_ch1()->receiving() && ep2_ch1()->writable(),
                           1000, 1000);
   TestSendRecv(1);
   // Make sure the P2PTransportChannels are actually using ports from the
   // pooled sessions.
   auto pooled_ports_1 = pooled_session_1->ReadyPorts();
   auto pooled_ports_2 = pooled_session_2->ReadyPorts();
   EXPECT_NE(pooled_ports_1.end(),
             std::find(pooled_ports_1.begin(), pooled_ports_1.end(),
-                      ep1_ch1()->best_connection()->port()));
+                      ep1_ch1()->selected_connection()->port()));
   EXPECT_NE(pooled_ports_2.end(),
             std::find(pooled_ports_2.begin(), pooled_ports_2.end(),
-                      ep2_ch1()->best_connection()->port()));
+                      ep2_ch1()->selected_connection()->port()));
 }
 
 // Test what happens when we have 2 users behind the same NAT. This can lead
 // to interesting behavior because the STUN server will only give out the
 // address of the outermost NAT.
 class P2PTransportChannelSameNatTest : public P2PTransportChannelTestBase {
  protected:
   void ConfigureEndpoints(Config nat_type, Config config1, Config config2) {
     ASSERT(nat_type >= NAT_FULL_CONE && nat_type <= NAT_SYMMETRIC);
     rtc::NATSocketServer::Translator* outer_nat =
@@ skipping 50 matching lines @@
   // Use only local ports for simplicity.
   SetAllocatorFlags(0, kOnlyLocalPorts);
   SetAllocatorFlags(1, kOnlyLocalPorts);
 
   // Create channels and let them go writable, as usual.
   CreateChannels(1);
 
   EXPECT_TRUE_WAIT_MARGIN(ep1_ch1()->receiving() && ep1_ch1()->writable() &&
                               ep2_ch1()->receiving() && ep2_ch1()->writable(),
                           1000, 1000);
-  EXPECT_TRUE(
-      ep1_ch1()->best_connection() && ep2_ch1()->best_connection() &&
-      LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) &&
-      RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1]));
+  EXPECT_TRUE(ep1_ch1()->selected_connection() &&
+              ep2_ch1()->selected_connection() &&
+              LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) &&
+              RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1]));
 
   // Make the receiving timeout shorter for testing.
   cricket::IceConfig config = CreateIceConfig(1000, false);
   ep1_ch1()->SetIceConfig(config);
   ep2_ch1()->SetIceConfig(config);
 
   // Blackhole any traffic to or from the public addrs.
   LOG(LS_INFO) << "Failing over...";
   fw()->AddRule(false, rtc::FP_ANY, rtc::FD_ANY, kPublicAddrs[1]);
   // The best connections will switch, so keep references to them.
-  const cricket::Connection* best_connection1 = ep1_ch1()->best_connection();
-  const cricket::Connection* best_connection2 = ep2_ch1()->best_connection();
+  const cricket::Connection* selected_connection1 =
+      ep1_ch1()->selected_connection();
+  const cricket::Connection* selected_connection2 =
+      ep2_ch1()->selected_connection();
   // We should detect loss of receiving within 1 second or so.
   EXPECT_TRUE_WAIT(
-      !best_connection1->receiving() && !best_connection2->receiving(), 3000);
+      !selected_connection1->receiving() && !selected_connection2->receiving(),
+      3000);
 
   // We should switch over to use the alternate addr immediately on both sides
   // when we are not receiving.
-  EXPECT_TRUE_WAIT(
-      ep1_ch1()->best_connection()->receiving() &&
-      ep2_ch1()->best_connection()->receiving(), 1000);
+  EXPECT_TRUE_WAIT(ep1_ch1()->selected_connection()->receiving() &&
+                       ep2_ch1()->selected_connection()->receiving(),
+                   1000);
   EXPECT_TRUE(LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]));
   EXPECT_TRUE(
       RemoteCandidate(ep1_ch1())->address().EqualIPs(kAlternateAddrs[1]));
   EXPECT_TRUE(
       LocalCandidate(ep2_ch1())->address().EqualIPs(kAlternateAddrs[1]));
 
   DestroyChannels();
 }
 
 // Test that we can quickly switch links if an interface goes down.
 // The controlling side has two interfaces and one will die.
 TEST_F(P2PTransportChannelMultihomedTest, TestFailoverControllingSide) {
   // Adding alternate address will make sure |kPublicAddrs| has the higher
   // priority than others. This is due to FakeNetwork::AddInterface method.
   AddAddress(0, kAlternateAddrs[0]);
   AddAddress(0, kPublicAddrs[0]);
   AddAddress(1, kPublicAddrs[1]);
 
   // Use only local ports for simplicity.
   SetAllocatorFlags(0, kOnlyLocalPorts);
   SetAllocatorFlags(1, kOnlyLocalPorts);
 
   // Create channels and let them go writable, as usual.
   CreateChannels(1);
   EXPECT_TRUE_WAIT_MARGIN(ep1_ch1()->receiving() && ep1_ch1()->writable() &&
                               ep2_ch1()->receiving() && ep2_ch1()->writable(),
                           1000, 1000);
-  EXPECT_TRUE(
-      ep1_ch1()->best_connection() && ep2_ch1()->best_connection() &&
-      LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) &&
-      RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1]));
+  EXPECT_TRUE(ep1_ch1()->selected_connection() &&
+              ep2_ch1()->selected_connection() &&
+              LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) &&
+              RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1]));
 
   // Make the receiving timeout shorter for testing.
   cricket::IceConfig config = CreateIceConfig(1000, false);
   ep1_ch1()->SetIceConfig(config);
   ep2_ch1()->SetIceConfig(config);
 
   // Blackhole any traffic to or from the public addrs.
   LOG(LS_INFO) << "Failing over...";
   fw()->AddRule(false, rtc::FP_ANY, rtc::FD_ANY, kPublicAddrs[0]);
   // The best connections will switch, so keep references to them.
-  const cricket::Connection* best_connection1 = ep1_ch1()->best_connection();
-  const cricket::Connection* best_connection2 = ep2_ch1()->best_connection();
+  const cricket::Connection* selected_connection1 =
+      ep1_ch1()->selected_connection();
+  const cricket::Connection* selected_connection2 =
+      ep2_ch1()->selected_connection();
   // We should detect loss of receiving within 1 second or so.
   EXPECT_TRUE_WAIT(
-      !best_connection1->receiving() && !best_connection2->receiving(), 3000);
+      !selected_connection1->receiving() && !selected_connection2->receiving(),
+      3000);
 
   // We should switch over to use the alternate addr immediately on both sides
   // when we are not receiving.
-  EXPECT_TRUE_WAIT(
-      ep1_ch1()->best_connection()->receiving() &&
-      ep2_ch1()->best_connection()->receiving(), 1000);
+  EXPECT_TRUE_WAIT(ep1_ch1()->selected_connection()->receiving() &&
+                       ep2_ch1()->selected_connection()->receiving(),
+                   1000);
   EXPECT_TRUE(
     LocalCandidate(ep1_ch1())->address().EqualIPs(kAlternateAddrs[0]));
   EXPECT_TRUE(RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1]));
   EXPECT_TRUE(
       RemoteCandidate(ep2_ch1())->address().EqualIPs(kAlternateAddrs[0]));
 
   DestroyChannels();
 }
 
 // Tests that a Wifi-Wifi connection has the highest precedence.
@@ skipping 11 matching lines @@
   SetAllocatorFlags(0, kOnlyLocalPorts);
   SetAllocatorFlags(1, kOnlyLocalPorts);
 
   // Create channels and let them go writable, as usual.
   CreateChannels(1);
 
   EXPECT_TRUE_WAIT_MARGIN(ep1_ch1()->receiving() && ep1_ch1()->writable() &&
                               ep2_ch1()->receiving() && ep2_ch1()->writable(),
                           1000, 1000);
   // Need to wait to make sure the connections on both networks are writable.
-  EXPECT_TRUE_WAIT(ep1_ch1()->best_connection() &&
+  EXPECT_TRUE_WAIT(ep1_ch1()->selected_connection() &&
                        LocalCandidate(ep1_ch1())->address().EqualIPs(wifi[0]) &&
                        RemoteCandidate(ep1_ch1())->address().EqualIPs(wifi[1]),
                    1000);
-  EXPECT_TRUE_WAIT(ep2_ch1()->best_connection() &&
+  EXPECT_TRUE_WAIT(ep2_ch1()->selected_connection() &&
                        LocalCandidate(ep2_ch1())->address().EqualIPs(wifi[1]) &&
                        RemoteCandidate(ep2_ch1())->address().EqualIPs(wifi[0]),
                    1000);
 }
 
 // Tests that a Wifi-Cellular connection has higher precedence than
 // a Cellular-Cellular connection.
 TEST_F(P2PTransportChannelMultihomedTest, TestPreferWifiOverCellularNetwork) {
   // The interface names are chosen so that |cellular| would have higher
   // candidate priority if it is not for the network type.
   auto& wifi = kAlternateAddrs;
   auto& cellular = kPublicAddrs;
   AddAddress(0, cellular[0], "test1", rtc::ADAPTER_TYPE_CELLULAR);
   AddAddress(1, wifi[1], "test0", rtc::ADAPTER_TYPE_WIFI);
   AddAddress(1, cellular[1], "test1", rtc::ADAPTER_TYPE_CELLULAR);
 
   // Use only local ports for simplicity.
   SetAllocatorFlags(0, kOnlyLocalPorts);
   SetAllocatorFlags(1, kOnlyLocalPorts);
 
   // Create channels and let them go writable, as usual.
   CreateChannels(1);
 
   EXPECT_TRUE_WAIT_MARGIN(ep1_ch1()->receiving() && ep1_ch1()->writable() &&
                               ep2_ch1()->receiving() && ep2_ch1()->writable(),
                           1000, 1000);
   // Need to wait to make sure the connections on both networks are writable.
-  EXPECT_TRUE_WAIT(ep1_ch1()->best_connection() &&
+  EXPECT_TRUE_WAIT(ep1_ch1()->selected_connection() &&
                        RemoteCandidate(ep1_ch1())->address().EqualIPs(wifi[1]),
                    1000);
-  EXPECT_TRUE_WAIT(ep2_ch1()->best_connection() &&
+  EXPECT_TRUE_WAIT(ep2_ch1()->selected_connection() &&
                        LocalCandidate(ep2_ch1())->address().EqualIPs(wifi[1]),
                    1000);
 }
 
 // Test that the backup connection is pinged at a rate no faster than
 // what was configured.
 TEST_F(P2PTransportChannelMultihomedTest, TestPingBackupConnectionRate) {
   AddAddress(0, kPublicAddrs[0]);
   // Adding alternate address will make sure |kPublicAddrs| has the higher
   // priority than others. This is due to FakeNetwork::AddInterface method.
@@ skipping 94 matching lines @@
   // Use only local ports for simplicity.
   SetAllocatorFlags(0, kOnlyLocalPorts);
   SetAllocatorFlags(1, kOnlyLocalPorts);
 
   // Create channels and let them go writable, as usual.
   CreateChannels(1);
   EXPECT_TRUE_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() &&
                    ep2_ch1()->receiving() && ep2_ch1()->writable(),
                    1000);
   EXPECT_TRUE(
-      ep1_ch1()->best_connection() && ep2_ch1()->best_connection() &&
+      ep1_ch1()->selected_connection() && ep2_ch1()->selected_connection() &&
       LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) &&
       RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1]));
 
 
   // Remove the public interface, add the alternate interface, and allocate
   // a new generation of candidates for the new interface (via
   // MaybeStartGathering()).
   LOG(LS_INFO) << "Draining...";
   AddAddress(1, kAlternateAddrs[1]);
   RemoveAddress(1, kPublicAddrs[1]);
   ep2_ch1()->MaybeStartGathering();
 
   // We should switch over to use the alternate address after
   // an exchange of pings.
   EXPECT_TRUE_WAIT(
-      ep1_ch1()->best_connection() && ep2_ch1()->best_connection() &&
+      ep1_ch1()->selected_connection() && ep2_ch1()->selected_connection() &&
       LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) &&
       RemoteCandidate(ep1_ch1())->address().EqualIPs(kAlternateAddrs[1]),
       3000);
 
   DestroyChannels();
 }
 
 */
 
 // A collection of tests which tests a single P2PTransportChannel by sending
@@ skipping 68 matching lines @@
 
   int SendData(cricket::TransportChannel& channel,
                const char* data,
                size_t len,
                int packet_id) {
     rtc::PacketOptions options;
     options.packet_id = packet_id;
     return channel.SendPacket(data, len, options, 0);
   }
 
+  cricket::Connection* CreateConnectionWithCandidate(
+      cricket::P2PTransportChannel& channel,
+      const std::string& ip_addr,
+      int port,
+      int priority,
+      bool writable) {
+    channel.AddRemoteCandidate(CreateHostCandidate(ip_addr, port, priority));
+    cricket::Connection* conn = WaitForConnectionTo(&channel, ip_addr, port);
+    if (conn && writable) {
+      conn->ReceivedPingResponse();  // make it writable
+    }
+    return conn;
+  }
+
+  void NominateConnection(cricket::Connection* conn) {
+    conn->set_nominated(true);
+    conn->SignalNominated(conn);
+  }
+
   void OnSelectedCandidatePairChanged(
       cricket::TransportChannel* transport_channel,
       cricket::CandidatePairInterface* selected_candidate_pair,
       int last_sent_packet_id) {
     last_selected_candidate_pair_ = selected_candidate_pair;
     last_sent_packet_id_ = last_sent_packet_id;
+    ++selected_candidate_pair_switches_;
   }
 
   void ReceivePingOnConnection(cricket::Connection* conn,
                                const std::string& remote_ufrag,
                                int priority) {
     cricket::IceMessage msg;
     msg.SetType(cricket::STUN_BINDING_REQUEST);
     msg.AddAttribute(new cricket::StunByteStringAttribute(
         cricket::STUN_ATTR_USERNAME,
         conn->local_candidate().username() + ":" + remote_ufrag));
@@ skipping 15 matching lines @@
     channel_state_ = channel->GetState();
   }
 
   cricket::CandidatePairInterface* last_selected_candidate_pair() {
     return last_selected_candidate_pair_;
   }
   int last_sent_packet_id() { return last_sent_packet_id_; }
   bool channel_ready_to_send() { return channel_ready_to_send_; }
   void reset_channel_ready_to_send() { channel_ready_to_send_ = false; }
   cricket::TransportChannelState channel_state() { return channel_state_; }
+  int get_and_reset_selected_candidate_pair_switches() {
+    int switches = selected_candidate_pair_switches_;
+    selected_candidate_pair_switches_ = 0;
+    return switches;
+  }
 
  private:
   std::unique_ptr<rtc::PhysicalSocketServer> pss_;
   std::unique_ptr<rtc::VirtualSocketServer> vss_;
   rtc::SocketServerScope ss_scope_;
   cricket::CandidatePairInterface* last_selected_candidate_pair_ = nullptr;
+  int selected_candidate_pair_switches_ = 0;
   int last_sent_packet_id_ = -1;
   bool channel_ready_to_send_ = false;
   cricket::TransportChannelState channel_state_ = cricket::STATE_INIT;
 };
 
 TEST_F(P2PTransportChannelPingTest, TestTriggeredChecks) {
   cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   cricket::P2PTransportChannel ch("trigger checks", 1, &pa);
   PrepareChannel(&ch);
   ch.Connect();
@@ skipping 202 matching lines @@
   EXPECT_EQ(500, ch.receiving_timeout());
   EXPECT_EQ(50, ch.check_receiving_interval());
   ch.Connect();
   ch.MaybeStartGathering();
   ch.AddRemoteCandidate(CreateHostCandidate("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.selected_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". Plus, SignalSelectedCandidatePair will be fired if the
 // best connection changes and SignalReadyToSend will be fired if the new best
 // connection is writable.
 TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) {
   cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   cricket::P2PTransportChannel ch("receiving state change", 1, &pa);
   PrepareChannel(&ch);
   ch.SetIceRole(cricket::ICEROLE_CONTROLLED);
   ch.Connect();
   ch.MaybeStartGathering();
   ch.AddRemoteCandidate(CreateHostCandidate("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());
+  EXPECT_EQ(conn1, ch.selected_connection());
   EXPECT_EQ(conn1, last_selected_candidate_pair());
   EXPECT_EQ(-1, last_sent_packet_id());
   // Channel is not ready to send because it is not writable.
   EXPECT_FALSE(channel_ready_to_send());
 
   int last_packet_id = 0;
   const char* data = "ABCDEFGH";
   int len = static_cast<int>(strlen(data));
   SendData(ch, data, len, ++last_packet_id);
   // When a higher priority candidate comes in, the new connection is chosen
   // as the best connection.
   ch.AddRemoteCandidate(CreateHostCandidate("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());
+  EXPECT_EQ(conn2, ch.selected_connection());
   EXPECT_EQ(conn2, last_selected_candidate_pair());
   EXPECT_EQ(last_packet_id, last_sent_packet_id());
   EXPECT_FALSE(channel_ready_to_send());
 
   // 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.
   SendData(ch, data, len, ++last_packet_id);
   ch.AddRemoteCandidate(CreateHostCandidate("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());
+  EXPECT_EQ(conn2, ch.selected_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());
+  EXPECT_EQ(conn3, ch.selected_connection());
   EXPECT_EQ(conn3, last_selected_candidate_pair());
   EXPECT_EQ(last_packet_id, last_sent_packet_id());
   EXPECT_TRUE(channel_ready_to_send());
 
   // 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.
   SendData(ch, data, len, ++last_packet_id);
   ch.AddRemoteCandidate(CreateHostCandidate("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());
+  EXPECT_EQ(conn3, ch.selected_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());
+  EXPECT_EQ(conn3, ch.selected_connection());
   reset_channel_ready_to_send();
   // The best connection switches after conn4 becomes writable.
   conn4->ReceivedPingResponse();
-  EXPECT_EQ(conn4, ch.best_connection());
+  EXPECT_EQ_WAIT(conn4, ch.selected_connection(), kDefaultTimeout);
   EXPECT_EQ(conn4, last_selected_candidate_pair());
   EXPECT_EQ(last_packet_id, last_sent_packet_id());
   // SignalReadyToSend is fired again because conn4 is writable.
   EXPECT_TRUE(channel_ready_to_send());
 }
 
 // The controlled side will select a connection as the "best connection" based
 // on requests from an unknown address before the controlling side nominates
 // a connection, and will nominate a connection from an unknown address if the
 // request contains the use_candidate attribute. Plus, it will also sends back
@@ skipping 12 matching lines @@
       cricket::STUN_ATTR_USERNAME, kIceUfrag[1]));
   uint32_t prflx_priority = cricket::ICE_TYPE_PREFERENCE_PRFLX << 24;
   request.AddAttribute(new cricket::StunUInt32Attribute(
       cricket::STUN_ATTR_PRIORITY, prflx_priority));
   cricket::TestUDPPort* port = static_cast<cricket::TestUDPPort*>(GetPort(&ch));
   port->SignalUnknownAddress(port, rtc::SocketAddress("1.1.1.1", 1),
                              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());
+  EXPECT_EQ(conn1, ch.selected_connection());
   conn1->ReceivedPingResponse();
-  EXPECT_EQ(conn1, ch.best_connection());
+  EXPECT_EQ(conn1, ch.selected_connection());
   port->set_sent_binding_response(false);
 
   // Another connection is nominated via use_candidate.
   ch.AddRemoteCandidate(CreateHostCandidate("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());
+  EXPECT_EQ(conn1, ch.selected_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());
+  EXPECT_EQ(conn2, ch.selected_connection());
 
   // Another request with unknown address, it will not be set as the best
   // connection because the best connection was nominated by the controlling
   // side.
   port->SignalUnknownAddress(port, rtc::SocketAddress("3.3.3.3", 3),
                              cricket::PROTO_UDP, &request, kIceUfrag[1], false);
   cricket::Connection* conn3 = WaitForConnectionTo(&ch, "3.3.3.3", 3);
   ASSERT_TRUE(conn3 != nullptr);
   EXPECT_TRUE(port->sent_binding_response());
   conn3->ReceivedPingResponse();  // Become writable.
-  EXPECT_EQ(conn2, ch.best_connection());
+  EXPECT_EQ(conn2, ch.selected_connection());
   port->set_sent_binding_response(false);
 
   // However if the request contains use_candidate attribute, it will be
   // selected as the best connection.
   request.AddAttribute(
       new cricket::StunByteStringAttribute(cricket::STUN_ATTR_USE_CANDIDATE));
   port->SignalUnknownAddress(port, rtc::SocketAddress("4.4.4.4", 4),
                              cricket::PROTO_UDP, &request, kIceUfrag[1], false);
   cricket::Connection* conn4 = WaitForConnectionTo(&ch, "4.4.4.4", 4);
   ASSERT_TRUE(conn4 != nullptr);
   EXPECT_TRUE(port->sent_binding_response());
   // conn4 is not the best connection yet because it is not writable.
-  EXPECT_EQ(conn2, ch.best_connection());
+  EXPECT_EQ(conn2, ch.selected_connection());
   conn4->ReceivedPingResponse();  // Become writable.
-  EXPECT_EQ(conn4, ch.best_connection());
+  EXPECT_EQ_WAIT(conn4, ch.selected_connection(), kDefaultTimeout);
 
   // Test that the request from an unknown address contains a ufrag from an old
   // generation.
   port->set_sent_binding_response(false);
   ch.SetRemoteIceCredentials(kIceUfrag[2], kIcePwd[2]);
   ch.SetRemoteIceCredentials(kIceUfrag[3], kIcePwd[3]);
   port->SignalUnknownAddress(port, rtc::SocketAddress("5.5.5.5", 5),
                              cricket::PROTO_UDP, &request, kIceUfrag[2], false);
   cricket::Connection* conn5 = WaitForConnectionTo(&ch, "5.5.5.5", 5);
   ASSERT_TRUE(conn5 != nullptr);
   EXPECT_TRUE(port->sent_binding_response());
   EXPECT_EQ(kIcePwd[2], conn5->remote_candidate().password());
 }
 
 // The controlled side will select a connection as the "best connection"
 // 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, &pa);
   PrepareChannel(&ch);
   ch.SetIceRole(cricket::ICEROLE_CONTROLLED);
   ch.Connect();
   ch.MaybeStartGathering();
   ch.AddRemoteCandidate(CreateHostCandidate("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());
+  EXPECT_EQ(conn1, ch.selected_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(CreateHostCandidate("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));
-  EXPECT_EQ(conn2, ch.best_connection());
+  EXPECT_EQ(conn2, ch.selected_connection());
+  conn2->ReceivedPingResponse();  // Make it writable.
 
   // Now another STUN message with an unknown address and use_candidate will
   // nominate the best connection.
   cricket::IceMessage request;
   request.SetType(cricket::STUN_BINDING_REQUEST);
   request.AddAttribute(new cricket::StunByteStringAttribute(
       cricket::STUN_ATTR_USERNAME, kIceUfrag[1]));
   uint32_t prflx_priority = cricket::ICE_TYPE_PREFERENCE_PRFLX << 24;
   request.AddAttribute(new cricket::StunUInt32Attribute(
       cricket::STUN_ATTR_PRIORITY, prflx_priority));
   request.AddAttribute(
       new cricket::StunByteStringAttribute(cricket::STUN_ATTR_USE_CANDIDATE));
   cricket::Port* port = GetPort(&ch);
   port->SignalUnknownAddress(port, rtc::SocketAddress("3.3.3.3", 3),
                              cricket::PROTO_UDP, &request, kIceUfrag[1], false);
   cricket::Connection* conn3 = WaitForConnectionTo(&ch, "3.3.3.3", 3);
   ASSERT_TRUE(conn3 != nullptr);
-  EXPECT_EQ(conn2, ch.best_connection());  // Not writable yet.
+  EXPECT_EQ(conn2, ch.selected_connection());  // Not writable yet.
   conn3->ReceivedPingResponse();           // Become writable.
-  EXPECT_EQ(conn3, ch.best_connection());
+  EXPECT_EQ_WAIT(conn3, ch.selected_connection(), kDefaultTimeout);
 
   // Now another data packet will not switch the best connection because the
   // best connection was nominated by the controlling side.
   conn2->ReceivedPing();
   conn2->ReceivedPingResponse();
   conn2->OnReadPacket("XYZ", 3, rtc::CreatePacketTime(0));
-  EXPECT_EQ(conn3, ch.best_connection());
+  EXPECT_EQ_WAIT(conn3, ch.selected_connection(), kDefaultTimeout);
+}
+
+TEST_F(P2PTransportChannelPingTest, TestDonotSwitchConnectionTooFrequently) {

[Taylor Brandstetter, 2016/06/21 18:33:26]

Rename test to ControlledAgentDoesntSwitchConnectionTooFrequently

[honghaiz3, 2016/06/22 08:03:16]

Done.

+  rtc::ScopedFakeClock clock;
+
+  cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
+  cricket::P2PTransportChannel ch("receiving state change", 1, &pa);
+  PrepareChannel(&ch);
+  ch.SetIceRole(cricket::ICEROLE_CONTROLLED);
+  ch.Connect();
+  ch.MaybeStartGathering();
+  // The connections have decreasing priority.
+  cricket::Connection* conn1 =
+      CreateConnectionWithCandidate(ch, "1.1.1.1", 1, 10, true);
+  ASSERT_TRUE(conn1 != nullptr);
+  cricket::Connection* conn2 =
+      CreateConnectionWithCandidate(ch, "2.2.2.2", 2, 9, true);
+  ASSERT_TRUE(conn2 != nullptr);
+  cricket::Connection* conn3 =
+      CreateConnectionWithCandidate(ch, "3.3.3.3", 3, 8, false);
+  ASSERT_TRUE(conn3 != nullptr);
+
+  // Simulate aggressive nomination on the controlling side.
+  get_and_reset_selected_candidate_pair_switches();
+  NominateConnection(conn2);
+  EXPECT_EQ(1, get_and_reset_selected_candidate_pair_switches());
+  EXPECT_EQ(conn2, last_selected_candidate_pair());
+
+  // Selected candidate pair will switch because conn1 has higher priority.
+  NominateConnection(conn1);
+  EXPECT_EQ(1, get_and_reset_selected_candidate_pair_switches());
+  EXPECT_EQ(conn1, last_selected_candidate_pair());
+
+  // Advance the clock by 1ms so that the last data receiving timestamp of
+  // conn2 is larger.
+  SIMULATED_WAIT(false, 1, clock);
+  // Selected candidate pair won't switch until conn2 receives data and becomes
+  // writable again because conn2 has lower priority and may have been pruned.
+  NominateConnection(conn2);
+  conn2->ReceivedPingResponse();
+  EXPECT_EQ(0, get_and_reset_selected_candidate_pair_switches());
+  // Will switch if conn2 receives data.
+  conn2->OnReadPacket("XYZ", 3, rtc::CreatePacketTime(0));
+  EXPECT_EQ(1, get_and_reset_selected_candidate_pair_switches());
+  EXPECT_EQ(conn2, last_selected_candidate_pair());
+
+  // Advance the clock by 1ms so that the last data receiving timestamp of
+  // conn3 is larger.
+  SIMULATED_WAIT(false, 1, clock);
+  // Selected candidate pair won't switch even if it is nominated and
+  // received data because it is not writable.
+  NominateConnection(conn3);
+  conn3->OnReadPacket("XYZ", 3, rtc::CreatePacketTime(0));
+  EXPECT_EQ(0, get_and_reset_selected_candidate_pair_switches());
+  // It will switch if it becomes writable
+  conn3->ReceivedPingResponse();
+  EXPECT_EQ_SIMULATED_WAIT(1, get_and_reset_selected_candidate_pair_switches(),
+                           kDefaultTimeout, clock);
+  EXPECT_EQ(conn3, last_selected_candidate_pair());
+
+  // Make sure sorting won't reselect candidate pair.
+  SIMULATED_WAIT(false, 10, clock);
+  EXPECT_EQ(0, get_and_reset_selected_candidate_pair_switches());
 }

[Taylor Brandstetter, 2016/06/21 18:33:26]

This test's expectations are good but it's testing way too many things. I think
there should be separate tests for:

1. First nominated connection is selected.
2. Higher priority nominated connection is selected.
3. Connection on which data was last received is selected.
4. Connection is selected once it becomes writable.

[honghaiz3, 2016/06/22 08:03:16]

I break this into three, making each of them smaller. 

 
 // Test that if a new remote candidate has the same address and port with
 // an old one, it will be used to create a new connection.
 TEST_F(P2PTransportChannelPingTest, TestAddRemoteCandidateWithAddressReuse) {
   cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   cricket::P2PTransportChannel ch("candidate reuse", 1, &pa);
   PrepareChannel(&ch);
   ch.Connect();
   ch.MaybeStartGathering();
   const std::string host_address = "1.1.1.1";
@@ skipping 32 matching lines @@
 TEST_F(P2PTransportChannelPingTest, TestDontPruneWhenWeak) {
   cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   cricket::P2PTransportChannel ch("test channel", 1, &pa);
   PrepareChannel(&ch);
   ch.SetIceRole(cricket::ICEROLE_CONTROLLED);
   ch.Connect();
   ch.MaybeStartGathering();
   ch.AddRemoteCandidate(CreateHostCandidate("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());
+  EXPECT_EQ(conn1, ch.selected_connection());
   conn1->ReceivedPingResponse();  // Becomes writable and receiving
 
   // When a higher-priority, nominated candidate comes in, the connections with
   // lower-priority are pruned.
   ch.AddRemoteCandidate(CreateHostCandidate("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);
@@ skipping 44 matching lines @@
 TEST_F(P2PTransportChannelPingTest, TestConnectionPrunedAgain) {
   cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   cricket::P2PTransportChannel ch("test channel", 1, &pa);
   PrepareChannel(&ch);
   ch.SetIceConfig(CreateIceConfig(1000, false));
   ch.Connect();
   ch.MaybeStartGathering();
   ch.AddRemoteCandidate(CreateHostCandidate("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());
+  EXPECT_EQ(conn1, ch.selected_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(CreateHostCandidate("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);
   ASSERT_TRUE(conn2 != nullptr);
   EXPECT_EQ_WAIT(cricket::Connection::STATE_INPROGRESS, conn2->state(), 1000);
   conn2->ReceivedPingResponse();
-  EXPECT_EQ_WAIT(conn2, ch.best_connection(), 1000);
+  EXPECT_EQ_WAIT(conn2, ch.selected_connection(), 1000);
   EXPECT_EQ(cricket::TransportChannelState::STATE_CONNECTING, ch.GetState());
 
   // When |conn1| comes back again, |conn2| will be pruned again.
   conn1->ReceivedPingResponse();
-  EXPECT_EQ_WAIT(conn1, ch.best_connection(), 1000);
+  EXPECT_EQ_WAIT(conn1, ch.selected_connection(), 1000);
   EXPECT_TRUE_WAIT(!conn2->active(), 1000);
   EXPECT_EQ(cricket::TransportChannelState::STATE_COMPLETED, ch.GetState());
 }
 
 // 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, &pa);
   PrepareChannel(&ch);
@@ skipping 169 matching lines @@
 
   TODO(honghaiz): Re-enable this once we use fake clock for this test to fix
   the flakiness. The following test becomes flaky because we now ping the
   connections with fast rates until every connection is pinged at least three
   times. The best connection may have been pinged before |max_strong_interval|,
   so it may not be the next connection to be pinged as expected in the test.
 
   // Verify that conn3 will be the "best connection" since it is readable and
   // writable. After |MAX_CURRENT_STRONG_INTERVAL|, it should be the next
   // pingable connection.
-  EXPECT_TRUE_WAIT(conn3 == ch.best_connection(), 5000);
+  EXPECT_TRUE_WAIT(conn3 == ch.selected_connection(), 5000);
   WAIT(false, max_strong_interval + 100);
   conn3->ReceivedPingResponse();
   ASSERT_TRUE(conn3->writable());
   EXPECT_EQ(conn3, FindNextPingableConnectionAndPingIt(&ch));
 
   */
 }
 
 // Test that Relay/Relay connections will be pinged first when everything has
 // been pinged even if the Relay/Relay connection wasn't the first to be pinged
@@ skipping 97 matching lines @@
 
   // 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);
 }