Index: webrtc/p2p/base/p2ptransportchannel_unittest.cc |
diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc |
index 45add0c733f03e100df0754478a406b6dc7133cd..38242a39c08e929f601cf8e5a2d1a8b59444dec3 100644 |
--- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc |
+++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc |
@@ -149,7 +149,6 @@ class P2PTransportChannelTestBase : public testing::Test, |
ss_.get(), kSocksProxyAddrs[0]), |
socks_server2_(ss_.get(), kSocksProxyAddrs[1], |
ss_.get(), kSocksProxyAddrs[1]), |
- clear_remote_candidates_ufrag_pwd_(false), |
force_relay_(false) { |
ep1_.role_ = cricket::ICEROLE_CONTROLLING; |
ep2_.role_ = cricket::ICEROLE_CONTROLLED; |
@@ -326,9 +325,7 @@ class P2PTransportChannelTestBase : public testing::Test, |
channel->SignalRoleConflict.connect( |
this, &P2PTransportChannelTestBase::OnRoleConflict); |
channel->SetIceCredentials(local_ice_ufrag, local_ice_pwd); |
- if (clear_remote_candidates_ufrag_pwd_) { |
- // This only needs to be set if we're clearing them from the |
- // candidates. Some unit tests rely on this not being set. |
+ if (remote_ice_credential_source_ == FROM_SETICECREDENTIALS) { |
channel->SetRemoteIceCredentials(remote_ice_ufrag, remote_ice_pwd); |
} |
channel->SetIceRole(GetEndpoint(endpoint)->ice_role()); |
@@ -705,7 +702,7 @@ class P2PTransportChannelTestBase : public testing::Test, |
static_cast<CandidatesData*>(msg->pdata)); |
cricket::P2PTransportChannel* rch = GetRemoteChannel(data->channel); |
for (auto& c : data->candidates) { |
- if (clear_remote_candidates_ufrag_pwd_) { |
+ if (remote_ice_credential_source_ != FROM_CANDIDATE) { |
c.set_username(""); |
c.set_password(""); |
} |
@@ -786,8 +783,13 @@ class P2PTransportChannelTestBase : public testing::Test, |
return GetChannelData(ch)->ch_packets_; |
} |
- void set_clear_remote_candidates_ufrag_pwd(bool clear) { |
Taylor Brandstetter
2016/06/21 00:33:05
I changed this method for readability. It wasn't c
|
- clear_remote_candidates_ufrag_pwd_ = clear; |
+ enum RemoteIceCredentialSource { FROM_CANDIDATE, FROM_SETICECREDENTIALS }; |
+ |
+ // How does the test pass ICE credentials to the P2PTransportChannel? |
+ // On the candidate itself, or through SetIceCredentials? |
+ // Goes through the candidate itself by default. |
+ void set_remote_ice_credential_source(RemoteIceCredentialSource source) { |
+ remote_ice_credential_source_ = source; |
} |
void set_force_relay(bool relay) { |
@@ -808,7 +810,7 @@ class P2PTransportChannelTestBase : public testing::Test, |
rtc::SocksProxyServer socks_server2_; |
Endpoint ep1_; |
Endpoint ep2_; |
- bool clear_remote_candidates_ufrag_pwd_; |
+ RemoteIceCredentialSource remote_ice_credential_source_ = FROM_CANDIDATE; |
bool force_relay_; |
}; |
@@ -887,7 +889,7 @@ class P2PTransportChannelTest : public P2PTransportChannelTestBase { |
SetAllocatorFlags(1, allocator_flags2); |
SetAllocationStepDelay(1, delay); |
- set_clear_remote_candidates_ufrag_pwd(true); |
+ set_remote_ice_credential_source(FROM_SETICECREDENTIALS); |
} |
void ConfigureEndpoint(int endpoint, Config config) { |
switch (config) { |
@@ -1105,7 +1107,7 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignaling) { |
ConfigureEndpoints(OPEN, OPEN, kDefaultPortAllocatorFlags, |
kDefaultPortAllocatorFlags); |
// Emulate no remote credentials coming in. |
- set_clear_remote_candidates_ufrag_pwd(false); |
+ set_remote_ice_credential_source(FROM_CANDIDATE); |
CreateChannels(1); |
// Only have remote credentials come in for ep2, not ep1. |
ep2_ch1()->SetRemoteIceCredentials(kIceUfrag[0], kIcePwd[0]); |
@@ -1157,7 +1159,7 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignalingWithNAT) { |
ConfigureEndpoints(OPEN, NAT_SYMMETRIC, kDefaultPortAllocatorFlags, |
kDefaultPortAllocatorFlags); |
// Emulate no remote credentials coming in. |
- set_clear_remote_candidates_ufrag_pwd(false); |
+ set_remote_ice_credential_source(FROM_CANDIDATE); |
CreateChannels(1); |
// Only have remote credentials come in for ep2, not ep1. |
ep2_ch1()->SetRemoteIceCredentials(kIceUfrag[0], kIcePwd[0]); |
@@ -1200,9 +1202,67 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignalingWithNAT) { |
DestroyChannels(); |
} |
+// Test that we properly create a connection on a STUN ping from unknown address |
+// when the signaling is slow, even if the new candidate is created due to the |
+// remote peer doing an ICE restart, pairing this candidate across generations. |
+// |
+// Previously this wasn't working due to a bug where the peer reflexive |
+// candidate was only updated for the newest generation candidate pairs, and |
+// not older-generation candidate pairs created by pairing candidates across |
+// generations. This resulted in the old-generation prflx candidate being |
+// prioritized above new-generation candidate pairs. |
+TEST_F(P2PTransportChannelTest, |
+ PeerReflexiveCandidateBeforeSignalingWithIceRestart) { |
+ ConfigureEndpoints(OPEN, OPEN, kDefaultPortAllocatorFlags, |
+ kDefaultPortAllocatorFlags); |
+ // Only gather relay candidates, so that when the prflx candidate arrives |
+ // it's prioritized above the current candidate pair. |
+ GetEndpoint(0)->allocator_->set_candidate_filter(cricket::CF_RELAY); |
+ GetEndpoint(1)->allocator_->set_candidate_filter(cricket::CF_RELAY); |
+ // Setting this allows us to control when SetRemoteIceCredentials is called. |
+ set_remote_ice_credential_source(FROM_CANDIDATE); |
+ CreateChannels(1); |
+ // Wait for the initial connection to be made. |
+ ep1_ch1()->SetRemoteIceCredentials(kIceUfrag[1], kIcePwd[1]); |
+ ep2_ch1()->SetRemoteIceCredentials(kIceUfrag[0], kIcePwd[0]); |
+ EXPECT_TRUE_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() && |
+ ep2_ch1()->receiving() && ep2_ch1()->writable(), |
+ kDefaultTimeout); |
+ |
+ // Simulate an ICE restart on ep2, but don't signal the candidate or new |
+ // ICE credentials until after a prflx connection has been made. |
+ PauseCandidates(1); |
+ ep2_ch1()->SetIceCredentials(kIceUfrag[3], kIcePwd[3]); |
+ ep1_ch1()->SetRemoteIceCredentials(kIceUfrag[3], kIcePwd[3]); |
+ ep2_ch1()->MaybeStartGathering(); |
+ |
+ // The caller should have the best connection connected to the peer reflexive |
+ // candidate. |
+ EXPECT_EQ_WAIT("prflx", |
+ ep1_ch1()->best_connection()->remote_candidate().type(), |
+ kDefaultTimeout); |
+ const cricket::Connection* prflx_best_connection = |
+ ep1_ch1()->best_connection(); |
+ |
+ // Now simulate the ICE restart on ep1. |
+ ep1_ch1()->SetIceCredentials(kIceUfrag[2], kIcePwd[2]); |
+ ep2_ch1()->SetRemoteIceCredentials(kIceUfrag[2], kIcePwd[2]); |
+ ep1_ch1()->MaybeStartGathering(); |
+ |
+ // Finally send the candidates from ep2's ICE restart and verify that ep1 uses |
+ // their information to update the peer reflexive candidate. |
+ ResumeCandidates(1); |
+ |
+ EXPECT_EQ_WAIT("relay", |
+ ep1_ch1()->best_connection()->remote_candidate().type(), |
+ kDefaultTimeout); |
+ EXPECT_EQ(prflx_best_connection, ep1_ch1()->best_connection()); |
+ DestroyChannels(); |
+} |
+ |
// Test that if remote candidates don't have ufrag and pwd, we still work. |
TEST_F(P2PTransportChannelTest, RemoteCandidatesWithoutUfragPwd) { |
- set_clear_remote_candidates_ufrag_pwd(true); |
+ set_remote_ice_credential_source(FROM_SETICECREDENTIALS); |
ConfigureEndpoints(OPEN, OPEN, kDefaultPortAllocatorFlags, |
kDefaultPortAllocatorFlags); |
CreateChannels(1); |