Start ICE connectivity checks as soon as the first pair is pingable.

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 337 matching lines @@
     channel->SignalReadPacket.connect(
         this, &P2PTransportChannelTestBase::OnReadPacket);
     channel->SignalRoleConflict.connect(
         this, &P2PTransportChannelTestBase::OnRoleConflict);
     channel->SetIceCredentials(local_ice_ufrag, local_ice_pwd);
     if (remote_ice_credential_source_ == FROM_SETICECREDENTIALS) {
       channel->SetRemoteIceCredentials(remote_ice_ufrag, remote_ice_pwd);
     }
     channel->SetIceRole(GetEndpoint(endpoint)->ice_role());
     channel->SetIceTiebreaker(GetEndpoint(endpoint)->GetIceTiebreaker());
-    channel->Connect();
     return channel;
   }
   void DestroyChannels() {
     ep1_.cd1_.ch_.reset();
     ep2_.cd1_.ch_.reset();
     ep1_.cd2_.ch_.reset();
     ep2_.cd2_.ch_.reset();
   }
   P2PTransportChannel* ep1_ch1() { return ep1_.cd1_.ch_.get(); }
   P2PTransportChannel* ep1_ch2() { return ep1_.cd2_.ch_.get(); }
   P2PTransportChannel* ep2_ch1() { return ep2_.cd1_.ch_.get(); }
   P2PTransportChannel* ep2_ch2() { return ep2_.cd2_.ch_.get(); }
 
   TestTurnServer* test_turn_server() { return &turn_server_; }
+  rtc::VirtualSocketServer* virtual_socket_server() { return vss_.get(); }
 
   // Common results.
   static const Result kLocalUdpToLocalUdp;
   static const Result kLocalUdpToStunUdp;
   static const Result kLocalUdpToPrflxUdp;
   static const Result kPrflxUdpToLocalUdp;
   static const Result kStunUdpToLocalUdp;
   static const Result kStunUdpToStunUdp;
   static const Result kPrflxUdpToStunUdp;
   static const Result kLocalUdpToRelayUdp;
