diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index e349da42c9..c31f67175b 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -2115,20 +2115,31 @@ void PeerConnection::OnSelectedCandidatePairChanged( Observer()->OnIceSelectedCandidatePairChanged(event); } -absl::optional PeerConnection::GetDataMid() const { +bool PeerConnection::CreateDataChannelTransport(absl::string_view mid) { RTC_DCHECK_RUN_ON(signaling_thread()); - return sctp_mid_s_; -} + RTC_DCHECK(!sctp_mid().has_value() || mid == sctp_mid().value()); + RTC_LOG(LS_INFO) << "Creating data channel, mid=" << mid; + + absl::optional transport_name = + network_thread()->BlockingCall([&] { + RTC_DCHECK_RUN_ON(network_thread()); + return SetupDataChannelTransport_n(mid); + }); + if (!transport_name) + return false; -void PeerConnection::SetSctpDataInfo(absl::string_view mid, - absl::string_view transport_name) { - RTC_DCHECK_RUN_ON(signaling_thread()); sctp_mid_s_ = std::string(mid); - SetSctpTransportName(std::string(transport_name)); + SetSctpTransportName(transport_name.value()); + + return true; } -void PeerConnection::ResetSctpDataInfo() { +void PeerConnection::DestroyDataChannelTransport(RTCError error) { RTC_DCHECK_RUN_ON(signaling_thread()); + network_thread()->BlockingCall([&] { + RTC_DCHECK_RUN_ON(network_thread()); + TeardownDataChannelTransport_n(error); + }); sctp_mid_s_.reset(); SetSctpTransportName(""); } diff --git a/pc/peer_connection.h b/pc/peer_connection.h index aac1635484..ea1a9d9d90 100644 --- a/pc/peer_connection.h +++ b/pc/peer_connection.h @@ -394,15 +394,8 @@ class PeerConnection : public PeerConnectionInternal, const std::map& bundle_groups_by_mid) override; - // Returns the MID for the data section associated with the - // SCTP data channel, if it has been set. If no data - // channels are configured this will return nullopt. - absl::optional GetDataMid() const override; - - void SetSctpDataInfo(absl::string_view mid, - absl::string_view transport_name) override; - - void ResetSctpDataInfo() override; + bool CreateDataChannelTransport(absl::string_view mid) override; + void DestroyDataChannelTransport(RTCError error) override; // Asynchronously calls SctpTransport::Start() on the network thread for // `sctp_mid()` if set. Called as part of setting the local description. @@ -430,9 +423,9 @@ class PeerConnection : public PeerConnectionInternal, // this session. bool SrtpRequired() const override; - absl::optional SetupDataChannelTransport_n( - absl::string_view mid) override RTC_RUN_ON(network_thread()); - void TeardownDataChannelTransport_n(RTCError error) override + absl::optional SetupDataChannelTransport_n(absl::string_view mid) + RTC_RUN_ON(network_thread()); + void TeardownDataChannelTransport_n(RTCError error) RTC_RUN_ON(network_thread()); const FieldTrialsView& trials() const override { return *trials_; } diff --git a/pc/peer_connection_internal.h b/pc/peer_connection_internal.h index c91a44a148..6fc1222804 100644 --- a/pc/peer_connection_internal.h +++ b/pc/peer_connection_internal.h @@ -95,7 +95,6 @@ class PeerConnectionSdpMethods { const std::map& bundle_groups_by_mid) = 0; - virtual absl::optional GetDataMid() const = 0; // Internal implementation for AddTransceiver family of methods. If // `fire_callback` is set, fires OnRenegotiationNeeded callback if successful. virtual RTCErrorOr> @@ -117,17 +116,14 @@ class PeerConnectionSdpMethods { // Returns true if SRTP (either using DTLS-SRTP or SDES) is required by // this session. virtual bool SrtpRequired() const = 0; - // Configures the data channel transport on the network thread. - // The return value will be unset if an error occurs. If the setup succeeded - // the return value will be set and contain the name of the transport - // (empty string if a name isn't available). - virtual absl::optional SetupDataChannelTransport_n( - absl::string_view mid) = 0; - virtual void TeardownDataChannelTransport_n(RTCError error) = 0; - virtual void SetSctpDataInfo(absl::string_view mid, - absl::string_view transport_name) = 0; - virtual void ResetSctpDataInfo() = 0; - + // Initializes the data channel transport for the peerconnection instance. + // This will have the effect that `sctp_mid()` and `sctp_transport_name()` + // will return a set value (even though it might be an empty string) and the + // dc transport will be initialized on the network thread. + virtual bool CreateDataChannelTransport(absl::string_view mid) = 0; + // Tears down the data channel transport state and clears the `sctp_mid()` and + // `sctp_transport_name()` properties. + virtual void DestroyDataChannelTransport(RTCError error) = 0; virtual const FieldTrialsView& trials() const = 0; virtual void ClearStatsCache() = 0; diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index 4a77b7546c..c795ccff6c 100644 --- a/pc/sdp_offer_answer.cc +++ b/pc/sdp_offer_answer.cc @@ -3791,13 +3791,15 @@ RTCError SdpOfferAnswerHandler::UpdateTransceiversAndDataChannels( return error; } } else if (media_type == cricket::MEDIA_TYPE_DATA) { - if (pc_->GetDataMid() && new_content.name != *(pc_->GetDataMid())) { + const auto data_mid = pc_->sctp_mid(); + if (data_mid && new_content.name != data_mid.value()) { // Ignore all but the first data section. RTC_LOG(LS_INFO) << "Ignoring data media section with MID=" << new_content.name; continue; } - RTCError error = UpdateDataChannel(source, new_content, bundle_group); + RTCError error = + UpdateDataChannelTransport(source, new_content, bundle_group); if (!error.ok()) { return error; } @@ -3979,7 +3981,7 @@ RTCError SdpOfferAnswerHandler::UpdateTransceiverChannel( return RTCError::OK(); } -RTCError SdpOfferAnswerHandler::UpdateDataChannel( +RTCError SdpOfferAnswerHandler::UpdateDataChannelTransport( cricket::ContentSource source, const cricket::ContentInfo& content, const cricket::ContentGroup* bundle_group) { @@ -3991,8 +3993,8 @@ RTCError SdpOfferAnswerHandler::UpdateDataChannel( sb << "Rejected data channel transport with mid=" << content.mid(); RTCError error(RTCErrorType::OPERATION_ERROR_WITH_DATA, sb.Release()); error.set_error_detail(RTCErrorDetailType::DATA_CHANNEL_FAILURE); - DestroyDataChannelTransport(error); - } else if (!CreateDataChannel(content.name)) { + pc_->DestroyDataChannelTransport(error); + } else if (!pc_->CreateDataChannelTransport(content.name)) { LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR, "Failed to create data channel."); } @@ -4323,8 +4325,9 @@ void SdpOfferAnswerHandler::GetOptionsForUnifiedPlanOffer( session_options->media_description_options.push_back( GetMediaDescriptionOptionsForRejectedData(mid)); } else { - RTC_CHECK(pc_->GetDataMid()); - if (mid == *(pc_->GetDataMid())) { + const auto data_mid = pc_->sctp_mid(); + RTC_CHECK(data_mid); + if (mid == data_mid.value()) { session_options->media_description_options.push_back( GetMediaDescriptionOptionsForActiveData(mid)); } else { @@ -4365,9 +4368,9 @@ void SdpOfferAnswerHandler::GetOptionsForUnifiedPlanOffer( } // Lastly, add a m-section if we have requested local data channels and an // m section does not already exist. - if (!pc_->GetDataMid() && data_channel_controller()->HasDataChannels()) { + if (!pc_->sctp_mid() && data_channel_controller()->HasDataChannels()) { // Attempt to recycle a stopped m-line. - // TODO(crbug.com/1442604): GetDataMid() should return the mid if one was + // TODO(crbug.com/1442604): sctp_mid() should return the mid if one was // ever created but rejected. bool recycled = false; for (size_t i = 0; i < session_options->media_description_options.size(); @@ -4510,7 +4513,7 @@ void SdpOfferAnswerHandler::GetOptionsForUnifiedPlanAnswer( // Reject all data sections if data channels are disabled. // Reject a data section if it has already been rejected. // Reject all data sections except for the first one. - if (content.rejected || content.name != *(pc_->GetDataMid())) { + if (content.rejected || content.name != *(pc_->sctp_mid())) { session_options->media_description_options.push_back( GetMediaDescriptionOptionsForRejectedData(content.name)); } else { @@ -5003,14 +5006,14 @@ void SdpOfferAnswerHandler::RemoveUnusedChannels( RTCError error(RTCErrorType::OPERATION_ERROR_WITH_DATA, "No data channel section in the description."); error.set_error_detail(RTCErrorDetailType::DATA_CHANNEL_FAILURE); - DestroyDataChannelTransport(error); + pc_->DestroyDataChannelTransport(error); } else if (data_info->rejected) { rtc::StringBuilder sb; sb << "Rejected data channel with mid=" << data_info->name << "."; RTCError error(RTCErrorType::OPERATION_ERROR_WITH_DATA, sb.Release()); error.set_error_detail(RTCErrorDetailType::DATA_CHANNEL_FAILURE); - DestroyDataChannelTransport(error); + pc_->DestroyDataChannelTransport(error); } } @@ -5193,7 +5196,7 @@ RTCError SdpOfferAnswerHandler::CreateChannels(const SessionDescription& desc) { } const cricket::ContentInfo* data = cricket::GetFirstDataContent(&desc); - if (data && !data->rejected && !CreateDataChannel(data->name)) { + if (data && !data->rejected && !pc_->CreateDataChannelTransport(data->name)) { LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR, "Failed to create data channel."); } @@ -5201,33 +5204,6 @@ RTCError SdpOfferAnswerHandler::CreateChannels(const SessionDescription& desc) { return RTCError::OK(); } -bool SdpOfferAnswerHandler::CreateDataChannel(const std::string& mid) { - RTC_DCHECK_RUN_ON(signaling_thread()); - RTC_DCHECK(!pc_->sctp_mid().has_value() || mid == pc_->sctp_mid().value()); - RTC_LOG(LS_INFO) << "Creating data channel, mid=" << mid; - - absl::optional transport_name = - context_->network_thread()->BlockingCall([&] { - RTC_DCHECK_RUN_ON(context_->network_thread()); - return pc_->SetupDataChannelTransport_n(mid); - }); - if (!transport_name) - return false; - - pc_->SetSctpDataInfo(mid, *transport_name); - return true; -} - -void SdpOfferAnswerHandler::DestroyDataChannelTransport(RTCError error) { - RTC_DCHECK_RUN_ON(signaling_thread()); - context_->network_thread()->BlockingCall( - [&, data_channel_controller = data_channel_controller()] { - RTC_DCHECK_RUN_ON(context_->network_thread()); - pc_->TeardownDataChannelTransport_n(error); - }); - pc_->ResetSctpDataInfo(); -} - void SdpOfferAnswerHandler::DestroyAllChannels() { RTC_DCHECK_RUN_ON(signaling_thread()); if (!transceivers()) { @@ -5252,7 +5228,7 @@ void SdpOfferAnswerHandler::DestroyAllChannels() { } } - DestroyDataChannelTransport({}); + pc_->DestroyDataChannelTransport({}); } void SdpOfferAnswerHandler::GenerateMediaDescriptionOptions( diff --git a/pc/sdp_offer_answer.h b/pc/sdp_offer_answer.h index 80a21391b9..eb7c705461 100644 --- a/pc/sdp_offer_answer.h +++ b/pc/sdp_offer_answer.h @@ -359,9 +359,9 @@ class SdpOfferAnswerHandler : public SdpStateProvider { // Either creates or destroys the local data channel according to the given // media section. - RTCError UpdateDataChannel(cricket::ContentSource source, - const cricket::ContentInfo& content, - const cricket::ContentGroup* bundle_group) + RTCError UpdateDataChannelTransport(cricket::ContentSource source, + const cricket::ContentInfo& content, + const cricket::ContentGroup* bundle_group) RTC_RUN_ON(signaling_thread()); // Check if a call to SetLocalDescription is acceptable with a session // description of the given type. @@ -526,12 +526,6 @@ class SdpOfferAnswerHandler : public SdpStateProvider { // This method will also delete any existing media channels before creating. RTCError CreateChannels(const cricket::SessionDescription& desc); - bool CreateDataChannel(const std::string& mid); - - // Destroys the RTP data channel transport and/or the SCTP data channel - // transport and clears it. - void DestroyDataChannelTransport(RTCError error); - // Generates MediaDescriptionOptions for the `session_opts` based on existing // local description or remote description. void GenerateMediaDescriptionOptions( diff --git a/pc/test/fake_peer_connection_base.h b/pc/test/fake_peer_connection_base.h index 743c18122a..a1c8dca12e 100644 --- a/pc/test/fake_peer_connection_base.h +++ b/pc/test/fake_peer_connection_base.h @@ -339,9 +339,6 @@ class FakePeerConnectionBase : public PeerConnectionInternal { return false; } - absl::optional GetDataMid() const override { - return absl::nullopt; - } RTCErrorOr> AddTransceiver( cricket::MediaType media_type, rtc::scoped_refptr track, @@ -358,14 +355,10 @@ class FakePeerConnectionBase : public PeerConnectionInternal { Call* call_ptr() override { return nullptr; } bool SrtpRequired() const override { return false; } - absl::optional SetupDataChannelTransport_n( - absl::string_view mid) override { - return absl::nullopt; + bool CreateDataChannelTransport(absl::string_view mid) override { + return false; } - void TeardownDataChannelTransport_n(RTCError error) override {} - void SetSctpDataInfo(absl::string_view mid, - absl::string_view transport_name) override {} - void ResetSctpDataInfo() override {} + void DestroyDataChannelTransport(RTCError error) override {} const FieldTrialsView& trials() const override { return field_trials_; } diff --git a/pc/test/mock_peer_connection_internal.h b/pc/test/mock_peer_connection_internal.h index 58d13ede2f..5fd7a50b4f 100644 --- a/pc/test/mock_peer_connection_internal.h +++ b/pc/test/mock_peer_connection_internal.h @@ -248,7 +248,6 @@ class MockPeerConnectionInternal : public PeerConnectionInternal { (const cricket::SessionDescription*, (const std::map&)), (override)); - MOCK_METHOD(absl::optional, GetDataMid, (), (const, override)); MOCK_METHOD(RTCErrorOr>, AddTransceiver, (cricket::MediaType, @@ -263,16 +262,11 @@ class MockPeerConnectionInternal : public PeerConnectionInternal { (override)); MOCK_METHOD(Call*, call_ptr, (), (override)); MOCK_METHOD(bool, SrtpRequired, (), (const, override)); - MOCK_METHOD(absl::optional, - SetupDataChannelTransport_n, - (absl::string_view mid), + MOCK_METHOD(bool, + CreateDataChannelTransport, + (absl::string_view), (override)); - MOCK_METHOD(void, TeardownDataChannelTransport_n, (RTCError), (override)); - MOCK_METHOD(void, - SetSctpDataInfo, - (absl::string_view, absl::string_view), - (override)); - MOCK_METHOD(void, ResetSctpDataInfo, (), (override)); + MOCK_METHOD(void, DestroyDataChannelTransport, (RTCError error), (override)); MOCK_METHOD(const FieldTrialsView&, trials, (), (const, override)); // PeerConnectionInternal