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

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 268 matching lines @@
   ASSERT(worker_thread_ == rtc::Thread::Current());
   if (!ports_.empty()) {
     LOG(LS_ERROR)
         << "Attempt to change tiebreaker after Port has been allocated.";
     return;
   }
 
   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 {
       LOG_J(LS_VERBOSE, this) << "Ice not completed yet for this channel as "
                               << network->ToString()
                               << " has more than 1 connection.";
       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,
                                             const std::string& ice_pwd) {
   ASSERT(worker_thread_ == rtc::Thread::Current());
   ice_ufrag_ = ice_ufrag;
   ice_pwd_ = ice_pwd;
   // Note: Candidate gathering will restart when MaybeStartGathering is next
   // called.
 }
@@ skipping 537 matching lines @@
   }
   return sent;
 }
 
 bool P2PTransportChannel::GetStats(ConnectionInfos *infos) {
   ASSERT(worker_thread_ == rtc::Thread::Current());
   // Gather connection 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();
     info.writable =
         (connection->write_state() == Connection::STATE_WRITABLE);
     info.timeout =
         (connection->write_state() == Connection::STATE_WRITE_TIMEOUT);
     info.new_connection = !connection->reported();
     connection->set_reported(true);
     info.rtt = connection->rtt();
@@ skipping 458 matching lines @@
   }
 }
 
 void P2PTransportChannel::OnReadyToSend(Connection* connection) {
   if (connection == best_connection_ && writable()) {
     SignalReadyToSend(this);
   }
 }
 
 }  // namespace cricket