@@ skipping 97 matching lines @@
     EXPECT_EQ(expected.local_type, local_type);
     EXPECT_EQ(expected.remote_type, remote_type);
     EXPECT_EQ(expected.local_proto, local_proto);
     EXPECT_EQ(expected.remote_proto, remote_proto);
   }
 
   // Return true if the approprite parts of the expected Result, based
   // on the local and remote candidate of ep2_ch1, match.  This can be
   // used in an EXPECT_TRUE_WAIT.
   bool CheckCandidate2(const Result& expected) {
-    const std::string& local_type = LocalCandidate(ep2_ch1())->type();
-    // const std::string& remote_type = RemoteCandidate(ep2_ch1())->type();
     const std::string& local_proto = LocalCandidate(ep2_ch1())->protocol();
     const std::string& remote_proto = RemoteCandidate(ep2_ch1())->protocol();
     // Removed remote_type comparision aginst selected connection remote
     // candidate. This is done to handle remote type discrepancy from
     // local to stun based on the test type.
     // For example in case of Open -> NAT, ep2 channels will have LULU
     // and in other cases like NAT -> NAT it will be LUSU. To avoid these
     // mismatches and we are doing comparision in different way.
     // i.e. when don't match its remote type is either local or stun.
+    //
+    // Update(deadbeef): Also had to remove local type comparison. There
+    // is currently an issue where the local type is not updated to stun.
+    // So one side may see local<->relay while the other sees relay<->stun.
+    // This mean the other side may prioritize prflx<->relay, and won't have
+    // a local type of relay no matter how long we wait.
+    // TODO(deadbeef): Fix this!! It's causing us to have very sparse test
+    // coverage and is a very real bug.
+    //

[pthatcher1, 2016/06/25 00:02:36]

Who is going to fix this, and how soon?

[Taylor Brandstetter, 2016/06/27 22:19:34]

Me, and this week if possible. I also plan to re-enable the 48 tests for NAT
combinations that are completely disabled.

     // TODO(ronghuawu): Refine the test criteria.
     // https://code.google.com/p/webrtc/issues/detail?id=1953
     return ((local_proto == expected.local_proto2 &&
-             remote_proto == expected.remote_proto2) &&
-            (local_type == expected.local_type2 ||
-             // Sometimes we expect local -> prflx or prflx -> local
-             // and instead get prflx -> local or local -> prflx, and
-             // that's OK.
-             (IsLocalToPrflxOrTheReverse(expected) &&
-              local_type == expected.remote_type2)));
+             remote_proto == expected.remote_proto2));
   }
 
   // EXPECT_EQ on the approprite parts of the expected Result, based
   // on the local and remote candidate of ep2_ch1.  This is like
   // CheckCandidate2, except that it will provide more detail about
   // what didn't match.
   void ExpectCandidate2(const Result& expected) {
     if (CheckCandidate2(expected)) {
       return;
     }
@@ skipping 534 matching lines @@
   FLAKY_P2P_TEST(x, NAT_PORT_RESTRICTED) \
   P2P_TEST(x, NAT_SYMMETRIC) \
   FLAKY_P2P_TEST(x, NAT_DOUBLE_CONE) \
   P2P_TEST(x, NAT_SYMMETRIC_THEN_CONE) \
   P2P_TEST(x, BLOCK_UDP) \
   P2P_TEST(x, BLOCK_UDP_AND_INCOMING_TCP) \
   P2P_TEST(x, BLOCK_ALL_BUT_OUTGOING_HTTP) \
   P2P_TEST(x, PROXY_HTTPS) \
   P2P_TEST(x, PROXY_SOCKS)
 
-#define FLAKY_P2P_TEST_SET(x) \
-  P2P_TEST(x, OPEN) \
-  P2P_TEST(x, NAT_FULL_CONE) \
-  P2P_TEST(x, NAT_ADDR_RESTRICTED) \
-  P2P_TEST(x, NAT_PORT_RESTRICTED) \
-  P2P_TEST(x, NAT_SYMMETRIC) \
-  P2P_TEST(x, NAT_DOUBLE_CONE) \
-  P2P_TEST(x, NAT_SYMMETRIC_THEN_CONE) \
-  P2P_TEST(x, BLOCK_UDP) \
-  P2P_TEST(x, BLOCK_UDP_AND_INCOMING_TCP) \
-  P2P_TEST(x, BLOCK_ALL_BUT_OUTGOING_HTTP) \
-  P2P_TEST(x, PROXY_HTTPS) \
-  P2P_TEST(x, PROXY_SOCKS)
-
 P2P_TEST_SET(OPEN)
 P2P_TEST_SET(NAT_FULL_CONE)
 P2P_TEST_SET(NAT_ADDR_RESTRICTED)
 P2P_TEST_SET(NAT_PORT_RESTRICTED)
 P2P_TEST_SET(NAT_SYMMETRIC)
 P2P_TEST_SET(NAT_DOUBLE_CONE)
 P2P_TEST_SET(NAT_SYMMETRIC_THEN_CONE)
 P2P_TEST_SET(BLOCK_UDP)
 P2P_TEST_SET(BLOCK_UDP_AND_INCOMING_TCP)
 P2P_TEST_SET(BLOCK_ALL_BUT_OUTGOING_HTTP)
@@ skipping 354 matching lines @@
     EXPECT_EQ(ICEROLE_CONTROLLING, ports_before[i]->GetIceRole());
     EXPECT_EQ(kTiebreaker1, ports_before[i]->IceTiebreaker());
   }
 
   ep1_ch1()->SetIceRole(ICEROLE_CONTROLLED);
   ep1_ch1()->SetIceTiebreaker(kTiebreaker2);
 
   const std::vector<PortInterface*> ports_after = ep1_ch1()->ports();
   for (size_t i = 0; i < ports_after.size(); ++i) {
     EXPECT_EQ(ICEROLE_CONTROLLED, ports_before[i]->GetIceRole());
-    // SetIceTiebreaker after Connect() has been called will fail. So expect the
+    // SetIceTiebreaker after ports have been created will fail. So expect the
     // 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);
 
@@ skipping 233 matching lines @@
   // Also expect that the channel instantly indicates that it's writable since
   // it has a TURN-TURN pair.
   EXPECT_TRUE(ep1_ch1()->writable());
   EXPECT_TRUE(GetEndpoint(0)->ready_to_send_);
 }
 
 // Test that a TURN/peer reflexive candidate pair is also presumed writable.
 TEST_F(P2PTransportChannelTest, TurnToPrflxPresumedWritable) {
   rtc::ScopedFakeClock fake_clock;
 
+  // We need to add artificial network delay to verify that the connection
+  // is presumed writable before it's actually writable. Without this delay
+  // it would become writable instantly.
+  virtual_socket_server()->set_delay_mean(50);
+  virtual_socket_server()->UpdateDelayDistribution();
+
   ConfigureEndpoints(NAT_SYMMETRIC, NAT_SYMMETRIC, kDefaultPortAllocatorFlags,
                      kDefaultPortAllocatorFlags);
   // We want the remote TURN candidate to show up as prflx. To do this we need
   // to configure the server to accept packets from an address we haven't
   // explicitly installed permission for.
   test_turn_server()->set_enable_permission_checks(false);
   IceConfig config;
   config.presume_writable_when_fully_relayed = true;
   GetEndpoint(0)->cd1_.ch_.reset(
       CreateChannel(0, ICE_CANDIDATE_COMPONENT_DEFAULT, kIceUfrag[0],
@@ skipping 567 matching lines @@
   int selected_candidate_pair_switches_ = 0;
   int last_sent_packet_id_ = -1;
   bool channel_ready_to_send_ = false;
   TransportChannelState channel_state_ = STATE_INIT;
 };
 
 TEST_F(P2PTransportChannelPingTest, TestTriggeredChecks) {
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("trigger checks", 1, &pa);
   PrepareChannel(&ch);
-  ch.Connect();
   ch.MaybeStartGathering();
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 1));
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "2.2.2.2", 2, 2));
 
   Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   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, FindNextPingableConnectionAndPingIt(&ch));
 
   // 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, FindNextPingableConnectionAndPingIt(&ch));
 }
 
 TEST_F(P2PTransportChannelPingTest, TestAllConnectionsPingedSufficiently) {
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("ping sufficiently", 1, &pa);
   PrepareChannel(&ch);
-  ch.Connect();
   ch.MaybeStartGathering();
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 1));
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "2.2.2.2", 2, 2));
 
   Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
   ASSERT_TRUE(conn1 != nullptr);
   ASSERT_TRUE(conn2 != nullptr);
 
   // Low-priority connection becomes writable so that the other connection
   // is not pruned.
   conn1->ReceivedPingResponse(LOW_RTT);
   EXPECT_TRUE_WAIT(
       conn1->num_pings_sent() >= MIN_PINGS_AT_WEAK_PING_INTERVAL &&
           conn2->num_pings_sent() >= MIN_PINGS_AT_WEAK_PING_INTERVAL,
       kDefaultTimeout);
 }
 
 // Verify that the connections are pinged at the right time.
 TEST_F(P2PTransportChannelPingTest, TestStunPingIntervals) {
   rtc::ScopedFakeClock clock;
   int RTT_RATIO = 4;
   int SCHEDULING_RANGE = 200;
   int RTT_RANGE = 10;
 
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("TestChannel", 1, &pa);
   PrepareChannel(&ch);
-  ch.Connect();
   ch.MaybeStartGathering();
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 1));
   Connection* conn = WaitForConnectionTo(&ch, "1.1.1.1", 1);
 
   ASSERT_TRUE(conn != nullptr);
   SIMULATED_WAIT(conn->num_pings_sent() == 1, kDefaultTimeout, clock);
 
   // Initializing.
 
   int64_t start = clock.TimeNanos();
