Do not switch a connection if the new connection is not ready to send packets.

webrtc/p2p/base/p2ptransportchannel_unittest.cc

Index: webrtc/p2p/base/p2ptransportchannel_unittest.cc
diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc
index 7cd23daa38d2567dbab7cb16647c197f2ff8c094..6154997bf47a1a273fc75c866cf006ea574be3d9 100644
--- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc
+++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc
@@ -857,6 +857,13 @@ class P2PTransportChannelTestBase : public testing::Test,
   void OnNominated(Connection* conn) { nominated_ = true; }
   bool nominated() { return nominated_; }
 
+  Connection* GetTopConnection(P2PTransportChannel* channel) {
+    if (channel->connections().empty()) {
+      return nullptr;
+    }
+    return channel->connections().front();
+  }

[Taylor Brandstetter, 2016/08/08 23:01:05]

I think these unit tests should move away from accessing "connections_"
directly, since it's an implementation detail. Could the tests that depend on
this method be rewritten so they don't need it?

[honghaiz3, 2016/08/11 22:45:53]

Done.

+
  private:
   rtc::Thread* main_;
   std::unique_ptr<rtc::PhysicalSocketServer> pss_;
@@ -1172,21 +1179,19 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignaling) {
   // candidate.
   PauseCandidates(1);
 
-  // The caller should have the selected connection connected to the peer
-  // reflexive candidate.
-  const Connection* selected_connection = NULL;
-  WAIT((selected_connection = ep1_ch1()->selected_connection()) != NULL, 2000);
-  EXPECT_EQ("prflx",
-            ep1_ch1()->selected_connection()->remote_candidate().type());
+  // The caller should have the top connection connected to the peer
+  // reflexive candidate. Note that the top connection is not selected yet as
+  // it is not ready to send.
+  const Connection* top_connection = NULL;
+  ASSERT_TRUE_WAIT((top_connection = GetTopConnection(ep1_ch1())) != NULL,

[Taylor Brandstetter, 2016/08/08 23:01:05]

For example, in this test I think we can do the EXPECTS after the connection is
selected (after SetRemoteIceCredentials but before ResumeCandidates). What this
test really wants to verify is that a prflx connection can be used before a
candidate is signaled, and that its remote candidate is replaced once one is
received.

[honghaiz3, 2016/08/11 22:45:53]

Done.

+                   2000);
+  EXPECT_EQ("prflx", top_connection->remote_candidate().type());
 
   // Because we don't have a remote pwd, we don't ping yet.
-  EXPECT_EQ(kIceUfrag[1],
-            ep1_ch1()->selected_connection()->remote_candidate().username());
-  EXPECT_EQ("",
-            ep1_ch1()->selected_connection()->remote_candidate().password());
+  EXPECT_EQ(kIceUfrag[1], top_connection->remote_candidate().username());
+  EXPECT_TRUE(top_connection->remote_candidate().password().empty());
   // Because we don't have ICE credentials yet, we don't know the generation.
-  EXPECT_EQ(0u,
-            ep1_ch1()->selected_connection()->remote_candidate().generation());
+  EXPECT_EQ(0u, top_connection->remote_candidate().generation());
   EXPECT_TRUE(nullptr == ep1_ch1()->FindNextPingableConnection());
 
   // Add two sets of remote ICE credentials, so that the ones used by the
@@ -1195,20 +1200,17 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignaling) {
   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()->selected_connection()->remote_candidate().password());
-  EXPECT_EQ(1u,
-            ep1_ch1()->selected_connection()->remote_candidate().generation());
+  EXPECT_EQ(kIcePwd[1], top_connection->remote_candidate().password());
+  EXPECT_EQ(1u, top_connection->remote_candidate().generation());

[Taylor Brandstetter, 2016/08/15 17:55:49]

Previously this tested that a prflx candidate's generation would be updated once
ICE credentials are received, allowing it to be replaced by a non-prflx
candidate (there used to be bugs for this flow). If that coverage is being
removed from these tests, could it be added as a separate test?

