Reland "Surface ICE candidates that match an updated candidate filter."
This is a reland of cd8d1cf68e4eeed71fba51c97006a91bfd41813d
Original change's description:
> Surface ICE candidates that match an updated candidate filter.
>
> After this change an ICE agent can surface candidates that do not match
> the previous filter but are allowed by the updated one. The candidate
> filter, as part of the internal implementation in the ICE transport,
> manifests the RTCIceTransportPolicy field in RTCConfiguration.
>
> This new feature would allow an ICE agent to gather new candidates when
> the transport policy changes from e.g. 'relay' to 'all' without an ICE
> restart.
>
> A caveat in the current implementation remains, and a candidate can
> surface multiple times if the transport policy, or the candidate filter
> directly, performs multiple transitions from a value that disallows to
> one that allows the underlying candidate type. For example, if the
> transport policy is updated by 'all' -> 'relay' -> 'all', the same host
> candidate can surface after the second update.
>
>
> Bug: webrtc:8939
> Change-Id: I92c2e07dafab225c702c5de28f47958a0d3270cc
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/132282
> Commit-Queue: Qingsi Wang <qingsi@webrtc.org>
> Reviewed-by: Jeroen de Borst <jeroendb@webrtc.org>
> Reviewed-by: Seth Hampson <shampson@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#27674}
Bug: webrtc:8939
Change-Id: I9c32b1ea05028ecd937ab4912779dd958faf734f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/133582
Reviewed-by: Seth Hampson <shampson@webrtc.org>
Reviewed-by: Jeroen de Borst <jeroendb@webrtc.org>
Commit-Queue: Qingsi Wang <qingsi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27694}diff --git a/p2p/client/basic_port_allocator_unittest.cc b/p2p/client/basic_port_allocator_unittest.cc
index 1682e9d..067e757 100644
--- a/p2p/client/basic_port_allocator_unittest.cc
+++ b/p2p/client/basic_port_allocator_unittest.cc
@@ -1266,7 +1266,7 @@
TEST_F(BasicPortAllocatorTest,
TestDisableAdapterEnumerationWithoutNatRelayTransportOnly) {
ResetWithStunServerNoNat(kStunAddr);
- allocator().set_candidate_filter(CF_RELAY);
+ allocator().SetCandidateFilter(CF_RELAY);
// Expect to see no ports and no candidates.
CheckDisableAdapterEnumeration(0U, rtc::IPAddress(), rtc::IPAddress(),
rtc::IPAddress(), rtc::IPAddress());
@@ -1527,7 +1527,7 @@
AddInterface(kClientAddr);
ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
// Set candidate filter *after* creating the session. Should have no effect.
- allocator().set_candidate_filter(CF_RELAY);
+ allocator().SetCandidateFilter(CF_RELAY);
session_->StartGettingPorts();
// 7 candidates and 4 ports is what we would normally get (see the
// TestGetAllPorts* tests).
@@ -1546,7 +1546,7 @@
AddInterface(kClientAddr);
// GTURN is not configured here.
ResetWithTurnServersNoNat(kTurnUdpIntAddr, rtc::SocketAddress());
- allocator().set_candidate_filter(CF_RELAY);
+ allocator().SetCandidateFilter(CF_RELAY);
ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
session_->StartGettingPorts();
EXPECT_TRUE_SIMULATED_WAIT(candidate_allocation_done_,
@@ -1565,7 +1565,7 @@
TEST_F(BasicPortAllocatorTest, TestCandidateFilterWithHostOnly) {
AddInterface(kClientAddr);
allocator().set_flags(PORTALLOCATOR_ENABLE_SHARED_SOCKET);
- allocator().set_candidate_filter(CF_HOST);
+ allocator().SetCandidateFilter(CF_HOST);
ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
session_->StartGettingPorts();
EXPECT_TRUE_SIMULATED_WAIT(candidate_allocation_done_,
@@ -1583,7 +1583,7 @@
ResetWithStunServerAndNat(kStunAddr);
allocator().set_flags(PORTALLOCATOR_ENABLE_SHARED_SOCKET);
- allocator().set_candidate_filter(CF_REFLEXIVE);
+ allocator().SetCandidateFilter(CF_REFLEXIVE);
ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
session_->StartGettingPorts();
EXPECT_TRUE_SIMULATED_WAIT(candidate_allocation_done_,
@@ -1602,7 +1602,7 @@
TEST_F(BasicPortAllocatorTest, TestCandidateFilterWithReflexiveOnlyAndNoNAT) {
AddInterface(kClientAddr);
allocator().set_flags(PORTALLOCATOR_ENABLE_SHARED_SOCKET);
- allocator().set_candidate_filter(CF_REFLEXIVE);
+ allocator().SetCandidateFilter(CF_REFLEXIVE);
ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
session_->StartGettingPorts();
EXPECT_TRUE_SIMULATED_WAIT(candidate_allocation_done_,
@@ -2133,7 +2133,7 @@
kDefaultAllocationTimeout, fake_clock);
size_t initial_candidates_size = peeked_session->ReadyCandidates().size();
size_t initial_ports_size = peeked_session->ReadyPorts().size();
- allocator_->set_candidate_filter(CF_RELAY);
+ allocator_->SetCandidateFilter(CF_RELAY);
// Assume that when TakePooledSession is called, the candidate filter will be
// applied to the pooled session. This is tested by PortAllocatorTest.
session_ =
@@ -2157,6 +2157,145 @@
}
}
+// Test that candidates that do not match a previous candidate filter can be
+// surfaced if they match the new one after setting the filter value.
+TEST_F(BasicPortAllocatorTest,
+ SurfaceNewCandidatesAfterSetCandidateFilterToAddCandidateTypes) {
+ // We would still surface a host candidate if the IP is public, even though it
+ // is disabled by the candidate filter. See
+ // BasicPortAllocatorSession::CheckCandidateFilter. Use the private address so
+ // that the srflx candidate is not equivalent to the host candidate.
+ AddInterface(kPrivateAddr);
+ ResetWithStunServerAndNat(kStunAddr);
+
+ AddTurnServers(kTurnUdpIntAddr, rtc::SocketAddress());
+
+ allocator_->set_flags(allocator().flags() |
+ PORTALLOCATOR_ENABLE_SHARED_SOCKET |
+ PORTALLOCATOR_DISABLE_TCP);
+
+ allocator_->SetCandidateFilter(CF_NONE);
+ ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
+ session_->StartGettingPorts();
+ EXPECT_TRUE_SIMULATED_WAIT(candidate_allocation_done_,
+ kDefaultAllocationTimeout, fake_clock);
+ EXPECT_TRUE(candidates_.empty());
+ EXPECT_TRUE(ports_.empty());
+
+ // Surface the relay candidate previously gathered but not signaled.
+ session_->SetCandidateFilter(CF_RELAY);
+ ASSERT_EQ_SIMULATED_WAIT(1u, candidates_.size(), kDefaultAllocationTimeout,
+ fake_clock);
+ EXPECT_EQ(RELAY_PORT_TYPE, candidates_.back().type());
+ EXPECT_EQ(1u, ports_.size());
+
+ // Surface the srflx candidate previously gathered but not signaled.
+ session_->SetCandidateFilter(CF_RELAY | CF_REFLEXIVE);
+ ASSERT_EQ_SIMULATED_WAIT(2u, candidates_.size(), kDefaultAllocationTimeout,
+ fake_clock);
+ EXPECT_EQ(STUN_PORT_TYPE, candidates_.back().type());
+ EXPECT_EQ(2u, ports_.size());
+
+ // Surface the srflx candidate previously gathered but not signaled.
+ session_->SetCandidateFilter(CF_ALL);
+ ASSERT_EQ_SIMULATED_WAIT(3u, candidates_.size(), kDefaultAllocationTimeout,
+ fake_clock);
+ EXPECT_EQ(LOCAL_PORT_TYPE, candidates_.back().type());
+ EXPECT_EQ(2u, ports_.size());
+}
+
+// This is a similar test as
+// SurfaceNewCandidatesAfterSetCandidateFilterToAddCandidateTypes, and we
+// test the transitions for which the new filter value is not a super set of the
+// previous value.
+TEST_F(
+ BasicPortAllocatorTest,
+ SurfaceNewCandidatesAfterSetCandidateFilterToAllowDifferentCandidateTypes) {
+ // We would still surface a host candidate if the IP is public, even though it
+ // is disabled by the candidate filter. See
+ // BasicPortAllocatorSession::CheckCandidateFilter. Use the private address so
+ // that the srflx candidate is not equivalent to the host candidate.
+ AddInterface(kPrivateAddr);
+ ResetWithStunServerAndNat(kStunAddr);
+
+ AddTurnServers(kTurnUdpIntAddr, rtc::SocketAddress());
+
+ allocator_->set_flags(allocator().flags() |
+ PORTALLOCATOR_ENABLE_SHARED_SOCKET |
+ PORTALLOCATOR_DISABLE_TCP);
+
+ allocator_->SetCandidateFilter(CF_NONE);
+ ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
+ session_->StartGettingPorts();
+ EXPECT_TRUE_SIMULATED_WAIT(candidate_allocation_done_,
+ kDefaultAllocationTimeout, fake_clock);
+ EXPECT_TRUE(candidates_.empty());
+ EXPECT_TRUE(ports_.empty());
+
+ // Surface the relay candidate previously gathered but not signaled.
+ session_->SetCandidateFilter(CF_RELAY);
+ EXPECT_EQ_SIMULATED_WAIT(1u, candidates_.size(), kDefaultAllocationTimeout,
+ fake_clock);
+ EXPECT_EQ(RELAY_PORT_TYPE, candidates_.back().type());
+ EXPECT_EQ(1u, ports_.size());
+
+ // Surface the srflx candidate previously gathered but not signaled.
+ session_->SetCandidateFilter(CF_REFLEXIVE);
+ EXPECT_EQ_SIMULATED_WAIT(2u, candidates_.size(), kDefaultAllocationTimeout,
+ fake_clock);
+ EXPECT_EQ(STUN_PORT_TYPE, candidates_.back().type());
+ EXPECT_EQ(2u, ports_.size());
+
+ // Surface the host candidate previously gathered but not signaled.
+ session_->SetCandidateFilter(CF_HOST);
+ EXPECT_EQ_SIMULATED_WAIT(3u, candidates_.size(), kDefaultAllocationTimeout,
+ fake_clock);
+ EXPECT_EQ(LOCAL_PORT_TYPE, candidates_.back().type());
+ // We use a shared socket and cricket::UDPPort handles the srflx candidate.
+ EXPECT_EQ(2u, ports_.size());
+}
+
+// Test that after an allocation session has stopped getting ports, changing the
+// candidate filter to allow new types of gathered candidates does not surface
+// any candidate.
+TEST_F(BasicPortAllocatorTest,
+ NoCandidateSurfacedWhenUpdatingCandidateFilterIfSessionStopped) {
+ AddInterface(kPrivateAddr);
+ ResetWithStunServerAndNat(kStunAddr);
+
+ AddTurnServers(kTurnUdpIntAddr, rtc::SocketAddress());
+
+ allocator_->set_flags(allocator().flags() |
+ PORTALLOCATOR_ENABLE_SHARED_SOCKET |
+ PORTALLOCATOR_DISABLE_TCP);
+
+ allocator_->SetCandidateFilter(CF_NONE);
+ ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
+ session_->StartGettingPorts();
+ EXPECT_TRUE_SIMULATED_WAIT(candidate_allocation_done_,
+ kDefaultAllocationTimeout, fake_clock);
+ auto test_invariants = [this]() {
+ EXPECT_TRUE(candidates_.empty());
+ EXPECT_TRUE(ports_.empty());
+ };
+
+ test_invariants();
+
+ session_->StopGettingPorts();
+
+ session_->SetCandidateFilter(CF_RELAY);
+ SIMULATED_WAIT(false, kDefaultAllocationTimeout, fake_clock);
+ test_invariants();
+
+ session_->SetCandidateFilter(CF_RELAY | CF_REFLEXIVE);
+ SIMULATED_WAIT(false, kDefaultAllocationTimeout, fake_clock);
+ test_invariants();
+
+ session_->SetCandidateFilter(CF_ALL);
+ SIMULATED_WAIT(false, kDefaultAllocationTimeout, fake_clock);
+ test_invariants();
+}
+
TEST_F(BasicPortAllocatorTest, SetStunKeepaliveIntervalForPorts) {
const int pool_size = 1;
const int expected_stun_keepalive_interval = 123;