@@ skipping 57 matching lines @@
   ping_interval_ms = (clock.TimeNanos() - start) / rtc::kNumNanosecsPerMillisec;
   EXPECT_GE(ping_interval_ms, STABILIZING_WRITABLE_CONNECTION_PING_INTERVAL);
   EXPECT_LE(ping_interval_ms,
             STABILIZING_WRITABLE_CONNECTION_PING_INTERVAL + SCHEDULING_RANGE);
 }
 
 TEST_F(P2PTransportChannelPingTest, TestNoTriggeredChecksWhenWritable) {
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("trigger checks", 1, &pa);
   PrepareChannel(&ch);
-  ch.Connect();
   ch.MaybeStartGathering();
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 1));
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "2.2.2.2", 2, 2));
 
   Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
   ASSERT_TRUE(conn1 != nullptr);
   ASSERT_TRUE(conn2 != nullptr);
 
   EXPECT_EQ(conn2, FindNextPingableConnectionAndPingIt(&ch));
   EXPECT_EQ(conn1, FindNextPingableConnectionAndPingIt(&ch));
   conn1->ReceivedPingResponse(LOW_RTT);
   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, FindNextPingableConnectionAndPingIt(&ch));
 }
 
 TEST_F(P2PTransportChannelPingTest, TestFailedConnectionNotPingable) {
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("Do not ping failed connections", 1, &pa);
   PrepareChannel(&ch);
-  ch.Connect();
   ch.MaybeStartGathering();
   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.FindNextPingableConnection());
   conn1->Prune();  // A pruned connection may still be pingable.
   EXPECT_EQ(conn1, ch.FindNextPingableConnection());
   conn1->FailAndPrune();
   EXPECT_TRUE(nullptr == ch.FindNextPingableConnection());
 }
 
 TEST_F(P2PTransportChannelPingTest, TestSignalStateChanged) {
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("state change", 1, &pa);
   PrepareChannel(&ch);
-  ch.Connect();
   ch.MaybeStartGathering();
   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);
   // Pruning the connection reduces the set of active connections and changes
   // the channel state.
   conn1->Prune();
   EXPECT_EQ_WAIT(STATE_FAILED, channel_state(), kDefaultTimeout);
 }
 
 // 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) {
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("add candidate", 1, &pa);
   PrepareChannel(&ch);
