Set the generation on peer reflexive candidates when created.

If an actual peer reflexive candidate was created (and not one that
would just be replaced by a different candidate later), we weren't
setting the generation value. This means that new-generation prflx
candidate pairs weren't being prioritized above the cross-generation
pairs, or above relay<->relay pairs.

R=honghaiz@webrtc.org, pthatcher@webrtc.org

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

Cr-Commit-Position: refs/heads/master@{#13259}
This commit is contained in:
Taylor Brandstetter 2016-06-22 13:13:55 -07:00
parent 329c9407e0
commit f7c15a9159
3 changed files with 30 additions and 38 deletions

View File

@ -577,56 +577,31 @@ void P2PTransportChannel::OnUnknownAddress(
// is currently available for. See if we already have a candidate with the
// address. If it isn't we need to create new candidate for it.
// Determine if the remote candidates use shared ufrag.
bool ufrag_per_port = false;
std::vector<RemoteCandidate>::iterator it;
if (remote_candidates_.size() > 0) {
it = remote_candidates_.begin();
std::string username = it->username();
for (; it != remote_candidates_.end(); ++it) {
if (it->username() != username) {
ufrag_per_port = true;
break;
}
}
}
const Candidate* candidate = NULL;
std::string remote_password;
for (it = remote_candidates_.begin(); it != remote_candidates_.end(); ++it) {
if (it->username() == remote_username) {
remote_password = it->password();
if (ufrag_per_port ||
(it->address() == address &&
it->protocol() == ProtoToString(proto))) {
candidate = &(*it);
break;
}
// We don't want to break here because we may find a match of the address
// later.
const Candidate* candidate = nullptr;
for (const Candidate& c : remote_candidates_) {
if (c.username() == remote_username && c.address() == address &&
c.protocol() == ProtoToString(proto)) {
candidate = &c;
break;
}
}
uint32_t remote_generation = 0;
std::string remote_password;
// The STUN binding request may arrive after setRemoteDescription and before
// adding remote candidate, so we need to set the password to the shared
// password if the user name matches.
if (remote_password.empty()) {
const IceParameters* ice_param =
FindRemoteIceFromUfrag(remote_username, &remote_generation);
// Note: if not found, the remote_generation will still be 0.
if (ice_param != nullptr) {
remote_password = ice_param->pwd;
}
// password and set the generation if the user name matches.
const IceParameters* ice_param =
FindRemoteIceFromUfrag(remote_username, &remote_generation);
// Note: if not found, the remote_generation will still be 0.
if (ice_param != nullptr) {
remote_password = ice_param->pwd;
}
Candidate remote_candidate;
bool remote_candidate_is_new = (candidate == nullptr);
if (!remote_candidate_is_new) {
remote_candidate = *candidate;
if (ufrag_per_port) {
remote_candidate.set_address(address);
}
} else {
// Create a new candidate with this address.
// The priority of the candidate is set to the PRIORITY attribute

View File

@ -1071,6 +1071,16 @@ TEST_F(P2PTransportChannelTest, HandleUfragPwdChange) {
DestroyChannels();
}
// Same as above test, but with a symmetric NAT.
// We should end up with relay<->prflx candidate pairs, with generation "1".
TEST_F(P2PTransportChannelTest, HandleUfragPwdChangeSymmetricNat) {
ConfigureEndpoints(NAT_SYMMETRIC, NAT_SYMMETRIC, kDefaultPortAllocatorFlags,
kDefaultPortAllocatorFlags);
CreateChannels(1);
TestHandleIceUfragPasswordChanged();
DestroyChannels();
}
// Test the operation of GetStats.
TEST_F(P2PTransportChannelTest, GetStats) {
ConfigureEndpoints(OPEN, OPEN, kDefaultPortAllocatorFlags,
@ -1333,6 +1343,11 @@ TEST_F(P2PTransportChannelTest, TestTcpConnectionsFromActiveToPassive) {
SetAllowTcpListen(0, true); // actpass.
SetAllowTcpListen(1, false); // active.
// We want SetRemoteIceCredentials to be called as it normally would.
// Otherwise we won't know what credentials to use for the expected
// prflx TCP candidates.
set_remote_ice_credential_source(FROM_SETICECREDENTIALS);
// Pause candidate so we could verify the candidate properties.
PauseCandidates(0);
PauseCandidates(1);
@ -1627,6 +1642,7 @@ class P2PTransportChannelSameNatTest : public P2PTransportChannelTestBase {
static_cast<rtc::NATType>(nat_type - NAT_FULL_CONE));
ConfigureEndpoint(outer_nat, 0, config1);
ConfigureEndpoint(outer_nat, 1, config2);
set_remote_ice_credential_source(FROM_SETICECREDENTIALS);
}
void ConfigureEndpoint(rtc::NATSocketServer::Translator* nat,
int endpoint, Config config) {

View File

@ -1485,6 +1485,7 @@ void Connection::MaybeAddPrflxCandidate(ConnectionRequest* request,
new_local_candidate.set_network_name(local_candidate().network_name());
new_local_candidate.set_network_type(local_candidate().network_type());
new_local_candidate.set_related_address(local_candidate().address());
new_local_candidate.set_generation(local_candidate().generation());
new_local_candidate.set_foundation(ComputeFoundation(
PRFLX_PORT_TYPE, local_candidate().protocol(),
local_candidate().relay_protocol(), local_candidate().address()));