Remove sigslot signals from TurnPort
Bug: webrtc:11943
Change-Id: If07749a4fa47bd06a2a11be9d334a4a39a1026b1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/272651
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Fredrik Solenberg <solenberg@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37998}
diff --git a/p2p/base/turn_port.cc b/p2p/base/turn_port.cc
index 1c9f3f3..5468451 100644
--- a/p2p/base/turn_port.cc
+++ b/p2p/base/turn_port.cc
@@ -104,41 +104,38 @@
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 @@
void OnChannelBindError(StunMessage* response, int code);
void OnChannelBindTimeout();
// Signal sent when TurnEntry is destroyed.
- sigslot::signal1<TurnEntry*> SignalDestroyed;
+ webrtc::CallbackList<TurnEntry*> 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 @@
"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 @@
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 @@
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 @@
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 @@
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 @@
<< 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 @@
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 @@
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::OnEntryDestroyed(TurnEntry* entry) {
- RTC_DCHECK(entry_ == entry);
- entry_ = NULL;
-}
-
TurnChannelBindRequest::TurnChannelBindRequest(
TurnPort* port,
TurnEntry* entry,
@@ -1705,8 +1712,11 @@
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 @@
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::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 @@
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 @@
"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 eaf796a..e514687 100644
--- a/p2p/base/turn_port.h
+++ b/p2p/base/turn_port.h
@@ -175,23 +175,6 @@
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<TurnPort*, const rtc::SocketAddress&, const rtc::SocketAddress&>
- SignalResolvedServerAddress;
-
- // Signal when TurnPort is closed,
- // e.g remote socket closed (TCP)
- // or receiveing a REFRESH response with lifetime 0.
- sigslot::signal1<TurnPort*> SignalTurnPortClosed;
-
- // All public methods/signals below are for testing only.
- sigslot::signal2<TurnPort*, int> SignalTurnRefreshResult;
- sigslot::signal3<TurnPort*, const rtc::SocketAddress&, int>
- SignalCreatePermissionResult;
-
bool HasRequests() { return !request_manager_.empty(); }
void set_credentials(const RelayCredentials& credentials) {
credentials_ = credentials;
@@ -204,6 +187,16 @@
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 @@
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 efb88f5..e6734a6 100644
--- a/p2p/base/turn_port_unittest.cc
+++ b/p2p/base/turn_port_unittest.cc
@@ -171,21 +171,15 @@
// 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 @@
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 @@
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 @@
&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 @@
TestTurnServer turn_server_;
std::unique_ptr<TurnPort> turn_port_;
std::unique_ptr<UDPPort> 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<rtc::Buffer> turn_packets_;
std::vector<rtc::Buffer> udp_packets_;
