Change the gating of surfacing candidates on ICE transport type change
from a field trial to RTCConfiguration.
The test coverage is also expanded for the underlying feature.
Bug: None
Change-Id: Ic9c1362867e4a956c5453be7a9355083b6a442f5
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/138980
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Alex Glaznev <glaznev@webrtc.org>
Reviewed-by: Kári Helgason <kthelgason@webrtc.org>
Reviewed-by: Amit Hilbuch <amithi@webrtc.org>
Commit-Queue: Qingsi Wang <qingsi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28143}
diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h
index 1994505..a9d9785 100644
--- a/api/peer_connection_interface.h
+++ b/api/peer_connection_interface.h
@@ -506,6 +506,17 @@
// re-determining was removed in ICEbis (ICE v2).
bool redetermine_role_on_ice_restart = true;
+ // This flag is only effective when |continual_gathering_policy| is
+ // GATHER_CONTINUALLY.
+ //
+ // If true, after the ICE transport type is changed such that new types of
+ // ICE candidates are allowed by the new transport type, e.g. from
+ // IceTransportsType::kRelay to IceTransportsType::kAll, candidates that
+ // have been gathered by the ICE transport but not matching the previous
+ // transport type and as a result not observed by PeerConnectionObserver,
+ // will be surfaced to the observer.
+ bool surface_ice_candidates_on_ice_transport_type_changed = false;
+
// The following fields define intervals in milliseconds at which ICE
// connectivity checks are sent.
//
diff --git a/p2p/base/ice_transport_internal.h b/p2p/base/ice_transport_internal.h
index 93eec14..89b2107 100644
--- a/p2p/base/ice_transport_internal.h
+++ b/p2p/base/ice_transport_internal.h
@@ -91,6 +91,13 @@
// candidate pairs will succeed, even before a binding response is received.
bool presume_writable_when_fully_relayed = false;
+ // If true, after the ICE transport type (as the candidate filter used by the
+ // port allocator) is changed such that new types of ICE candidates are
+ // allowed by the new filter, e.g. from CF_RELAY to CF_ALL, candidates that
+ // have been gathered by the ICE transport but filtered out and not signaled
+ // to the upper layers, will be surfaced.
+ bool surface_ice_candidates_on_ice_transport_type_changed = false;
+
// Interval to check on all networks and to perform ICE regathering on any
// active network having no connection on it.
absl::optional<int> regather_on_failed_networks_interval;
diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc
index 57e0e12..d469ff7 100644
--- a/p2p/base/p2p_transport_channel.cc
+++ b/p2p/base/p2p_transport_channel.cc
@@ -557,6 +557,15 @@
}
}
+ config_.surface_ice_candidates_on_ice_transport_type_changed =
+ config.surface_ice_candidates_on_ice_transport_type_changed;
+ if (config_.surface_ice_candidates_on_ice_transport_type_changed &&
+ config_.continual_gathering_policy != GATHER_CONTINUALLY) {
+ RTC_LOG(LS_WARNING)
+ << "surface_ice_candidates_on_ice_transport_type_changed is "
+ "ineffective since we do not gather continually.";
+ }
+
if (config_.regather_on_failed_networks_interval !=
config.regather_on_failed_networks_interval) {
config_.regather_on_failed_networks_interval =
@@ -1030,10 +1039,11 @@
void P2PTransportChannel::OnCandidateFilterChanged(uint32_t prev_filter,
uint32_t cur_filter) {
+ RTC_DCHECK_RUN_ON(network_thread_);
if (prev_filter == cur_filter || allocator_session() == nullptr) {
return;
}
- if (webrtc::field_trial::IsEnabled("WebRTC-GatherOnCandidateFilterChanged")) {
+ if (config_.surface_ice_candidates_on_ice_transport_type_changed) {
allocator_session()->SetCandidateFilter(cur_filter);
}
}
diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc
index e20f92d..c5bcbdd 100644
--- a/p2p/base/p2p_transport_channel_unittest.cc
+++ b/p2p/base/p2p_transport_channel_unittest.cc
@@ -4950,8 +4950,6 @@
// removed and are still usable for necessary route switching.
TEST_F(P2PTransportChannelTest,
SurfaceHostCandidateOnCandidateFilterChangeFromRelayToAll) {
- webrtc::test::ScopedFieldTrials field_trials(
- "WebRTC-GatherOnCandidateFilterChanged/Enabled/");
rtc::ScopedFakeClock clock;
ConfigureEndpoints(
@@ -4962,9 +4960,11 @@
auto* ep2 = GetEndpoint(1);
ep1->allocator_->SetCandidateFilter(CF_RELAY);
ep2->allocator_->SetCandidateFilter(CF_RELAY);
- IceConfig continual_gathering_config =
- CreateIceConfig(1000, GATHER_CONTINUALLY);
- CreateChannels(continual_gathering_config, continual_gathering_config);
+ // 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;
+ CreateChannels(ice_config, ice_config);
ASSERT_TRUE_SIMULATED_WAIT(ep1_ch1()->selected_connection() != nullptr,
kDefaultTimeout, clock);
ASSERT_TRUE_SIMULATED_WAIT(ep2_ch1()->selected_connection() != nullptr,
@@ -5014,8 +5014,6 @@
// changing the candidate filter.
TEST_F(P2PTransportChannelTest,
SurfaceSrflxCandidateOnCandidateFilterChangeFromRelayToNoHost) {
- webrtc::test::ScopedFieldTrials field_trials(
- "WebRTC-GatherOnCandidateFilterChanged/Enabled/");
rtc::ScopedFakeClock clock;
// We need an actual NAT so that the host candidate is not equivalent to the
// srflx candidate; otherwise, the host candidate would still surface even
@@ -5032,9 +5030,11 @@
auto* ep2 = GetEndpoint(1);
ep1->allocator_->SetCandidateFilter(CF_RELAY);
ep2->allocator_->SetCandidateFilter(CF_RELAY);
- IceConfig continual_gathering_config =
- CreateIceConfig(1000, GATHER_CONTINUALLY);
- CreateChannels(continual_gathering_config, continual_gathering_config);
+ // 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;
+ CreateChannels(ice_config, ice_config);
ASSERT_TRUE_SIMULATED_WAIT(ep1_ch1()->selected_connection() != nullptr,
kDefaultTimeout, clock);
ASSERT_TRUE_SIMULATED_WAIT(ep2_ch1()->selected_connection() != nullptr,
@@ -5075,13 +5075,49 @@
ep1_ch1()->selected_connection()->remote_candidate().type());
}
+// This is the complement to
+// SurfaceHostCandidateOnCandidateFilterChangeFromRelayToAll, and instead of
+// gathering continually we only gather once, which makes the config
+// |surface_ice_candidates_on_ice_transport_type_changed| ineffective after the
+// gathering stopped.
+TEST_F(P2PTransportChannelTest,
+ CannotSurfaceTheNewlyAllowedOnFilterChangeIfNotGatheringContinually) {
+ 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_RELAY);
+ // Only gather once.
+ IceConfig ice_config = CreateIceConfig(1000, GATHER_ONCE);
+ ice_config.surface_ice_candidates_on_ice_transport_type_changed = true;
+ CreateChannels(ice_config, ice_config);
+ ASSERT_TRUE_SIMULATED_WAIT(ep1_ch1()->selected_connection() != nullptr,
+ kDefaultTimeout, clock);
+ ASSERT_TRUE_SIMULATED_WAIT(ep2_ch1()->selected_connection() != nullptr,
+ kDefaultTimeout, clock);
+ // Loosen the candidate filter at ep1.
+ ep1->allocator_->SetCandidateFilter(CF_ALL);
+ // Wait for a period for any potential surfacing of new candidates.
+ SIMULATED_WAIT(false, kDefaultTimeout, clock);
+ EXPECT_EQ(RELAY_PORT_TYPE,
+ ep1_ch1()->selected_connection()->local_candidate().type());
+
+ // Loosen the candidate filter at ep2.
+ ep2->allocator_->SetCandidateFilter(CF_ALL);
+ EXPECT_EQ(RELAY_PORT_TYPE,
+ ep2_ch1()->selected_connection()->local_candidate().type());
+}
+
// Test that when the candidate filter is updated to be more restrictive,
// candidates that 1) have already been gathered and signaled 2) but no longer
// match the filter, are not removed.
TEST_F(P2PTransportChannelTest,
RestrictingCandidateFilterDoesNotRemoveRegatheredCandidates) {
- webrtc::test::ScopedFieldTrials field_trials(
- "WebRTC-GatherOnCandidateFilterChanged/Enabled/");
rtc::ScopedFakeClock clock;
ConfigureEndpoints(
@@ -5092,14 +5128,16 @@
auto* ep2 = GetEndpoint(1);
ep1->allocator_->SetCandidateFilter(CF_ALL);
ep2->allocator_->SetCandidateFilter(CF_ALL);
- IceConfig continual_gathering_config =
- CreateIceConfig(1000, GATHER_CONTINUALLY);
+ // 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 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(continual_gathering_config, continual_gathering_config);
+ CreateChannels(ice_config, ice_config);
// We have gathered host, srflx and relay candidates.
EXPECT_TRUE_SIMULATED_WAIT(ep1->saved_candidates_.size() == 3u,
diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc
index dcb6c8a..0b7c9c8 100644
--- a/pc/peer_connection.cc
+++ b/pc/peer_connection.cc
@@ -749,6 +749,7 @@
bool presume_writable_when_fully_relayed;
bool enable_ice_renomination;
bool redetermine_role_on_ice_restart;
+ bool surface_ice_candidates_on_ice_transport_type_changed;
absl::optional<int> ice_check_interval_strong_connectivity;
absl::optional<int> ice_check_interval_weak_connectivity;
absl::optional<int> ice_check_min_interval;
@@ -804,6 +805,8 @@
o.presume_writable_when_fully_relayed &&
enable_ice_renomination == o.enable_ice_renomination &&
redetermine_role_on_ice_restart == o.redetermine_role_on_ice_restart &&
+ surface_ice_candidates_on_ice_transport_type_changed ==
+ o.surface_ice_candidates_on_ice_transport_type_changed &&
ice_check_interval_strong_connectivity ==
o.ice_check_interval_strong_connectivity &&
ice_check_interval_weak_connectivity ==
@@ -5749,6 +5752,8 @@
ice_config.continual_gathering_policy = gathering_policy;
ice_config.presume_writable_when_fully_relayed =
config.presume_writable_when_fully_relayed;
+ ice_config.surface_ice_candidates_on_ice_transport_type_changed =
+ config.surface_ice_candidates_on_ice_transport_type_changed;
ice_config.ice_check_interval_strong_connectivity =
config.ice_check_interval_strong_connectivity;
ice_config.ice_check_interval_weak_connectivity =
diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc
index 7b3163b..395c437 100644
--- a/pc/peer_connection_integrationtest.cc
+++ b/pc/peer_connection_integrationtest.cc
@@ -5086,8 +5086,6 @@
}
TEST_P(PeerConnectionIntegrationTest, RegatherAfterChangingIceTransportType) {
- webrtc::test::ScopedFieldTrials field_trials(
- "WebRTC-GatherOnCandidateFilterChanged/Enabled/");
static const rtc::SocketAddress turn_server_internal_address{"88.88.88.0",
3478};
static const rtc::SocketAddress turn_server_external_address{"88.88.88.1", 0};
@@ -5103,11 +5101,13 @@
caller_config.servers.push_back(ice_server);
caller_config.type = webrtc::PeerConnectionInterface::kRelay;
caller_config.continual_gathering_policy = PeerConnection::GATHER_CONTINUALLY;
+ caller_config.surface_ice_candidates_on_ice_transport_type_changed = true;
PeerConnectionInterface::RTCConfiguration callee_config;
callee_config.servers.push_back(ice_server);
callee_config.type = webrtc::PeerConnectionInterface::kRelay;
callee_config.continual_gathering_policy = PeerConnection::GATHER_CONTINUALLY;
+ callee_config.surface_ice_candidates_on_ice_transport_type_changed = true;
ASSERT_TRUE(
CreatePeerConnectionWrappersWithConfig(caller_config, callee_config));
diff --git a/sdk/android/api/org/webrtc/PeerConnection.java b/sdk/android/api/org/webrtc/PeerConnection.java
index 5f802fb..a44969d 100644
--- a/sdk/android/api/org/webrtc/PeerConnection.java
+++ b/sdk/android/api/org/webrtc/PeerConnection.java
@@ -450,6 +450,7 @@
public int iceCandidatePoolSize;
public boolean pruneTurnPorts;
public boolean presumeWritableWhenFullyRelayed;
+ public boolean surfaceIceCandidatesOnIceTransportTypeChanged;
// The following fields define intervals in milliseconds at which ICE
// connectivity checks are sent.
//
@@ -552,6 +553,7 @@
iceCandidatePoolSize = 0;
pruneTurnPorts = false;
presumeWritableWhenFullyRelayed = false;
+ surfaceIceCandidatesOnIceTransportTypeChanged = false;
iceCheckIntervalStrongConnectivityMs = null;
iceCheckIntervalWeakConnectivityMs = null;
iceCheckMinInterval = null;
@@ -658,6 +660,11 @@
return presumeWritableWhenFullyRelayed;
}
+ @CalledByNative("RTCConfiguration")
+ boolean getSurfaceIceCandidatesOnIceTransportTypeChanged() {
+ return surfaceIceCandidatesOnIceTransportTypeChanged;
+ }
+
@Nullable
@CalledByNative("RTCConfiguration")
Integer getIceCheckIntervalStrongConnectivity() {
diff --git a/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java b/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java
index a1a11a5..3836f50 100644
--- a/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java
+++ b/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java
@@ -45,9 +45,11 @@
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
+import org.webrtc.Logging;
import org.webrtc.Metrics.HistogramInfo;
import org.webrtc.PeerConnection.IceConnectionState;
import org.webrtc.PeerConnection.IceGatheringState;
+import org.webrtc.PeerConnection.IceTransportsType;
import org.webrtc.PeerConnection.PeerConnectionState;
import org.webrtc.PeerConnection.SignalingState;
import org.webrtc.PeerConnection.TlsCertPolicy;
@@ -59,7 +61,9 @@
/** End-to-end tests for PeerConnection.java. */
@RunWith(BaseJUnit4ClassRunner.class)
public class PeerConnectionTest {
- private static final int TIMEOUT_SECONDS = 20;
+ private static final String TAG = "PeerConnectionTest";
+ private static final int DEFAULT_TIMEOUT_SECONDS = 20;
+ private static final int SHORT_TIMEOUT_SECONDS = 5;
private @Nullable TreeSet<String> threadsBeforeTest;
@Before
@@ -124,6 +128,7 @@
// TODO(bugs.webrtc.org/8491): Remove NoSynchronizedMethodCheck suppression.
@SuppressWarnings("NoSynchronizedMethodCheck")
public synchronized void onIceCandidate(IceCandidate candidate) {
+ Logging.d(TAG, "onIceCandidate: " + candidate.toString());
--expectedIceCandidates;
// We don't assert expectedIceCandidates >= 0 because it's hard to know
@@ -201,7 +206,7 @@
}
if (expectedIceConnectionChanges.isEmpty()) {
- System.out.println(name + "Got an unexpected ICE connection change " + newState);
+ Logging.d(TAG, name + "Got an unexpected ICE connection change " + newState);
return;
}
@@ -217,8 +222,7 @@
}
if (expectedIceConnectionChanges.isEmpty()) {
- System.out.println(
- name + "Got an unexpected standardized ICE connection change " + newState);
+ Logging.d(TAG, name + "Got an unexpected standardized ICE connection change " + newState);
return;
}
@@ -236,7 +240,7 @@
@SuppressWarnings("NoSynchronizedMethodCheck")
public synchronized void onConnectionChange(PeerConnectionState newState) {
if (expectedConnectionChanges.isEmpty()) {
- System.out.println(name + " got an unexpected DTLS connection change " + newState);
+ Logging.d(TAG, name + " got an unexpected DTLS connection change " + newState);
return;
}
@@ -247,7 +251,7 @@
// TODO(bugs.webrtc.org/8491): Remove NoSynchronizedMethodCheck suppression.
@SuppressWarnings("NoSynchronizedMethodCheck")
public synchronized void onIceConnectionReceivingChange(boolean receiving) {
- System.out.println(name + " got an ICE connection receiving change " + receiving);
+ Logging.d(TAG, name + " got an ICE connection receiving change " + receiving);
}
// TODO(bugs.webrtc.org/8491): Remove NoSynchronizedMethodCheck suppression.
@@ -267,7 +271,7 @@
return;
}
if (expectedIceGatheringChanges.isEmpty()) {
- System.out.println(name + "Got an unexpected ICE gathering change " + newState);
+ Logging.d(TAG, name + "Got an unexpected ICE gathering change " + newState);
}
assertEquals(expectedIceGatheringChanges.remove(), newState);
}
@@ -530,12 +534,14 @@
TreeSet<String> stillWaitingForExpectations = unsatisfiedExpectations();
while (!stillWaitingForExpectations.isEmpty()) {
if (!stillWaitingForExpectations.equals(prev)) {
- System.out.println(name + " still waiting at\n " + (new Throwable()).getStackTrace()[1]
- + "\n for: " + Arrays.toString(stillWaitingForExpectations.toArray()));
+ Logging.d(TAG,
+ name + " still waiting at\n " + (new Throwable()).getStackTrace()[1]
+ + "\n for: " + Arrays.toString(stillWaitingForExpectations.toArray()));
}
if (endTime < System.currentTimeMillis()) {
- System.out.println(name + " timed out waiting for: "
- + Arrays.toString(stillWaitingForExpectations.toArray()));
+ Logging.d(TAG,
+ name + " timed out waiting for: "
+ + Arrays.toString(stillWaitingForExpectations.toArray()));
return false;
}
try {
@@ -547,8 +553,8 @@
stillWaitingForExpectations = unsatisfiedExpectations();
}
if (prev == null) {
- System.out.println(
- name + " didn't need to wait at\n " + (new Throwable()).getStackTrace()[1]);
+ Logging.d(
+ TAG, name + " didn't need to wait at\n " + (new Throwable()).getStackTrace()[1]);
}
return true;
}
@@ -1062,8 +1068,8 @@
offeringPC.addIceCandidate(candidate);
}
- assertTrue(offeringExpectations.waitForAllExpectationsToBeSatisfied(TIMEOUT_SECONDS));
- assertTrue(answeringExpectations.waitForAllExpectationsToBeSatisfied(TIMEOUT_SECONDS));
+ assertTrue(offeringExpectations.waitForAllExpectationsToBeSatisfied(DEFAULT_TIMEOUT_SECONDS));
+ assertTrue(answeringExpectations.waitForAllExpectationsToBeSatisfied(DEFAULT_TIMEOUT_SECONDS));
assertEquals(PeerConnection.SignalingState.STABLE, offeringPC.signalingState());
assertEquals(PeerConnection.SignalingState.STABLE, answeringPC.signalingState());
@@ -1119,7 +1125,7 @@
DataChannel.Buffer buffer =
new DataChannel.Buffer(ByteBuffer.wrap("hello!".getBytes(Charset.forName("UTF-8"))), false);
assertTrue(offeringExpectations.dataChannel.send(buffer));
- assertTrue(answeringExpectations.waitForAllExpectationsToBeSatisfied(TIMEOUT_SECONDS));
+ assertTrue(answeringExpectations.waitForAllExpectationsToBeSatisfied(DEFAULT_TIMEOUT_SECONDS));
// Construct this binary message two different ways to ensure no
// shortcuts are taken.
@@ -1131,7 +1137,7 @@
offeringExpectations.expectMessage(expectedBinaryMessage, true);
assertTrue(answeringExpectations.dataChannel.send(
new DataChannel.Buffer(ByteBuffer.wrap(new byte[] {1, 2, 3, 4, 5}), true)));
- assertTrue(offeringExpectations.waitForAllExpectationsToBeSatisfied(TIMEOUT_SECONDS));
+ assertTrue(offeringExpectations.waitForAllExpectationsToBeSatisfied(DEFAULT_TIMEOUT_SECONDS));
offeringExpectations.expectStateChange(DataChannel.State.CLOSING);
answeringExpectations.expectStateChange(DataChannel.State.CLOSING);
@@ -1139,8 +1145,8 @@
answeringExpectations.expectStateChange(DataChannel.State.CLOSED);
answeringExpectations.dataChannel.close();
offeringExpectations.dataChannel.close();
- assertTrue(offeringExpectations.waitForAllExpectationsToBeSatisfied(TIMEOUT_SECONDS));
- assertTrue(answeringExpectations.waitForAllExpectationsToBeSatisfied(TIMEOUT_SECONDS));
+ assertTrue(offeringExpectations.waitForAllExpectationsToBeSatisfied(DEFAULT_TIMEOUT_SECONDS));
+ assertTrue(answeringExpectations.waitForAllExpectationsToBeSatisfied(DEFAULT_TIMEOUT_SECONDS));
// Test SetBitrate.
assertTrue(offeringPC.setBitrate(100000, 5000000, 500000000));
@@ -1279,8 +1285,8 @@
offeringPC.addIceCandidate(candidate);
}
- assertTrue(offeringExpectations.waitForAllExpectationsToBeSatisfied(TIMEOUT_SECONDS));
- assertTrue(answeringExpectations.waitForAllExpectationsToBeSatisfied(TIMEOUT_SECONDS));
+ assertTrue(offeringExpectations.waitForAllExpectationsToBeSatisfied(DEFAULT_TIMEOUT_SECONDS));
+ assertTrue(answeringExpectations.waitForAllExpectationsToBeSatisfied(DEFAULT_TIMEOUT_SECONDS));
assertEquals(PeerConnection.SignalingState.STABLE, offeringPC.signalingState());
assertEquals(PeerConnection.SignalingState.STABLE, answeringPC.signalingState());
@@ -1291,7 +1297,7 @@
DataChannel.Buffer buffer =
new DataChannel.Buffer(ByteBuffer.wrap("hello!".getBytes(Charset.forName("UTF-8"))), false);
assertTrue(offeringExpectations.dataChannel.send(buffer));
- assertTrue(answeringExpectations.waitForAllExpectationsToBeSatisfied(TIMEOUT_SECONDS));
+ assertTrue(answeringExpectations.waitForAllExpectationsToBeSatisfied(DEFAULT_TIMEOUT_SECONDS));
// Construct this binary message two different ways to ensure no
// shortcuts are taken.
@@ -1303,7 +1309,7 @@
offeringExpectations.expectMessage(expectedBinaryMessage, true);
assertTrue(answeringExpectations.dataChannel.send(
new DataChannel.Buffer(ByteBuffer.wrap(new byte[] {1, 2, 3, 4, 5}), true)));
- assertTrue(offeringExpectations.waitForAllExpectationsToBeSatisfied(TIMEOUT_SECONDS));
+ assertTrue(offeringExpectations.waitForAllExpectationsToBeSatisfied(DEFAULT_TIMEOUT_SECONDS));
offeringExpectations.expectStateChange(DataChannel.State.CLOSING);
answeringExpectations.expectStateChange(DataChannel.State.CLOSING);
@@ -1311,8 +1317,8 @@
answeringExpectations.expectStateChange(DataChannel.State.CLOSED);
answeringExpectations.dataChannel.close();
offeringExpectations.dataChannel.close();
- assertTrue(offeringExpectations.waitForAllExpectationsToBeSatisfied(TIMEOUT_SECONDS));
- assertTrue(answeringExpectations.waitForAllExpectationsToBeSatisfied(TIMEOUT_SECONDS));
+ assertTrue(offeringExpectations.waitForAllExpectationsToBeSatisfied(DEFAULT_TIMEOUT_SECONDS));
+ assertTrue(answeringExpectations.waitForAllExpectationsToBeSatisfied(DEFAULT_TIMEOUT_SECONDS));
// Free the Java-land objects and collect them.
shutdownPC(offeringPC, offeringExpectations);
@@ -1323,6 +1329,72 @@
System.gc();
}
+ // Tests that ICE candidates that are not allowed by an ICE transport type, thus not being
+ // signaled to the gathering PeerConnection, can be surfaced via configuration if allowed by the
+ // new ICE transport type, when RTCConfiguration.surfaceIceCandidatesOnIceTransportTypeChanged is
+ // true.
+ @Test
+ @SmallTest
+ public void testSurfaceIceCandidatesWhenIceTransportTypeChanged() throws Exception {
+ // For this test, we only need one PeerConnection to observe the behavior of gathering, and we
+ // create only the offering PC below.
+ //
+ // Allow loopback interfaces too since our Android devices often don't
+ // have those.
+ PeerConnectionFactory.Options options = new PeerConnectionFactory.Options();
+ options.networkIgnoreMask = 0;
+ PeerConnectionFactory factory =
+ PeerConnectionFactory.builder().setOptions(options).createPeerConnectionFactory();
+
+ PeerConnection.RTCConfiguration rtcConfig =
+ new PeerConnection.RTCConfiguration(Arrays.asList());
+ // NONE would prevent any candidate being signaled to the PC.
+ rtcConfig.iceTransportsType = PeerConnection.IceTransportsType.NONE;
+ // We must have the continual gathering enabled to allow the surfacing of candidates on the ICE
+ // transport type change.
+ rtcConfig.continualGatheringPolicy = PeerConnection.ContinualGatheringPolicy.GATHER_CONTINUALLY;
+ rtcConfig.surfaceIceCandidatesOnIceTransportTypeChanged = true;
+
+ ObserverExpectations offeringExpectations = new ObserverExpectations("PCTest:offerer");
+ PeerConnection offeringPC = factory.createPeerConnection(rtcConfig, offeringExpectations);
+ assertNotNull(offeringPC);
+
+ // Create a data channel and set local description to kick off the ICE candidate gathering.
+ offeringExpectations.expectRenegotiationNeeded();
+ DataChannel offeringDC = offeringPC.createDataChannel("offeringDC", new DataChannel.Init());
+ assertEquals("offeringDC", offeringDC.label());
+
+ offeringExpectations.setDataChannel(offeringDC);
+ SdpObserverLatch sdpLatch = new SdpObserverLatch();
+ offeringPC.createOffer(sdpLatch, new MediaConstraints());
+ assertTrue(sdpLatch.await());
+ SessionDescription offerSdp = sdpLatch.getSdp();
+ assertEquals(offerSdp.type, SessionDescription.Type.OFFER);
+ assertFalse(offerSdp.description.isEmpty());
+
+ sdpLatch = new SdpObserverLatch();
+ offeringExpectations.expectSignalingChange(SignalingState.HAVE_LOCAL_OFFER);
+ offeringPC.setLocalDescription(sdpLatch, offerSdp);
+ assertTrue(sdpLatch.await());
+ assertNull(sdpLatch.getSdp());
+
+ assertEquals(offeringPC.getLocalDescription().type, offerSdp.type);
+
+ // Wait until we satisfy all expectations in the setup.
+ assertTrue(offeringExpectations.waitForAllExpectationsToBeSatisfied(DEFAULT_TIMEOUT_SECONDS));
+
+ // Add the expectation of gathering at least one candidate, which should however fail because of
+ // the transport type NONE.
+ offeringExpectations.expectIceCandidates(1);
+ assertFalse(offeringExpectations.waitForAllExpectationsToBeSatisfied(SHORT_TIMEOUT_SECONDS));
+
+ // Change the transport type and we should be able to meet the expectation of gathering this
+ // time.
+ rtcConfig.iceTransportsType = PeerConnection.IceTransportsType.ALL;
+ offeringPC.setConfiguration(rtcConfig);
+ assertTrue(offeringExpectations.waitForAllExpectationsToBeSatisfied(DEFAULT_TIMEOUT_SECONDS));
+ }
+
@Test
@MediumTest
public void testTrackRemovalAndAddition() throws Exception {
@@ -1464,8 +1536,8 @@
offeringExpectations.expectFramesDelivered(1);
answeringExpectations.expectFramesDelivered(1);
- assertTrue(offeringExpectations.waitForAllExpectationsToBeSatisfied(TIMEOUT_SECONDS));
- assertTrue(answeringExpectations.waitForAllExpectationsToBeSatisfied(TIMEOUT_SECONDS));
+ assertTrue(offeringExpectations.waitForAllExpectationsToBeSatisfied(DEFAULT_TIMEOUT_SECONDS));
+ assertTrue(answeringExpectations.waitForAllExpectationsToBeSatisfied(DEFAULT_TIMEOUT_SECONDS));
assertEquals(PeerConnection.SignalingState.STABLE, offeringPC.signalingState());
assertEquals(PeerConnection.SignalingState.STABLE, answeringPC.signalingState());
@@ -1820,36 +1892,36 @@
// Call getStats (old implementation) before shutting down PC.
expectations.expectOldStatsCallback();
assertTrue(pc.getStats(expectations, null /* track */));
- assertTrue(expectations.waitForAllExpectationsToBeSatisfied(TIMEOUT_SECONDS));
+ assertTrue(expectations.waitForAllExpectationsToBeSatisfied(DEFAULT_TIMEOUT_SECONDS));
// Call the new getStats implementation as well.
expectations.expectNewStatsCallback();
pc.getStats(expectations);
- assertTrue(expectations.waitForAllExpectationsToBeSatisfied(TIMEOUT_SECONDS));
+ assertTrue(expectations.waitForAllExpectationsToBeSatisfied(DEFAULT_TIMEOUT_SECONDS));
expectations.expectIceConnectionChange(IceConnectionState.CLOSED);
expectations.expectStandardizedIceConnectionChange(IceConnectionState.CLOSED);
expectations.expectConnectionChange(PeerConnectionState.CLOSED);
expectations.expectSignalingChange(SignalingState.CLOSED);
pc.close();
- assertTrue(expectations.waitForAllExpectationsToBeSatisfied(TIMEOUT_SECONDS));
+ assertTrue(expectations.waitForAllExpectationsToBeSatisfied(DEFAULT_TIMEOUT_SECONDS));
// Call getStats (old implementation) after calling close(). Should still
// work.
expectations.expectOldStatsCallback();
assertTrue(pc.getStats(expectations, null /* track */));
- assertTrue(expectations.waitForAllExpectationsToBeSatisfied(TIMEOUT_SECONDS));
+ assertTrue(expectations.waitForAllExpectationsToBeSatisfied(DEFAULT_TIMEOUT_SECONDS));
- System.out.println("FYI stats: ");
+ Logging.d(TAG, "FYI stats: ");
int reportIndex = -1;
for (StatsReport[] reports : expectations.takeStatsReports()) {
- System.out.println(" Report #" + (++reportIndex));
+ Logging.d(TAG, " Report #" + (++reportIndex));
for (int i = 0; i < reports.length; ++i) {
- System.out.println(" " + reports[i].toString());
+ Logging.d(TAG, " " + reports[i].toString());
}
}
assertEquals(1, reportIndex);
- System.out.println("End stats.");
+ Logging.d(TAG, "End stats.");
pc.dispose();
}
diff --git a/sdk/android/src/jni/pc/peer_connection.cc b/sdk/android/src/jni/pc/peer_connection.cc
index 98c3dd0..1e3a73d 100644
--- a/sdk/android/src/jni/pc/peer_connection.cc
+++ b/sdk/android/src/jni/pc/peer_connection.cc
@@ -186,6 +186,9 @@
rtc_config->presume_writable_when_fully_relayed =
Java_RTCConfiguration_getPresumeWritableWhenFullyRelayed(jni,
j_rtc_config);
+ rtc_config->surface_ice_candidates_on_ice_transport_type_changed =
+ Java_RTCConfiguration_getSurfaceIceCandidatesOnIceTransportTypeChanged(
+ jni, j_rtc_config);
ScopedJavaLocalRef<jobject> j_ice_check_interval_strong_connectivity =
Java_RTCConfiguration_getIceCheckIntervalStrongConnectivity(jni,
j_rtc_config);
diff --git a/sdk/objc/api/peerconnection/RTCConfiguration.h b/sdk/objc/api/peerconnection/RTCConfiguration.h
index f9e6edf..2c32311 100644
--- a/sdk/objc/api/peerconnection/RTCConfiguration.h
+++ b/sdk/objc/api/peerconnection/RTCConfiguration.h
@@ -140,6 +140,18 @@
*/
@property(nonatomic, assign) BOOL shouldPresumeWritableWhenFullyRelayed;
+/* This flag is only effective when |continualGatheringPolicy| is
+ * RTCContinualGatheringPolicyGatherContinually.
+ *
+ * If YES, after the ICE transport type is changed such that new types of
+ * ICE candidates are allowed by the new transport type, e.g. from
+ * RTCIceTransportPolicyRelay to RTCIceTransportPolicyAll, candidates that
+ * have been gathered by the ICE transport but not matching the previous
+ * transport type and as a result not observed by PeerConnectionDelegateAdapter,
+ * will be surfaced to the delegate.
+ */
+@property(nonatomic, assign) BOOL shouldSurfaceIceCandidatesOnIceTransportTypeChanged;
+
/** If set to non-nil, controls the minimal interval between consecutive ICE
* check packets.
*/
diff --git a/sdk/objc/api/peerconnection/RTCConfiguration.mm b/sdk/objc/api/peerconnection/RTCConfiguration.mm
index 83ae575..c2ff8bf 100644
--- a/sdk/objc/api/peerconnection/RTCConfiguration.mm
+++ b/sdk/objc/api/peerconnection/RTCConfiguration.mm
@@ -45,6 +45,8 @@
@synthesize shouldPruneTurnPorts = _shouldPruneTurnPorts;
@synthesize shouldPresumeWritableWhenFullyRelayed =
_shouldPresumeWritableWhenFullyRelayed;
+@synthesize shouldSurfaceIceCandidatesOnIceTransportTypeChanged =
+ _shouldSurfaceIceCandidatesOnIceTransportTypeChanged;
@synthesize iceCheckMinInterval = _iceCheckMinInterval;
@synthesize iceRegatherIntervalRange = _iceRegatherIntervalRange;
@synthesize sdpSemantics = _sdpSemantics;
@@ -109,6 +111,8 @@
_shouldPruneTurnPorts = config.prune_turn_ports;
_shouldPresumeWritableWhenFullyRelayed =
config.presume_writable_when_fully_relayed;
+ _shouldSurfaceIceCandidatesOnIceTransportTypeChanged =
+ config.surface_ice_candidates_on_ice_transport_type_changed;
if (config.ice_check_min_interval) {
_iceCheckMinInterval =
[NSNumber numberWithInt:*config.ice_check_min_interval];
@@ -160,6 +164,7 @@
_iceCandidatePoolSize,
_shouldPruneTurnPorts,
_shouldPresumeWritableWhenFullyRelayed,
+ _shouldSurfaceIceCandidatesOnIceTransportTypeChanged,
_iceCheckMinInterval,
_iceRegatherIntervalRange,
_disableLinkLocalNetworks,
@@ -239,6 +244,8 @@
nativeConfig->prune_turn_ports = _shouldPruneTurnPorts ? true : false;
nativeConfig->presume_writable_when_fully_relayed =
_shouldPresumeWritableWhenFullyRelayed ? true : false;
+ nativeConfig->surface_ice_candidates_on_ice_transport_type_changed =
+ _shouldSurfaceIceCandidatesOnIceTransportTypeChanged ? true : false;
if (_iceCheckMinInterval != nil) {
nativeConfig->ice_check_min_interval = absl::optional<int>(_iceCheckMinInterval.intValue);
}