[honghaiz3, 2016/08/16 17:29:15]

It is more natural to test all these (ufrag, pwd, and generation) at one place
when ice_credentials are updated. And if we want to do the same test as the
original one (basically the client get the stun ping request before getting any
signaling (offer/candidate)), it is best to just peek into the internal  state
(i.e., connection() and TopConnection) as I did in the first revision. And I do
not think it is super-critical to avoid that. WDYT?

[Taylor Brandstetter, 2016/08/16 18:48:31]

I don't think we need to peek at the internal state for this. If we want to wait
until endpoint 1 receives a STUN ping, we can just wait for endpoint 2 to become
writable, or even use GetStats.

[honghaiz3, 2016/08/16 21:11:36]

Done. True if we only need to check the ICE generation after remote ICE
candidate is received. 

   EXPECT_TRUE(nullptr != ep1_ch1()->FindNextPingableConnection());
 
   ResumeCandidates(1);
 
-  WAIT(ep2_ch1()->selected_connection() != NULL, 2000);
-  // Verify ep1's selected connection is updated to use the 'local' candidate.
+  EXPECT_TRUE_WAIT(ep2_ch1()->selected_connection() != NULL, 2000);
+  // Verify ep1's top connection is updated to use the 'local' candidate.
   EXPECT_EQ_WAIT("local",
-                 ep1_ch1()->selected_connection()->remote_candidate().type(),
-                 2000);
-  EXPECT_EQ(selected_connection, ep1_ch1()->selected_connection());
+                 GetTopConnection(ep1_ch1())->remote_candidate().type(), 2000);
+  EXPECT_EQ(top_connection, GetTopConnection(ep1_ch1()));
   DestroyChannels();
 }
 
@@ -1226,20 +1228,18 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignalingWithNAT) {
   // candidate.
   PauseCandidates(1);
 
-  // The caller should have the selected connection connected to the peer
-  // reflexive candidate.
-  WAIT(ep1_ch1()->selected_connection() != NULL, 2000);
-  EXPECT_EQ("prflx",
-            ep1_ch1()->selected_connection()->remote_candidate().type());
+  // The caller's top connection should be connected to the peer reflexive
+  // candidate.
+  Connection* top_connection = nullptr;
+  ASSERT_TRUE_WAIT((top_connection = GetTopConnection(ep1_ch1())) != nullptr,
+                   2000);
+  EXPECT_EQ("prflx", top_connection->remote_candidate().type());
 
   // Because we don't have a remote pwd, we don't ping yet.
-  EXPECT_EQ(kIceUfrag[1],
-            ep1_ch1()->selected_connection()->remote_candidate().username());
-  EXPECT_EQ("",
-            ep1_ch1()->selected_connection()->remote_candidate().password());
+  EXPECT_EQ(kIceUfrag[1], top_connection->remote_candidate().username());
+  EXPECT_EQ("", top_connection->remote_candidate().password());
   // Because we don't have ICE credentials yet, we don't know the generation.
-  EXPECT_EQ(0u,
-            ep1_ch1()->selected_connection()->remote_candidate().generation());
+  EXPECT_EQ(0u, top_connection->remote_candidate().generation());
   EXPECT_TRUE(nullptr == ep1_ch1()->FindNextPingableConnection());
 
   // Add two sets of remote ICE credentials, so that the ones used by the
@@ -1248,21 +1248,20 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignalingWithNAT) {
   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()->selected_connection()->remote_candidate().password());
-  EXPECT_EQ(1u,
-            ep1_ch1()->selected_connection()->remote_candidate().generation());
+  EXPECT_EQ(kIcePwd[1], top_connection->remote_candidate().password());
+  EXPECT_EQ(1u, top_connection->remote_candidate().generation());
 
   ResumeCandidates(1);
 
   const Connection* selected_connection = NULL;