-  ch.Connect();
   ch.MaybeStartGathering();
   // Add a candidate with a future ufrag.
   ch.AddRemoteCandidate(
       CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 1, kIceUfrag[2]));
   Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   ASSERT_TRUE(conn1 != nullptr);
   const Candidate& candidate = conn1->remote_candidate();
   EXPECT_EQ(kIceUfrag[2], candidate.username());
   EXPECT_TRUE(candidate.password().empty());
   EXPECT_TRUE(FindNextPingableConnectionAndPingIt(&ch) == nullptr);
@@ skipping 32 matching lines @@
     } else if (candidate.username() == kIceUfrag[2]) {
       EXPECT_EQ(kIcePwd[2], candidate.password());
     }
   }
 }
 
 TEST_F(P2PTransportChannelPingTest, ConnectionResurrection) {
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("connection resurrection", 1, &pa);
   PrepareChannel(&ch);
-  ch.Connect();
   ch.MaybeStartGathering();
 
   // Create conn1 and keep track of original candidate priority.
   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);
   uint32_t remote_priority = conn1->remote_candidate().priority();
 
   // Create a higher priority candidate and make the connection
   // receiving/writable. This will prune conn1.
@@ skipping 39 matching lines @@
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("receiving state change", 1, &pa);
   PrepareChannel(&ch);
   // Default receiving timeout and checking receiving interval should not be too
   // small.
   EXPECT_LE(1000, ch.receiving_timeout());
   EXPECT_LE(200, ch.check_receiving_interval());
   ch.SetIceConfig(CreateIceConfig(500, false));
   EXPECT_EQ(500, ch.receiving_timeout());
   EXPECT_EQ(50, ch.check_receiving_interval());
-  ch.Connect();
   ch.MaybeStartGathering();
   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);
 
   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);
 }
 
 // The controlled side will select a connection as the "selected connection"
 // based on priority until the controlling side nominates a connection, at which
 // point the controlled side will select that connection as the
 // "selected connection". Plus, SignalSelectedCandidatePair will be fired if the
 // selected connection changes and SignalReadyToSend will be fired if the new
 // selected connection is writable.
 TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) {
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("receiving state change", 1, &pa);
   PrepareChannel(&ch);
   ch.SetIceRole(ICEROLE_CONTROLLED);
-  ch.Connect();
   ch.MaybeStartGathering();
   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());
 
