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

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

Issue 1356103002: Revert of 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
« no previous file with comments | « webrtc/p2p/base/port.h ('k') | webrtc/p2p/base/port_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: webrtc/p2p/base/port.cc
diff --git a/webrtc/p2p/base/port.cc b/webrtc/p2p/base/port.cc
index d6bc27ba023bdf156dd8dd0aeca87f07a355a880..da66928db284ae8810acac8299ec89dbe2d53ea1 100644
--- a/webrtc/p2p/base/port.cc
+++ b/webrtc/p2p/base/port.cc
@@ -782,8 +782,8 @@
: 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),
@@ -800,8 +800,7 @@
sent_packets_discarded_(0),
sent_packets_total_(0),
reported_(false),
- state_(STATE_WAITING),
- receiving_timeout_(WEAK_CONNECTION_RECEIVE_TIMEOUT) {
+ state_(STATE_WAITING) {
// All of our connections start in WAITING state.
// TODO(mallinath) - Start connections from STATE_FROZEN.
// Wire up to send stun packets
@@ -842,21 +841,22 @@
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;
if (value != old_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);
CheckTimeout();
}
@@ -902,17 +902,27 @@
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
- // 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);
+
+ // 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.";
}
} else if (!msg) {
// The packet was STUN, but failed a check and was handled internally.
@@ -982,7 +992,12 @@
// Otherwise we can mark connection to read timeout. No response will be
// sent in this scenario.
case STUN_BINDING_INDICATION:
- ReceivedPing();
+ if (read_state_ == STATE_READABLE) {
+ ReceivedPing();
+ } else {
+ LOG_J(LS_WARNING, this) << "Received STUN binding indication "
+ << "from an unreadable connection.";
+ }
break;
default:
@@ -1009,7 +1024,8 @@
void Connection::Destroy() {
LOG_J(LS_VERBOSE, this) << "Connection destroyed";
- port_->thread()->Post(this, MSG_DELETE);
+ set_read_state(STATE_READ_TIMEOUT);
+ set_write_state(STATE_WRITE_TIMEOUT);
}
void Connection::PrintPingsSinceLastResponse(std::string* s, size_t max) {
@@ -1073,6 +1089,7 @@
<< " rtt=" << rtt;
set_write_state(STATE_WRITE_UNRELIABLE);
}
+
if ((write_state_ == STATE_WRITE_UNRELIABLE ||
write_state_ == STATE_WRITE_INIT) &&
TooLongWithoutResponse(pings_since_last_response_,
@@ -1084,11 +1101,6 @@
<< ", 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);
}
void Connection::Ping(uint32 now) {
@@ -1102,8 +1114,8 @@
}
void Connection::ReceivedPing() {
- set_receiving(true);
last_ping_received_ = rtc::Time();
+ set_read_state(STATE_READABLE);
}
void Connection::ReceivedPingResponse() {
@@ -1112,7 +1124,6 @@
// 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();
@@ -1130,9 +1141,10 @@
'-', // not connected (false)
'C', // connected (true)
};
- const char RECEIVE_STATE_ABBREV[2] = {
- '-', // not receiving (false)
- 'R', // receiving (true)
+ const char READ_STATE_ABBREV[3] = {
+ '-', // STATE_READ_INIT
+ 'R', // STATE_READABLE
+ 'x', // STATE_READ_TIMEOUT
};
const char WRITE_STATE_ABBREV[4] = {
'W', // STATE_WRITABLE
@@ -1160,7 +1172,7 @@
<< ":" << remote.type() << ":"
<< remote.protocol() << ":" << remote.address().ToSensitiveString() << "|"
<< CONNECT_STATE_ABBREV[connected()]
- << RECEIVE_STATE_ABBREV[receiving()]
+ << READ_STATE_ABBREV[read_state()]
<< WRITE_STATE_ABBREV[write_state()]
<< ICESTATE[state()] << "|"
<< priority() << "|";
@@ -1185,6 +1197,11 @@
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 = (
@@ -1252,8 +1269,14 @@
}
void Connection::CheckTimeout() {
- // If write has timed out and it is not receiving, remove the connection.
- if (!receiving_ && write_state_ == STATE_WRITE_TIMEOUT) {
+ // 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);
}
}
« no previous file with comments | « webrtc/p2p/base/port.h ('k') | webrtc/p2p/base/port_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698