diff --git a/pc/data_channel_controller.cc b/pc/data_channel_controller.cc index b1ebfc05f9..3545546da6 100644 --- a/pc/data_channel_controller.cc +++ b/pc/data_channel_controller.cc @@ -49,24 +49,27 @@ RTCError DataChannelController::SendData( StreamId sid, const SendDataParams& params, const rtc::CopyOnWriteBuffer& payload) { - if (data_channel_transport()) - return DataChannelSendData(sid, params, payload); - RTC_LOG(LS_ERROR) << "SendData called before transport is ready"; - return RTCError(RTCErrorType::INVALID_STATE); + RTC_DCHECK_RUN_ON(network_thread()); + if (!data_channel_transport_) { + RTC_LOG(LS_ERROR) << "SendData called before transport is ready"; + return RTCError(RTCErrorType::INVALID_STATE); + } + return data_channel_transport_->SendData(sid.stream_id_int(), params, + payload); } void DataChannelController::AddSctpDataStream(StreamId sid) { RTC_DCHECK_RUN_ON(network_thread()); RTC_DCHECK(sid.HasValue()); - if (data_channel_transport()) { - data_channel_transport()->OpenChannel(sid.stream_id_int()); + if (data_channel_transport_) { + data_channel_transport_->OpenChannel(sid.stream_id_int()); } } void DataChannelController::RemoveSctpDataStream(StreamId sid) { RTC_DCHECK_RUN_ON(network_thread()); - if (data_channel_transport()) { - data_channel_transport()->CloseChannel(sid.stream_id_int()); + if (data_channel_transport_) { + data_channel_transport_->CloseChannel(sid.stream_id_int()); } } @@ -178,11 +181,11 @@ void DataChannelController::TeardownDataChannelTransport_n() { void DataChannelController::OnTransportChanged( DataChannelTransportInterface* new_data_channel_transport) { RTC_DCHECK_RUN_ON(network_thread()); - if (data_channel_transport() && - data_channel_transport() != new_data_channel_transport) { + if (data_channel_transport_ && + data_channel_transport_ != new_data_channel_transport) { // Changed which data channel transport is used for `sctp_mid_` (eg. now // it's bundled). - data_channel_transport()->SetDataSink(nullptr); + data_channel_transport_->SetDataSink(nullptr); set_data_channel_transport(new_data_channel_transport); if (new_data_channel_transport) { new_data_channel_transport->SetDataSink(this); @@ -418,33 +421,15 @@ void DataChannelController::OnTransportChannelClosed(RTCError error) { } } -DataChannelTransportInterface* DataChannelController::data_channel_transport() - const { - // TODO(bugs.webrtc.org/11547): Only allow this accessor to be called on the - // network thread. - // RTC_DCHECK_RUN_ON(network_thread()); - return data_channel_transport_; -} - void DataChannelController::set_data_channel_transport( DataChannelTransportInterface* transport) { RTC_DCHECK_RUN_ON(network_thread()); data_channel_transport_ = transport; } -RTCError DataChannelController::DataChannelSendData( - StreamId sid, - const SendDataParams& params, - const rtc::CopyOnWriteBuffer& payload) { - RTC_DCHECK_RUN_ON(network_thread()); - RTC_DCHECK(data_channel_transport()); - return data_channel_transport()->SendData(sid.stream_id_int(), params, - payload); -} - void DataChannelController::NotifyDataChannelsOfTransportCreated() { RTC_DCHECK_RUN_ON(network_thread()); - RTC_DCHECK(data_channel_transport()); + RTC_DCHECK(data_channel_transport_); for (const auto& channel : sctp_data_channels_n_) { if (channel->sid_n().HasValue()) diff --git a/pc/data_channel_controller.h b/pc/data_channel_controller.h index 074b1feadb..edd21c2390 100644 --- a/pc/data_channel_controller.h +++ b/pc/data_channel_controller.h @@ -94,7 +94,6 @@ class DataChannelController : public SctpDataChannelControllerInterface, bool HasUsedDataChannels() const; // Accessors - DataChannelTransportInterface* data_channel_transport() const; void set_data_channel_transport(DataChannelTransportInterface* transport); // Called when the transport for the data channels is closed or destroyed. @@ -135,11 +134,6 @@ class DataChannelController : public SctpDataChannelControllerInterface, absl::optional fallback_ssl_role) RTC_RUN_ON(network_thread()); - // Called from SendData when data_channel_transport() is true. - RTCError DataChannelSendData(StreamId sid, - const SendDataParams& params, - const rtc::CopyOnWriteBuffer& payload); - // Called when all data channels need to be notified of a transport channel // (calls OnTransportChannelCreated on the signaling thread). void NotifyDataChannelsOfTransportCreated(); @@ -147,9 +141,8 @@ class DataChannelController : public SctpDataChannelControllerInterface, // Plugin transport used for data channels. Pointer may be accessed and // checked from any thread, but the object may only be touched on the // network thread. - // TODO(bugs.webrtc.org/9987): Accessed on both signaling and network - // thread. - DataChannelTransportInterface* data_channel_transport_ = nullptr; + DataChannelTransportInterface* data_channel_transport_ + RTC_GUARDED_BY(network_thread()) = nullptr; SctpSidAllocator sid_allocator_ RTC_GUARDED_BY(network_thread()); std::vector> sctp_data_channels_n_ RTC_GUARDED_BY(network_thread()); diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index 90a6cd2b92..0363042687 100644 --- a/pc/sdp_offer_answer.cc +++ b/pc/sdp_offer_answer.cc @@ -3890,14 +3890,9 @@ RTCError SdpOfferAnswerHandler::UpdateDataChannel( RTCError error(RTCErrorType::OPERATION_ERROR_WITH_DATA, sb.Release()); error.set_error_detail(RTCErrorDetailType::DATA_CHANNEL_FAILURE); DestroyDataChannelTransport(error); - } else { - if (!data_channel_controller()->data_channel_transport()) { - RTC_LOG(LS_INFO) << "Creating data channel, mid=" << content.mid(); - if (!CreateDataChannel(content.name)) { - return RTCError(RTCErrorType::INTERNAL_ERROR, - "Failed to create data channel."); - } - } + } else if (!CreateDataChannel(content.name)) { + return RTCError(RTCErrorType::INTERNAL_ERROR, + "Failed to create data channel."); } return RTCError::OK(); } @@ -5081,12 +5076,9 @@ RTCError SdpOfferAnswerHandler::CreateChannels(const SessionDescription& desc) { } const cricket::ContentInfo* data = cricket::GetFirstDataContent(&desc); - if (data && !data->rejected && - !data_channel_controller()->data_channel_transport()) { - if (!CreateDataChannel(data->name)) { - return RTCError(RTCErrorType::INTERNAL_ERROR, - "Failed to create data channel."); - } + if (data && !data->rejected && !CreateDataChannel(data->name)) { + return RTCError(RTCErrorType::INTERNAL_ERROR, + "Failed to create data channel."); } return RTCError::OK(); @@ -5094,6 +5086,13 @@ RTCError SdpOfferAnswerHandler::CreateChannels(const SessionDescription& desc) { bool SdpOfferAnswerHandler::CreateDataChannel(const std::string& mid) { RTC_DCHECK_RUN_ON(signaling_thread()); + if (pc_->sctp_mid().has_value()) { + RTC_DCHECK_EQ(mid, *pc_->sctp_mid()); + return true; // data channel already created. + } + + RTC_LOG(LS_INFO) << "Creating data channel, mid=" << mid; + if (!context_->network_thread()->BlockingCall([this, &mid] { RTC_DCHECK_RUN_ON(context_->network_thread()); return pc_->SetupDataChannelTransport_n(mid);