@@ skipping 58 matching lines @@
 // 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 a ping response and set the ICE pwd in the remote candidate
 // appropriately.
 TEST_F(P2PTransportChannelPingTest, TestSelectConnectionFromUnknownAddress) {
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("receiving state change", 1, &pa);
   PrepareChannel(&ch);
   ch.SetIceRole(ICEROLE_CONTROLLED);
-  ch.Connect();
   ch.MaybeStartGathering();
   // A minimal STUN message with prflx priority.
   IceMessage request;
   request.SetType(STUN_BINDING_REQUEST);
   request.AddAttribute(
       new StunByteStringAttribute(STUN_ATTR_USERNAME, kIceUfrag[1]));
   uint32_t prflx_priority = ICE_TYPE_PREFERENCE_PRFLX << 24;
   request.AddAttribute(
       new StunUInt32Attribute(STUN_ATTR_PRIORITY, prflx_priority));
   TestUDPPort* port = static_cast<TestUDPPort*>(GetPort(&ch));
@@ skipping 60 matching lines @@
 
 // The controlled side will select a connection as the "selected connection"
 // based on media received until the controlling side nominates a connection,
 // at which point the controlled side will select that connection as
 // the "selected connection".
 TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBasedOnMediaReceived) {
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("receiving state change", 1, &pa);
   PrepareChannel(&ch);
   ch.SetIceRole(ICEROLE_CONTROLLED);
-  ch.Connect();
   ch.MaybeStartGathering();
   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());
 
   // If a data packet is received on conn2, the selected connection should
   // switch to conn2 because the controlled side must mirror the media path
   // chosen by the controlling side.
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "2.2.2.2", 2, 1));
@@ skipping 32 matching lines @@
 }
 
 TEST_F(P2PTransportChannelPingTest,
        TestControlledAgentDataReceivingTakesHigherPrecedenceThanPriority) {
   rtc::ScopedFakeClock clock;
 
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("SwitchSelectedConnection", 1, &pa);
   PrepareChannel(&ch);
   ch.SetIceRole(ICEROLE_CONTROLLED);
-  ch.Connect();
   ch.MaybeStartGathering();
   // The connections have decreasing priority.
   Connection* conn1 =
       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, true);
   ASSERT_TRUE(conn2 != nullptr);
 
   // Initially, connections are selected based on priority.
@@ skipping 19 matching lines @@
 }
 
 TEST_F(P2PTransportChannelPingTest,
        TestControlledAgentNominationTakesHigherPrecedenceThanDataReceiving) {
   rtc::ScopedFakeClock clock;
 
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("SwitchSelectedConnection", 1, &pa);
   PrepareChannel(&ch);
   ch.SetIceRole(ICEROLE_CONTROLLED);
-  ch.Connect();
   ch.MaybeStartGathering();
   // The connections have decreasing priority.
   Connection* conn1 =
       CreateConnectionWithCandidate(ch, clock, "1.1.1.1", 1, 10, false);
   ASSERT_TRUE(conn1 != nullptr);
   Connection* conn2 =
       CreateConnectionWithCandidate(ch, clock, "2.2.2.2", 2, 9, false);
   ASSERT_TRUE(conn2 != nullptr);
 
   // conn1 received data; it is the selected connection.
@@ skipping 25 matching lines @@
 }
 
 TEST_F(P2PTransportChannelPingTest,
        TestControlledAgentWriteStateTakesHigherPrecedenceThanNomination) {
   rtc::ScopedFakeClock clock;
 
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("SwitchSelectedConnection", 1, &pa);
   PrepareChannel(&ch);
   ch.SetIceRole(ICEROLE_CONTROLLED);
-  ch.Connect();
   ch.MaybeStartGathering();
   // The connections have decreasing priority.
   Connection* conn1 =
       CreateConnectionWithCandidate(ch, clock, "1.1.1.1", 1, 10, false);
   ASSERT_TRUE(conn1 != nullptr);
   Connection* conn2 =
       CreateConnectionWithCandidate(ch, clock, "2.2.2.2", 2, 9, false);
   ASSERT_TRUE(conn2 != nullptr);
 
   NominateConnection(conn1);
@@ skipping 18 matching lines @@
   SIMULATED_WAIT(false, 10, clock);
   EXPECT_EQ(0, get_and_reset_selected_candidate_pair_switches());
 }
 
 // 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) {
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("candidate reuse", 1, &pa);
   PrepareChannel(&ch);
-  ch.Connect();
   ch.MaybeStartGathering();
   const std::string host_address = "1.1.1.1";
   const int port_num = 1;
 
   // kIceUfrag[1] is the current generation ufrag.
   Candidate candidate = CreateUdpCandidate(LOCAL_PORT_TYPE, host_address,
                                            port_num, 1, kIceUfrag[1]);
   ch.AddRemoteCandidate(candidate);
   Connection* conn1 = WaitForConnectionTo(&ch, host_address, port_num);
   ASSERT_TRUE(conn1 != nullptr);
