Make PortInterface signals protected
This flushes out downstream usage of these objects.
"Protected" is used because the Subscribe and Notify functions
are defined on Port, not PortInterface.
Bug: webrtc:42222066
Change-Id: Icb6ec4e723d97f82b6eaee4221a05209834d4e86
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/421800
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#46125}
diff --git a/p2p/BUILD.gn b/p2p/BUILD.gn
index 6a7457d..4f4cf32 100644
--- a/p2p/BUILD.gn
+++ b/p2p/BUILD.gn
@@ -674,6 +674,7 @@
"../rtc_base:dscp",
"../rtc_base:net_helper",
"../rtc_base:network",
+ "../rtc_base:sigslot_trampoline",
"../rtc_base:socket",
"../rtc_base:socket_address",
"../rtc_base/network:sent_packet",
diff --git a/p2p/base/port.cc b/p2p/base/port.cc
index 1b2d53c..9f7c06f 100644
--- a/p2p/base/port.cc
+++ b/p2p/base/port.cc
@@ -109,9 +109,6 @@
shared_socket_(shared_socket),
network_cost_(args.network->GetCost(env_.field_trials())),
role_conflict_callback_(nullptr),
- unknown_address_trampoline_(this),
- read_packet_trampoline_(this),
- sent_packet_trampoline_(this),
weak_factory_(this) {
RTC_DCHECK_RUN_ON(thread_);
RTC_DCHECK(factory_ != nullptr);
diff --git a/p2p/base/port.h b/p2p/base/port.h
index 0ab339e..5a00aa0 100644
--- a/p2p/base/port.h
+++ b/p2p/base/port.h
@@ -43,7 +43,6 @@
#include "rtc_base/network.h"
#include "rtc_base/network/received_packet.h"
#include "rtc_base/network/sent_packet.h"
-#include "rtc_base/sigslot_trampoline.h"
#include "rtc_base/socket_address.h"
#include "rtc_base/system/rtc_export.h"
#include "rtc_base/third_party/sigslot/sigslot.h"
@@ -585,10 +584,6 @@
// Signals and trampolines. These will eventually be removed and replaced
// with straight CallbackLists (or simple callbacks).
// TODO: https://issues.webrtc.org/42222066 - replace and delete.
-
- // Downstream code uses this signal. We will continue firing it along with the
- // callback list. The signal can be deleted once all downstream usages are
- // replaced with the new CallbackList implementation.
sigslot::signal2<Port*, const Candidate&> SignalCandidateReady;
sigslot::signal1<Port*> SignalPortComplete;
// Downstream code uses this signal. We will continue firing it along with the
@@ -596,13 +591,6 @@
// replaced with the new CallbackList implementation.
sigslot::signal1<Port*> SignalPortError;
- SignalTrampoline<PortInterface, &PortInterface::SignalUnknownAddress>
- unknown_address_trampoline_;
- SignalTrampoline<PortInterface, &PortInterface::SignalReadPacket>
- read_packet_trampoline_;
- SignalTrampoline<PortInterface, &PortInterface::SignalSentPacket>
- sent_packet_trampoline_;
-
// Keep as the last member variable.
WeakPtrFactory<Port> weak_factory_ RTC_GUARDED_BY(thread_);
};
diff --git a/p2p/base/port_interface.cc b/p2p/base/port_interface.cc
index b123fd2..d442ad4 100644
--- a/p2p/base/port_interface.cc
+++ b/p2p/base/port_interface.cc
@@ -14,7 +14,10 @@
namespace webrtc {
-PortInterface::PortInterface() = default;
+PortInterface::PortInterface()
+ : unknown_address_trampoline_(this),
+ read_packet_trampoline_(this),
+ sent_packet_trampoline_(this) {}
PortInterface::~PortInterface() = default;
diff --git a/p2p/base/port_interface.h b/p2p/base/port_interface.h
index 55b6650..15141d4 100644
--- a/p2p/base/port_interface.h
+++ b/p2p/base/port_interface.h
@@ -30,6 +30,7 @@
#include "rtc_base/net_helper.h"
#include "rtc_base/network.h"
#include "rtc_base/network/sent_packet.h"
+#include "rtc_base/sigslot_trampoline.h"
#include "rtc_base/socket.h"
#include "rtc_base/socket_address.h"
#include "rtc_base/third_party/sigslot/sigslot.h"
@@ -97,13 +98,6 @@
// Indicates that we received a successful STUN binding request from an
// address that doesn't correspond to any current connection. To turn this
// into a real connection, call CreateConnection.
- sigslot::signal6<PortInterface*,
- const SocketAddress&,
- ProtocolType,
- IceMessage*,
- const std::string&,
- bool>
- SignalUnknownAddress;
virtual void SubscribeUnknownAddress(
absl::AnyInvocable<void(PortInterface*,
const SocketAddress&,
@@ -131,8 +125,6 @@
std::function<void(PortInterface*)> callback) = 0;
// Signaled when Port discovers ice role conflict with the peer.
- // TODO: bugs.webrtc.org/42222066 - remove slot.
- sigslot::signal1<PortInterface*> SignalRoleConflict;
virtual void SubscribeRoleConflict(absl::AnyInvocable<void()> callback) = 0;
virtual void NotifyRoleConflict() = 0;
@@ -141,8 +133,6 @@
// through their respective connection and instead delivers every packet
// through this port.
virtual void EnablePortPackets() = 0;
- sigslot::signal4<PortInterface*, const char*, size_t, const SocketAddress&>
- SignalReadPacket;
virtual void SubscribeReadPacket(
absl::AnyInvocable<
void(PortInterface*, const char*, size_t, const SocketAddress&)>
@@ -153,7 +143,6 @@
const SocketAddress&) = 0;
// Emitted each time a packet is sent on this port.
- sigslot::signal1<const SentPacketInfo&> SignalSentPacket;
virtual void SubscribeSentPacket(
absl::AnyInvocable<void(const SentPacketInfo&)> callback) = 0;
virtual void NotifySentPacket(const SentPacketInfo& packet) = 0;
@@ -218,6 +207,26 @@
absl::string_view remote_ufrag) = 0;
virtual int16_t network_cost() const = 0;
+ // Since the Subscribe and Notify methods are defined on subclasses'
+ // interfaces, these signals need to be "protected", not "private".
+ // TODO: https://issues.webrtc.org/42222066 - replace and delete.
+ sigslot::signal6<PortInterface*,
+ const SocketAddress&,
+ ProtocolType,
+ IceMessage*,
+ const std::string&,
+ bool>
+ SignalUnknownAddress;
+ sigslot::signal4<PortInterface*, const char*, size_t, const SocketAddress&>
+ SignalReadPacket;
+ sigslot::signal1<const SentPacketInfo&> SignalSentPacket;
+ SignalTrampoline<PortInterface, &PortInterface::SignalUnknownAddress>
+ unknown_address_trampoline_;
+ SignalTrampoline<PortInterface, &PortInterface::SignalReadPacket>
+ read_packet_trampoline_;
+ SignalTrampoline<PortInterface, &PortInterface::SignalSentPacket>
+ sent_packet_trampoline_;
+ sigslot::signal1<PortInterface*> SignalRoleConflict;
// Connection and Port are entangled; functions exposed to Port only
// should not be public.
diff --git a/p2p/base/port_unittest.cc b/p2p/base/port_unittest.cc
index 341379f..3f1cb15 100644
--- a/p2p/base/port_unittest.cc
+++ b/p2p/base/port_unittest.cc
@@ -3992,32 +3992,6 @@
EXPECT_TRUE(port->GetConnection(address) != nullptr);
}
-#if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
-class DeathChannel : public sigslot::has_slots<> {
- public:
- explicit DeathChannel(std::unique_ptr<Port> port) : port_(std::move(port)) {}
- void IgnoredSlot(PortInterface* /* port */) {}
- void AddSignal() {
- port_->SignalRoleConflict.connect(this, &DeathChannel::IgnoredSlot);
- }
- void AddCallback() {
- port_->SubscribeRoleConflict([]() {});
- }
-
- private:
- std::unique_ptr<Port> port_;
-};
-
-class PortDeathTest : public PortTest {};
-
-TEST_F(PortDeathTest, AddSignalThenCallback) {
- DeathChannel dc(CreateRawTestPort());
- dc.AddSignal();
- EXPECT_DEATH(dc.AddCallback(), "");
-}
-
-#endif // RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
-
// TODO(webrtc:11463) : Move Connection tests into separate unit test
// splitting out shared test code as needed.
diff --git a/p2p/base/tcp_port_unittest.cc b/p2p/base/tcp_port_unittest.cc
index 1b8136e..e98f685 100644
--- a/p2p/base/tcp_port_unittest.cc
+++ b/p2p/base/tcp_port_unittest.cc
@@ -249,7 +249,8 @@
class SentPacketCounter : public sigslot::has_slots<> {
public:
explicit SentPacketCounter(TCPPort* p) {
- p->SignalSentPacket.connect(this, &SentPacketCounter::OnSentPacket);
+ p->SubscribeSentPacket(
+ [this](const webrtc::SentPacketInfo& info) { OnSentPacket(info); });
}
int sent_packets() const { return sent_packets_; }
