diff --git a/pc/jsep_transport.cc b/pc/jsep_transport.cc index 7beeba5881..c385fd1cb8 100644 --- a/pc/jsep_transport.cc +++ b/pc/jsep_transport.cc @@ -168,23 +168,20 @@ webrtc::RTCError JsepTransport::SetLocalJsepTransportDescription( } // If doing SDES, setup the SDES crypto parameters. - { - webrtc::MutexLock lock(&accessor_lock_); - if (sdes_transport_) { - RTC_DCHECK(!unencrypted_rtp_transport_); - RTC_DCHECK(!dtls_srtp_transport_); - if (!SetSdes(jsep_description.cryptos, - jsep_description.encrypted_header_extension_ids, type, - ContentSource::CS_LOCAL)) { - return webrtc::RTCError(webrtc::RTCErrorType::INVALID_PARAMETER, - "Failed to setup SDES crypto parameters."); - } - } else if (dtls_srtp_transport_) { - RTC_DCHECK(!unencrypted_rtp_transport_); - RTC_DCHECK(!sdes_transport_); - dtls_srtp_transport_->UpdateRecvEncryptedHeaderExtensionIds( - jsep_description.encrypted_header_extension_ids); + if (sdes_transport_) { + RTC_DCHECK(!unencrypted_rtp_transport_); + RTC_DCHECK(!dtls_srtp_transport_); + if (!SetSdes(jsep_description.cryptos, + jsep_description.encrypted_header_extension_ids, type, + ContentSource::CS_LOCAL)) { + return webrtc::RTCError(webrtc::RTCErrorType::INVALID_PARAMETER, + "Failed to setup SDES crypto parameters."); } + } else if (dtls_srtp_transport_) { + RTC_DCHECK(!unencrypted_rtp_transport_); + RTC_DCHECK(!sdes_transport_); + dtls_srtp_transport_->UpdateRecvEncryptedHeaderExtensionIds( + jsep_description.encrypted_header_extension_ids); } bool ice_restarting = local_description_ != nullptr && @@ -205,18 +202,18 @@ webrtc::RTCError JsepTransport::SetLocalJsepTransportDescription( return error; } } - { - webrtc::MutexLock lock(&accessor_lock_); RTC_DCHECK(rtp_dtls_transport_->internal()); rtp_dtls_transport_->internal()->ice_transport()->SetIceParameters( ice_parameters); - if (rtcp_dtls_transport_) { - RTC_DCHECK(rtcp_dtls_transport_->internal()); - rtcp_dtls_transport_->internal()->ice_transport()->SetIceParameters( - ice_parameters); + { + webrtc::MutexLock lock(&accessor_lock_); + if (rtcp_dtls_transport_) { + RTC_DCHECK(rtcp_dtls_transport_->internal()); + rtcp_dtls_transport_->internal()->ice_transport()->SetIceParameters( + ice_parameters); + } } - } // If PRANSWER/ANSWER is set, we should decide transport protocol type. if (type == SdpType::kPrAnswer || type == SdpType::kAnswer) { error = NegotiateAndSetDtlsParameters(type); @@ -260,27 +257,24 @@ webrtc::RTCError JsepTransport::SetRemoteJsepTransportDescription( } // If doing SDES, setup the SDES crypto parameters. - { - webrtc::MutexLock lock(&accessor_lock_); - if (sdes_transport_) { - RTC_DCHECK(!unencrypted_rtp_transport_); - RTC_DCHECK(!dtls_srtp_transport_); - if (!SetSdes(jsep_description.cryptos, - jsep_description.encrypted_header_extension_ids, type, - ContentSource::CS_REMOTE)) { - return webrtc::RTCError(webrtc::RTCErrorType::INVALID_PARAMETER, - "Failed to setup SDES crypto parameters."); - } - sdes_transport_->CacheRtpAbsSendTimeHeaderExtension( - jsep_description.rtp_abs_sendtime_extn_id); - } else if (dtls_srtp_transport_) { - RTC_DCHECK(!unencrypted_rtp_transport_); - RTC_DCHECK(!sdes_transport_); - dtls_srtp_transport_->UpdateSendEncryptedHeaderExtensionIds( - jsep_description.encrypted_header_extension_ids); - dtls_srtp_transport_->CacheRtpAbsSendTimeHeaderExtension( - jsep_description.rtp_abs_sendtime_extn_id); + if (sdes_transport_) { + RTC_DCHECK(!unencrypted_rtp_transport_); + RTC_DCHECK(!dtls_srtp_transport_); + if (!SetSdes(jsep_description.cryptos, + jsep_description.encrypted_header_extension_ids, type, + ContentSource::CS_REMOTE)) { + return webrtc::RTCError(webrtc::RTCErrorType::INVALID_PARAMETER, + "Failed to setup SDES crypto parameters."); } + sdes_transport_->CacheRtpAbsSendTimeHeaderExtension( + jsep_description.rtp_abs_sendtime_extn_id); + } else if (dtls_srtp_transport_) { + RTC_DCHECK(!unencrypted_rtp_transport_); + RTC_DCHECK(!sdes_transport_); + dtls_srtp_transport_->UpdateSendEncryptedHeaderExtensionIds( + jsep_description.encrypted_header_extension_ids); + dtls_srtp_transport_->CacheRtpAbsSendTimeHeaderExtension( + jsep_description.rtp_abs_sendtime_extn_id); } remote_description_.reset(new JsepTransportDescription(jsep_description)); @@ -341,7 +335,6 @@ void JsepTransport::SetNeedsIceRestartFlag() { absl::optional JsepTransport::GetDtlsRole() const { RTC_DCHECK_RUN_ON(network_thread_); - webrtc::MutexLock lock(&accessor_lock_); RTC_DCHECK(rtp_dtls_transport_); RTC_DCHECK(rtp_dtls_transport_->internal()); rtc::SSLRole dtls_role; @@ -354,14 +347,17 @@ absl::optional JsepTransport::GetDtlsRole() const { bool JsepTransport::GetStats(TransportStats* stats) { RTC_DCHECK_RUN_ON(network_thread_); - webrtc::MutexLock lock(&accessor_lock_); stats->transport_name = mid(); stats->channel_stats.clear(); RTC_DCHECK(rtp_dtls_transport_->internal()); - bool ret = GetTransportStats(rtp_dtls_transport_->internal(), stats); + bool ret = GetTransportStats(rtp_dtls_transport_->internal(), + ICE_CANDIDATE_COMPONENT_RTP, stats); + + webrtc::MutexLock lock(&accessor_lock_); if (rtcp_dtls_transport_) { RTC_DCHECK(rtcp_dtls_transport_->internal()); - ret &= GetTransportStats(rtcp_dtls_transport_->internal(), stats); + ret &= GetTransportStats(rtcp_dtls_transport_->internal(), + ICE_CANDIDATE_COMPONENT_RTCP, stats); } return ret; } @@ -396,7 +392,6 @@ webrtc::RTCError JsepTransport::VerifyCertificateFingerprint( void JsepTransport::SetActiveResetSrtpParams(bool active_reset_srtp_params) { RTC_DCHECK_RUN_ON(network_thread_); - webrtc::MutexLock lock(&accessor_lock_); if (dtls_srtp_transport_) { RTC_LOG(INFO) << "Setting active_reset_srtp_params of DtlsSrtpTransport to: " @@ -471,29 +466,25 @@ bool JsepTransport::SetRtcpMux(bool enable, } void JsepTransport::ActivateRtcpMux() { - { - // Don't hold the network_thread_ lock while calling other functions, - // since they might call other functions that call RTC_DCHECK_RUN_ON. - // TODO(https://crbug.com/webrtc/10318): Simplify when possible. - RTC_DCHECK_RUN_ON(network_thread_); + RTC_DCHECK_RUN_ON(network_thread_); + + if (unencrypted_rtp_transport_) { + RTC_DCHECK(!sdes_transport_); + RTC_DCHECK(!dtls_srtp_transport_); + unencrypted_rtp_transport_->SetRtcpPacketTransport(nullptr); + } else if (sdes_transport_) { + RTC_DCHECK(!unencrypted_rtp_transport_); + RTC_DCHECK(!dtls_srtp_transport_); + sdes_transport_->SetRtcpPacketTransport(nullptr); + } else if (dtls_srtp_transport_) { + RTC_DCHECK(dtls_srtp_transport_); + RTC_DCHECK(!unencrypted_rtp_transport_); + RTC_DCHECK(!sdes_transport_); + dtls_srtp_transport_->SetDtlsTransports(rtp_dtls_transport(), + /*rtcp_dtls_transport=*/nullptr); } { webrtc::MutexLock lock(&accessor_lock_); - if (unencrypted_rtp_transport_) { - RTC_DCHECK(!sdes_transport_); - RTC_DCHECK(!dtls_srtp_transport_); - unencrypted_rtp_transport_->SetRtcpPacketTransport(nullptr); - } else if (sdes_transport_) { - RTC_DCHECK(!unencrypted_rtp_transport_); - RTC_DCHECK(!dtls_srtp_transport_); - sdes_transport_->SetRtcpPacketTransport(nullptr); - } else if (dtls_srtp_transport_) { - RTC_DCHECK(dtls_srtp_transport_); - RTC_DCHECK(!unencrypted_rtp_transport_); - RTC_DCHECK(!sdes_transport_); - dtls_srtp_transport_->SetDtlsTransports(rtp_dtls_transport_locked(), - /*rtcp_dtls_transport=*/nullptr); - } rtcp_dtls_transport_ = nullptr; // Destroy this reference. } // Notify the JsepTransportController to update the aggregate states. @@ -687,17 +678,12 @@ webrtc::RTCError JsepTransport::NegotiateDtlsRole( } bool JsepTransport::GetTransportStats(DtlsTransportInternal* dtls_transport, + int component, TransportStats* stats) { RTC_DCHECK_RUN_ON(network_thread_); RTC_DCHECK(dtls_transport); TransportChannelStats substats; - if (rtcp_dtls_transport_) { - substats.component = dtls_transport == rtcp_dtls_transport_->internal() - ? ICE_CANDIDATE_COMPONENT_RTCP - : ICE_CANDIDATE_COMPONENT_RTP; - } else { - substats.component = ICE_CANDIDATE_COMPONENT_RTP; - } + substats.component = component; dtls_transport->GetSslVersionBytes(&substats.ssl_version_bytes); dtls_transport->GetSrtpCryptoSuite(&substats.srtp_crypto_suite); dtls_transport->GetSslCipherSuite(&substats.ssl_cipher_suite); diff --git a/pc/jsep_transport.h b/pc/jsep_transport.h index ad7faa0a55..56fc2d9a74 100644 --- a/pc/jsep_transport.h +++ b/pc/jsep_transport.h @@ -131,9 +131,8 @@ class JsepTransport : public sigslot::has_slots<> { // that are part of this Transport. webrtc::RTCError SetRemoteJsepTransportDescription( const JsepTransportDescription& jsep_description, - webrtc::SdpType type) RTC_LOCKS_EXCLUDED(accessor_lock_); - webrtc::RTCError AddRemoteCandidates(const Candidates& candidates) - RTC_LOCKS_EXCLUDED(accessor_lock_); + webrtc::SdpType type); + webrtc::RTCError AddRemoteCandidates(const Candidates& candidates); // Set the "needs-ice-restart" flag as described in JSEP. After the flag is // set, offers should generate new ufrags/passwords until an ICE restart @@ -152,8 +151,7 @@ class JsepTransport : public sigslot::has_slots<> { // Returns role if negotiated, or empty absl::optional if it hasn't been // negotiated yet. - absl::optional GetDtlsRole() const - RTC_LOCKS_EXCLUDED(accessor_lock_); + absl::optional GetDtlsRole() const; // TODO(deadbeef): Make this const. See comment in transportcontroller.h. bool GetStats(TransportStats* stats) RTC_LOCKS_EXCLUDED(accessor_lock_); @@ -168,26 +166,32 @@ class JsepTransport : public sigslot::has_slots<> { return remote_description_.get(); } - webrtc::RtpTransportInternal* rtp_transport() const - RTC_LOCKS_EXCLUDED(accessor_lock_) { - webrtc::MutexLock lock(&accessor_lock_); - return default_rtp_transport(); + // Returns the rtp transport, if any. + webrtc::RtpTransportInternal* rtp_transport() const { + if (dtls_srtp_transport_) { + return dtls_srtp_transport_.get(); + } + if (sdes_transport_) { + return sdes_transport_.get(); + } + if (unencrypted_rtp_transport_) { + return unencrypted_rtp_transport_.get(); + } + return nullptr; } - const DtlsTransportInternal* rtp_dtls_transport() const - RTC_LOCKS_EXCLUDED(accessor_lock_) { - webrtc::MutexLock lock(&accessor_lock_); + const DtlsTransportInternal* rtp_dtls_transport() const { if (rtp_dtls_transport_) { return rtp_dtls_transport_->internal(); - } else { - return nullptr; } + return nullptr; } - DtlsTransportInternal* rtp_dtls_transport() - RTC_LOCKS_EXCLUDED(accessor_lock_) { - webrtc::MutexLock lock(&accessor_lock_); - return rtp_dtls_transport_locked(); + DtlsTransportInternal* rtp_dtls_transport() { + if (rtp_dtls_transport_) { + return rtp_dtls_transport_->internal(); + } + return nullptr; } const DtlsTransportInternal* rtcp_dtls_transport() const @@ -195,9 +199,8 @@ class JsepTransport : public sigslot::has_slots<> { webrtc::MutexLock lock(&accessor_lock_); if (rtcp_dtls_transport_) { return rtcp_dtls_transport_->internal(); - } else { - return nullptr; } + return nullptr; } DtlsTransportInternal* rtcp_dtls_transport() @@ -205,28 +208,21 @@ class JsepTransport : public sigslot::has_slots<> { webrtc::MutexLock lock(&accessor_lock_); if (rtcp_dtls_transport_) { return rtcp_dtls_transport_->internal(); - } else { - return nullptr; } + return nullptr; } - rtc::scoped_refptr RtpDtlsTransport() - RTC_LOCKS_EXCLUDED(accessor_lock_) { - webrtc::MutexLock lock(&accessor_lock_); + rtc::scoped_refptr RtpDtlsTransport() { return rtp_dtls_transport_; } - rtc::scoped_refptr SctpTransport() const - RTC_LOCKS_EXCLUDED(accessor_lock_) { - webrtc::MutexLock lock(&accessor_lock_); + rtc::scoped_refptr SctpTransport() const { return sctp_transport_; } // TODO(bugs.webrtc.org/9719): Delete method, update callers to use // SctpTransport() instead. - webrtc::DataChannelTransportInterface* data_channel_transport() const - RTC_LOCKS_EXCLUDED(accessor_lock_) { - webrtc::MutexLock lock(&accessor_lock_); + webrtc::DataChannelTransportInterface* data_channel_transport() const { if (sctp_data_channel_transport_) { return sctp_data_channel_transport_.get(); } @@ -248,19 +244,9 @@ class JsepTransport : public sigslot::has_slots<> { const rtc::RTCCertificate* certificate, const rtc::SSLFingerprint* fingerprint) const; - void SetActiveResetSrtpParams(bool active_reset_srtp_params) - RTC_LOCKS_EXCLUDED(accessor_lock_); + void SetActiveResetSrtpParams(bool active_reset_srtp_params); private: - DtlsTransportInternal* rtp_dtls_transport_locked() - RTC_EXCLUSIVE_LOCKS_REQUIRED(accessor_lock_) { - if (rtp_dtls_transport_) { - return rtp_dtls_transport_->internal(); - } else { - return nullptr; - } - } - bool SetRtcpMux(bool enable, webrtc::SdpType type, ContentSource source); void ActivateRtcpMux() RTC_LOCKS_EXCLUDED(accessor_lock_); @@ -268,8 +254,7 @@ class JsepTransport : public sigslot::has_slots<> { bool SetSdes(const std::vector& cryptos, const std::vector& encrypted_extension_ids, webrtc::SdpType type, - ContentSource source) - RTC_EXCLUSIVE_LOCKS_REQUIRED(accessor_lock_); + ContentSource source); // Negotiates and sets the DTLS parameters based on the current local and // remote transport description, such as the DTLS role to use, and whether @@ -286,8 +271,7 @@ class JsepTransport : public sigslot::has_slots<> { webrtc::SdpType local_description_type, ConnectionRole local_connection_role, ConnectionRole remote_connection_role, - absl::optional* negotiated_dtls_role) - RTC_LOCKS_EXCLUDED(accessor_lock_); + absl::optional* negotiated_dtls_role); // Pushes down the ICE parameters from the remote description. void SetRemoteIceParameters(const IceParameters& ice_parameters, @@ -300,22 +284,8 @@ class JsepTransport : public sigslot::has_slots<> { rtc::SSLFingerprint* remote_fingerprint); bool GetTransportStats(DtlsTransportInternal* dtls_transport, - TransportStats* stats) - RTC_EXCLUSIVE_LOCKS_REQUIRED(accessor_lock_); - - // Returns the default (non-datagram) rtp transport, if any. - webrtc::RtpTransportInternal* default_rtp_transport() const - RTC_EXCLUSIVE_LOCKS_REQUIRED(accessor_lock_) { - if (dtls_srtp_transport_) { - return dtls_srtp_transport_.get(); - } else if (sdes_transport_) { - return sdes_transport_.get(); - } else if (unencrypted_rtp_transport_) { - return unencrypted_rtp_transport_.get(); - } else { - return nullptr; - } - } + int component, + TransportStats* stats); // Owning thread, for safety checks const rtc::Thread* const network_thread_; @@ -345,6 +315,8 @@ class JsepTransport : public sigslot::has_slots<> { const std::unique_ptr dtls_srtp_transport_; const rtc::scoped_refptr rtp_dtls_transport_; + // TODO(tommi): Restrict use to network thread, or make const. And delete the + // `accessor_lock_`. rtc::scoped_refptr rtcp_dtls_transport_ RTC_GUARDED_BY(accessor_lock_);