@@ skipping 19 matching lines @@
   EXPECT_TRUE(conn2->last_ping_received() > 0);
 }
 
 // When the current selected connection is strong, lower-priority connections
 // will be pruned. Otherwise, lower-priority connections are kept.
 TEST_F(P2PTransportChannelPingTest, TestDontPruneWhenWeak) {
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("test channel", 1, &pa);
   PrepareChannel(&ch);
   ch.SetIceRole(ICEROLE_CONTROLLED);
-  ch.Connect();
   ch.MaybeStartGathering();
   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());
   conn1->ReceivedPingResponse(LOW_RTT);  // 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));
@@ skipping 17 matching lines @@
   WAIT(conn3->pruned(), 1000);
   EXPECT_FALSE(conn3->pruned());
 }
 
 TEST_F(P2PTransportChannelPingTest, TestDontPruneHighPriorityConnections) {
   rtc::ScopedFakeClock clock;
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("test channel", 1, &pa);
   PrepareChannel(&ch);
   ch.SetIceRole(ICEROLE_CONTROLLED);
-  ch.Connect();
   ch.MaybeStartGathering();
   Connection* conn1 =
       CreateConnectionWithCandidate(ch, clock, "1.1.1.1", 1, 100, true);
   ASSERT_TRUE(conn1 != nullptr);
   Connection* conn2 =
       CreateConnectionWithCandidate(ch, clock, "2.2.2.2", 2, 200, false);
   ASSERT_TRUE(conn2 != nullptr);
   // Even if conn1 is writable, nominated, receiving data, it should not prune
   // conn2.
   NominateConnection(conn1);
   SIMULATED_WAIT(false, 1, clock);
   conn1->OnReadPacket("XYZ", 3, rtc::CreatePacketTime(0));
   SIMULATED_WAIT(conn2->pruned(), 100, clock);
   EXPECT_FALSE(conn2->pruned());
 }
 
 // Test that GetState returns the state correctly.
 TEST_F(P2PTransportChannelPingTest, TestGetState) {
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("test channel", 1, &pa);
   PrepareChannel(&ch);
-  ch.Connect();
   ch.MaybeStartGathering();
   EXPECT_EQ(TransportChannelState::STATE_INIT, ch.GetState());
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 100));
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "2.2.2.2", 2, 1));
   Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   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(TransportChannelState::STATE_CONNECTING, ch.GetState());
   // |conn1| becomes writable and receiving; it then should prune |conn2|.
   conn1->ReceivedPingResponse(LOW_RTT);
   EXPECT_TRUE_WAIT(conn2->pruned(), 1000);
   EXPECT_EQ(TransportChannelState::STATE_COMPLETED, ch.GetState());
   conn1->Prune();  // All connections are pruned.
   // Need to wait until the channel state is updated.
   EXPECT_EQ_WAIT(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) {
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("test channel", 1, &pa);
   PrepareChannel(&ch);
   ch.SetIceConfig(CreateIceConfig(1000, false));
-  ch.Connect();
   ch.MaybeStartGathering();
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 100));
   Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   ASSERT_TRUE(conn1 != nullptr);
   EXPECT_EQ(conn1, ch.selected_connection());
   conn1->ReceivedPingResponse(LOW_RTT);  // 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 selected connection becomes not
   // receiving, |conn2| will start to ping and upon receiving the ping response,
@@ skipping 21 matching lines @@
   EXPECT_TRUE_WAIT(!conn2->active(), 1000);
   EXPECT_EQ(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) {
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("test channel", 1, &pa);
   PrepareChannel(&ch);
-  ch.Connect();
   ch.MaybeStartGathering();
   // Have one connection only but later becomes write-time-out.
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 100));
   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.
@@ skipping 13 matching lines @@
 
 // Tests that after a port allocator session is started, it will be stopped
 // when a new connection becomes writable and receiving. Also tests that if a
 // connection belonging to an old session becomes writable, it won't stop
 // the current port allocator session.
 TEST_F(P2PTransportChannelPingTest, TestStopPortAllocatorSessions) {
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("test channel", 1, &pa);
   PrepareChannel(&ch);
   ch.SetIceConfig(CreateIceConfig(2000, false));
-  ch.Connect();
   ch.MaybeStartGathering();
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 100));
   Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   ASSERT_TRUE(conn1 != nullptr);
   conn1->ReceivedPingResponse(LOW_RTT);  // Becomes writable and receiving
   EXPECT_TRUE(!ch.allocator_session()->IsGettingPorts());
 
   // Start a new session. Even though conn1, which belongs to an older
   // session, becomes unwritable and writable again, it should not stop the
   // current session.
