From b594041ec8a3ae9f501260e2456d9d5ce6482819 Mon Sep 17 00:00:00 2001 From: Guo-wei Shieh Date: Mon, 24 Aug 2015 11:58:03 -0700 Subject: [PATCH] TcpPort Reconnect should inform upper layer to start sending again. During the reconnection phase, EWOULDBLOCK has been returned to upper layer which stops the sending of video stream. BUG=webrtc:4930 R=pthatcher@webrtc.org Review URL: https://codereview.webrtc.org/1288553010 . Cr-Commit-Position: refs/heads/master@{#9767} --- webrtc/p2p/base/port_unittest.cc | 28 ++++++++++++++++++++++++++++ webrtc/p2p/base/tcpport.cc | 12 +++++++++--- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/webrtc/p2p/base/port_unittest.cc b/webrtc/p2p/base/port_unittest.cc index 2c122da6fc..b0ba89f21e 100644 --- a/webrtc/p2p/base/port_unittest.cc +++ b/webrtc/p2p/base/port_unittest.cc @@ -226,6 +226,9 @@ class TestChannel : public sigslot::has_slots<> { conn_->SignalStateChange.connect( this, &TestChannel::OnConnectionStateChange); conn_->SignalDestroyed.connect(this, &TestChannel::OnDestroyed); + conn_->SignalReadyToSend.connect(this, + &TestChannel::OnConnectionReadyToSend); + connection_ready_to_send_ = false; } void OnConnectionStateChange(Connection* conn) { if (conn->write_state() == Connection::STATE_WRITABLE) { @@ -307,7 +310,20 @@ class TestChannel : public sigslot::has_slots<> { bool nominated() const { return nominated_; } + void set_connection_ready_to_send(bool ready) { + connection_ready_to_send_ = ready; + } + bool connection_ready_to_send() const { + return connection_ready_to_send_; + } + private: + // ReadyToSend will only issue after a Connection recovers from EWOULDBLOCK. + void OnConnectionReadyToSend(Connection* conn) { + ASSERT_EQ(conn, conn_); + connection_ready_to_send_ = true; + } + IceMode ice_mode_; rtc::scoped_ptr src_; Port* dst_; @@ -318,6 +334,7 @@ class TestChannel : public sigslot::has_slots<> { rtc::scoped_ptr remote_request_; std::string remote_frag_; bool nominated_; + bool connection_ready_to_send_ = false; }; class PortTest : public testing::Test, public sigslot::has_slots<> { @@ -618,6 +635,9 @@ class PortTest : public testing::Test, public sigslot::has_slots<> { static_cast(ch2.conn()) ->set_reconnection_timeout(kTcpReconnectTimeout); + EXPECT_FALSE(ch1.connection_ready_to_send()); + EXPECT_FALSE(ch2.connection_ready_to_send()); + // Once connected, disconnect them. DisconnectTcpTestChannels(&ch1, &ch2); @@ -638,11 +658,19 @@ class PortTest : public testing::Test, public sigslot::has_slots<> { // Verify that we could still connect channels. ConnectStartedChannels(&ch1, &ch2); + EXPECT_TRUE_WAIT(ch1.connection_ready_to_send(), + kTcpReconnectTimeout); + // Channel2 is the passive one so a new connection is created during + // reconnect. This new connection should never have issued EWOULDBLOCK + // hence the connection_ready_to_send() should be false. + EXPECT_FALSE(ch2.connection_ready_to_send()); } else { EXPECT_EQ(ch1.conn()->write_state(), Connection::STATE_WRITABLE); EXPECT_TRUE_WAIT( ch1.conn()->write_state() == Connection::STATE_WRITE_TIMEOUT, kTcpReconnectTimeout + kTimeout); + EXPECT_FALSE(ch1.connection_ready_to_send()); + EXPECT_FALSE(ch2.connection_ready_to_send()); } // Tear down and ensure that goes smoothly. diff --git a/webrtc/p2p/base/tcpport.cc b/webrtc/p2p/base/tcpport.cc index 6dc66d26cf..7dc416f923 100644 --- a/webrtc/p2p/base/tcpport.cc +++ b/webrtc/p2p/base/tcpport.cc @@ -354,10 +354,16 @@ int TCPConnection::GetError() { void TCPConnection::OnConnectionRequestResponse(ConnectionRequest* req, StunMessage* response) { - // Once we receive a binding response, we are really writable, and not just - // pretending to be writable. - pretending_to_be_writable_ = false; + // Process the STUN response before we inform upper layer ready to send. Connection::OnConnectionRequestResponse(req, response); + + // If we're in the state of pretending to be writeable, we should inform the + // upper layer it's ready to send again as previous EWOULDLBLOCK from socket + // would have stopped the outgoing stream. + if (pretending_to_be_writable_) { + Connection::OnReadyToSend(); + } + pretending_to_be_writable_ = false; ASSERT(write_state() == STATE_WRITABLE); }