-  WAIT((selected_connection = ep2_ch1()->selected_connection()) != NULL, 2000);
+  EXPECT_TRUE_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()->selected_connection(), selected_connection);
+  EXPECT_TRUE_WAIT(ep1_ch1()->writable(), 2000);
   EXPECT_EQ("prflx",
             ep1_ch1()->selected_connection()->remote_candidate().type());
+  EXPECT_EQ(selected_connection, ep2_ch1()->selected_connection());
   DestroyChannels();
 }
 
@@ -2557,8 +2556,14 @@ class P2PTransportChannelPingTest : public testing::Test,
 
   Connection* WaitForConnectionTo(P2PTransportChannel* ch,
                                   const std::string& ip,
-                                  int port_num) {
-    EXPECT_TRUE_WAIT(GetConnectionTo(ch, ip, port_num) != nullptr, 3000);
+                                  int port_num,
+                                  rtc::FakeClock* clock = nullptr) {
+    if (clock == nullptr) {
+      EXPECT_TRUE_WAIT(GetConnectionTo(ch, ip, port_num) != nullptr, 3000);
+    } else {
+      EXPECT_TRUE_SIMULATED_WAIT(GetConnectionTo(ch, ip, port_num) != nullptr,
+                                 3000, *clock);
+    }
     return GetConnectionTo(ch, ip, port_num);
   }
 
@@ -3043,7 +3048,6 @@ TEST_F(P2PTransportChannelPingTest, TestReceivingStateChange) {
 
   conn1->ReceivedPing();
   conn1->OnReadPacket("ABC", 3, rtc::CreatePacketTime(0));
-  EXPECT_TRUE_WAIT(ch.selected_connection() != nullptr, 1000);
   EXPECT_TRUE_WAIT(ch.receiving(), 1000);
   EXPECT_TRUE_WAIT(!ch.receiving(), 1000);
 }
@@ -3063,30 +3067,35 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) {
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 1));
   Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   ASSERT_TRUE(conn1 != nullptr);
-  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));
   EXPECT_EQ(-1, SendData(ch, data, len, ++last_packet_id));
+  EXPECT_EQ(-1, last_sent_packet_id());
+
+  // A connection need to be writable before it is selected for transmission.

[Taylor Brandstetter, 2016/08/08 23:01:05]

nit: "needs"

[honghaiz3, 2016/08/11 22:45:53]

Done.

+  conn1->ReceivedPingResponse(LOW_RTT, "id");
+  EXPECT_EQ_WAIT(conn1, ch.selected_connection(), kDefaultTimeout);
+  EXPECT_EQ(conn1, last_selected_candidate_pair());
+  EXPECT_EQ(len, SendData(ch, data, len, ++last_packet_id));
+
   // When a higher priority candidate comes in, the new connection is chosen
   // as the selected connection.
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "2.2.2.2", 2, 10));
   Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
   ASSERT_TRUE(conn2 != nullptr);
-  EXPECT_EQ(conn2, ch.selected_connection());
+  conn2->ReceivedPingResponse(LOW_RTT, "id");
+  EXPECT_EQ_WAIT(conn2, ch.selected_connection(), kDefaultTimeout);
   EXPECT_EQ(conn2, last_selected_candidate_pair());
-  EXPECT_EQ(-1, last_sent_packet_id());
-  EXPECT_FALSE(channel_ready_to_send());
+  EXPECT_TRUE(channel_ready_to_send());
+  EXPECT_EQ(last_packet_id, last_sent_packet_id());
 
   // If a stun request with use-candidate attribute arrives, the receiving
   // connection will be set as the selected connection, even though
   // its priority is lower.
-  EXPECT_EQ(-1, SendData(ch, data, len, ++last_packet_id));
+  EXPECT_EQ(len, SendData(ch, data, len, ++last_packet_id));
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "3.3.3.3", 3, 1));
   Connection* conn3 = WaitForConnectionTo(&ch, "3.3.3.3", 3);
   ASSERT_TRUE(conn3 != nullptr);
