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 200 matching lines @@
       allocator_(allocator),
       worker_thread_(rtc::Thread::Current()),
       incoming_only_(false),
       error_(0),
       best_connection_(NULL),
       pending_best_connection_(NULL),
       sort_dirty_(false),
       remote_ice_mode_(ICEMODE_FULL),
       ice_role_(ICEROLE_UNKNOWN),
       tiebreaker_(0),
-      remote_candidate_generation_(0),
       gathering_state_(kIceGatheringNew),
       check_receiving_delay_(MIN_CHECK_RECEIVING_DELAY * 5),
       receiving_timeout_(MIN_CHECK_RECEIVING_DELAY * 50),
       backup_connection_ping_interval_(0) {
   uint32_t weak_ping_delay = ::strtoul(
       webrtc::field_trial::FindFullName("WebRTC-StunInterPacketDelay").c_str(),
       nullptr, 10);
   if (weak_ping_delay) {
     weak_ping_delay_ =  weak_ping_delay;
   }
@@ skipping 108 matching lines @@
   ASSERT(worker_thread_ == rtc::Thread::Current());
   ice_ufrag_ = ice_ufrag;
   ice_pwd_ = ice_pwd;
   // Note: Candidate gathering will restart when MaybeStartGathering is next
   // called.
 }
 
 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 (remote_ice_generations_.empty() || (remote_ice_ufrag() != ice_ufrag) ||
+      (remote_ice_pwd() != ice_pwd)) {
+    // Keep the old-generation ICE credentials so that newer connections
+    // are prioritized over the older ones.
+    remote_ice_generations_.push_back(IceGeneration(ice_ufrag, ice_pwd));
   }

[pthatcher1, 2015/12/10 22:08:06]

I think this might be a little better using an optional and operator==:

Optional<IceParameters> gen1 = remote_ice();
IceParaemeters gen2 = IceParameters(ice_ufrag, ice_pwd);
if (!gen1 || *gen1 != gen2) {
  remote_ice_generations_.push_back(gen2);
}

[honghaiz3, 2015/12/11 04:47:29]

Done.

 
-  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) {
-    // We need to keep track of the remote ice restart so newer
-    // connections are prioritized over the older.
-    ++remote_candidate_generation_;
-  }
 }
 
 void P2PTransportChannel::SetRemoteIceMode(IceMode mode) {
   remote_ice_mode_ = mode;
 }
 
 void P2PTransportChannel::SetIceConfig(const IceConfig& config) {
   gather_continually_ = config.gather_continually;
   LOG(LS_INFO) << "Set gather_continually to " << gather_continually_;
 
@@ skipping 149 matching lines @@
         break;
       }
       // We don't want to break here because we may find a match of the address
       // later.
     }
   }
 
   // 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() && remote_username == remote_ice_ufrag_) {
-    remote_password = remote_ice_pwd_;
+  if (remote_password.empty() && !remote_ice_generations_.empty() &&
+      remote_username == remote_ice_ufrag()) {
+    remote_password = remote_ice_pwd();
   }

[pthatcher1, 2015/12/10 22:08:06]

Same here, with optional:

if (remote_password.empty() && remote_ice()) {
  remote_password = remote_ice().pwd;
}

[honghaiz3, 2015/12/11 04:47:29]

Done.

 
   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.
@@ skipping 96 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_) {
-    LOG(LS_WARNING) << "Dropping a remote candidate because its generation "
-                    << generation
-                    << " is lower than the current remote generation "
-                    << remote_candidate_generation_;
+  int generation = GetRemoteCandidateGeneration(candidate);
+  // If a remote candidate with a previous generation arrives, drop it.
+  if (generation < remote_ice_generation()) {
+    LOG(LS_WARNING) << "Dropping a remote candidate because its ufrag "
+                    << candidate.username()
+                    << " indicates it was for an previous generation.";
     return;
   }
 
+  Candidate new_remote_candidate(candidate);
+  new_remote_candidate.set_generation(generation);
+  // 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_ice_generations_.empty()) {
+    if (candidate.username().empty()) {
+      new_remote_candidate.set_username(remote_ice_ufrag());
+    }
+    if (new_remote_candidate.username() == remote_ice_ufrag()) {
+      if (candidate.password().empty()) {
+        new_remote_candidate.set_password(remote_ice_pwd());
+      }
+    } else {
+      // The candidate belongs to the next generation. Its pwd will be set
+      // when the new remote ICE credentials arrive.
+      LOG(LS_WARNING) << "A remote candidate arrives with an unknown ufrag: "
+                      << candidate.username();
+    }
+  }
+
   // Create connections to this remote candidate.
