From a09b921dd46cdeef45b253aaef1eb9e7811b0fd1 Mon Sep 17 00:00:00 2001 From: Philipp Hancke Date: Wed, 22 Jun 2022 07:41:22 +0200 Subject: [PATCH] pc: flush getStats cache in addIceCandidate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BUG=webrtc:14190 Change-Id: I6faf35af7b124f4d5258204f7813cedcf3275f42 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/265878 Commit-Queue: Philipp Hancke Reviewed-by: Harald Alvestrand Reviewed-by: Henrik Boström Commit-Queue: Henrik Boström Cr-Commit-Position: refs/heads/main@{#37297} --- pc/peer_connection.cc | 7 +++- pc/peer_connection_integrationtest.cc | 51 ++++++++++++++++++++++++--- 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 11004ba743..3bd62c89f3 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -1614,6 +1614,7 @@ RTCError PeerConnection::SetConfiguration( bool PeerConnection::AddIceCandidate( const IceCandidateInterface* ice_candidate) { RTC_DCHECK_RUN_ON(signaling_thread()); + ClearStatsCache(); return sdp_handler_->AddIceCandidate(ice_candidate); } @@ -1621,7 +1622,11 @@ void PeerConnection::AddIceCandidate( std::unique_ptr candidate, std::function callback) { RTC_DCHECK_RUN_ON(signaling_thread()); - sdp_handler_->AddIceCandidate(std::move(candidate), callback); + sdp_handler_->AddIceCandidate(std::move(candidate), + [this, callback](webrtc::RTCError result) { + ClearStatsCache(); + callback(result); + }); } bool PeerConnection::RemoveIceCandidates( diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc index d6ac0c82c9..19db051629 100644 --- a/pc/peer_connection_integrationtest.cc +++ b/pc/peer_connection_integrationtest.cc @@ -25,6 +25,7 @@ #include #include "absl/algorithm/container.h" +#include "absl/memory/memory.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" #include "api/async_resolver_factory.h" @@ -2432,11 +2433,9 @@ TEST_P(PeerConnectionIntegrationTestWithFakeClock, first_report->GetStatsOfType(); ASSERT_EQ(first_candidate_stats.size(), 0u); - // Start candidate gathering and wait for it to complete. + // Create an offer at the caller and set it as remote description on the + // callee. caller()->CreateAndSetAndSignalOffer(); - ASSERT_TRUE_SIMULATED_WAIT(caller()->IceGatheringStateComplete(), - kDefaultTimeout, FakeClock()); - // Call getStats again, assert there are candidates now. rtc::scoped_refptr second_report = caller()->NewGetStats(); @@ -2450,6 +2449,50 @@ TEST_P(PeerConnectionIntegrationTestWithFakeClock, EXPECT_EQ(first_report->timestamp_us(), second_report->timestamp_us()); } +TEST_P(PeerConnectionIntegrationTestWithFakeClock, + AddIceCandidateFlushesGetStatsCache) { + ASSERT_TRUE(CreatePeerConnectionWrappers()); + ConnectFakeSignalingForSdpOnly(); + caller()->AddAudioTrack(); + + // Start candidate gathering and wait for it to complete. Candidates are not + // signalled. + caller()->CreateAndSetAndSignalOffer(); + ASSERT_TRUE_SIMULATED_WAIT(caller()->IceGatheringStateComplete(), + kDefaultTimeout, FakeClock()); + + // Call getStats, assert there are no candidates. + rtc::scoped_refptr first_report = + caller()->NewGetStats(); + ASSERT_TRUE(first_report); + auto first_candidate_stats = + first_report->GetStatsOfType(); + ASSERT_EQ(first_candidate_stats.size(), 0u); + + // Add a "fake" candidate. + absl::optional result; + caller()->pc()->AddIceCandidate( + absl::WrapUnique(webrtc::CreateIceCandidate( + "", 0, + "candidate:2214029314 1 udp 2122260223 127.0.0.1 49152 typ host", + nullptr)), + [&result](RTCError r) { result = r; }); + ASSERT_TRUE_WAIT(result.has_value(), kDefaultTimeout); + ASSERT_TRUE(result.value().ok()); + + // Call getStats again, assert there is a remote candidate now. + rtc::scoped_refptr second_report = + caller()->NewGetStats(); + ASSERT_TRUE(second_report); + auto second_candidate_stats = + second_report->GetStatsOfType(); + ASSERT_EQ(second_candidate_stats.size(), 1u); + + // The fake clock ensures that no time has passed so the cache must have been + // explicitly invalidated. + EXPECT_EQ(first_report->timestamp_us(), second_report->timestamp_us()); +} + #endif // !defined(THREAD_SANITIZER) // Verify that a TurnCustomizer passed in through RTCConfiguration