Add ufrag to the ICE candidate signaling.

webrtc/p2p/base/p2ptransportchannel.cc

 /*
  *  Copyright 2004 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 330 matching lines @@
 
 void P2PTransportChannel::SetRemoteIceCredentials(const std::string& ice_ufrag,
                                                   const std::string& ice_pwd) {
   ASSERT(worker_thread_ == rtc::Thread::Current());
   bool ice_restart = false;
   if (!remote_ice_ufrag_.empty() && !remote_ice_pwd_.empty()) {
     ice_restart = (remote_ice_ufrag_ != ice_ufrag) ||
                   (remote_ice_pwd_!= ice_pwd);
   }
 
+  if (ice_restart) {
+    old_remote_ufrags_.insert(remote_ice_ufrag_);
+  }
   remote_ice_ufrag_ = ice_ufrag;
   remote_ice_pwd_ = ice_pwd;
 
   // We need to update the credentials for any peer reflexive candidates.
   std::vector<Connection*>::iterator it = connections_.begin();
   for (; it != connections_.end(); ++it) {
     (*it)->MaybeSetRemoteIceCredentials(ice_ufrag, ice_pwd);
   }
 
   if (ice_restart) {
@@ skipping 274 matching lines @@
   } else {
     LOG(LS_INFO) << "Not switching the best connection on controlled side yet,"
                  << " because it's not writable: " << conn->ToString();
     pending_best_connection_ = conn;
   }
 }
 
 void P2PTransportChannel::AddRemoteCandidate(const Candidate& candidate) {
   ASSERT(worker_thread_ == rtc::Thread::Current());
 
-  uint32_t generation = candidate.generation();
   // Network may not guarantee the order of the candidate delivery. If a
   // remote candidate with an older generation arrives, drop it.
-  if (generation != 0 && generation < remote_candidate_generation_) {
+  const std::string& ufrag = candidate.username();
+  uint32_t generation = candidate.generation();
+  if (!ufrag.empty()) {
+    if (old_remote_ufrags_.find(ufrag) != old_remote_ufrags_.end()) {
+      LOG(LS_WARNING) << "Dropping a remote candidate because its ICE ufrag "
+                      << ufrag << " is old.";

[pthatcher1, 2015/12/04 20:43:50]

Perhaps say "because its ufrag indicates it was for a previous ICE generation"

[honghaiz3, 2015/12/09 23:57:54]

Done.

+      return;
+    }
+    // TODO(honghaiz): Get rid of the checking using generation when all clients
+    // include ufrag in the candidate signaling.
+  } else if (generation != 0 && generation < remote_candidate_generation_) {
     LOG(LS_WARNING) << "Dropping a remote candidate because its generation "
                     << generation
                     << " is lower than the current remote generation "
                     << remote_candidate_generation_;
     return;
   }
 
   // Create connections to this remote candidate.
   CreateConnections(candidate, NULL);
 
   // Resort the connections list, which may have new elements.
   SortConnections();
 }
 
 // Creates connections from all of the ports that we care about to the given
 // remote candidate.  The return value is true if we created a connection from
 // the origin port.
 bool P2PTransportChannel::CreateConnections(const Candidate& remote_candidate,
                                             PortInterface* origin_port) {
   ASSERT(worker_thread_ == rtc::Thread::Current());
 
   Candidate new_remote_candidate(remote_candidate);
+  // Candidate generation is still used for determining the priority.

[pthatcher1, 2015/12/04 20:43:50]

You can remove the "is still"

[honghaiz3, 2015/12/09 23:57:54]

Moved set_generation to AddRemoteIceCandidate()

   new_remote_candidate.set_generation(
       GetRemoteCandidateGeneration(remote_candidate));
   // ICE candidates don't need to have username and password set, but
   // the code below this (specifically, ConnectionRequest::Prepare in
   // port.cc) uses the remote candidates's username.  So, we set it
   // here.
   if (remote_candidate.username().empty()) {
     new_remote_candidate.set_username(remote_ice_ufrag_);
   }
-  if (remote_candidate.password().empty()) {
-    new_remote_candidate.set_password(remote_ice_pwd_);
+  if (new_remote_candidate.username() == remote_ice_ufrag_) {
+    if (remote_candidate.password().empty()) {
+      new_remote_candidate.set_password(remote_ice_pwd_);
+    }
+  } else {
+    LOG(LS_WARNING)
+        << "A remote candidate arrives with a ufrag that was not seen before: "
+        << remote_candidate.username();
   }

[pthatcher1, 2015/12/04 20:43:50]

When is the pwd on the candidate going to be filled in?  I see that it is in the
unit test, but it's not clear when that happens.  A comment explaining when that
happens would be useful.

[honghaiz3, 2015/12/09 23:57:54]

Done.

 
   // If we've already seen the new remote candidate (in the current candidate
   // generation), then we shouldn't try creating connections for it.
   // We either already have a connection for it, or we previously created one
   // and then later pruned it. If we don't return, the channel will again
   // re-create any connections that were previously pruned, which will then
   // immediately be re-pruned, churning the network for no purpose.
   // This only applies to candidates received over signaling (i.e. origin_port
   // is NULL).
   if (!origin_port && IsDuplicateRemoteCandidate(new_remote_candidate)) {
@@ skipping 73 matching lines @@
     cricket::Connection* connection) const {
   std::vector<Connection*>::const_iterator citer =
       std::find(connections_.begin(), connections_.end(), connection);
   return citer != connections_.end();
 }
 
 uint32_t P2PTransportChannel::GetRemoteCandidateGeneration(
     const Candidate& candidate) {
   // We need to keep track of the remote ice restart so newer
   // connections are prioritized over the older.
-  ASSERT(candidate.generation() == 0 ||
-         candidate.generation() == remote_candidate_generation_);
-  return remote_candidate_generation_;
+  // The candidate generation should be one of 0, remote_candidate_generation_,
+  // and remote_candidate_generation_ + 1.
+  return std::max(remote_candidate_generation_, candidate.generation());

[pthatcher1, 2015/12/04 20:43:50]

I think this should go lookup the generation by looking in remote_generations_
for which ufrag matches.  If none do, then use candidate.generation().  If
that's not set, then assume it's the latest.

[honghaiz3, 2015/12/09 23:57:54]

Done.

 }
 
 // Check if remote candidate is already cached.
 bool P2PTransportChannel::IsDuplicateRemoteCandidate(
     const Candidate& candidate) {
   for (size_t i = 0; i < remote_candidates_.size(); ++i) {
     if (remote_candidates_[i].IsEquivalent(candidate)) {
       return true;
     }
   }
   return false;
 }
 
 // Maintain our remote candidate list, adding this new remote one.
 void P2PTransportChannel::RememberRemoteCandidate(
     const Candidate& remote_candidate, PortInterface* origin_port) {
   // Remove any candidates whose generation is older than this one.  The
   // presence of a new generation indicates that the old ones are not useful.
+  ASSERT(!remote_candidate.username().empty());
   size_t i = 0;
   while (i < remote_candidates_.size()) {
-    if (remote_candidates_[i].generation() < remote_candidate.generation()) {
+    if (remote_candidates_[i].username() != remote_candidate.username()) {
       LOG(INFO) << "Pruning candidate from old generation: "
                 << remote_candidates_[i].address().ToSensitiveString();
       remote_candidates_.erase(remote_candidates_.begin() + i);
     } else {
       i += 1;
     }
   }
 
   // Make sure this candidate is not a duplicate.
   if (IsDuplicateRemoteCandidate(remote_candidate)) {
@@ skipping 544 matching lines @@
   SignalSentPacket(this, sent_packet);
 }
 
 void P2PTransportChannel::OnReadyToSend(Connection* connection) {
   if (connection == best_connection_ && writable()) {
     SignalReadyToSend(this);
   }
 }
 
 }  // namespace cricket