Delete a connection after it is pruned or becomes write_timeout

webrtc/p2p/base/port.cc

Index: webrtc/p2p/base/port.cc
diff --git a/webrtc/p2p/base/port.cc b/webrtc/p2p/base/port.cc
index ddf38dcb189cfe90e1a1ecfa0278a8837d109271..02c67e0c26b1be5fea3608823b7a0b4d9386e749 100644
--- a/webrtc/p2p/base/port.cc
+++ b/webrtc/p2p/base/port.cc
@@ -1124,12 +1124,14 @@ void Connection::ReceivedPingResponse() {
 }
 
 bool Connection::dead(uint32_t now) const {
-  if (now < (time_created_ms_ + MIN_CONNECTION_LIFETIME)) {
-    // A connection that hasn't passed its minimum lifetime is still alive.
-    // We do this to prevent connections from being pruned too quickly
-    // during a network change event when two networks would be up
-    // simultaneously but only for a brief period.
-    return false;
+  if (last_received() == 0) {
+    // If a connection has never received anything, it will be dead only if
+    // it is inactive and has passed its minimum lifetime. We do this to prevent
+    // a connection may be deleted too quickly because it has not got a chance
+    // to be pinged, or it may be pruned too quickly during a network change
+    // event when two networks would be up simultaneously but only for a
+    // brief period.
+    return !active() && now > (time_created_ms_ + MIN_CONNECTION_LIFETIME);
   }

[pthatcher1, 2015/12/30 17:12:28]

Let me see if I understand the purpose of this CL correctly:  Currently, we are
creating a connection, it's still actively pinging, but it hasn't received
anything yet, and we decide it's dead, perhaps because it never had enough time
to ping and receive something.  

In that case, I think something like this might be more readable:

if (last_received() > 0) {
  // If it has ever received anything, we keep it alive until it hasn't received
anything for DEAD_CONNECTION_RECEIVE_TIMEOUT
  // This covers the normal case of a successfully used connection that stops
working.
  // This also allows a remote peer to continue pinging over a locally inactive
(pruned) connection.
  return (now > (last_received() + DEAD_CONNECTION_RECEIVE_TIMEOUT)); 
}

if (active()) {
  // If it has never received anything, but it is actively pinging (not pruned),

  // it is alive at least as long as it is pinging.  Otherwise, the connection
might be deleted
  // before it has a chance to ping.
  // This is the normal case for a new connection that is pinging but hasn't
received anything yet.
  return false;
}

// if it has never received anything and it not actively pinging (pruned), but
we keep it around at least MIN_CONNECTION_LIFETIME
// to prevent connections from being pruned too quickly during a network change
event when two networks would be up
// simultaneously but only for a brief period.
return now > (time_created_ms_ + MIN_CONNECTION_LIFETIME);

[honghaiz3, 2015/12/30 19:38:18]

Yes. More precisely, having 10 seconds for the minimum lifetime is not enough in
some cases. Some good connection may not have a chance to send a ping before it
is deleted.
This happened sometimes when both sides have two networks (so more connections
are created). 

 
   // It is dead if it has not received anything for