diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc index 9bf0b23db6..998f4a9f6f 100644 --- a/p2p/base/p2p_transport_channel.cc +++ b/p2p/base/p2p_transport_channel.cc @@ -703,7 +703,10 @@ void P2PTransportChannel::SetIceConfig(const IceConfig& config) { "send_ping_on_nomination_ice_controlled", &field_trials_.send_ping_on_nomination_ice_controlled, // Allow connections to live untouched longer that 30s. - "dead_connection_timeout_ms", &field_trials_.dead_connection_timeout_ms) + "dead_connection_timeout_ms", &field_trials_.dead_connection_timeout_ms, + // Stop gathering on strongly connected. + "stop_gather_on_strongly_connected", + &field_trials_.stop_gather_on_strongly_connected) ->Parse(webrtc::field_trial::FindFullName("WebRTC-IceFieldTrials")); if (field_trials_.dead_connection_timeout_ms < 30000) { @@ -2015,11 +2018,13 @@ void P2PTransportChannel::OnConnectionStateChange(Connection* connection) { // the connection is at the latest generation. It is not enough to check // that the connection becomes weakly connected because the connection may be // changing from (writable, receiving) to (writable, not receiving). - bool strongly_connected = !connection->weak(); - bool latest_generation = connection->local_candidate().generation() >= - allocator_session()->generation(); - if (strongly_connected && latest_generation) { - MaybeStopPortAllocatorSessions(); + if (field_trials_.stop_gather_on_strongly_connected) { + bool strongly_connected = !connection->weak(); + bool latest_generation = connection->local_candidate().generation() >= + allocator_session()->generation(); + if (strongly_connected && latest_generation) { + MaybeStopPortAllocatorSessions(); + } } // We have to unroll the stack before doing this because we may be changing // the state of connections while sorting. diff --git a/p2p/base/p2p_transport_channel_ice_field_trials.h b/p2p/base/p2p_transport_channel_ice_field_trials.h index 00e1151ba3..82dc580c1e 100644 --- a/p2p/base/p2p_transport_channel_ice_field_trials.h +++ b/p2p/base/p2p_transport_channel_ice_field_trials.h @@ -58,6 +58,9 @@ struct IceFieldTrials { // The timeout after which the connection will be considered dead if no // traffic is received. int dead_connection_timeout_ms = 30000; + + // Stop gathering when having a strong connection. + bool stop_gather_on_strongly_connected = true; }; } // namespace cricket diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc index 0e3a009bca..2af2a7412e 100644 --- a/p2p/base/p2p_transport_channel_unittest.cc +++ b/p2p/base/p2p_transport_channel_unittest.cc @@ -5978,4 +5978,134 @@ TEST_F(P2PTransportChannelTest, EnableDnsLookupsWithTransportPolicyNoHost) { DestroyChannels(); } +class GatherAfterConnectedTest : public P2PTransportChannelTest, + public ::testing::WithParamInterface {}; + +TEST_P(GatherAfterConnectedTest, GatherAfterConnected) { + const bool stop_gather_on_strongly_connected = GetParam(); + const std::string field_trial = + std::string("WebRTC-IceFieldTrials/stop_gather_on_strongly_connected:") + + (stop_gather_on_strongly_connected ? "true/" : "false/"); + webrtc::test::ScopedFieldTrials field_trials(field_trial); + + rtc::ScopedFakeClock clock; + // Use local + relay + constexpr uint32_t flags = + kDefaultPortAllocatorFlags | PORTALLOCATOR_ENABLE_SHARED_SOCKET | + PORTALLOCATOR_DISABLE_STUN | PORTALLOCATOR_DISABLE_TCP; + ConfigureEndpoints(OPEN, OPEN, flags, flags); + auto* ep1 = GetEndpoint(0); + auto* ep2 = GetEndpoint(1); + ep1->allocator_->SetCandidateFilter(CF_ALL); + ep2->allocator_->SetCandidateFilter(CF_ALL); + + // Use step delay 3s which is long enough for + // connection to be established before managing to gather relay candidates. + int delay = 3000; + SetAllocationStepDelay(0, delay); + SetAllocationStepDelay(1, delay); + IceConfig ice_config = CreateIceConfig(1000, GATHER_CONTINUALLY); + CreateChannels(ice_config, ice_config); + + PauseCandidates(0); + PauseCandidates(1); + + // We have gathered host candidates but not relay. + ASSERT_TRUE_SIMULATED_WAIT(ep1->saved_candidates_.size() == 1u && + ep2->saved_candidates_.size() == 1u, + kDefaultTimeout, clock); + + ResumeCandidates(0); + ResumeCandidates(1); + + PauseCandidates(0); + PauseCandidates(1); + + ASSERT_TRUE_SIMULATED_WAIT(ep1_ch1()->remote_candidates().size() == 1 && + ep2_ch1()->remote_candidates().size() == 1, + kDefaultTimeout, clock); + + ASSERT_TRUE_SIMULATED_WAIT( + ep1_ch1()->selected_connection() && ep2_ch1()->selected_connection(), + kDefaultTimeout, clock); + + clock.AdvanceTime(webrtc::TimeDelta::Millis(10 * delay)); + + if (stop_gather_on_strongly_connected) { + // The relay candiates gathered has not been propagated to channel. + EXPECT_EQ(ep1->saved_candidates_.size(), 0u); + EXPECT_EQ(ep2->saved_candidates_.size(), 0u); + } else { + // The relay candiates gathered has been propagated to channel. + EXPECT_EQ(ep1->saved_candidates_.size(), 1u); + EXPECT_EQ(ep2->saved_candidates_.size(), 1u); + } +} + +TEST_P(GatherAfterConnectedTest, GatherAfterConnectedMultiHomed) { + const bool stop_gather_on_strongly_connected = GetParam(); + const std::string field_trial = + std::string("WebRTC-IceFieldTrials/stop_gather_on_strongly_connected:") + + (stop_gather_on_strongly_connected ? "true/" : "false/"); + webrtc::test::ScopedFieldTrials field_trials(field_trial); + + rtc::ScopedFakeClock clock; + // Use local + relay + constexpr uint32_t flags = + kDefaultPortAllocatorFlags | PORTALLOCATOR_ENABLE_SHARED_SOCKET | + PORTALLOCATOR_DISABLE_STUN | PORTALLOCATOR_DISABLE_TCP; + AddAddress(0, kAlternateAddrs[0]); + ConfigureEndpoints(OPEN, OPEN, flags, flags); + auto* ep1 = GetEndpoint(0); + auto* ep2 = GetEndpoint(1); + ep1->allocator_->SetCandidateFilter(CF_ALL); + ep2->allocator_->SetCandidateFilter(CF_ALL); + + // Use step delay 3s which is long enough for + // connection to be established before managing to gather relay candidates. + int delay = 3000; + SetAllocationStepDelay(0, delay); + SetAllocationStepDelay(1, delay); + IceConfig ice_config = CreateIceConfig(1000, GATHER_CONTINUALLY); + CreateChannels(ice_config, ice_config); + + PauseCandidates(0); + PauseCandidates(1); + + // We have gathered host candidates but not relay. + ASSERT_TRUE_SIMULATED_WAIT(ep1->saved_candidates_.size() == 2u && + ep2->saved_candidates_.size() == 1u, + kDefaultTimeout, clock); + + ResumeCandidates(0); + ResumeCandidates(1); + + PauseCandidates(0); + PauseCandidates(1); + + ASSERT_TRUE_SIMULATED_WAIT(ep1_ch1()->remote_candidates().size() == 1 && + ep2_ch1()->remote_candidates().size() == 2, + kDefaultTimeout, clock); + + ASSERT_TRUE_SIMULATED_WAIT( + ep1_ch1()->selected_connection() && ep2_ch1()->selected_connection(), + kDefaultTimeout, clock); + + clock.AdvanceTime(webrtc::TimeDelta::Millis(10 * delay)); + + if (stop_gather_on_strongly_connected) { + // The relay candiates gathered has not been propagated to channel. + EXPECT_EQ(ep1->saved_candidates_.size(), 0u); + EXPECT_EQ(ep2->saved_candidates_.size(), 0u); + } else { + // The relay candiates gathered has been propagated. + EXPECT_EQ(ep1->saved_candidates_.size(), 2u); + EXPECT_EQ(ep2->saved_candidates_.size(), 1u); + } +} + +INSTANTIATE_TEST_SUITE_P(GatherAfterConnectedTest, + GatherAfterConnectedTest, + ::testing::Values(true, false)); + } // namespace cricket