A few updates on connection states

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 d6bc27ba023bdf156dd8dd0aeca87f07a355a880..e03c0e60b5b3654b468e8d30b1f9531f8cc95175 100644
--- a/webrtc/p2p/base/port.cc
+++ b/webrtc/p2p/base/port.cc
@@ -791,6 +791,7 @@ Connection::Connection(Port* port,
       remote_ice_mode_(ICEMODE_FULL),
       requests_(port->thread()),
       rtt_(DEFAULT_RTT),
+      time_created_(rtc::Time()),
       last_ping_sent_(0),
       last_ping_received_(0),
       last_data_received_(0),
@@ -849,7 +850,6 @@ void Connection::set_write_state(WriteState value) {
     LOG_J(LS_VERBOSE, this) << "set_write_state from: " << old_value << " to "
                             << value;
     SignalStateChange(this);
-    CheckTimeout();
   }
 }
 
@@ -858,7 +858,6 @@ void Connection::set_receiving(bool value) {
     LOG_J(LS_VERBOSE, this) << "set_receiving to " << value;
     receiving_ = value;
     SignalStateChange(this);
-    CheckTimeout();
   }
 }
 
@@ -913,6 +912,7 @@ void Connection::OnReadPacket(
       LOG(LS_WARNING) << "Received a data packet on a timed-out Connection. "
                       << "Resetting state to STATE_WRITE_INIT.";
       set_write_state(STATE_WRITE_INIT);
+      Unprune();

[pthatcher1, 2015/09/21 23:49:18]

if (!pruned_) Unprune()?

Why would we unprune if we're not pruned?

[honghaiz3, 2015/09/22 19:35:08]

If it is not pruned, then you don't need to unprune it. 
This makes it similar to the original behavior that when a connection receives a
data packet, it will be pulled out of the pruned state, so it will be available
for pinging again. 

     }
   } else if (!msg) {
     // The packet was STUN, but failed a check and was handled internally.
@@ -1003,7 +1003,13 @@ void Connection::Prune() {
     LOG_J(LS_VERBOSE, this) << "Connection pruned";
     pruned_ = true;
     requests_.Clear();
-    set_write_state(STATE_WRITE_TIMEOUT);
+  }
+}
+
+void Connection::Unprune() {
+  if (pruned_) {
+    LOG_J(LS_VERBOSE, this) << "Connection un-pruned";
+    pruned_ = false;
   }
 }
 
@@ -1045,6 +1051,8 @@ void Connection::UpdateState(uint32 now) {
   }
 
   // Check the writable state.  (The order of these checks is important.)
+  // If the connection has not sent stun ping for a while (mostly because

[pthatcher1, 2015/09/21 23:49:18]

sent stun ping => sent a stun ping

[honghaiz3, 2015/09/22 19:35:08]

Done.

+  // it is pruned), it is considered stale.
   //
   // Before becoming unwritable, we allow for a fixed number of pings to fail
   // (i.e., receive no response).  We also have to give the response time to
@@ -1053,7 +1061,12 @@ void Connection::UpdateState(uint32 now) {
   // Before timing out writability, we give a fixed amount of time.  This is to
   // allow for changes in network conditions.
 
-  if ((write_state_ == STATE_WRITABLE) &&
+  if (write_state_ == STATE_WRITABLE &&
+      last_ping_sent_ + CONNECTION_WRITE_STALE_TIMEOUT < now) {
+    LOG_J(LS_INFO, this) << "WriteState becomes stale";
+    set_write_state(STATE_WRITE_STALE);
+  }
+  if ((write_state_ == STATE_WRITABLE || write_state_ == STATE_WRITE_STALE) &&
       TooManyFailures(pings_since_last_response_,
                       CONNECTION_WRITE_CONNECT_FAILURES,
                       rtt,
@@ -1089,6 +1102,16 @@ void Connection::UpdateState(uint32 now) {
   uint32 last_recv_time = last_received();
   bool receiving = now <= last_recv_time + receiving_timeout_;
   set_receiving(receiving);
+  if (!receiving) {
+    // If this connection has not received anything for a long time,
+    // destroy the connection. If the connection has never received anything,
+    // use the connection creating time as the starting time for calculating
+    // the receiving timeout.
+    if (now > std::max(last_recv_time, time_created_) +
+        DEAD_CONNECTION_RECEIVE_TIMEOUT) {
+      Destroy();
+    }
+  }
 }
 
 void Connection::Ping(uint32 now) {
@@ -1134,8 +1157,9 @@ std::string Connection::ToString() const {
     '-',  // not receiving (false)
     'R',  // receiving (true)
   };
-  const char WRITE_STATE_ABBREV[4] = {
+  const char WRITE_STATE_ABBREV[5] = {
     'W',  // STATE_WRITABLE
+    's',  // STATE_WRITE_STALE

[pthatcher1, 2015/09/21 23:49:18]

Maybe a '?' would be good, since it means we don't know since we're not probing
any more.

[honghaiz3, 2015/09/22 19:35:08]

Dropped this (saved for a separate CL). 

     'w',  // STATE_WRITE_UNRELIABLE
     '-',  // STATE_WRITE_INIT
     'x',  // STATE_WRITE_TIMEOUT
@@ -1251,13 +1275,6 @@ void Connection::OnConnectionRequestSent(ConnectionRequest* request) {
                     << ", use_candidate=" << use_candidate;
 }
 
-void Connection::CheckTimeout() {
-  // If write has timed out and it is not receiving, remove the connection.
-  if (!receiving_ && write_state_ == STATE_WRITE_TIMEOUT) {
-    port_->thread()->Post(this, MSG_DELETE);
-  }
-}
-
 void Connection::HandleRoleConflictFromPeer() {
   port_->SignalRoleConflict(port_);
 }