-  CreateConnections(candidate, NULL);
+  CreateConnections(new_remote_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);
-  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 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)) {
+  if (!origin_port && IsDuplicateRemoteCandidate(remote_candidate)) {
     // return true to indicate success, without creating any new connections.
     return true;
   }
 
   // Add a new connection for this candidate to every port that allows such a
   // connection (i.e., if they have compatible protocols) and that does not
   // already have a connection to an equivalent candidate.  We must be careful
   // to make sure that the origin port is included, even if it was pruned,
   // since that may be the only port that can create this connection.
   bool created = false;
   std::vector<PortInterface *>::reverse_iterator it;
   for (it = ports_.rbegin(); it != ports_.rend(); ++it) {
-    if (CreateConnection(*it, new_remote_candidate, origin_port)) {
+    if (CreateConnection(*it, remote_candidate, origin_port)) {
       if (*it == origin_port)
         created = true;
     }
   }
 
   if ((origin_port != NULL) &&
       std::find(ports_.begin(), ports_.end(), origin_port) == ports_.end()) {
-    if (CreateConnection(origin_port, new_remote_candidate, origin_port))
+    if (CreateConnection(origin_port, remote_candidate, origin_port))
       created = true;
   }
 
   // Remember this remote candidate so that we can add it to future ports.
-  RememberRemoteCandidate(new_remote_candidate, origin_port);
+  RememberRemoteCandidate(remote_candidate, origin_port);
 
   return created;
 }
 
 // Setup a connection object for the local and remote candidate combination.
 // And then listen to connection object for changes.
 bool P2PTransportChannel::CreateConnection(PortInterface* port,
                                            const Candidate& remote_candidate,
                                            PortInterface* origin_port) {
   // Look for an existing connection with this remote address.  If one is not
@@ skipping 37 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_;
+  const std::string& ufrag = candidate.username();
+  int generation = 0;
+  if (!ufrag.empty()) {
+    // If remote side sets the ufrag, we use that to determine the candidate
+    // generation.
+    generation = remote_ice_generations_.size() - 1;
+    for (; generation >= 0; --generation) {
+      if (ufrag == remote_ice_generations_[generation].ufrag) {
+        return generation;
+      }
+    }
+    // If not found, this is the next generation candidate.
+    return remote_ice_generations_.size();
+  }

[pthatcher1, 2015/12/10 22:08:06]

I think this could be more readable by using std::find_if;

const auto& params = remote_ice_parameters_;
if (!candidate.username().empty()) {
  auto it = std::find_if(params.begin(), params.end(), [candidate](const
IceParameters& ice) {
    return ice.ufrag = candidate.username();
  });
  if (it == params.end()) {
    // If not found, assume it's the next (future) generation;
    return remote_ice_generation() + 1;
  }
  return it - params.begin();
}

[honghaiz3, 2015/12/11 04:47:29]

Done.

+  // Use the generation set in the candidate.
+  generation = candidate.generation();
+  int cur_remote_generation = remote_ice_generation();
+  if (generation == 0 && cur_remote_generation > 0) {
+    // 0 indicates it may not be set by the peer.
+    generation = cur_remote_generation;
+  }
+  return generation;

[pthatcher1, 2015/12/10 22:08:06]

I think this would be readable as:

// If candidate generation is set, use that.
if (candidate.generation() > 0) {
  return candidate.generation();
}

// If we have remote parameters, use that.  
if (!params.empty()) {
  return remote_ice_generation();
}

// Otherwise, assume it's generation 0.
return 0;

[honghaiz3, 2015/12/11 04:47:29]

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;
     }
   }
@@ skipping 592 matching lines @@
   SignalSentPacket(this, sent_packet);
 }
 
 void P2PTransportChannel::OnReadyToSend(Connection* connection) {
   if (connection == best_connection_ && writable()) {
     SignalReadyToSend(this);
   }
 }
 
 }  // namespace cricket