commit e2cf0181938747a50ac8541ae9bad70afd07a4e1 parent 35f6a63ac9de0fc466228497e65ef6a7c999bd32 Author: Michael Froman <mfroman@mozilla.com> Date: Wed, 8 Oct 2025 23:10:33 -0500 Bug 1993083 - Vendor libwebrtc from 4fad261111 Upstream commit: https://webrtc.googlesource.com/src/+/4fad261111feaad6d16d2de094dd1b53568c6237 Add PeerConnection::RemoveIceCandidate for IceCandidate This also removes internal usage of the RemoveIceCandidates (plural) method and marks it as deprecated. There still remain code paths related to candidate removal (including callbacks) where the Candidate::transport_name() property is still needed. Once we remove the deprecated method and the callback code paths, that property can be removed too. No-Iwyu: Unrelated include fixes needed. Bug: webrtc:8395 Change-Id: I54ce6257afe0794b90ee41c736be3abef3b4c3f4 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/394220 Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org> Reviewed-by: Harald Alvestrand <hta@webrtc.org> Cr-Commit-Position: refs/heads/main@{#45103} Diffstat:
17 files changed, 130 insertions(+), 68 deletions(-)
diff --git a/third_party/libwebrtc/README.mozilla.last-vendor b/third_party/libwebrtc/README.mozilla.last-vendor @@ -1,4 +1,4 @@ # ./mach python dom/media/webrtc/third_party_build/vendor-libwebrtc.py --from-local /home/mfroman/mozilla/elm/.moz-fast-forward/moz-libwebrtc --commit mozpatches libwebrtc -libwebrtc updated from /home/mfroman/mozilla/elm/.moz-fast-forward/moz-libwebrtc commit mozpatches on 2025-10-09T04:09:19.575408+00:00. +libwebrtc updated from /home/mfroman/mozilla/elm/.moz-fast-forward/moz-libwebrtc commit mozpatches on 2025-10-09T04:10:23.508359+00:00. # base of lastest vendoring -a53fa19c7f +4fad261111 diff --git a/third_party/libwebrtc/api/jsep_session_description.h b/third_party/libwebrtc/api/jsep_session_description.h @@ -75,8 +75,9 @@ class JsepSessionDescription : public SessionDescriptionInterface { SdpType type_; std::vector<JsepCandidateCollection> candidate_collection_; - bool GetMediasectionIndex(const IceCandidate* candidate, size_t* index); - int GetMediasectionIndex(const Candidate& candidate); + bool IsValidMLineIndex(int index) const; + bool GetMediasectionIndex(const IceCandidate* candidate, size_t* index) const; + int GetMediasectionIndex(absl::string_view mid) const; }; } // namespace webrtc diff --git a/third_party/libwebrtc/api/peer_connection_interface.h b/third_party/libwebrtc/api/peer_connection_interface.h @@ -1124,8 +1124,9 @@ class RTC_EXPORT PeerConnectionInterface : public RefCountInterface { virtual bool AddIceCandidate(const IceCandidate* candidate) = 0; // TODO(hbos): Remove default implementation once implemented by downstream // projects. - virtual void AddIceCandidate(std::unique_ptr<IceCandidate> /* candidate */, - std::function<void(RTCError)> /* callback */) {} + virtual void AddIceCandidate(std::unique_ptr<IceCandidate> candidate, + std::function<void(RTCError)> callback) {} + virtual bool RemoveIceCandidate(const IceCandidate* candidate) = 0; // Removes a group of remote candidates from the ICE agent. Needed mainly for // continual gathering, to avoid an ever-growing list of candidates as @@ -1133,6 +1134,7 @@ class RTC_EXPORT PeerConnectionInterface : public RefCountInterface { // to the MID of the m= section that generated the candidate. // TODO(bugs.webrtc.org/8395): Use IceCandidate instead of // Candidate, which would avoid the transport_name oddity. + [[deprecated("Use IceCandidate version")]] virtual bool RemoveIceCandidates( const std::vector<Candidate>& candidates) = 0; diff --git a/third_party/libwebrtc/api/test/mock_peerconnectioninterface.h b/third_party/libwebrtc/api/test/mock_peerconnectioninterface.h @@ -199,6 +199,10 @@ class MockPeerConnectionInterface : public PeerConnectionInterface { RemoveIceCandidates, (const std::vector<Candidate>&), (override)); + MOCK_METHOD(bool, + RemoveIceCandidate, + (const IceCandidate* candidate), + (override)); MOCK_METHOD(RTCError, SetBitrate, (const BitrateSettings&), (override)); MOCK_METHOD(void, ReconfigureBandwidthEstimation, diff --git a/third_party/libwebrtc/pc/jsep_session_description.cc b/third_party/libwebrtc/pc/jsep_session_description.cc @@ -10,7 +10,9 @@ #include "api/jsep_session_description.h" +#include <algorithm> #include <cstddef> +#include <iterator> #include <memory> #include <optional> #include <string> @@ -101,7 +103,6 @@ void UpdateConnectionAddress( } media_desc->set_connection_address(connection_addr); } - } // namespace // TODO(steveanton): Remove this default implementation once Chromium has been @@ -282,7 +283,7 @@ size_t JsepSessionDescription::RemoveCandidates( const std::vector<Candidate>& candidates) { size_t num_removed = 0; for (auto& candidate : candidates) { - int mediasection_index = GetMediasectionIndex(candidate); + int mediasection_index = GetMediasectionIndex(candidate.transport_name()); if (mediasection_index < 0) { // Not found. continue; @@ -316,51 +317,33 @@ bool JsepSessionDescription::ToString(std::string* out) const { return !out->empty(); } -bool JsepSessionDescription::GetMediasectionIndex(const IceCandidate* candidate, - size_t* index) { - if (!candidate || !index) { - return false; - } +bool JsepSessionDescription::IsValidMLineIndex(int index) const { + RTC_DCHECK(description_); + return index >= 0 && + index < static_cast<int>(description_->contents().size()); +} - // If the candidate has no valid mline index or sdp_mid, it is impossible - // to find a match. - if (candidate->sdp_mid().empty() && - (candidate->sdp_mline_index() < 0 || - static_cast<size_t>(candidate->sdp_mline_index()) >= - description_->contents().size())) { +bool JsepSessionDescription::GetMediasectionIndex(const IceCandidate* candidate, + size_t* index) const { + if (!candidate || !index || !description_) { return false; } - if (candidate->sdp_mline_index() >= 0) + auto mid = candidate->sdp_mid(); + if (!mid.empty()) { + *index = GetMediasectionIndex(mid); + } else { + // An sdp_mline_index of -1 will be treated as invalid. *index = static_cast<size_t>(candidate->sdp_mline_index()); - if (description_ && !candidate->sdp_mid().empty()) { - bool found = false; - // Try to match the sdp_mid with content name. - for (size_t i = 0; i < description_->contents().size(); ++i) { - if (candidate->sdp_mid() == description_->contents().at(i).mid()) { - *index = i; - found = true; - break; - } - } - if (!found) { - // If the sdp_mid is presented but we can't find a match, we consider - // this as an error. - return false; - } } - return true; + return IsValidMLineIndex(*index); } -int JsepSessionDescription::GetMediasectionIndex(const Candidate& candidate) { - // Find the description with a matching transport name of the candidate. - const std::string& transport_name = candidate.transport_name(); - for (size_t i = 0; i < description_->contents().size(); ++i) { - if (transport_name == description_->contents().at(i).mid()) { - return static_cast<int>(i); - } - } - return -1; +int JsepSessionDescription::GetMediasectionIndex(absl::string_view mid) const { + const auto& contents = description_->contents(); + auto it = + std::find_if(contents.begin(), contents.end(), + [&](const auto& content) { return mid == content.mid(); }); + return it == contents.end() ? -1 : std::distance(contents.begin(), it); } - } // namespace webrtc diff --git a/third_party/libwebrtc/pc/jsep_session_description_unittest.cc b/third_party/libwebrtc/pc/jsep_session_description_unittest.cc @@ -166,6 +166,16 @@ TEST_F(JsepSessionDescriptionTest, CheckSessionDescription) { EXPECT_EQ(2u, jsep_desc_->number_of_mediasections()); } +TEST_F(JsepSessionDescriptionTest, IsValidMLineIndex) { + ASSERT_GT(jsep_desc_->number_of_mediasections(), 0u); + // Create a candidate with no mid and an illegal index. + IceCandidate ice_candidate("", -1, candidate_); + EXPECT_FALSE(jsep_desc_->AddCandidate(&ice_candidate)); + IceCandidate ice_candidate2("", jsep_desc_->number_of_mediasections(), + candidate_); + EXPECT_FALSE(jsep_desc_->AddCandidate(&ice_candidate2)); +} + // Test that we can add a candidate to a session description without MID. TEST_F(JsepSessionDescriptionTest, AddCandidateWithoutMid) { IceCandidate jsep_candidate("", 0, candidate_); @@ -226,9 +236,7 @@ TEST_F(JsepSessionDescriptionTest, AddAndRemoveCandidatesWithMid) { // The mline index should have been updated according to mid. EXPECT_EQ(1, ice_candidate->sdp_mline_index()); - std::vector<Candidate> candidates(1, candidate_); - candidates[0].set_transport_name(mid); - EXPECT_EQ(1u, jsep_desc_->RemoveCandidates(candidates)); + EXPECT_EQ(1u, jsep_desc_->RemoveCandidate(ice_candidate)); EXPECT_EQ(0u, jsep_desc_->candidates(0)->count()); EXPECT_EQ(0u, jsep_desc_->candidates(1)->count()); } diff --git a/third_party/libwebrtc/pc/jsep_transport_controller.cc b/third_party/libwebrtc/pc/jsep_transport_controller.cc @@ -456,6 +456,34 @@ RTCError JsepTransportController::RemoveRemoteCandidates( return RTCError::OK(); } +bool JsepTransportController::RemoveRemoteCandidate(const IceCandidate* c) { + if (!network_thread_->IsCurrent()) { + return network_thread_->BlockingCall( + [&] { return RemoveRemoteCandidate(c); }); + } + + RTC_DCHECK_RUN_ON(network_thread_); + std::string mid = c->sdp_mid(); + if (!VerifyCandidate(c->candidate()).ok() || mid.empty()) { + RTC_LOG(LS_ERROR) << "Candidate invalid or missing sdp_mid: " + << c->candidate().ToSensitiveString(); + return false; + } + JsepTransport* jsep_transport = GetJsepTransportForMid(mid); + if (!jsep_transport) { + RTC_LOG(LS_WARNING) << "No Transport for mid=" << mid; + return false; + } + DtlsTransportInternal* dtls = + c->candidate().component() == ICE_CANDIDATE_COMPONENT_RTP + ? jsep_transport->rtp_dtls_transport() + : jsep_transport->rtcp_dtls_transport(); + if (dtls) { + dtls->ice_transport()->RemoveRemoteCandidate(c->candidate()); + } + return true; +} + bool JsepTransportController::GetStats(const std::string& transport_name, TransportStats* stats) const { RTC_DCHECK_RUN_ON(network_thread_); diff --git a/third_party/libwebrtc/pc/jsep_transport_controller.h b/third_party/libwebrtc/pc/jsep_transport_controller.h @@ -211,6 +211,7 @@ class JsepTransportController : public PayloadTypeSuggester, RTCError AddRemoteCandidates(const std::string& mid, const std::vector<Candidate>& candidates); RTCError RemoveRemoteCandidates(const std::vector<Candidate>& candidates); + bool RemoveRemoteCandidate(const IceCandidate* candidate); /********************** * DTLS-related methods diff --git a/third_party/libwebrtc/pc/peer_connection.cc b/third_party/libwebrtc/pc/peer_connection.cc @@ -1578,6 +1578,12 @@ void PeerConnection::AddIceCandidate(std::unique_ptr<IceCandidate> candidate, }); } +bool PeerConnection::RemoveIceCandidate(const IceCandidate* candidate) { + TRACE_EVENT0("webrtc", "PeerConnection::RemoveIceCandidate"); + RTC_DCHECK_RUN_ON(signaling_thread()); + return sdp_handler_->RemoveIceCandidate(candidate); +} + bool PeerConnection::RemoveIceCandidates( const std::vector<Candidate>& candidates) { TRACE_EVENT0("webrtc", "PeerConnection::RemoveIceCandidates"); diff --git a/third_party/libwebrtc/pc/peer_connection.h b/third_party/libwebrtc/pc/peer_connection.h @@ -236,6 +236,7 @@ class PeerConnection : public PeerConnectionInternal, bool AddIceCandidate(const IceCandidate* candidate) override; void AddIceCandidate(std::unique_ptr<IceCandidate> candidate, std::function<void(RTCError)> callback) override; + bool RemoveIceCandidate(const IceCandidate* candidate) override; bool RemoveIceCandidates(const std::vector<Candidate>& candidates) override; RTCError SetBitrate(const BitrateSettings& bitrate) override; diff --git a/third_party/libwebrtc/pc/peer_connection_ice_unittest.cc b/third_party/libwebrtc/pc/peer_connection_ice_unittest.cc @@ -87,6 +87,7 @@ using RTCOfferAnswerOptions = PeerConnectionInterface::RTCOfferAnswerOptions; using ::testing::Combine; using ::testing::ElementsAre; +using ::testing::IsEmpty; using ::testing::NotNull; using ::testing::Pair; using ::testing::SizeIs; @@ -583,8 +584,7 @@ TEST_P(PeerConnectionIceTest, ASSERT_TRUE(caller->pc()->AddIceCandidate(ice_candidate.get())); caller->pc()->Close(); - - EXPECT_FALSE(caller->pc()->RemoveIceCandidates({candidate})); + EXPECT_FALSE(caller->pc()->RemoveIceCandidate(ice_candidate.get())); } TEST_P(PeerConnectionIceTest, @@ -605,7 +605,7 @@ TEST_P(PeerConnectionIceTest, std::unique_ptr<IceCandidate> ice_candidate = CreateIceCandidate(audio_content->mid(), 0, candidate); EXPECT_TRUE(caller->pc()->AddIceCandidate(ice_candidate.get())); - EXPECT_TRUE(caller->pc()->RemoveIceCandidates({candidate})); + EXPECT_TRUE(caller->pc()->RemoveIceCandidate(ice_candidate.get())); } TEST_P(PeerConnectionIceTest, RemoveCandidateRemovesFromRemoteDescription) { @@ -619,8 +619,11 @@ TEST_P(PeerConnectionIceTest, RemoveCandidateRemovesFromRemoteDescription) { caller->SetRemoteDescription(callee->CreateAnswerAndSetAsLocal())); Candidate candidate = CreateLocalUdpCandidate(kCalleeAddress); - ASSERT_TRUE(caller->AddIceCandidate(&candidate)); - EXPECT_TRUE(caller->pc()->RemoveIceCandidates({candidate})); + std::unique_ptr<IceCandidateInterface> ice_candidate = + caller->CreateJsepCandidateForFirstTransport(&candidate); + + ASSERT_TRUE(caller->pc()->AddIceCandidate(ice_candidate.get())); + EXPECT_TRUE(caller->pc()->RemoveIceCandidate(ice_candidate.get())); EXPECT_EQ(0u, caller->GetIceCandidatesFromRemoteDescription().size()); } @@ -1616,12 +1619,13 @@ TEST_P(PeerConnectionIceTest, PrefersMidOverMLineIndex) { // `candidate.transport_name()` is empty. Candidate candidate = CreateLocalUdpCandidate(kCalleeAddress); + ASSERT_THAT(candidate.transport_name(), IsEmpty()); auto* audio_content = GetFirstAudioContent(caller->pc()->local_description()->description()); std::unique_ptr<IceCandidate> ice_candidate = CreateIceCandidate(audio_content->mid(), 65535, candidate); EXPECT_TRUE(caller->pc()->AddIceCandidate(ice_candidate.get())); - EXPECT_TRUE(caller->pc()->RemoveIceCandidates({candidate})); + EXPECT_TRUE(caller->pc()->RemoveIceCandidate(ice_candidate.get())); } } // namespace webrtc diff --git a/third_party/libwebrtc/pc/peer_connection_proxy.h b/third_party/libwebrtc/pc/peer_connection_proxy.h @@ -158,7 +158,11 @@ PROXY_METHOD2(void, AddIceCandidate, std::unique_ptr<IceCandidate>, std::function<void(RTCError)>) +PROXY_METHOD1(bool, RemoveIceCandidate, const IceCandidate*) +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" PROXY_METHOD1(bool, RemoveIceCandidates, const std::vector<Candidate>&) +#pragma clang diagnostic pop PROXY_METHOD1(RTCError, SetBitrate, const BitrateSettings&) PROXY_METHOD1(void, ReconfigureBandwidthEstimation, diff --git a/third_party/libwebrtc/pc/sdp_offer_answer.cc b/third_party/libwebrtc/pc/sdp_offer_answer.cc @@ -2992,6 +2992,22 @@ void SdpOfferAnswerHandler::AddIceCandidate( }); } +bool SdpOfferAnswerHandler::RemoveIceCandidate(const IceCandidate* candidate) { + TRACE_EVENT0("webrtc", "SdpOfferAnswerHandler::RemoveIceCandidate"); + RTC_DCHECK_RUN_ON(signaling_thread()); + if (pc_->IsClosed() || !remote_description() || !candidate) { + RTC_LOG(LS_ERROR) << "RemoveIceCandidate: PeerConnection is closed."; + return false; + } + + bool removed = mutable_remote_description()->RemoveCandidate(candidate); + // Remove the candidates from the transport controller. + // This involves a hop to the network thread - should we rather do this + // asynchronously? + transport_controller_s()->RemoveRemoteCandidate(candidate); + return removed; +} + bool SdpOfferAnswerHandler::RemoveIceCandidates( const std::vector<Candidate>& candidates) { TRACE_EVENT0("webrtc", "SdpOfferAnswerHandler::RemoveIceCandidates"); @@ -3002,23 +3018,22 @@ bool SdpOfferAnswerHandler::RemoveIceCandidates( } if (!remote_description()) { - RTC_LOG(LS_ERROR) << "RemoveIceCandidates: ICE candidates can't be removed " - "without any remote session description."; + RTC_LOG(LS_ERROR) << "RemoveIceCandidates: No remote description."; return false; } if (candidates.empty()) { - RTC_LOG(LS_ERROR) << "RemoveIceCandidates: candidates are empty."; + RTC_LOG(LS_ERROR) << "RemoveIceCandidates: No candidates."; return false; } - size_t number_removed = + const size_t number_removed = mutable_remote_description()->RemoveCandidates(candidates); if (number_removed != candidates.size()) { RTC_LOG(LS_ERROR) << "RemoveIceCandidates: Failed to remove candidates. Requested " << candidates.size() << " but only " << number_removed - << " are removed."; + << " were removed."; } // Remove the candidates from the transport controller. @@ -3028,6 +3043,8 @@ bool SdpOfferAnswerHandler::RemoveIceCandidates( << "RemoveIceCandidates: Error when removing remote candidates: " << error.message(); } + // Technically it would be more correct to return `number_removed != 0u` here, + // but some downstream code needs to be updated first. return true; } diff --git a/third_party/libwebrtc/pc/sdp_offer_answer.h b/third_party/libwebrtc/pc/sdp_offer_answer.h @@ -149,6 +149,7 @@ class SdpOfferAnswerHandler : public SdpStateProvider { bool AddIceCandidate(const IceCandidate* candidate); void AddIceCandidate(std::unique_ptr<IceCandidate> candidate, std::function<void(RTCError)> callback); + bool RemoveIceCandidate(const IceCandidate* candidate); bool RemoveIceCandidates(const std::vector<Candidate>& candidates); // Adds a locally generated candidate to the local description. void AddLocalIceCandidate(const IceCandidate* candidate); diff --git a/third_party/libwebrtc/pc/test/fake_peer_connection_base.h b/third_party/libwebrtc/pc/test/fake_peer_connection_base.h @@ -229,6 +229,10 @@ class FakePeerConnectionBase : public PeerConnectionInternal { bool AddIceCandidate(const IceCandidate* candidate) override { return false; } + bool RemoveIceCandidate(const IceCandidate* candidate) override { + return false; + } + bool RemoveIceCandidates(const std::vector<Candidate>& candidates) override { return false; } diff --git a/third_party/libwebrtc/pc/test/mock_peer_connection_internal.h b/third_party/libwebrtc/pc/test/mock_peer_connection_internal.h @@ -211,6 +211,10 @@ class MockPeerConnectionInternal : public PeerConnectionInternal { (override)); MOCK_METHOD(bool, AddIceCandidate, (const IceCandidate*), (override)); MOCK_METHOD(bool, + RemoveIceCandidate, + (const IceCandidate* candidate), + (override)); + MOCK_METHOD(bool, RemoveIceCandidates, (const std::vector<webrtc::Candidate>&), (override)); diff --git a/third_party/libwebrtc/sdk/objc/api/peerconnection/RTCPeerConnection.mm b/third_party/libwebrtc/sdk/objc/api/peerconnection/RTCPeerConnection.mm @@ -636,19 +636,13 @@ void PeerConnectionDelegateAdapter::OnRemoveTrack( } - (void)removeIceCandidates: (NSArray<RTC_OBJC_TYPE(RTCIceCandidate) *> *)iceCandidates { - std::vector<webrtc::Candidate> candidates; for (RTC_OBJC_TYPE(RTCIceCandidate) * iceCandidate in iceCandidates) { - std::unique_ptr<const webrtc::IceCandidate> candidate( + std::unique_ptr<webrtc::IceCandidate> candidate( iceCandidate.nativeCandidate); if (candidate) { - candidates.push_back(candidate->candidate()); - // Need to fill the transport name from the sdp_mid. - candidates.back().set_transport_name(candidate->sdp_mid()); + _peerConnection->RemoveIceCandidate(candidate.get()); } } - if (!candidates.empty()) { - _peerConnection->RemoveIceCandidates(candidates); - } } - (void)addStream:(RTC_OBJC_TYPE(RTCMediaStream) *)stream {