Create a new connection if a candidate reuses an address

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 837 matching lines @@
 
 // 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) {
   if (!port->SupportsProtocol(remote_candidate.protocol())) {
     return false;
   }
   // Look for an existing connection with this remote address.  If one is not
-  // found, then we can create a new connection for this address.
+  // found or it is found but the existing remote candidate has an older
+  // generation, then we can create a new connection for this address.
   Connection* connection = port->GetConnection(remote_candidate.address());
-  if (connection != NULL) {
-    connection->MaybeUpdatePeerReflexiveCandidate(remote_candidate);
-
-    // It is not legal to try to change any of the parameters of an existing
-    // connection; however, the other side can send a duplicate candidate.
-    if (!remote_candidate.IsEquivalent(connection->remote_candidate())) {
-      LOG(INFO) << "Attempt to change a remote candidate."
-                << " Existing remote candidate: "
-                << connection->remote_candidate().ToString()
-                << "New remote candidate: "
-                << remote_candidate.ToString();
+  if (connection == nullptr ||
+      connection->remote_candidate().generation() <
+          remote_candidate.generation()) {
+    // Don't create a connection if this is a candidate we received in a
+    // message and we are not allowed to make outgoing connections.
+    PortInterface::CandidateOrigin origin = GetOrigin(port, origin_port);
+    if (origin == PortInterface::ORIGIN_MESSAGE && incoming_only_) {
       return false;
     }
-  } else {
-    PortInterface::CandidateOrigin origin = GetOrigin(port, origin_port);
-
-    // Don't create connection if this is a candidate we received in a
-    // message and we are not allowed to make outgoing connections.
-    if (origin == PortInterface::ORIGIN_MESSAGE && incoming_only_)
+    Connection* connection = port->CreateConnection(remote_candidate, origin);
+    if (!connection) {
       return false;
-
-    connection = port->CreateConnection(remote_candidate, origin);
-    if (!connection)
-      return false;
-
+    }
     AddConnection(connection);

[pthatcher1, 2016/05/31 20:09:29]

I think this should also be AddOrReplaceConnectionk, since that's what it does.

[honghaiz3, 2016/05/31 23:25:23]

It might look strange to use AddOrRepalceConnection here if you look at the
implementation of the method. We are not replacing anything directly in the
method. It will get replaced simply because the old connection (on the same
remote address) was destroyed and removed from the list. 
WDYT?

[pthatcher1, 2016/06/01 01:57:04]

Ah, you're completely right.  Please keep it AddConnection.

[honghaiz3, 2016/06/01 16:37:59]

Acknowledged.

-
     LOG_J(LS_INFO, this) << "Created connection with origin=" << origin << ", ("
                          << connections_.size() << " total)";
+    return true;
   }
 
-  return true;
+  // No new connection was created.
+  // Check if this is a peer reflexive candidate.
+  connection->MaybeUpdatePeerReflexiveCandidate(remote_candidate);
+
+  // It is not legal to try to change any of the parameters of an existing
+  // connection; however, the other side can send a duplicate candidate.
+  if (!remote_candidate.IsEquivalent(connection->remote_candidate())) {
+    LOG(INFO) << "Attempt to change a remote candidate."
+              << " Existing remote candidate: "
+              << connection->remote_candidate().ToString()
+              << "New remote candidate: " << remote_candidate.ToString();
+  }
+  return false;
 }
 
 bool P2PTransportChannel::FindConnection(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) {
@@ skipping 793 matching lines @@
 
   // During the initial state when nothing has been pinged yet, return the first
   // one in the ordered |connections_|.
   return *(std::find_if(connections_.begin(), connections_.end(),
                         [conn1, conn2](Connection* conn) {
                           return conn == conn1 || conn == conn2;
                         }));
 }
 
 }  // namespace cricket