From e309651f3330bc56ee0e504d833e639a613d725d Mon Sep 17 00:00:00 2001 From: Jonas Oreland Date: Wed, 27 May 2020 09:01:05 +0200 Subject: Don't SetNeedsIceRestartFlag if widening candidate filter when surface_ice_candidates_on_ice_transport_type_changed This patch fixes a minor bug in the implementation of surface_ice_candidates_on_ice_transport_type_changed. The existing implementation correctly handles the surfacing, but accidentally also set the SetNeedsIceRestartFlag, which made _next_ offer contain a ice restart. Modified existing testcase to verify this. Bug: webrtc:8939 Change-Id: If566e3249296467668627e5941495f6036cbd903 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/176127 Commit-Queue: Jonas Oreland Reviewed-by: Harald Alvestrand Cr-Commit-Position: refs/heads/master@{#31363} --- p2p/base/p2p_transport_channel_unittest.cc | 70 ++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) (limited to 'p2p') diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc index a3cf9370e2..ea8d66f890 100644 --- a/p2p/base/p2p_transport_channel_unittest.cc +++ b/p2p/base/p2p_transport_channel_unittest.cc @@ -5546,6 +5546,76 @@ TEST_F(P2PTransportChannelTest, DestroyChannels(); } +// Verify that things break unless +// - both parties use the surface_ice_candidates_on_ice_transport_type_changed +// - both parties loosen candidate filter at the same time (approx.). +// +// i.e surface_ice_candidates_on_ice_transport_type_changed requires +// coordination outside of webrtc to function properly. +TEST_F(P2PTransportChannelTest, SurfaceRequiresCoordination) { + webrtc::test::ScopedFieldTrials field_trials( + "WebRTC-IceFieldTrials/skip_relay_to_non_relay_connections:true/"); + rtc::ScopedFakeClock clock; + + ConfigureEndpoints( + OPEN, OPEN, + kDefaultPortAllocatorFlags | PORTALLOCATOR_ENABLE_SHARED_SOCKET, + kDefaultPortAllocatorFlags | PORTALLOCATOR_ENABLE_SHARED_SOCKET); + auto* ep1 = GetEndpoint(0); + auto* ep2 = GetEndpoint(1); + ep1->allocator_->SetCandidateFilter(CF_RELAY); + ep2->allocator_->SetCandidateFilter(CF_ALL); + // Enable continual gathering and also resurfacing gathered candidates upon + // the candidate filter changed in the ICE configuration. + IceConfig ice_config = CreateIceConfig(1000, GATHER_CONTINUALLY); + ice_config.surface_ice_candidates_on_ice_transport_type_changed = true; + // Pause candidates gathering so we can gather all types of candidates. See + // P2PTransportChannel::OnConnectionStateChange, where we would stop the + // gathering when we have a strongly connected candidate pair. + PauseCandidates(0); + PauseCandidates(1); + CreateChannels(ice_config, ice_config); + + // On the caller we only have relay, + // on the callee we have host, srflx and relay. + EXPECT_TRUE_SIMULATED_WAIT(ep1->saved_candidates_.size() == 1u, + kDefaultTimeout, clock); + EXPECT_TRUE_SIMULATED_WAIT(ep2->saved_candidates_.size() == 3u, + kDefaultTimeout, clock); + + ResumeCandidates(0); + ResumeCandidates(1); + ASSERT_TRUE_SIMULATED_WAIT( + ep1_ch1()->selected_connection() != nullptr && + RELAY_PORT_TYPE == + ep1_ch1()->selected_connection()->local_candidate().type() && + ep2_ch1()->selected_connection() != nullptr && + RELAY_PORT_TYPE == + ep1_ch1()->selected_connection()->remote_candidate().type(), + kDefaultTimeout, clock); + ASSERT_TRUE_SIMULATED_WAIT(ep2_ch1()->selected_connection() != nullptr, + kDefaultTimeout, clock); + + // Wait until the callee discards it's candidates + // since they don't manage to connect. + SIMULATED_WAIT(false, 300000, clock); + + // And then loosen caller candidate filter. + ep1->allocator_->SetCandidateFilter(CF_ALL); + + SIMULATED_WAIT(false, kDefaultTimeout, clock); + + // No p2p connection will be made, it will remain on relay. + EXPECT_TRUE(ep1_ch1()->selected_connection() != nullptr && + RELAY_PORT_TYPE == + ep1_ch1()->selected_connection()->local_candidate().type() && + ep2_ch1()->selected_connection() != nullptr && + RELAY_PORT_TYPE == + ep1_ch1()->selected_connection()->remote_candidate().type()); + + DestroyChannels(); +} + TEST_F(P2PTransportChannelPingTest, TestInitialSelectDampening0) { webrtc::test::ScopedFieldTrials field_trials( "WebRTC-IceFieldTrials/initial_select_dampening:0/"); -- cgit v1.2.3