@@ -3098,7 +3107,7 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) {
   NominateConnection(conn3);
   EXPECT_EQ(conn3, ch.selected_connection());
   EXPECT_EQ(conn3, last_selected_candidate_pair());
-  EXPECT_EQ(-1, last_sent_packet_id());
+  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
@@ -3150,9 +3159,9 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionFromUnknownAddress) {
   Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   ASSERT_TRUE(conn1 != nullptr);
   EXPECT_TRUE(port->sent_binding_response());
-  EXPECT_EQ(conn1, ch.selected_connection());
+  EXPECT_NE(conn1, ch.selected_connection());
   conn1->ReceivedPingResponse(LOW_RTT, "id");
-  EXPECT_EQ(conn1, ch.selected_connection());
+  EXPECT_EQ_WAIT(conn1, ch.selected_connection(), kDefaultTimeout);
   port->set_sent_binding_response(false);
 
   // Another connection is nominated via use_candidate.
@@ -3218,7 +3227,8 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBasedOnMediaReceived) {
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 10));
   Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   ASSERT_TRUE(conn1 != nullptr);
-  EXPECT_EQ(conn1, ch.selected_connection());
+  conn1->ReceivedPingResponse(LOW_RTT, "id");
+  EXPECT_EQ_WAIT(conn1, ch.selected_connection(), kDefaultTimeout);
 
   // If a data packet is received on conn2, the selected connection should
   // switch to conn2 because the controlled side must mirror the media path
@@ -3226,7 +3236,7 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBasedOnMediaReceived) {
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "2.2.2.2", 2, 1));
   Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
   ASSERT_TRUE(conn2 != nullptr);
-  conn2->ReceivedPing();  // Start receiving.
+  conn2->ReceivedPingResponse(LOW_RTT, "id");  // Become writable and receiving.
   conn2->OnReadPacket("ABC", 3, rtc::CreatePacketTime(0));
   EXPECT_EQ(conn2, ch.selected_connection());
   conn2->ReceivedPingResponse(LOW_RTT, "id");  // Become writable.
