diff --git a/p2p/base/turn_port.cc b/p2p/base/turn_port.cc index 1c9f3f34ea..54684516e3 100644 --- a/p2p/base/turn_port.cc +++ b/p2p/base/turn_port.cc @@ -104,41 +104,38 @@ class TurnRefreshRequest : public StunRequest { TurnPort* port_; }; -class TurnCreatePermissionRequest : public StunRequest, - public sigslot::has_slots<> { +class TurnCreatePermissionRequest : public StunRequest { public: TurnCreatePermissionRequest(TurnPort* port, TurnEntry* entry, const rtc::SocketAddress& ext_addr, absl::string_view remote_ufrag); + ~TurnCreatePermissionRequest() override; void OnSent() override; void OnResponse(StunMessage* response) override; void OnErrorResponse(StunMessage* response) override; void OnTimeout() override; private: - void OnEntryDestroyed(TurnEntry* entry); - TurnPort* port_; TurnEntry* entry_; rtc::SocketAddress ext_addr_; std::string remote_ufrag_; }; -class TurnChannelBindRequest : public StunRequest, public sigslot::has_slots<> { +class TurnChannelBindRequest : public StunRequest { public: TurnChannelBindRequest(TurnPort* port, TurnEntry* entry, int channel_id, const rtc::SocketAddress& ext_addr); + ~TurnChannelBindRequest() override; void OnSent() override; void OnResponse(StunMessage* response) override; void OnErrorResponse(StunMessage* response) override; void OnTimeout() override; private: - void OnEntryDestroyed(TurnEntry* entry); - TurnPort* port_; TurnEntry* entry_; int channel_id_; @@ -191,7 +188,7 @@ class TurnEntry : public sigslot::has_slots<> { void OnChannelBindError(StunMessage* response, int code); void OnChannelBindTimeout(); // Signal sent when TurnEntry is destroyed. - sigslot::signal1 SignalDestroyed; + webrtc::CallbackList destroyed_callback_list_; const std::string& get_remote_ufrag() const { return remote_ufrag_; } void set_remote_ufrag(absl::string_view remote_ufrag) { @@ -843,10 +840,6 @@ void TurnPort::ResolveTurnAddress(const rtc::SocketAddress& address) { "TURN host lookup received error."); return; } - // Signal needs both resolved and unresolved address. After signal is sent - // we can copy resolved address back into `server_address_`. - SignalResolvedServerAddress(this, server_address_.address, - resolved_address); server_address_.address = resolved_address; PrepareAddress(); }); @@ -947,8 +940,9 @@ void TurnPort::Close() { state_ = STATE_DISCONNECTED; // Delete all existing connections; stop sending data. DestroyAllConnections(); - - SignalTurnPortClosed(this); + if (callbacks_for_test_) { + callbacks_for_test_->OnTurnPortClosed(); + } } rtc::DiffServCodePoint TurnPort::StunDscpValue() const { @@ -1252,7 +1246,7 @@ bool TurnPort::CreateOrRefreshEntry(const rtc::SocketAddress& addr, void TurnPort::DestroyEntry(TurnEntry* entry) { RTC_DCHECK(entry != NULL); - entry->SignalDestroyed(entry); + entry->destroyed_callback_list_.Send(entry); entries_.remove(entry); delete entry; } @@ -1288,6 +1282,11 @@ void TurnPort::HandleConnectionDestroyed(Connection* conn) { kTurnPermissionTimeout); } +void TurnPort::SetCallbacksForTest(CallbacksForTest* callbacks) { + RTC_DCHECK(!callbacks_for_test_); + callbacks_for_test_ = callbacks; +} + bool TurnPort::SetEntryChannelId(const rtc::SocketAddress& address, int channel_id) { TurnEntry* entry = FindEntry(address); @@ -1595,7 +1594,9 @@ void TurnRefreshRequest::OnResponse(StunMessage* response) { SafeTask(port->task_safety_.flag(), [port] { port->Close(); })); } - port_->SignalTurnRefreshResult(port_, TURN_SUCCESS_RESULT_CODE); + if (port_->callbacks_for_test_) { + port_->callbacks_for_test_->OnTurnRefreshResult(TURN_SUCCESS_RESULT_CODE); + } } void TurnRefreshRequest::OnErrorResponse(StunMessage* response) { @@ -1612,7 +1613,9 @@ void TurnRefreshRequest::OnErrorResponse(StunMessage* response) { << rtc::hex_encode(id()) << ", code=" << error_code << ", rtt=" << Elapsed(); port_->OnRefreshError(); - port_->SignalTurnRefreshResult(port_, error_code); + if (port_->callbacks_for_test_) { + port_->callbacks_for_test_->OnTurnRefreshResult(error_code); + } } } @@ -1634,8 +1637,11 @@ TurnCreatePermissionRequest::TurnCreatePermissionRequest( entry_(entry), ext_addr_(ext_addr), remote_ufrag_(remote_ufrag) { - entry_->SignalDestroyed.connect( - this, &TurnCreatePermissionRequest::OnEntryDestroyed); + RTC_DCHECK(entry_); + entry_->destroyed_callback_list_.AddReceiver(this, [this](TurnEntry* entry) { + RTC_DCHECK(entry_ == entry); + entry_ = nullptr; + }); StunMessage* message = mutable_msg(); // Create the request as indicated in RFC5766, Section 9.1. RTC_DCHECK_EQ(message->type(), TURN_CREATE_PERMISSION_REQUEST); @@ -1649,6 +1655,12 @@ TurnCreatePermissionRequest::TurnCreatePermissionRequest( port_->TurnCustomizerMaybeModifyOutgoingStunMessage(message); } +TurnCreatePermissionRequest::~TurnCreatePermissionRequest() { + if (entry_) { + entry_->destroyed_callback_list_.RemoveReceivers(this); + } +} + void TurnCreatePermissionRequest::OnSent() { RTC_LOG(LS_INFO) << port_->ToString() << ": TURN create permission request sent, id=" @@ -1689,11 +1701,6 @@ void TurnCreatePermissionRequest::OnTimeout() { } } -void TurnCreatePermissionRequest::OnEntryDestroyed(TurnEntry* entry) { - RTC_DCHECK(entry_ == entry); - entry_ = NULL; -} - TurnChannelBindRequest::TurnChannelBindRequest( TurnPort* port, TurnEntry* entry, @@ -1705,8 +1712,11 @@ TurnChannelBindRequest::TurnChannelBindRequest( entry_(entry), channel_id_(channel_id), ext_addr_(ext_addr) { - entry_->SignalDestroyed.connect(this, - &TurnChannelBindRequest::OnEntryDestroyed); + RTC_DCHECK(entry_); + entry_->destroyed_callback_list_.AddReceiver(this, [this](TurnEntry* entry) { + RTC_DCHECK(entry_ == entry); + entry_ = nullptr; + }); StunMessage* message = mutable_msg(); // Create the request as indicated in RFC5766, Section 11.1. RTC_DCHECK_EQ(message->type(), TURN_CHANNEL_BIND_REQUEST); @@ -1718,6 +1728,12 @@ TurnChannelBindRequest::TurnChannelBindRequest( port_->TurnCustomizerMaybeModifyOutgoingStunMessage(message); } +TurnChannelBindRequest::~TurnChannelBindRequest() { + if (entry_) { + entry_->destroyed_callback_list_.RemoveReceivers(this); + } +} + void TurnChannelBindRequest::OnSent() { RTC_LOG(LS_INFO) << port_->ToString() << ": TURN channel bind request sent, id=" @@ -1765,11 +1781,6 @@ void TurnChannelBindRequest::OnTimeout() { } } -void TurnChannelBindRequest::OnEntryDestroyed(TurnEntry* entry) { - RTC_DCHECK(entry_ == entry); - entry_ = NULL; -} - TurnEntry::TurnEntry(TurnPort* port, int channel_id, const rtc::SocketAddress& ext_addr, @@ -1834,8 +1845,10 @@ int TurnEntry::Send(const void* data, void TurnEntry::OnCreatePermissionSuccess() { RTC_LOG(LS_INFO) << port_->ToString() << ": Create permission for " << ext_addr_.ToSensitiveString() << " succeeded"; - port_->SignalCreatePermissionResult(port_, ext_addr_, - TURN_SUCCESS_RESULT_CODE); + if (port_->callbacks_for_test_) { + port_->callbacks_for_test_->OnTurnCreatePermissionResult( + TURN_SUCCESS_RESULT_CODE); + } // If `state_` is STATE_BOUND, the permission will be refreshed // by ChannelBindRequest. @@ -1862,8 +1875,9 @@ void TurnEntry::OnCreatePermissionError(StunMessage* response, int code) { "code=" << code << "; pruned connection."; } - // Send signal with error code. - port_->SignalCreatePermissionResult(port_, ext_addr_, code); + } + if (port_->callbacks_for_test_) { + port_->callbacks_for_test_->OnTurnCreatePermissionResult(code); } } diff --git a/p2p/base/turn_port.h b/p2p/base/turn_port.h index eaf796a2f0..e51468770a 100644 --- a/p2p/base/turn_port.h +++ b/p2p/base/turn_port.h @@ -175,23 +175,6 @@ class TurnPort : public Port { rtc::AsyncPacketSocket* socket() const { return socket_; } StunRequestManager& request_manager() { return request_manager_; } - // Signal with resolved server address. - // Parameters are port, server address and resolved server address. - // This signal will be sent only if server address is resolved successfully. - sigslot:: - signal3 - SignalResolvedServerAddress; - - // Signal when TurnPort is closed, - // e.g remote socket closed (TCP) - // or receiveing a REFRESH response with lifetime 0. - sigslot::signal1 SignalTurnPortClosed; - - // All public methods/signals below are for testing only. - sigslot::signal2 SignalTurnRefreshResult; - sigslot::signal3 - SignalCreatePermissionResult; - bool HasRequests() { return !request_manager_.empty(); } void set_credentials(const RelayCredentials& credentials) { credentials_ = credentials; @@ -204,6 +187,16 @@ class TurnPort : public Port { void CloseForTest() { Close(); } + // TODO(solenberg): Tests should be refactored to not peek at internal state. + class CallbacksForTest { + public: + virtual ~CallbacksForTest() {} + virtual void OnTurnCreatePermissionResult(int code) = 0; + virtual void OnTurnRefreshResult(int code) = 0; + virtual void OnTurnPortClosed() = 0; + }; + void SetCallbacksForTest(CallbacksForTest* callbacks); + protected: TurnPort(webrtc::TaskQueueBase* thread, rtc::PacketSocketFactory* factory, @@ -371,6 +364,8 @@ class TurnPort : public Port { webrtc::ScopedTaskSafety task_safety_; + CallbacksForTest* callbacks_for_test_ = nullptr; + friend class TurnEntry; friend class TurnAllocateRequest; friend class TurnRefreshRequest; diff --git a/p2p/base/turn_port_unittest.cc b/p2p/base/turn_port_unittest.cc index efb88f5058..e6734a645a 100644 --- a/p2p/base/turn_port_unittest.cc +++ b/p2p/base/turn_port_unittest.cc @@ -171,21 +171,15 @@ class TestConnectionWrapper : public sigslot::has_slots<> { // Note: This test uses a fake clock with a simulated network round trip // (between local port and TURN server) of kSimulatedRtt. -class TurnPortTest : public ::testing::Test, public sigslot::has_slots<> { +class TurnPortTest : public ::testing::Test, + public TurnPort::CallbacksForTest, + public sigslot::has_slots<> { public: TurnPortTest() : ss_(new TurnPortTestVirtualSocketServer()), main_(ss_.get()), socket_factory_(ss_.get()), - turn_server_(&main_, ss_.get(), kTurnUdpIntAddr, kTurnUdpExtAddr), - turn_ready_(false), - turn_error_(false), - turn_unknown_address_(false), - turn_create_permission_success_(false), - turn_port_closed_(false), - turn_port_destroyed_(false), - udp_ready_(false), - test_finish_(false) { + turn_server_(&main_, ss_.get(), kTurnUdpIntAddr, kTurnUdpExtAddr) { // Some code uses "last received time == 0" to represent "nothing received // so far", so we need to start the fake clock at a nonzero time... // TODO(deadbeef): Fix this. @@ -206,16 +200,6 @@ class TurnPortTest : public ::testing::Test, public sigslot::has_slots<> { bool /*port_muxed*/) { turn_unknown_address_ = true; } - void OnTurnCreatePermissionResult(TurnPort* port, - const SocketAddress& addr, - int code) { - // Ignoring the address. - turn_create_permission_success_ = (code == 0); - } - - void OnTurnRefreshResult(TurnPort* port, int code) { - turn_refresh_success_ = (code == 0); - } void OnTurnReadPacket(Connection* conn, const char* data, size_t size, @@ -237,9 +221,17 @@ class TurnPortTest : public ::testing::Test, public sigslot::has_slots<> { turn_port_->HandleIncomingPacket(socket, data, size, remote_addr, packet_time_us); } - void OnTurnPortClosed(TurnPort* port) { turn_port_closed_ = true; } void OnTurnPortDestroyed(PortInterface* port) { turn_port_destroyed_ = true; } + // TurnPort::TestCallbacks + void OnTurnCreatePermissionResult(int code) override { + turn_create_permission_success_ = (code == 0); + } + void OnTurnRefreshResult(int code) override { + turn_refresh_success_ = (code == 0); + } + void OnTurnPortClosed() override { turn_port_closed_ = true; } + rtc::Socket* CreateServerSocket(const SocketAddress addr) { rtc::Socket* socket = ss_->CreateSocket(AF_INET, SOCK_STREAM); EXPECT_GE(socket->Bind(addr), 0); @@ -352,14 +344,9 @@ class TurnPortTest : public ::testing::Test, public sigslot::has_slots<> { &TurnPortTest::OnCandidateError); turn_port_->SignalUnknownAddress.connect( this, &TurnPortTest::OnTurnUnknownAddress); - turn_port_->SignalCreatePermissionResult.connect( - this, &TurnPortTest::OnTurnCreatePermissionResult); - turn_port_->SignalTurnRefreshResult.connect( - this, &TurnPortTest::OnTurnRefreshResult); - turn_port_->SignalTurnPortClosed.connect(this, - &TurnPortTest::OnTurnPortClosed); turn_port_->SubscribePortDestroyed( [this](PortInterface* port) { OnTurnPortDestroyed(port); }); + turn_port_->SetCallbacksForTest(this); } void CreateUdpPort() { CreateUdpPort(kLocalAddr2); } @@ -783,14 +770,14 @@ class TurnPortTest : public ::testing::Test, public sigslot::has_slots<> { TestTurnServer turn_server_; std::unique_ptr turn_port_; std::unique_ptr udp_port_; - bool turn_ready_; - bool turn_error_; - bool turn_unknown_address_; - bool turn_create_permission_success_; - bool turn_port_closed_; - bool turn_port_destroyed_; - bool udp_ready_; - bool test_finish_; + bool turn_ready_ = false; + bool turn_error_ = false; + bool turn_unknown_address_ = false; + bool turn_create_permission_success_ = false; + bool turn_port_closed_ = false; + bool turn_port_destroyed_ = false; + bool udp_ready_ = false; + bool test_finish_ = false; bool turn_refresh_success_ = false; std::vector turn_packets_; std::vector udp_packets_;