Replace readable with receiving where receiving means receiving anything (stun ping, response or da…

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 da66928db284ae8810acac8299ec89dbe2d53ea1..940664e9ef1745102e5cf395f2361615bdd2f0c8 100644
--- a/webrtc/p2p/base/port.cc
+++ b/webrtc/p2p/base/port.cc
@@ -782,8 +782,8 @@ Connection::Connection(Port* port,
     : port_(port),
       local_candidate_index_(index),
       remote_candidate_(remote_candidate),
-      read_state_(STATE_READ_INIT),
       write_state_(STATE_WRITE_INIT),
+      receiving_(false),
       connected_(true),
       pruned_(false),
       use_candidate_attr_(false),
@@ -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),
@@ -800,7 +801,8 @@ Connection::Connection(Port* port,
       sent_packets_discarded_(0),
       sent_packets_total_(0),
       reported_(false),
-      state_(STATE_WAITING) {
+      state_(STATE_WAITING),
+      receiving_timeout_(WEAK_CONNECTION_RECEIVE_TIMEOUT) {
   // All of our connections start in WAITING state.
   // TODO(mallinath) - Start connections from STATE_FROZEN.
   // Wire up to send stun packets
@@ -841,16 +843,6 @@ uint64 Connection::priority() const {
   return priority;
 }
 
-void Connection::set_read_state(ReadState value) {
-  ReadState old_value = read_state_;
-  read_state_ = value;
-  if (value != old_value) {
-    LOG_J(LS_VERBOSE, this) << "set_read_state";
-    SignalStateChange(this);
-    CheckTimeout();
-  }
-}
-
 void Connection::set_write_state(WriteState value) {
   WriteState old_value = write_state_;
   write_state_ = value;
@@ -862,6 +854,15 @@ void Connection::set_write_state(WriteState value) {
   }
 }
 
+void Connection::set_receiving(bool value) {
+  if (value != receiving_) {
+    LOG_J(LS_VERBOSE, this) << "set_receiving to " << value;
+    receiving_ = value;
+    SignalStateChange(this);
+    CheckTimeout();
+  }
+}
+
 void Connection::set_state(State state) {
   State old_state = state_;
   state_ = state;
@@ -902,27 +903,17 @@ void Connection::OnReadPacket(
   const rtc::SocketAddress& addr(remote_candidate_.address());
   if (!port_->GetStunMessage(data, size, addr, msg.accept(), &remote_ufrag)) {
     // The packet did not parse as a valid STUN message
-
-    // If this connection is readable, then pass along the packet.
-    if (read_state_ == STATE_READABLE) {
-      // readable means data from this address is acceptable
-      // Send it on!
-      last_data_received_ = rtc::Time();
-      recv_rate_tracker_.AddSamples(size);
-      SignalReadPacket(this, data, size, packet_time);
-
-      // If timed out sending writability checks, start up again
-      if (!pruned_ && (write_state_ == STATE_WRITE_TIMEOUT)) {
-        LOG(LS_WARNING) << "Received a data packet on a timed-out Connection. "
-                        << "Resetting state to STATE_WRITE_INIT.";
-        set_write_state(STATE_WRITE_INIT);
-      }
-    } else {
-      // Not readable means the remote address hasn't sent a valid
-      // binding request yet.
-
-      LOG_J(LS_WARNING, this)
-        << "Received non-STUN packet from an unreadable connection.";
+    // This is a data packet, pass it along.
+    set_receiving(true);
+    last_data_received_ = rtc::Time();
+    recv_rate_tracker_.AddSamples(size);
+    SignalReadPacket(this, data, size, packet_time);
+
+    // If timed out sending writability checks, start up again
+    if (!pruned_ && (write_state_ == STATE_WRITE_TIMEOUT)) {
+      LOG(LS_WARNING) << "Received a data packet on a timed-out Connection. "
+                      << "Resetting state to STATE_WRITE_INIT.";
+      set_write_state(STATE_WRITE_INIT);

[pthatcher1, 2015/09/18 21:51:33]

Interesting.  This basically means "ping every connection for which we are
receiving data".  That's kind of like "unprune if we receive data".  We should
remember to address this in your second CL when we change the definition of
"pruned".

[honghaiz3, 2015/09/18 22:03:02]

We need to un-prune sometimes. We also need this in other cases. 

     }
   } else if (!msg) {
     // The packet was STUN, but failed a check and was handled internally.
@@ -992,12 +983,7 @@ void Connection::OnReadPacket(
       // Otherwise we can mark connection to read timeout. No response will be
       // sent in this scenario.
       case STUN_BINDING_INDICATION:
-        if (read_state_ == STATE_READABLE) {
-          ReceivedPing();
-        } else {
-          LOG_J(LS_WARNING, this) << "Received STUN binding indication "
-                                  << "from an unreadable connection.";
-        }
+        ReceivedPing();
         break;
 
       default:
@@ -1024,8 +1010,7 @@ void Connection::Prune() {
 
 void Connection::Destroy() {
   LOG_J(LS_VERBOSE, this) << "Connection destroyed";
-  set_read_state(STATE_READ_TIMEOUT);
-  set_write_state(STATE_WRITE_TIMEOUT);
+  port_->thread()->Post(this, MSG_DELETE);
 }
 
 void Connection::PrintPingsSinceLastResponse(std::string* s, size_t max) {
@@ -1089,7 +1074,6 @@ void Connection::UpdateState(uint32 now) {
                          << " rtt=" << rtt;
     set_write_state(STATE_WRITE_UNRELIABLE);
   }
-
   if ((write_state_ == STATE_WRITE_UNRELIABLE ||
        write_state_ == STATE_WRITE_INIT) &&
       TooLongWithoutResponse(pings_since_last_response_,
@@ -1101,6 +1085,21 @@ void Connection::UpdateState(uint32 now) {
                          << ", rtt=" << rtt;
     set_write_state(STATE_WRITE_TIMEOUT);
   }
+
+  // Check the receiving state.
+  uint32 last_recv_time = last_received();
+  bool receiving = now <= last_recv_time + receiving_timeout_;
+  set_receiving(receiving);
+  if (receiving) {
+    return;
+  }
+  // If this connection has never received anything, use the connection
+  // creating time as the starting time for connection timeout.
+  if (now > std::max(last_recv_time, time_created_) +
+      DEAD_CONNECTION_RECEIVE_TIMEOUT) {
+    // Delete self.
+    Destroy();
+  }

[pthatcher1, 2015/09/18 21:51:33]

Doesn't CheckTimeout() handle this?  I think we can leave this part off (and
time_created_ also).

[honghaiz3, 2015/09/18 22:03:02]

Done.

 }
 
 void Connection::Ping(uint32 now) {
@@ -1114,8 +1113,8 @@ void Connection::Ping(uint32 now) {
 }
 
 void Connection::ReceivedPing() {
+  set_receiving(true);
   last_ping_received_ = rtc::Time();
-  set_read_state(STATE_READABLE);
 }
 
 void Connection::ReceivedPingResponse() {
@@ -1124,6 +1123,7 @@ void Connection::ReceivedPingResponse() {
   // So if we're not already, become writable. We may be bringing a pruned
   // connection back to life, but if we don't really want it, we can always
   // prune it again.
+  set_receiving(true);
   set_write_state(STATE_WRITABLE);
   set_state(STATE_SUCCEEDED);
   pings_since_last_response_.clear();
@@ -1141,10 +1141,9 @@ std::string Connection::ToString() const {
     '-',  // not connected (false)
     'C',  // connected (true)
   };
-  const char READ_STATE_ABBREV[3] = {
-    '-',  // STATE_READ_INIT
-    'R',  // STATE_READABLE
-    'x',  // STATE_READ_TIMEOUT
+  const char RECEIVE_STATE_ABBREV[2] = {
+    '-',  // not receiving (false)
+    'R',  // receiving (true)
   };
   const char WRITE_STATE_ABBREV[4] = {
     'W',  // STATE_WRITABLE
@@ -1172,7 +1171,7 @@ std::string Connection::ToString() const {
      << ":" << remote.type() << ":"
      << remote.protocol() << ":" << remote.address().ToSensitiveString() << "|"
      << CONNECT_STATE_ABBREV[connected()]
-     << READ_STATE_ABBREV[read_state()]
+     << RECEIVE_STATE_ABBREV[receiving()]
      << WRITE_STATE_ABBREV[write_state()]
      << ICESTATE[state()] << "|"
      << priority() << "|";
@@ -1197,11 +1196,6 @@ void Connection::OnConnectionRequestResponse(ConnectionRequest* request,
   uint32 rtt = request->Elapsed();
 
   ReceivedPingResponse();
-  if (remote_ice_mode_ == ICEMODE_LITE) {
-    // A ice-lite end point never initiates ping requests. This will allow
-    // us to move to STATE_READABLE without an incoming ping request.
-    set_read_state(STATE_READABLE);
-  }
 
   if (LOG_CHECK_LEVEL_V(sev)) {
     bool use_candidate = (
@@ -1269,14 +1263,8 @@ void Connection::OnConnectionRequestSent(ConnectionRequest* request) {
 }
 
 void Connection::CheckTimeout() {
-  // If both read and write have timed out or read has never initialized, then
-  // this connection can contribute no more to p2p socket unless at some later
-  // date readability were to come back.  However, we gave readability a long
-  // time to timeout, so at this point, it seems fair to get rid of this
-  // connection.
-  if ((read_state_ == STATE_READ_TIMEOUT ||
-       read_state_ == STATE_READ_INIT) &&
-      write_state_ == STATE_WRITE_TIMEOUT) {
+  // 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);
   }
 }