Record DTLS handshake errors without referencing the PeerConnection object JsepTransportController invokes the DTLS handshake error callback on the network thread. The previous callback checked a PeerConnection WeakPtr there, which binds WeakPtr validity checks to the network thread and can DCHECK when PeerConnection is destroyed on the signaling thread. Record the existing histogram directly from the callback instead. The callback no longer captures PeerConnection state, so it is safe when invoked on the network thread or during PeerConnection teardown. Remove the now-unused PeerConnection WeakPtrFactory. Bug: webrtc:514547040 Change-Id: I92251ceb1235d219a8c1d62c9ff9f7dd09a591eb Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/473760 Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org> Reviewed-by: Harald Alvestrand <hta@webrtc.org> Commit-Queue: Boris Tsirkin <btsirkin@meta.com> Cr-Commit-Position: refs/heads/main@{#47749}
diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index d85270a..d391a25 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc
@@ -494,8 +494,7 @@ data_channel_controller_(this), message_handler_(signaling_thread()), codec_lookup_helper_( - std::make_unique<CodecLookupHelperForPeerConnection>(this)), - weak_factory_(this) { + std::make_unique<CodecLookupHelperForPeerConnection>(this)) { // Field trials specific to the peerconnection should be owned by the `env`, RTC_DCHECK(dependencies.trials == nullptr); @@ -631,12 +630,11 @@ config.ice_transport_factory = ice_transport_factory_.get(); config.dtls_transport_factory = dtls_transport_factory_.get(); config.rtp_transport_factory = rtp_transport_factory_.get(); - config.on_dtls_handshake_error = - [weak_ptr = weak_factory_.GetWeakPtr()](SSLHandshakeError s) { - if (weak_ptr) { - weak_ptr->OnTransportControllerDtlsHandshakeError(s); - } - }; + config.on_dtls_handshake_error = [](SSLHandshakeError s) { + RTC_HISTOGRAM_ENUMERATION("WebRTC.PeerConnection.DtlsHandshakeError", + static_cast<int>(s), + static_cast<int>(SSLHandshakeError::MAX_VALUE)); + }; config.signal_ice_candidates_gathered = [this](absl::string_view transport, const std::vector<Candidate>& candidates) { @@ -2544,13 +2542,6 @@ OnSelectedCandidatePairChanged(event); } -void PeerConnection::OnTransportControllerDtlsHandshakeError( - SSLHandshakeError error) { - RTC_HISTOGRAM_ENUMERATION("WebRTC.PeerConnection.DtlsHandshakeError", - static_cast<int>(error), - static_cast<int>(SSLHandshakeError::MAX_VALUE)); -} - // Returns the media index for a local ice candidate given the content name. bool PeerConnection::GetLocalCandidateMediaIndex(absl::string_view content_name, int* sdp_mline_index) {
diff --git a/pc/peer_connection.h b/pc/peer_connection.h index e1d1353..67748f5 100644 --- a/pc/peer_connection.h +++ b/pc/peer_connection.h
@@ -98,7 +98,6 @@ #include "rtc_base/system/plan_b_only.h" #include "rtc_base/thread.h" #include "rtc_base/thread_annotations.h" -#include "rtc_base/weak_ptr.h" namespace webrtc { @@ -586,7 +585,6 @@ const std::vector<Candidate>& candidates) RTC_RUN_ON(signaling_thread()); void OnTransportControllerCandidateChanged( const CandidatePairChangeEvent& event) RTC_RUN_ON(signaling_thread()); - void OnTransportControllerDtlsHandshakeError(SSLHandshakeError error); // Invoked when TransportController connection completion is signaled. // Reports stats for all transports in use. @@ -778,9 +776,6 @@ pending_local_description_clone_; mutable std::unique_ptr<SessionDescriptionInterface> pending_remote_description_clone_; - - // This variable needs to be the last one in the class. - WeakPtrFactory<PeerConnection> weak_factory_; }; } // namespace webrtc