Create a new connection if a candidate reuses an address

webrtc/p2p/base/p2ptransportchannel.cc

Index: webrtc/p2p/base/p2ptransportchannel.cc
diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc
index b1de3e91d3d4a3ee5bca7221e515a10c4aae090c..a2fb774456d3b71d6478677657fa3a12bde994ad 100644
--- a/webrtc/p2p/base/p2ptransportchannel.cc
+++ b/webrtc/p2p/base/p2ptransportchannel.cc
@@ -855,40 +855,41 @@ bool P2PTransportChannel::CreateConnection(PortInterface* port,
     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();
-      return false;
-    }
-  } else {
-    PortInterface::CandidateOrigin origin = GetOrigin(port, origin_port);
-
-    // Don't create connection if this is a candidate we received in a
+  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.
-    if (origin == PortInterface::ORIGIN_MESSAGE && incoming_only_)
+    PortInterface::CandidateOrigin origin = GetOrigin(port, origin_port);
+    if (origin == PortInterface::ORIGIN_MESSAGE && incoming_only_) {
       return false;
-
-    connection = port->CreateConnection(remote_candidate, origin);
-    if (!connection)
+    }
+    Connection* 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 {