Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(319)

Unified Diff: webrtc/p2p/base/port.cc

Issue 1351673003: Replace readable with receiving where receiving means receiving anything (stun ping, response or da… (Closed) Base URL: https://chromium.googlesource.com/external/webrtc@master
Patch Set: Created 5 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: webrtc/p2p/base/port.cc
diff --git a/webrtc/p2p/base/port.cc b/webrtc/p2p/base/port.cc
index 8048201754b737dabdfa1901e020521cacc4ef84..c29b85062ab77cc4b980fea2790178e28f24378d 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),
@@ -798,7 +799,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
@@ -839,16 +841,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;
@@ -856,7 +848,14 @@ void Connection::set_write_state(WriteState value) {
LOG_J(LS_VERBOSE, this) << "set_write_state from: " << old_value << " to "
<< value;
SignalStateChange(this);
- CheckTimeout();
+ }
+}
+
+void Connection::set_receiving(bool value) {
+ if (value != receiving_) {
+ LOG_J(LS_VERBOSE, this) << "set_receiving to " << value;
+ receiving_ = value;
+ SignalStateChange(this);
}
}
@@ -901,10 +900,12 @@ void Connection::OnReadPacket(
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!
+ // If this connection received a ping or a ping response before, then pass
+ // along the packet. If the remote side is ICE-LITE, it may never receive
+ // a ping, but it needs to receive a ping response before receiving a data
+ // packet.
+ if (last_ping_received_ != 0 || last_ping_response_received_ != 0) {
+ set_receiving(true);
last_data_received_ = rtc::Time();
recv_rate_tracker_.Update(size);
SignalReadPacket(this, data, size, packet_time);
@@ -990,12 +991,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:
@@ -1022,8 +1018,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) {
@@ -1087,7 +1082,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_,
@@ -1099,6 +1093,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();
+ }
}
void Connection::Ping(uint32 now) {
@@ -1112,8 +1121,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() {
@@ -1122,6 +1131,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();
@@ -1139,10 +1149,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
@@ -1170,7 +1179,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() << "|";
@@ -1195,11 +1204,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 = (
@@ -1266,19 +1270,6 @@ void Connection::OnConnectionRequestSent(ConnectionRequest* request) {
<< ", use_candidate=" << use_candidate;
}
-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) {
- port_->thread()->Post(this, MSG_DELETE);
- }
-}
-
void Connection::HandleRoleConflictFromPeer() {
port_->SignalRoleConflict(port_);
}

Powered by Google App Engine
This is Rietveld 408576698