admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Update prflx candidates' generation when setting ICE credentials.

Browse Source 
If a STUN ping arrives before the remote description does, a prflx
candidate will be created with an unknown generation.

Once the remote description does arrive, the candidate's generation
should be set so it can be sorted properly, and replaced by a non-prflx
candidate once the candidate is signaled.

BUG=webrtc:5752
R=honghaiz@webrtc.org, pthatcher@webrtc.org

Review URL: https://codereview.webrtc.org/1868353004 .

Cr-Commit-Position: refs/heads/master@{#12433}
This commit is contained in:
Taylor Brandstetter 2016-04-19 18:03:26 -07:00
parent 0e533ef487
commit 0a1bc53758
6 changed files with 56 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
webrtc/api/peerconnection_unittest.cc
View File

@ -1187,10 +1187,7 @@ class P2PTestConductor : public testing::Test {
// This test sets up a call between two parties. Both parties send static
// frames to each other. Once the test is finished the number of sent frames
// is compared to the number of received frames.
void LocalP2PTest() { LocalP2PTest(false); }
// TODO(perkj); Remove the flag bug5752 when
// https://bugs.chromium.org/p/webrtc/issues/detail?id=5752 is fixed.
void LocalP2PTest(bool bug5752) {
void LocalP2PTest() {
if (initiating_client_->NumberOfLocalMediaStreams() == 0) {
initiating_client_->AddMediaStream(true, true);
}
@ -1219,18 +1216,9 @@ class P2PTestConductor : public testing::Test {
// Completed.
// Note: These tests have been observed to fail under heavy load at
// shorter timeouts, so they may be flaky.
if (bug5752) {
EXPECT_TRUE_WAIT(
initiating_client_->ice_connection_state() ==
webrtc::PeerConnectionInterface::kIceConnectionCompleted ||
initiating_client_->ice_connection_state() ==
webrtc::PeerConnectionInterface::kIceConnectionConnected,
kMaxWaitForFramesMs);
} else {
EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionCompleted,
initiating_client_->ice_connection_state(),
kMaxWaitForFramesMs);
}
EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionCompleted,
initiating_client_->ice_connection_state(),
kMaxWaitForFramesMs);
EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionConnected,
receiving_client_->ice_connection_state(),
kMaxWaitForFramesMs);
@ -1461,9 +1449,7 @@ TEST_F(P2PTestConductor, LocalP2PTestDtlsTransferCaller) {
SetSignalingReceivers();
initializing_client()->IceRestart();
// TODO(perkj): Remove the flag bug5752 when
// https://bugs.chromium.org/p/webrtc/issues/detail?id=5752 is fixed.
LocalP2PTest(true /* bug5752 */);
LocalP2PTest();
VerifyRenderedSize(640, 480);
}

3
webrtc/p2p/base/candidate.h
View File

@ -249,7 +249,8 @@ class Candidate {
ost << "Cand[" << transport_name_ << ":" << foundation_ << ":" << component_
<< ":" << protocol_ << ":" << priority_ << ":" << address << ":"
<< type_ << ":" << related_address_ << ":" << username_ << ":"
<< password_ << ":" << network_id_ << ":" << network_cost_ << "]";
<< password_ << ":" << network_id_ << ":" << network_cost_ << ":"
<< generation_ << "]";
return ost.str();
}

9
webrtc/p2p/base/p2ptransportchannel.cc
View File

@ -390,10 +390,15 @@ void P2PTransportChannel::SetRemoteIceCredentials(const std::string& ice_ufrag,
candidate.set_password(ice_pwd);
}
}
// We need to update the credentials for any peer reflexive candidates.
// We need to update the credentials and generation for any peer reflexive
// candidates.
for (Connection* conn : connections_) {
conn->MaybeSetRemoteIceCredentials(ice_ufrag, ice_pwd);
conn->MaybeSetRemoteIceCredentialsAndGeneration(
ice_ufrag, ice_pwd,
static_cast<int>(remote_ice_parameters_.size() - 1));
}
// Updating the remote ICE candidate generation could change the sort order.
RequestSort();
}
void P2PTransportChannel::SetRemoteIceMode(IceMode mode) {

29
webrtc/p2p/base/p2ptransportchannel_unittest.cc
View File

@ -1193,7 +1193,7 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignaling) {
set_clear_remote_candidates_ufrag_pwd(false);
CreateChannels(1);
// Only have remote credentials come in for ep2, not ep1.
ep2_ch1()->SetRemoteIceCredentials(kIceUfrag[3], kIcePwd[3]);
ep2_ch1()->SetRemoteIceCredentials(kIceUfrag[0], kIcePwd[0]);
// Pause sending ep2's candidates to ep1 until ep1 receives the peer reflexive
// candidate.
@ -1209,15 +1209,23 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignaling) {
EXPECT_EQ(kIceUfrag[1],
ep1_ch1()->best_connection()->remote_candidate().username());
EXPECT_EQ("", ep1_ch1()->best_connection()->remote_candidate().password());
// Because we don't have ICE credentials yet, we don't know the generation.
EXPECT_EQ(0u, ep1_ch1()->best_connection()->remote_candidate().generation());
EXPECT_TRUE(nullptr == ep1_ch1()->FindNextPingableConnection());
// Add two sets of remote ICE credentials, so that the ones used by the
// candidate will be generation 1 instead of 0.
ep1_ch1()->SetRemoteIceCredentials(kIceUfrag[3], kIcePwd[3]);
ep1_ch1()->SetRemoteIceCredentials(kIceUfrag[1], kIcePwd[1]);
ResumeCandidates(1);
// After setting the remote ICE credentials, the password and generation
// of the peer reflexive candidate should be updated.
EXPECT_EQ(kIcePwd[1],
ep1_ch1()->best_connection()->remote_candidate().password());
EXPECT_EQ(1u, ep1_ch1()->best_connection()->remote_candidate().generation());
EXPECT_TRUE(nullptr != ep1_ch1()->FindNextPingableConnection());
ResumeCandidates(1);
WAIT(ep2_ch1()->best_connection() != NULL, 2000);
// Verify ep1's best connection is updated to use the 'local' candidate.
EXPECT_EQ_WAIT(
@ -1237,7 +1245,7 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignalingWithNAT) {
set_clear_remote_candidates_ufrag_pwd(false);
CreateChannels(1);
// Only have remote credentials come in for ep2, not ep1.
ep2_ch1()->SetRemoteIceCredentials(kIceUfrag[3], kIcePwd[3]);
ep2_ch1()->SetRemoteIceCredentials(kIceUfrag[0], kIcePwd[0]);
// Pause sending ep2's candidates to ep1 until ep1 receives the peer reflexive
// candidate.
PauseCandidates(1);
@ -1251,14 +1259,21 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignalingWithNAT) {
EXPECT_EQ(kIceUfrag[1],
ep1_ch1()->best_connection()->remote_candidate().username());
EXPECT_EQ("", ep1_ch1()->best_connection()->remote_candidate().password());
// Because we don't have ICE credentials yet, we don't know the generation.
EXPECT_EQ(0u, ep1_ch1()->best_connection()->remote_candidate().generation());
EXPECT_TRUE(nullptr == ep1_ch1()->FindNextPingableConnection());
// Add two sets of remote ICE credentials, so that the ones used by the
// candidate will be generation 1 instead of 0.
ep1_ch1()->SetRemoteIceCredentials(kIceUfrag[3], kIcePwd[3]);
ep1_ch1()->SetRemoteIceCredentials(kIceUfrag[1], kIcePwd[1]);
ResumeCandidates(1);
// After setting the remote ICE credentials, the password and generation
// of the peer reflexive candidate should be updated.
EXPECT_EQ(kIcePwd[1],
ep1_ch1()->best_connection()->remote_candidate().password());
EXPECT_TRUE(nullptr != ep1_ch1()->FindNextPingableConnection());
EXPECT_EQ(1u, ep1_ch1()->best_connection()->remote_candidate().generation());
ResumeCandidates(1);
const cricket::Connection* best_connection = NULL;
WAIT((best_connection = ep2_ch1()->best_connection()) != NULL, 2000);

14
webrtc/p2p/base/port.cc
View File

@ -1298,12 +1298,22 @@ void Connection::HandleRoleConflictFromPeer() {
port_->SignalRoleConflict(port_);
}
void Connection::MaybeSetRemoteIceCredentials(const std::string& ice_ufrag,
const std::string& ice_pwd) {
void Connection::MaybeSetRemoteIceCredentialsAndGeneration(
const std::string& ice_ufrag,
const std::string& ice_pwd,
int generation) {
if (remote_candidate_.username() == ice_ufrag &&
remote_candidate_.password().empty()) {
remote_candidate_.set_password(ice_pwd);
}
// TODO(deadbeef): A value of '0' for the generation is used for both
// generation 0 and "generation unknown". It should be changed to an
// rtc::Optional to fix this.
if (remote_candidate_.username() == ice_ufrag &&
remote_candidate_.password() == ice_pwd &&
remote_candidate_.generation() == 0) {
remote_candidate_.set_generation(generation);
}
}
void Connection::MaybeUpdatePeerReflexiveCandidate(

12
webrtc/p2p/base/port.h
View File

@ -574,10 +574,14 @@ class Connection : public CandidatePairInterface,
uint32_t ComputeNetworkCost() const;
// Update the ICE password of the remote candidate if |ice_ufrag| matches
// the candidate's ufrag, and the candidate's passwrod has not been set.
void MaybeSetRemoteIceCredentials(const std::string& ice_ufrag,
const std::string& ice_pwd);
// Update the ICE password and/or generation of the remote candidate if a
// ufrag in |remote_ice_parameters| matches the candidate's ufrag, and the
// candidate's password and/or ufrag has not been set.
// |remote_ice_parameters| should be a list of known ICE parameters ordered
// by generation.
void MaybeSetRemoteIceCredentialsAndGeneration(const std::string& ice_ufrag,
const std::string& ice_pwd,
int generation);
// If |remote_candidate_| is peer reflexive and is equivalent to
// |new_candidate| except the type, update |remote_candidate_| to