@@ -3308,10 +3318,10 @@ TEST_F(P2PTransportChannelPingTest,
   ch.MaybeStartGathering();
   // The connections have decreasing priority.
   Connection* conn1 =
-      CreateConnectionWithCandidate(ch, clock, "1.1.1.1", 1, 10, false);
+      CreateConnectionWithCandidate(ch, clock, "1.1.1.1", 1, 10, true);
   ASSERT_TRUE(conn1 != nullptr);
   Connection* conn2 =
-      CreateConnectionWithCandidate(ch, clock, "2.2.2.2", 2, 9, false);
+      CreateConnectionWithCandidate(ch, clock, "2.2.2.2", 2, 9, true);
   ASSERT_TRUE(conn2 != nullptr);
 
   // conn1 received data; it is the selected connection.
@@ -3326,17 +3336,11 @@ TEST_F(P2PTransportChannelPingTest,
   EXPECT_EQ(1, reset_selected_candidate_pair_switches());
   EXPECT_EQ(conn2, last_selected_candidate_pair());
 
+  // conn1 is selected because it has higher priority and also nominated.
   NominateConnection(conn1);
   EXPECT_EQ(1, reset_selected_candidate_pair_switches());
   EXPECT_EQ(conn1, last_selected_candidate_pair());
 
-  // conn2 received data more recently; it is selected now because it
-  // received data more recently.
-  SIMULATED_WAIT(false, 1, clock);
-  conn2->OnReadPacket("XYZ", 3, rtc::CreatePacketTime(0));
-  EXPECT_EQ(1, reset_selected_candidate_pair_switches());
-  EXPECT_EQ(conn2, last_selected_candidate_pair());

[Taylor Brandstetter, 2016/08/08 23:01:05]

Why were these expectations removed?

[honghaiz3, 2016/08/11 22:45:53]

I thought it is kind of redundant with the previous test because both check that
Data receiving has higher precedence than priority. 
But it has slight difference. In this test, both connections are nominated while
they are not in the previous one. 
So I put it back (with a little fix). 

-
   // Make sure sorting won't reselect candidate pair.
   SIMULATED_WAIT(false, 10, clock);
   EXPECT_EQ(0, reset_selected_candidate_pair_switches());
@@ -3353,10 +3357,10 @@ TEST_F(P2PTransportChannelPingTest,
   ch.MaybeStartGathering();
   // The connections have decreasing priority.
   Connection* conn1 =
-      CreateConnectionWithCandidate(ch, clock, "1.1.1.1", 1, 10, false);
+      CreateConnectionWithCandidate(ch, clock, "1.1.1.1", 1, 10, true);
   ASSERT_TRUE(conn1 != nullptr);
   Connection* conn2 =
-      CreateConnectionWithCandidate(ch, clock, "2.2.2.2", 2, 9, false);
+      CreateConnectionWithCandidate(ch, clock, "2.2.2.2", 2, 9, true);
   ASSERT_TRUE(conn2 != nullptr);
 
   // conn1 is the selected connection because it has a higher priority,
@@ -3419,11 +3423,11 @@ TEST_F(P2PTransportChannelPingTest,
   ASSERT_TRUE(conn2 != nullptr);
 
   NominateConnection(conn1);
-  EXPECT_EQ(1, reset_selected_candidate_pair_switches());
+  // There is no selected connection because no connection is writable.
+  EXPECT_EQ(0, reset_selected_candidate_pair_switches());
 
   // conn2 becomes writable; it is selected even though it is not nominated.
   conn2->ReceivedPingResponse(LOW_RTT, "id");
-
   EXPECT_EQ_SIMULATED_WAIT(1, reset_selected_candidate_pair_switches(),
                            kDefaultTimeout, clock);
   EXPECT_EQ_SIMULATED_WAIT(conn2, last_selected_candidate_pair(),
@@ -3482,6 +3486,7 @@ TEST_F(P2PTransportChannelPingTest, TestAddRemoteCandidateWithAddressReuse) {
 // When the current selected connection is strong, lower-priority connections
 // will be pruned. Otherwise, lower-priority connections are kept.
 TEST_F(P2PTransportChannelPingTest, TestDontPruneWhenWeak) {
+  rtc::ScopedFakeClock clock;
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("test channel", 1, &pa);
   PrepareChannel(&ch);
@@ -3490,29 +3495,29 @@ TEST_F(P2PTransportChannelPingTest, TestDontPruneWhenWeak) {
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 1));
   Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   ASSERT_TRUE(conn1 != nullptr);
-  EXPECT_EQ(conn1, ch.selected_connection());
+  EXPECT_EQ(nullptr, ch.selected_connection());
   conn1->ReceivedPingResponse(LOW_RTT, "id");  // Becomes writable and receiving
 
   // When a higher-priority, nominated candidate comes in, the connections with
   // lower-priority are pruned.
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "2.2.2.2", 2, 10));
-  Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
+  Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2, &clock);
   ASSERT_TRUE(conn2 != nullptr);
   conn2->ReceivedPingResponse(LOW_RTT, "id");  // Becomes writable and receiving
   NominateConnection(conn2);
-  EXPECT_TRUE_WAIT(conn1->pruned(), 3000);
+  EXPECT_TRUE_SIMULATED_WAIT(conn1->pruned(), 3000, clock);
 
   ch.SetIceConfig(CreateIceConfig(500, GATHER_ONCE));
   // Wait until conn2 becomes not receiving.
-  EXPECT_TRUE_WAIT(!conn2->receiving(), 3000);
+  EXPECT_TRUE_SIMULATED_WAIT(!conn2->receiving(), 3000, clock);
 
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "3.3.3.3", 3, 1));
-  Connection* conn3 = WaitForConnectionTo(&ch, "3.3.3.3", 3);
+  Connection* conn3 = WaitForConnectionTo(&ch, "3.3.3.3", 3, &clock);
   ASSERT_TRUE(conn3 != nullptr);
   // The selected connection should still be conn2. Even through conn3 has lower
   // priority and is not receiving/writable, it is not pruned because the
   // selected connection is not receiving.
-  WAIT(conn3->pruned(), 1000);
+  SIMULATED_WAIT(conn3->pruned(), 1000, clock);
   EXPECT_FALSE(conn3->pruned());
 }
 
