diff --git a/pc/data_channel_controller.cc b/pc/data_channel_controller.cc index 1cad4eacd6..c8579de953 100644 --- a/pc/data_channel_controller.cc +++ b/pc/data_channel_controller.cc @@ -41,32 +41,16 @@ bool DataChannelController::SendData(int sid, 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 @@ void DataChannelController::OnDataReceived( 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 @@ void DataChannelController::OnChannelClosing(int channel_id) { 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 @@ void DataChannelController::OnChannelClosed(int channel_id) { // 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 @@ void DataChannelController::OnReadyToSend() { 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 5fd0afb1df..c79192474f 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 @@ class DataChannelController : public SctpDataChannelControllerInterface, std::vector> 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 SignalDataChannelTransportWritable_s - RTC_GUARDED_BY(signaling_thread()); - sigslot::signal2 - SignalDataChannelTransportReceivedData_s - RTC_GUARDED_BY(signaling_thread()); - sigslot::signal1 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 529cb6b44b..e783dce0af 100644 --- a/pc/data_channel_unittest.cc +++ b/pc/data_channel_unittest.cc @@ -433,21 +433,6 @@ TEST_F(SctpDataChannelTest, SendDataId) { 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 be2a480a1a..ccd9c70aee 100644 --- a/pc/sctp_data_channel.cc +++ b/pc/sctp_data_channel.cc @@ -369,9 +369,9 @@ void SctpDataChannel::SetSctpSid(const StreamId& sid) { 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 @@ DataChannelStats SctpDataChannel::GetStats() const { 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 17cad2698e..6289801812 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 @@ class SctpSidAllocator { // 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 @@ class SctpSidAllocator { // 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 Create( rtc::WeakPtr controller, @@ -188,9 +186,10 @@ class SctpDataChannel : public DataChannelInterface, // 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.