@@ skipping 17 matching lines @@
 // a correct role, in case the network becomes active before the connection is
 // destroyed.
 TEST_F(P2PTransportChannelPingTest,
        TestIceRoleUpdatedOnPortAfterSignalNetworkInactive) {
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("test channel", ICE_CANDIDATE_COMPONENT_DEFAULT, &pa);
   // Starts with ICEROLE_CONTROLLING.
   PrepareChannel(&ch);
   IceConfig config = CreateIceConfig(1000, true);
   ch.SetIceConfig(config);
-  ch.Connect();
   ch.MaybeStartGathering();
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 1));
 
   Connection* conn = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   ASSERT_TRUE(conn != nullptr);
 
   // Make the fake port signal that its network is inactive, then change the
   // ICE role and expect it to be updated.
   conn->port()->SignalNetworkInactive(conn->port());
   ch.SetIceRole(ICEROLE_CONTROLLED);
   EXPECT_EQ(ICEROLE_CONTROLLED, conn->port()->GetIceRole());
 }
 
 // Test that the ICE role is updated even on ports with inactive networks.
 // These ports may still have connections that need a correct role, for the
 // pings sent by those connections until they're replaced by newer-generation
 // connections.
 TEST_F(P2PTransportChannelPingTest, TestIceRoleUpdatedOnPortAfterIceRestart) {
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("test channel", ICE_CANDIDATE_COMPONENT_DEFAULT, &pa);
   // Starts with ICEROLE_CONTROLLING.
   PrepareChannel(&ch);
-  ch.Connect();
   ch.MaybeStartGathering();
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 1));
 
   Connection* conn = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   ASSERT_TRUE(conn != nullptr);
 
   // Do an ICE restart, change the role, and expect the old port to have its
   // role updated.
   ch.SetIceCredentials(kIceUfrag[1], kIcePwd[1]);
   ch.MaybeStartGathering();
   ch.SetIceRole(ICEROLE_CONTROLLED);
   EXPECT_EQ(ICEROLE_CONTROLLED, conn->port()->GetIceRole());
 }
 
 // Test that after some amount of time without receiving data, the connection
 // and port are destroyed.
 TEST_F(P2PTransportChannelPingTest, TestPortDestroyedAfterTimeout) {
   rtc::ScopedFakeClock fake_clock;
 
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("test channel", ICE_CANDIDATE_COMPONENT_DEFAULT, &pa);
   PrepareChannel(&ch);
   // Only a controlled channel should expect its ports to be destroyed.
   ch.SetIceRole(ICEROLE_CONTROLLED);
-  ch.Connect();
   ch.MaybeStartGathering();
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 1));
 
   Connection* conn = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   ASSERT_TRUE(conn != nullptr);
 
   // Simulate 2 minutes going by. This should be enough time for the port to
   // time out.
   for (int second = 0; second < 120; ++second) {
     fake_clock.AdvanceTime(rtc::TimeDelta::FromSeconds(1));
@@ skipping 24 matching lines @@
       bool prioritize_most_likely_to_work,
       int stable_writable_connection_ping_interval) {
     channel_.reset(new P2PTransportChannel("checks", 1, nullptr, allocator()));
     IceConfig config = channel_->config();
     config.prioritize_most_likely_candidate_pairs =
         prioritize_most_likely_to_work;
     config.stable_writable_connection_ping_interval =
         stable_writable_connection_ping_interval;
     channel_->SetIceConfig(config);
     PrepareChannel(channel_.get());
-    channel_->Connect();
     channel_->MaybeStartGathering();
     return *channel_.get();
   }
 
   BasicPortAllocator* allocator() { return allocator_.get(); }
   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.
@@ skipping 163 matching lines @@
 
   // TCP Relay/Relay is the next.
   VerifyNextPingableConnection(RELAY_PORT_TYPE, RELAY_PORT_TYPE,
                                TCP_PROTOCOL_NAME);
 
   // Finally, Local/Relay will be pinged.
   VerifyNextPingableConnection(LOCAL_PORT_TYPE, RELAY_PORT_TYPE);
 }
 
 }  // namespace cricket {