@@ -3565,6 +3570,7 @@ TEST_F(P2PTransportChannelPingTest, TestGetState) {
 // 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) {
+  rtc::ScopedFakeClock clock;
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("test channel", 1, &pa);
   PrepareChannel(&ch);
@@ -3573,36 +3579,41 @@ TEST_F(P2PTransportChannelPingTest, TestConnectionPrunedAgain) {
   ch.SetIceConfig(config);
   ch.MaybeStartGathering();
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 100));
-  Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
+  Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1, &clock);
   ASSERT_TRUE(conn1 != nullptr);
-  EXPECT_EQ(conn1, ch.selected_connection());
+  EXPECT_EQ(nullptr, ch.selected_connection());
   conn1->ReceivedPingResponse(LOW_RTT, "id");  // Becomes writable and receiving
+  EXPECT_EQ_SIMULATED_WAIT(conn1, ch.selected_connection(), kDefaultTimeout,
+                           clock);
 
   // Add a low-priority connection |conn2|, which will be pruned, but it will
   // not be deleted right away. Once the current selected connection becomes not
   // receiving, |conn2| will start to ping and upon receiving the ping response,
   // it will become the selected connection.
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "2.2.2.2", 2, 1));
-  Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
+  Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2, &clock);
   ASSERT_TRUE(conn2 != nullptr);
-  EXPECT_TRUE_WAIT(!conn2->active(), 1000);
+  EXPECT_TRUE_SIMULATED_WAIT(!conn2->active(), kDefaultTimeout, clock);
   // |conn2| should not send a ping yet.
   EXPECT_EQ(Connection::STATE_WAITING, conn2->state());
   EXPECT_EQ(TransportChannelState::STATE_COMPLETED, ch.GetState());
   // Wait for |conn1| becoming not receiving.
-  EXPECT_TRUE_WAIT(!conn1->receiving(), 3000);
+  EXPECT_TRUE_SIMULATED_WAIT(!conn1->receiving(), 3000, clock);
   // Make sure conn2 is not deleted.
-  conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
+  conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2, &clock);
   ASSERT_TRUE(conn2 != nullptr);
-  EXPECT_EQ_WAIT(Connection::STATE_INPROGRESS, conn2->state(), 1000);
+  EXPECT_EQ_SIMULATED_WAIT(Connection::STATE_INPROGRESS, conn2->state(),
+                           kDefaultTimeout, clock);
   conn2->ReceivedPingResponse(LOW_RTT, "id");
-  EXPECT_EQ_WAIT(conn2, ch.selected_connection(), 1000);
+  EXPECT_EQ_SIMULATED_WAIT(conn2, ch.selected_connection(), kDefaultTimeout,
+                           clock);
   EXPECT_EQ(TransportChannelState::STATE_CONNECTING, ch.GetState());
 
   // When |conn1| comes back again, |conn2| will be pruned again.
   conn1->ReceivedPingResponse(LOW_RTT, "id");
-  EXPECT_EQ_WAIT(conn1, ch.selected_connection(), 1000);
-  EXPECT_TRUE_WAIT(!conn2->active(), 1000);
+  EXPECT_EQ_SIMULATED_WAIT(conn1, ch.selected_connection(), kDefaultTimeout,
+                           clock);
+  EXPECT_TRUE_SIMULATED_WAIT(!conn2->active(), kDefaultTimeout, clock);
   EXPECT_EQ(TransportChannelState::STATE_COMPLETED, ch.GetState());
 }