From dd7fb43f28410e00039660e31e130568c901d03e Mon Sep 17 00:00:00 2001 From: deadbeef Date: Fri, 30 Sep 2016 15:16:48 -0700 Subject: [PATCH] Emit SignalReadyToSend even for "presumed writable" connections. The Connection class will now blindly forward SignalReadyToSend, and P2PTransportChannel will decide whether to forward it further (which it was already doing). BUG=webrtc:6448 Review-Url: https://codereview.webrtc.org/2374183005 Cr-Commit-Position: refs/heads/master@{#14462} --- .../p2p/base/p2ptransportchannel_unittest.cc | 39 +++++++++++++++++++ webrtc/p2p/base/port.cc | 4 +- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc index d7ee87300d..70ef00701e 100644 --- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc +++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc @@ -1672,6 +1672,9 @@ TEST_F(P2PTransportChannelTest, TestUsingPooledSessionAfterDoneGathering) { // Test that when the "presume_writable_when_fully_relayed" flag is set to // true and there's a TURN-TURN candidate pair, it's presumed to be writable // as soon as it's created. +// TODO(deadbeef): Move this and other "presumed writable" tests into a test +// class that operates on a single P2PTransportChannel, once an appropriate one +// (which supports TURN servers and TURN candidate gathering) is available. TEST_F(P2PTransportChannelTest, TurnToTurnPresumedWritable) { ConfigureEndpoints(OPEN, OPEN, kDefaultPortAllocatorFlags, kDefaultPortAllocatorFlags); @@ -1797,6 +1800,42 @@ TEST_F(P2PTransportChannelTest, PresumedWritablePreferredOverUnreliable) { DestroyChannels(); } +// Ensure that "SignalReadyToSend" is fired as expected with a "presumed +// writable" connection. Previously this did not work. +TEST_F(P2PTransportChannelTest, SignalReadyToSendWithPresumedWritable) { + ConfigureEndpoints(OPEN, OPEN, kDefaultPortAllocatorFlags, + kDefaultPortAllocatorFlags); + // Only test one endpoint, so we can ensure the connection doesn't receive a + // binding response and advance beyond being "presumed" writable. + GetEndpoint(0)->cd1_.ch_.reset(CreateChannel( + 0, ICE_CANDIDATE_COMPONENT_DEFAULT, kIceParams[0], kIceParams[1])); + IceConfig config; + config.presume_writable_when_fully_relayed = true; + ep1_ch1()->SetIceConfig(config); + ep1_ch1()->MaybeStartGathering(); + EXPECT_EQ_WAIT(IceGatheringState::kIceGatheringComplete, + ep1_ch1()->gathering_state(), kDefaultTimeout); + ep1_ch1()->AddRemoteCandidate( + CreateUdpCandidate(RELAY_PORT_TYPE, "1.1.1.1", 1, 0)); + // Sanity checking the type of the connection. + EXPECT_TRUE(ep1_ch1()->selected_connection() != nullptr); + EXPECT_EQ(RELAY_PORT_TYPE, LocalCandidate(ep1_ch1())->type()); + EXPECT_EQ(RELAY_PORT_TYPE, RemoteCandidate(ep1_ch1())->type()); + + // Tell the socket server to block packets (returning EWOULDBLOCK). + virtual_socket_server()->SetSendingBlocked(true); + const char* data = "test"; + int len = static_cast(strlen(data)); + EXPECT_EQ(-1, SendData(ep1_ch1(), data, len)); + + // Reset |ready_to_send_| flag, which is set to true if the event fires as it + // should. + GetEndpoint(0)->ready_to_send_ = false; + virtual_socket_server()->SetSendingBlocked(false); + EXPECT_TRUE(GetEndpoint(0)->ready_to_send_); + EXPECT_EQ(len, SendData(ep1_ch1(), data, len)); +} + // Test what happens when we have 2 users behind the same NAT. This can lead // to interesting behavior because the STUN server will only give out the // address of the outermost NAT. diff --git a/webrtc/p2p/base/port.cc b/webrtc/p2p/base/port.cc index 2f487dc179..8742d0f9e2 100644 --- a/webrtc/p2p/base/port.cc +++ b/webrtc/p2p/base/port.cc @@ -1106,9 +1106,7 @@ void Connection::HandleBindingRequest(IceMessage* msg) { } void Connection::OnReadyToSend() { - if (write_state_ == STATE_WRITABLE) { - SignalReadyToSend(this); - } + SignalReadyToSend(this); } void Connection::Prune() {