From 8597543ae845073ca32876bfaaff207017b6c0eb Mon Sep 17 00:00:00 2001 From: Honghai Zhang Date: Thu, 12 Nov 2015 11:07:12 -0800 Subject: [PATCH] Schedule a CreatePermissionRequest after the success of a previous request unless a channel binding request is already scheduled. BUG=5178 R=pthatcher@webrtc.org Review URL: https://codereview.webrtc.org/1434603002 . Cr-Commit-Position: refs/heads/master@{#10625} --- webrtc/p2p/base/stunrequest.cc | 8 ++++++++ webrtc/p2p/base/stunrequest.h | 3 +++ webrtc/p2p/base/turnport.cc | 23 +++++++++++++++++------ webrtc/p2p/base/turnport.h | 2 ++ webrtc/p2p/base/turnport_unittest.cc | 22 ++++++++++++++++++++++ 5 files changed, 52 insertions(+), 6 deletions(-) diff --git a/webrtc/p2p/base/stunrequest.cc b/webrtc/p2p/base/stunrequest.cc index df5614d3cc..0a0b1a8997 100644 --- a/webrtc/p2p/base/stunrequest.cc +++ b/webrtc/p2p/base/stunrequest.cc @@ -53,6 +53,14 @@ void StunRequestManager::SendDelayed(StunRequest* request, int delay) { } } +void StunRequestManager::Flush() { + for (const auto kv : requests_) { + StunRequest* request = kv.second; + thread_->Clear(request, MSG_STUN_SEND); + thread_->Send(request, MSG_STUN_SEND, NULL); + } +} + void StunRequestManager::Remove(StunRequest* request) { ASSERT(request->manager() == this); RequestMap::iterator iter = requests_.find(request->id()); diff --git a/webrtc/p2p/base/stunrequest.h b/webrtc/p2p/base/stunrequest.h index 267b4a1959..15ea0c73b7 100644 --- a/webrtc/p2p/base/stunrequest.h +++ b/webrtc/p2p/base/stunrequest.h @@ -32,6 +32,9 @@ class StunRequestManager { void Send(StunRequest* request); void SendDelayed(StunRequest* request, int delay); + // Sends all pending requests right away. Only for testing. + void Flush(); + // Removes a stun request that was added previously. This will happen // automatically when a request succeeds, fails, or times out. void Remove(StunRequest* request); diff --git a/webrtc/p2p/base/turnport.cc b/webrtc/p2p/base/turnport.cc index 1cc885e27d..32f63bed2a 100644 --- a/webrtc/p2p/base/turnport.cc +++ b/webrtc/p2p/base/turnport.cc @@ -141,7 +141,7 @@ class TurnEntry : public sigslot::has_slots<> { BindState state() const { return state_; } // Helper methods to send permission and channel bind requests. - void SendCreatePermissionRequest(); + void SendCreatePermissionRequest(int delay); void SendChannelBindRequest(int delay); // Sends a packet to the given destination address. // This will wrap the packet in STUN if necessary. @@ -1289,12 +1289,12 @@ TurnEntry::TurnEntry(TurnPort* port, int channel_id, ext_addr_(ext_addr), state_(STATE_UNBOUND) { // Creating permission for |ext_addr_|. - SendCreatePermissionRequest(); + SendCreatePermissionRequest(0); } -void TurnEntry::SendCreatePermissionRequest() { - port_->SendRequest(new TurnCreatePermissionRequest( - port_, this, ext_addr_), 0); +void TurnEntry::SendCreatePermissionRequest(int delay) { + port_->SendRequest(new TurnCreatePermissionRequest(port_, this, ext_addr_), + delay); } void TurnEntry::SendChannelBindRequest(int delay) { @@ -1337,12 +1337,23 @@ void TurnEntry::OnCreatePermissionSuccess() { << " succeeded"; // For success result code will be 0. port_->SignalCreatePermissionResult(port_, ext_addr_, 0); + + // If |state_| is STATE_BOUND, the permission will be refreshed + // by ChannelBindRequest. + if (state_ != STATE_BOUND) { + // Refresh the permission request about 1 minute before the permission + // times out. + int delay = TURN_PERMISSION_TIMEOUT - 60000; + SendCreatePermissionRequest(delay); + LOG_J(LS_INFO, port_) << "Scheduled create-permission-request in " + << delay << "ms."; + } } void TurnEntry::OnCreatePermissionError(StunMessage* response, int code) { if (code == STUN_ERROR_STALE_NONCE) { if (port_->UpdateNonce(response)) { - SendCreatePermissionRequest(); + SendCreatePermissionRequest(0); } } else { // Send signal with error code. diff --git a/webrtc/p2p/base/turnport.h b/webrtc/p2p/base/turnport.h index 3bca727346..3a1b4320e4 100644 --- a/webrtc/p2p/base/turnport.h +++ b/webrtc/p2p/base/turnport.h @@ -132,6 +132,8 @@ class TurnPort : public Port { // This signal is only for testing purpose. sigslot::signal3 SignalCreatePermissionResult; + // For testing only. + void FlushRequests() { request_manager_.Flush(); } protected: TurnPort(rtc::Thread* thread, diff --git a/webrtc/p2p/base/turnport_unittest.cc b/webrtc/p2p/base/turnport_unittest.cc index 724485ddde..484b152430 100644 --- a/webrtc/p2p/base/turnport_unittest.cc +++ b/webrtc/p2p/base/turnport_unittest.cc @@ -715,6 +715,28 @@ TEST_F(TurnPortTest, TestTurnConnectionUsingOTUNonce) { TestTurnConnection(); } +// Test that CreatePermissionRequest will be scheduled after the success +// of the first create permission request. +TEST_F(TurnPortTest, TestRefreshCreatePermissionRequest) { + CreateTurnPort(kTurnUsername, kTurnPassword, kTurnUdpProtoAddr); + + ASSERT_TRUE(turn_port_ != NULL); + turn_port_->PrepareAddress(); + ASSERT_TRUE_WAIT(turn_ready_, kTimeout); + CreateUdpPort(); + udp_port_->PrepareAddress(); + ASSERT_TRUE_WAIT(udp_ready_, kTimeout); + + Connection* conn = turn_port_->CreateConnection(udp_port_->Candidates()[0], + Port::ORIGIN_MESSAGE); + ASSERT_TRUE(conn != NULL); + ASSERT_TRUE_WAIT(turn_create_permission_success_, kTimeout); + turn_create_permission_success_ = false; + // A create-permission-request should be pending. + turn_port_->FlushRequests(); + ASSERT_TRUE_WAIT(turn_create_permission_success_, kTimeout); +} + // Do a TURN allocation, establish a UDP connection, and send some data. TEST_F(TurnPortTest, TestTurnSendDataTurnUdpToUdp) { // Create ports and prepare addresses.