Revert "Replace use of sigslot with CallbackList in data_channel_controller"
This reverts commit 8efc914cf353cea138a453c45e970e589bec0834.
Reason for revert: Breaks downstream project.
Original change's description:
> Replace use of sigslot with CallbackList in data_channel_controller
>
> This is a straightforward replacement; simplifications due to the ability
> to inline functions in the lambdas are for a later CL.
>
> Bug: webrtc:11943
> Change-Id: I7274cedde507b954f1d8aa8bc560861102eeb264
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/250540
> Reviewed-by: Niels Moller <nisse@webrtc.org>
> Commit-Queue: Harald Alvestrand <hta@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#35936}
TBR=nisse@webrtc.org,hta@webrtc.org,webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com
Change-Id: I8fd0f32ceec866bfd9a08cac1108b559bf03caac
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:11943
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/251280
Owners-Override: Mirko Bonadei <mbonadei@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35941}
diff --git a/pc/data_channel_controller.cc b/pc/data_channel_controller.cc
index a0df409..e11647f 100644
--- a/pc/data_channel_controller.cc
+++ b/pc/data_channel_controller.cc
@@ -49,33 +49,14 @@
// whether or not the underlying transport is ready.
return false;
}
- data_transport_writable_callbacks_.AddReceiver(
- webrtc_data_channel, [webrtc_data_channel](bool ready) {
- webrtc_data_channel->OnTransportReady(ready);
- });
- data_channel_transport_received_data_callbacks_.AddReceiver(
- webrtc_data_channel,
- [webrtc_data_channel](const cricket::ReceiveDataParams& params,
- const rtc::CopyOnWriteBuffer& data) {
- webrtc_data_channel->OnDataReceived(params, data);
- });
- data_channel_transport_channel_closing_callbacks_.AddReceiver(
- webrtc_data_channel, [webrtc_data_channel](int num) {
- webrtc_data_channel->OnClosingProcedureStartedRemotely(num);
- });
- // When a datachannel is closed, it may get deleted, so we have to make
- // sure the closed callback isn't called again.
- // This takes advantage of the fact that a channel is never closed twice.
- // Unfortunately it doesn't work for pre-opened datachannels, since these
- // have id = -1 (unassigned) at registration time, so they must be called
- // upon anyway.
- int channel_id = webrtc_data_channel->id();
- data_channel_transport_channel_closed_callbacks_.AddReceiver(
- webrtc_data_channel, [webrtc_data_channel, channel_id](int num) {
- if (num == channel_id || channel_id < 0) {
- webrtc_data_channel->OnClosingProcedureComplete(num);
- }
- });
+ SignalDataChannelTransportWritable_s.connect(
+ webrtc_data_channel, &SctpDataChannel::OnTransportReady);
+ SignalDataChannelTransportReceivedData_s.connect(
+ webrtc_data_channel, &SctpDataChannel::OnDataReceived);
+ SignalDataChannelTransportChannelClosing_s.connect(
+ webrtc_data_channel, &SctpDataChannel::OnClosingProcedureStartedRemotely);
+ SignalDataChannelTransportChannelClosed_s.connect(
+ webrtc_data_channel, &SctpDataChannel::OnClosingProcedureComplete);
return true;
}
@@ -87,24 +68,10 @@
<< "DisconnectDataChannel called when sctp_transport_ is NULL.";
return;
}
- data_transport_writable_callbacks_.RemoveReceivers(webrtc_data_channel);
- data_channel_transport_received_data_callbacks_.RemoveReceivers(
- webrtc_data_channel);
- data_channel_transport_channel_closing_callbacks_.RemoveReceivers(
- webrtc_data_channel);
- // This function is being called from the OnClosingProcedureComplete
- // function, which is called from the data_channel_transport_channel_closed
- // CallbackList.
- // Since CallbackList does not permit removing receivers in a callback,
- // schedule the disconnect to happen later.
- signaling_thread()->PostTask(ToQueuedTask([self = weak_factory_.GetWeakPtr(),
- webrtc_data_channel]() {
- if (self) {
- RTC_DCHECK_RUN_ON(self->signaling_thread());
- self->data_channel_transport_channel_closed_callbacks_.RemoveReceivers(
- webrtc_data_channel);
- }
- }));
+ SignalDataChannelTransportWritable_s.disconnect(webrtc_data_channel);
+ SignalDataChannelTransportReceivedData_s.disconnect(webrtc_data_channel);
+ SignalDataChannelTransportChannelClosing_s.disconnect(webrtc_data_channel);
+ SignalDataChannelTransportChannelClosed_s.disconnect(webrtc_data_channel);
}
void DataChannelController::AddSctpDataStream(int sid) {
@@ -153,8 +120,7 @@
// SignalDataChannelTransportReceivedData_s to
// SignalDataChannelTransportReceivedData_n).
if (!self->HandleOpenMessage_s(params, buffer)) {
- self->data_channel_transport_received_data_callbacks_.Send(params,
- buffer);
+ self->SignalDataChannelTransportReceivedData_s(params, buffer);
}
}
}));
@@ -166,8 +132,7 @@
ToQueuedTask([self = weak_factory_.GetWeakPtr(), channel_id] {
if (self) {
RTC_DCHECK_RUN_ON(self->signaling_thread());
- self->data_channel_transport_channel_closing_callbacks_.Send(
- channel_id);
+ self->SignalDataChannelTransportChannelClosing_s(channel_id);
}
}));
}
@@ -178,8 +143,7 @@
ToQueuedTask([self = weak_factory_.GetWeakPtr(), channel_id] {
if (self) {
RTC_DCHECK_RUN_ON(self->signaling_thread());
- self->data_channel_transport_channel_closed_callbacks_.Send(
- channel_id);
+ self->SignalDataChannelTransportChannelClosed_s(channel_id);
}
}));
}
@@ -191,7 +155,7 @@
if (self) {
RTC_DCHECK_RUN_ON(self->signaling_thread());
self->data_channel_transport_ready_to_send_ = true;
- self->data_transport_writable_callbacks_.Send(
+ self->SignalDataChannelTransportWritable_s(
self->data_channel_transport_ready_to_send_);
}
}));
@@ -334,9 +298,8 @@
return nullptr;
}
sctp_data_channels_.push_back(channel);
- channel->SignalClosed.connect(
- pc_, &PeerConnectionInternal::OnSctpDataChannelClosed);
- sctp_data_channel_created_callbacks_.Send(channel.get());
+ channel->SignalClosed.connect(pc_, &PeerConnection::OnSctpDataChannelClosed);
+ SignalSctpDataChannelCreated_(channel.get());
return channel;
}
diff --git a/pc/data_channel_controller.h b/pc/data_channel_controller.h
index 6b3cd1b..af0e063 100644
--- a/pc/data_channel_controller.h
+++ b/pc/data_channel_controller.h
@@ -27,21 +27,23 @@
#include "media/base/stream_params.h"
#include "pc/channel.h"
#include "pc/data_channel_utils.h"
-#include "pc/peer_connection_internal.h"
#include "pc/sctp_data_channel.h"
#include "rtc_base/checks.h"
#include "rtc_base/copy_on_write_buffer.h"
#include "rtc_base/ssl_stream_adapter.h"
+#include "rtc_base/third_party/sigslot/sigslot.h"
#include "rtc_base/thread.h"
#include "rtc_base/thread_annotations.h"
#include "rtc_base/weak_ptr.h"
namespace webrtc {
+class PeerConnection;
+
class DataChannelController : public SctpDataChannelProviderInterface,
public DataChannelSink {
public:
- explicit DataChannelController(PeerConnectionInternal* pc) : pc_(pc) {}
+ explicit DataChannelController(PeerConnection* pc) : pc_(pc) {}
// Not copyable or movable.
DataChannelController(DataChannelController&) = delete;
@@ -104,12 +106,10 @@
DataChannelTransportInterface* data_channel_transport() const;
void set_data_channel_transport(DataChannelTransportInterface* transport);
- template <typename F>
- void SubscribeDataChannelCreated(F&& callback) {
+ sigslot::signal1<SctpDataChannel*>& SignalSctpDataChannelCreated() {
RTC_DCHECK_RUN_ON(signaling_thread());
- sctp_data_channel_created_callbacks_.AddReceiver(callback);
+ return SignalSctpDataChannelCreated_;
}
-
// Called when the transport for the data channels is closed or destroyed.
void OnTransportChannelClosed(RTCError error);
@@ -165,21 +165,22 @@
// signaling thread.
// TODO(bugs.webrtc.org/11547): These '_s' signals likely all belong on the
// network thread.
- CallbackList<bool> data_transport_writable_callbacks_
+ sigslot::signal1<bool> SignalDataChannelTransportWritable_s
RTC_GUARDED_BY(signaling_thread());
- CallbackList<const cricket::ReceiveDataParams&, rtc::CopyOnWriteBuffer>
- data_channel_transport_received_data_callbacks_
+ sigslot::signal2<const cricket::ReceiveDataParams&,
+ const rtc::CopyOnWriteBuffer&>
+ SignalDataChannelTransportReceivedData_s
RTC_GUARDED_BY(signaling_thread());
- CallbackList<int> data_channel_transport_channel_closing_callbacks_
+ sigslot::signal1<int> SignalDataChannelTransportChannelClosing_s
RTC_GUARDED_BY(signaling_thread());
- CallbackList<int> data_channel_transport_channel_closed_callbacks_
+ sigslot::signal1<int> SignalDataChannelTransportChannelClosed_s
RTC_GUARDED_BY(signaling_thread());
- // Callback listened to for data channel creation.
- CallbackList<SctpDataChannel*> sctp_data_channel_created_callbacks_
+ sigslot::signal1<SctpDataChannel*> SignalSctpDataChannelCreated_
RTC_GUARDED_BY(signaling_thread());
+
// Owning PeerConnection.
- PeerConnectionInternal* const pc_;
+ PeerConnection* const pc_;
// The weak pointers must be dereferenced and invalidated on the signalling
// thread only.
rtc::WeakPtrFactory<DataChannelController> weak_factory_{this};
diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc
index f6cf544..4e951a5 100644
--- a/pc/peer_connection.cc
+++ b/pc/peer_connection.cc
@@ -2175,11 +2175,6 @@
return false;
}
-void PeerConnection::SubscribeDataChannelCreated(
- std::function<void(SctpDataChannel*)> callback) {
- data_channel_controller()->SubscribeDataChannelCreated(callback);
-}
-
bool PeerConnection::GetTransportDescription(
const SessionDescription* description,
const std::string& content_name,
diff --git a/pc/peer_connection.h b/pc/peer_connection.h
index 4cafe23..3f3dda6 100644
--- a/pc/peer_connection.h
+++ b/pc/peer_connection.h
@@ -117,7 +117,8 @@
// - The ICE state machine.
// - Generating stats.
class PeerConnection : public PeerConnectionInternal,
- public JsepTransportController::Observer {
+ public JsepTransportController::Observer,
+ public sigslot::has_slots<> {
public:
// Creates a PeerConnection and initializes it with the given values.
// If the initialization fails, the function releases the PeerConnection
@@ -288,6 +289,11 @@
return rtp_manager()->transceivers()->List();
}
+ sigslot::signal1<SctpDataChannel*>& SignalSctpDataChannelCreated() override {
+ RTC_DCHECK_RUN_ON(signaling_thread());
+ return data_channel_controller_.SignalSctpDataChannelCreated();
+ }
+
std::vector<DataChannelStats> GetDataChannelStats() const override;
absl::optional<std::string> sctp_transport_name() const override;
@@ -306,10 +312,9 @@
bool IceRestartPending(const std::string& content_name) const override;
bool NeedsIceRestart(const std::string& content_name) const override;
bool GetSslRole(const std::string& content_name, rtc::SSLRole* role) override;
- void SubscribeDataChannelCreated(
- std::function<void(SctpDataChannel*)> callback) override;
+
// Functions needed by DataChannelController
- void NoteDataAddedEvent() override { NoteUsageEvent(UsageEvent::DATA_ADDED); }
+ void NoteDataAddedEvent() { NoteUsageEvent(UsageEvent::DATA_ADDED); }
// Returns the observer. Will crash on CHECK if the observer is removed.
PeerConnectionObserver* Observer() const override;
bool IsClosed() const override {
@@ -320,7 +325,7 @@
// Get current SSL role used by SCTP's underlying transport.
bool GetSctpSslRole(rtc::SSLRole* role) override;
// Handler for the "channel closed" signal
- void OnSctpDataChannelClosed(DataChannelInterface* channel) override;
+ void OnSctpDataChannelClosed(DataChannelInterface* channel);
bool ShouldFireNegotiationNeededEvent(uint32_t event_id) override;
diff --git a/pc/peer_connection_internal.h b/pc/peer_connection_internal.h
index e10468d..3b82dc5 100644
--- a/pc/peer_connection_internal.h
+++ b/pc/peer_connection_internal.h
@@ -127,8 +127,7 @@
// Functions defined in this class are called by other objects,
// but not by SdpOfferAnswerHandler.
class PeerConnectionInternal : public PeerConnectionInterface,
- public PeerConnectionSdpMethods,
- public sigslot::has_slots<> {
+ public PeerConnectionSdpMethods {
public:
virtual rtc::Thread* network_thread() const = 0;
virtual rtc::Thread* worker_thread() const = 0;
@@ -140,6 +139,9 @@
rtc::scoped_refptr<RtpTransceiverProxyWithInternal<RtpTransceiver>>>
GetTransceiversInternal() const = 0;
+ virtual sigslot::signal1<SctpDataChannel*>&
+ SignalSctpDataChannelCreated() = 0;
+
// Call on the network thread to fetch stats for all the data channels.
// TODO(tommi): Make pure virtual after downstream updates.
virtual std::vector<DataChannelStats> GetDataChannelStats() const {
@@ -170,14 +172,6 @@
// Get SSL role for an arbitrary m= section (handles bundling correctly).
virtual bool GetSslRole(const std::string& content_name,
rtc::SSLRole* role) = 0;
-
- // Subscribe to the creation of datachannels. Used by the rtc-stats module.
- virtual void SubscribeDataChannelCreated(
- std::function<void(SctpDataChannel*)> callback) = 0;
- // Functions needed by DataChannelController
- virtual void NoteDataAddedEvent() = 0;
- // Handler for the "channel closed" signal
- virtual void OnSctpDataChannelClosed(DataChannelInterface* channel) = 0;
};
} // namespace webrtc
diff --git a/pc/rtc_stats_collector.cc b/pc/rtc_stats_collector.cc
index bc24d5b..7e9807e 100644
--- a/pc/rtc_stats_collector.cc
+++ b/pc/rtc_stats_collector.cc
@@ -1173,8 +1173,8 @@
RTC_DCHECK(worker_thread_);
RTC_DCHECK(network_thread_);
RTC_DCHECK_GE(cache_lifetime_us_, 0);
- pc_->SubscribeDataChannelCreated(
- [this](SctpDataChannel* channel) { OnSctpDataChannelCreated(channel); });
+ pc_->SignalSctpDataChannelCreated().connect(
+ this, &RTCStatsCollector::OnSctpDataChannelCreated);
}
RTCStatsCollector::~RTCStatsCollector() {
diff --git a/pc/rtc_stats_collector.h b/pc/rtc_stats_collector.h
index e75e82b..c84e6d3 100644
--- a/pc/rtc_stats_collector.h
+++ b/pc/rtc_stats_collector.h
@@ -84,10 +84,6 @@
// completed. Must be called on the signaling thread.
void WaitForPendingRequest();
- // Callback that is called on data channel creation.
- // Exposed for testing.
- void OnSctpDataChannelCreated(SctpDataChannel* channel);
-
protected:
RTCStatsCollector(PeerConnectionInternal* pc, int64_t cache_lifetime_us);
~RTCStatsCollector();
@@ -242,6 +238,8 @@
// This is a NO-OP if `network_report_` is null.
void MergeNetworkReport_s();
+ // Slots for signals (sigslot) that are wired up to `pc_`.
+ void OnSctpDataChannelCreated(SctpDataChannel* channel);
// Slots for signals (sigslot) that are wired up to `channel`.
void OnDataChannelOpened(DataChannelInterface* channel);
void OnDataChannelClosed(DataChannelInterface* channel);
diff --git a/pc/rtc_stats_collector_unittest.cc b/pc/rtc_stats_collector_unittest.cc
index ac0253c..8f0936c 100644
--- a/pc/rtc_stats_collector_unittest.cc
+++ b/pc/rtc_stats_collector_unittest.cc
@@ -1602,11 +1602,11 @@
rtc::scoped_refptr<SctpDataChannel> dummy_channel_a = SctpDataChannel::Create(
&provider, "DummyChannelA", InternalDataChannelInit(),
rtc::Thread::Current(), rtc::Thread::Current());
- stats_->stats_collector()->OnSctpDataChannelCreated(dummy_channel_a.get());
+ pc_->SignalSctpDataChannelCreated()(dummy_channel_a.get());
rtc::scoped_refptr<SctpDataChannel> dummy_channel_b = SctpDataChannel::Create(
&provider, "DummyChannelB", InternalDataChannelInit(),
rtc::Thread::Current(), rtc::Thread::Current());
- stats_->stats_collector()->OnSctpDataChannelCreated(dummy_channel_b.get());
+ pc_->SignalSctpDataChannelCreated()(dummy_channel_b.get());
dummy_channel_a->SignalOpened(dummy_channel_a.get());
// Closing a channel that is not opened should not affect the counts.
diff --git a/pc/test/fake_peer_connection_base.h b/pc/test/fake_peer_connection_base.h
index ff4f33d..3462c8c 100644
--- a/pc/test/fake_peer_connection_base.h
+++ b/pc/test/fake_peer_connection_base.h
@@ -247,6 +247,10 @@
return {};
}
+ sigslot::signal1<SctpDataChannel*>& SignalSctpDataChannelCreated() override {
+ return SignalSctpDataChannelCreated_;
+ }
+
absl::optional<std::string> sctp_transport_name() const override {
return absl::nullopt;
}
@@ -285,9 +289,6 @@
rtc::SSLRole* role) override {
return false;
}
- void SubscribeDataChannelCreated(
- std::function<void(SctpDataChannel*)> callback) override {}
-
const PeerConnectionInterface::RTCConfiguration* configuration()
const override {
return nullptr;
@@ -355,8 +356,6 @@
void TeardownDataChannelTransport_n() override {}
void SetSctpDataMid(const std::string& mid) override {}
void ResetSctpDataMid() override {}
- void NoteDataAddedEvent() override {}
- void OnSctpDataChannelClosed(DataChannelInterface* channel) override {}
protected:
sigslot::signal1<SctpDataChannel*> SignalSctpDataChannelCreated_;
