Remove remaining sigslots from DataChannelController
This includes:
* SignalDataChannelTransportWritable_s
* SignalDataChannelTransportReceivedData_s
* SignalDataChannelTransportChannelClosing_s
* Removing sigslot::has_slots<> inheritance from SctpDataChannel
Instead, we use the existing sctp_data_channels_ vector of channels
known to the DCC to deliver the callbacks.
Bug: webrtc:11943, webrtc:11547
Change-Id: I7935d7505856eedf04981b8ba665ef8419166c1d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297100
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39557}
diff --git a/pc/data_channel_controller.cc b/pc/data_channel_controller.cc
index 1cad4ea..c8579de 100644
--- a/pc/data_channel_controller.cc
+++ b/pc/data_channel_controller.cc
@@ -41,32 +41,16 @@
bool DataChannelController::ConnectDataChannel(
SctpDataChannel* webrtc_data_channel) {
RTC_DCHECK_RUN_ON(signaling_thread());
- if (!data_channel_transport()) {
- // Don't log an error here, because DataChannels are expected to call
- // ConnectDataChannel in this state. It's the only way to initially tell
- // whether or not the underlying transport is ready.
- return false;
- }
- SignalDataChannelTransportWritable_s.connect(
- webrtc_data_channel, &SctpDataChannel::OnTransportReady);
- SignalDataChannelTransportReceivedData_s.connect(
- webrtc_data_channel, &SctpDataChannel::OnDataReceived);
- SignalDataChannelTransportChannelClosing_s.connect(
- webrtc_data_channel, &SctpDataChannel::OnClosingProcedureStartedRemotely);
- return true;
+ // TODO(bugs.webrtc.org/11547): This method can be removed once not
+ // needed by `SctpDataChannel`.
+ return data_channel_transport() ? true : false;
}
void DataChannelController::DisconnectDataChannel(
SctpDataChannel* webrtc_data_channel) {
RTC_DCHECK_RUN_ON(signaling_thread());
- if (!data_channel_transport()) {
- RTC_LOG(LS_ERROR)
- << "DisconnectDataChannel called when sctp_transport_ is NULL.";
- return;
- }
- SignalDataChannelTransportWritable_s.disconnect(webrtc_data_channel);
- SignalDataChannelTransportReceivedData_s.disconnect(webrtc_data_channel);
- SignalDataChannelTransportChannelClosing_s.disconnect(webrtc_data_channel);
+ // TODO(bugs.webrtc.org/11547): This method can be removed once not
+ // needed by `SctpDataChannel`.
}
void DataChannelController::AddSctpDataStream(int sid) {
@@ -120,10 +104,11 @@
SafeTask(signaling_safety_.flag(), [this, params, buffer] {
RTC_DCHECK_RUN_ON(signaling_thread());
// TODO(bugs.webrtc.org/11547): The data being received should be
- // delivered on the network thread (change
- // SignalDataChannelTransportReceivedData_s to
- // SignalDataChannelTransportReceivedData_n).
- SignalDataChannelTransportReceivedData_s(params, buffer);
+ // delivered on the network thread.
+ for (const auto& channel : sctp_data_channels_) {
+ if (channel->id() == params.sid)
+ channel->OnDataReceived(params, buffer);
+ }
}));
}
@@ -132,7 +117,11 @@
signaling_thread()->PostTask(
SafeTask(signaling_safety_.flag(), [this, channel_id] {
RTC_DCHECK_RUN_ON(signaling_thread());
- SignalDataChannelTransportChannelClosing_s(channel_id);
+ // TODO(bugs.webrtc.org/11547): Should run on the network thread.
+ for (const auto& channel : sctp_data_channels_) {
+ if (channel->id() == channel_id)
+ channel->OnClosingProcedureStartedRemotely();
+ }
}));
}
@@ -151,8 +140,6 @@
// Note: this causes OnSctpDataChannelClosed() to not do anything
// when called from within `OnClosingProcedureComplete`.
sctp_data_channels_.erase(it);
-
- DisconnectDataChannel(channel.get());
sid_allocator_.ReleaseSid(channel->sid());
channel->OnClosingProcedureComplete();
@@ -165,7 +152,8 @@
signaling_thread()->PostTask(SafeTask(signaling_safety_.flag(), [this] {
RTC_DCHECK_RUN_ON(signaling_thread());
data_channel_transport_ready_to_send_ = true;
- SignalDataChannelTransportWritable_s(data_channel_transport_ready_to_send_);
+ for (const auto& channel : sctp_data_channels_)
+ channel->OnTransportReady(true);
}));
}
diff --git a/pc/data_channel_controller.h b/pc/data_channel_controller.h
index 5fd0afb..c791924 100644
--- a/pc/data_channel_controller.h
+++ b/pc/data_channel_controller.h
@@ -26,7 +26,6 @@
#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"
@@ -151,19 +150,6 @@
std::vector<rtc::scoped_refptr<SctpDataChannel>> sctp_data_channels_
RTC_GUARDED_BY(signaling_thread());
- // Signals from `data_channel_transport_`. These are invoked on the
- // signaling thread.
- // TODO(bugs.webrtc.org/11547): These '_s' signals likely all belong on the
- // network thread.
- sigslot::signal1<bool> SignalDataChannelTransportWritable_s
- RTC_GUARDED_BY(signaling_thread());
- sigslot::signal2<const cricket::ReceiveDataParams&,
- const rtc::CopyOnWriteBuffer&>
- SignalDataChannelTransportReceivedData_s
- RTC_GUARDED_BY(signaling_thread());
- sigslot::signal1<int> SignalDataChannelTransportChannelClosing_s
- RTC_GUARDED_BY(signaling_thread());
-
// Owning PeerConnection.
PeerConnectionInternal* const pc_;
// The weak pointers must be dereferenced and invalidated on the signalling
diff --git a/pc/data_channel_unittest.cc b/pc/data_channel_unittest.cc
index 529cb6b..e783dce 100644
--- a/pc/data_channel_unittest.cc
+++ b/pc/data_channel_unittest.cc
@@ -433,21 +433,6 @@
EXPECT_EQ(1, controller_->last_sid());
}
-// Tests that the incoming messages with wrong ids are rejected.
-TEST_F(SctpDataChannelTest, ReceiveDataWithInvalidId) {
- webrtc_data_channel_->SetSctpSid(StreamId(1));
- SetChannelReady();
-
- AddObserver();
-
- cricket::ReceiveDataParams params;
- params.sid = 0;
- DataBuffer buffer("abcd");
- webrtc_data_channel_->OnDataReceived(params, buffer.data);
-
- EXPECT_EQ(0U, observer_->messages_received());
-}
-
// Tests that the incoming messages with right ids are accepted.
TEST_F(SctpDataChannelTest, ReceiveDataWithValidId) {
webrtc_data_channel_->SetSctpSid(StreamId(1));
diff --git a/pc/sctp_data_channel.cc b/pc/sctp_data_channel.cc
index be2a480..ccd9c70 100644
--- a/pc/sctp_data_channel.cc
+++ b/pc/sctp_data_channel.cc
@@ -369,9 +369,9 @@
controller_->AddSctpDataStream(sid.stream_id_int());
}
-void SctpDataChannel::OnClosingProcedureStartedRemotely(int sid) {
+void SctpDataChannel::OnClosingProcedureStartedRemotely() {
RTC_DCHECK_RUN_ON(signaling_thread_);
- if (id_.stream_id_int() == sid && state_ != kClosing && state_ != kClosed) {
+ if (state_ != kClosing && state_ != kClosed) {
// Don't bother sending queued data since the side that initiated the
// closure wouldn't receive it anyway. See crbug.com/559394 for a lengthy
// discussion about this.
@@ -429,9 +429,7 @@
void SctpDataChannel::OnDataReceived(const cricket::ReceiveDataParams& params,
const rtc::CopyOnWriteBuffer& payload) {
RTC_DCHECK_RUN_ON(signaling_thread_);
- if (id_.stream_id_int() != params.sid) {
- return;
- }
+ RTC_DCHECK_EQ(id_.stream_id_int(), params.sid);
if (params.type == DataMessageType::kControl) {
if (handshake_state_ != kHandshakeWaitingForAck) {
diff --git a/pc/sctp_data_channel.h b/pc/sctp_data_channel.h
index 17cad26..6289801 100644
--- a/pc/sctp_data_channel.h
+++ b/pc/sctp_data_channel.h
@@ -29,7 +29,6 @@
#include "rtc_base/containers/flat_set.h"
#include "rtc_base/copy_on_write_buffer.h"
#include "rtc_base/ssl_stream_adapter.h" // For SSLRole
-#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"
@@ -100,7 +99,7 @@
// SctpDataChannel is an implementation of the DataChannelInterface based on
// SctpTransport. It provides an implementation of unreliable or
-// reliabledata channels.
+// reliable data channels.
// DataChannel states:
// kConnecting: The channel has been created the transport might not yet be
@@ -122,8 +121,7 @@
// 5. Bob sends outgoing stream reset.
// 6. Alice receives incoming reset, Bob receives acknowledgement. Both receive
// OnClosingProcedureComplete callback and transition to kClosed.
-class SctpDataChannel : public DataChannelInterface,
- public sigslot::has_slots<> {
+class SctpDataChannel : public DataChannelInterface {
public:
static rtc::scoped_refptr<SctpDataChannel> Create(
rtc::WeakPtr<SctpDataChannelControllerInterface> controller,
@@ -188,9 +186,10 @@
// Sets the SCTP sid and adds to transport layer if not set yet. Should only
// be called once.
void SetSctpSid(const StreamId& sid);
+
// The remote side started the closing procedure by resetting its outgoing
// stream (our incoming stream). Sets state to kClosing.
- void OnClosingProcedureStartedRemotely(int sid);
+ void OnClosingProcedureStartedRemotely();
// The closing procedure is complete; both incoming and outgoing stream
// resets are done and the channel can transition to kClosed. Called
// asynchronously after RemoveSctpDataStream.
