Delete a connection only if it has timed out on writing and not receiving for 10 seconds.

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 15ac006872e9087bf8af8cbe1fde783fd29f1160..bbd7c1cf0478fe0501d5e32ed9e2cdc95b0b74cb 100644
--- a/webrtc/p2p/base/p2ptransportchannel.cc
+++ b/webrtc/p2p/base/p2ptransportchannel.cc
@@ -286,23 +286,18 @@ void P2PTransportChannel::SetIceTiebreaker(uint64 tiebreaker) {
   tiebreaker_ = tiebreaker;
 }
 
-// Currently a channel is considered ICE completed once there is no
-// more than one connection per Network. This works for a single NIC
-// with both IPv4 and IPv6 enabled. However, this condition won't
-// happen when there are multiple NICs and all of them have
-// connectivity.
-// TODO(guoweis): Change Completion to be driven by a channel level
-// timer.
+// Currently a channel is considered ICE completed once there is at least one

[pthatcher1, 2015/09/28 23:07:27]

Just remove "Currently"  and say "A channel ..."

[honghaiz3, 2015/09/29 18:47:49]

Done.

+// connection that has not timed out on writing and there is no more than one
+// such connection per Network.

[pthatcher1, 2015/09/28 23:07:27]

This would be more clear as "once there is at least one active connection and at
most one active connection per network".

[honghaiz3, 2015/09/29 18:47:49]

Done.

 TransportChannelState P2PTransportChannel::GetState() const {
+  bool has_non_write_timeout_connections = false;

[pthatcher1, 2015/09/28 23:07:27]

I think this would be much more clear as:

bool Connection::Active() {
  return write_state() != WRITE_STATE_TIMEOUT;
}


if (!had_connection_) {
  return TransportChannelState::STATE_INIT;
}

std::vector<Connection*> active_connections;
for (Connection connection : connections_) {
  if (connection->Active()) {
    active_connections.push_back(connection);
  }
}
if (active_connections.empty()) {
  return TransportChannelState::STATE_FAILED;
}

for (Connection* connection : active_connections) {
// ... rest of the method as is

[honghaiz3, 2015/09/29 18:47:49]

Done.

   std::set<rtc::Network*> networks;
-
-  if (connections_.empty()) {
-    return had_connection_ ? TransportChannelState::STATE_FAILED
-                           : TransportChannelState::STATE_INIT;
-  }
-
-  for (uint32 i = 0; i < connections_.size(); ++i) {
-    rtc::Network* network = connections_[i]->port()->Network();
+  for (Connection* connection : connections_) {
+    if (connection->write_state() == Connection::STATE_WRITE_TIMEOUT) {
+      continue;
+    }
+    has_non_write_timeout_connections = true;
+    rtc::Network* network = connection->port()->Network();
     if (networks.find(network) == networks.end()) {
       networks.insert(network);
     } else {
@@ -312,9 +307,12 @@ TransportChannelState P2PTransportChannel::GetState() const {
       return TransportChannelState::STATE_CONNECTING;
     }
   }
-  LOG_J(LS_VERBOSE, this) << "Ice is completed for this channel.";
-
-  return TransportChannelState::STATE_COMPLETED;
+  if (has_non_write_timeout_connections) {
+    LOG_J(LS_VERBOSE, this) << "Ice is completed for this channel.";
+    return TransportChannelState::STATE_COMPLETED;
+  }
+  return had_connection_ ? TransportChannelState::STATE_FAILED
+                         : TransportChannelState::STATE_INIT;
 }
 
 void P2PTransportChannel::SetIceCredentials(const std::string& ice_ufrag,
@@ -872,8 +870,11 @@ bool P2PTransportChannel::GetStats(ConnectionInfos *infos) {
   infos->clear();
 
   std::vector<Connection *>::const_iterator it;
-  for (it = connections_.begin(); it != connections_.end(); ++it) {
-    Connection* connection = *it;
+  for (Connection* connection : connections_) {
+    if (connection->write_state() == Connection::STATE_WRITE_TIMEOUT &&
+        !connection->receiving()) {

[pthatcher1, 2015/09/28 23:07:27]

This is basically saying "Don't report stats for connections that we keep around
only because they haven't had a minimum lifetime".  Why wouldn't we keep around
stats for such connections?

[honghaiz3, 2015/09/29 18:47:49]

I wanted to keep the same app behavior, but think it is OK to keep them around.
I think it will be dropped anyway later in webrtcsession, though. 

+      continue;
+    }
     ConnectionInfo info;
     info.best_connection = (best_connection_ == connection);
     info.receiving = connection->receiving();