diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 37f19e09a0..4953494d88 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -3695,6 +3695,13 @@ bool PeerConnection::AddIceCandidate( if (ready) { bool result = UseCandidate(ice_candidate); if (result) { + NoteUsageEvent(UsageEvent::REMOTE_CANDIDATE_ADDED); + if (ice_candidate->candidate().address().IsUnresolvedIP()) { + NoteUsageEvent(UsageEvent::REMOTE_MDNS_CANDIDATE_ADDED); + } + if (ice_candidate->candidate().address().IsPrivateIP()) { + NoteUsageEvent(UsageEvent::REMOTE_PRIVATE_CANDIDATE_ADDED); + } NoteAddIceCandidateResult(kAddIceCandidateSuccess); } else { NoteAddIceCandidateResult(kAddIceCandidateFailNotUsable); @@ -6337,13 +6344,6 @@ bool PeerConnection::UseCandidate(const IceCandidateInterface* candidate) { RTCError error = transport_controller_->AddRemoteCandidates( result.value()->name, candidates); if (error.ok()) { - NoteUsageEvent(UsageEvent::REMOTE_CANDIDATE_ADDED); - if (candidate->candidate().address().IsUnresolvedIP()) { - NoteUsageEvent(UsageEvent::REMOTE_MDNS_CANDIDATE_ADDED); - } - if (candidate->candidate().address().IsPrivateIP()) { - NoteUsageEvent(UsageEvent::REMOTE_PRIVATE_CANDIDATE_ADDED); - } // Candidates successfully submitted for checking. if (ice_connection_state_ == PeerConnectionInterface::kIceConnectionNew || ice_connection_state_ == diff --git a/pc/peer_connection_histogram_unittest.cc b/pc/peer_connection_histogram_unittest.cc index cb8210c3f0..2f997aecbd 100644 --- a/pc/peer_connection_histogram_unittest.cc +++ b/pc/peer_connection_histogram_unittest.cc @@ -18,7 +18,6 @@ #include "absl/types/optional.h" #include "api/call/call_factory_interface.h" #include "api/jsep.h" -#include "api/jsep_session_description.h" #include "api/peer_connection_interface.h" #include "api/peer_connection_proxy.h" #include "api/rtc_error.h" @@ -33,7 +32,6 @@ #include "pc/peer_connection_wrapper.h" #include "pc/sdp_utils.h" #include "pc/test/mock_peer_connection_observers.h" -#include "pc/webrtc_sdp.h" #include "rtc_base/arraysize.h" #include "rtc_base/checks.h" #include "rtc_base/fake_mdns_responder.h" @@ -208,10 +206,6 @@ class PeerConnectionWrapperForUsageHistogramTest return true; } - webrtc::PeerConnectionInterface::IceGatheringState ice_gathering_state() { - return pc()->ice_gathering_state(); - } - private: // Candidates that have been sent but not yet configured std::vector> @@ -669,77 +663,6 @@ TEST_F(PeerConnectionUsageHistogramTest, FingerprintWithPrivateIPCallee) { #ifndef WEBRTC_ANDROID #ifdef HAVE_SCTP -// Test that the usage pattern bits for adding remote private or mDNS candidates -// are set when the remote candidates are retrieved from Offer/Answer SDP -// instead of trickled ICE messages. -TEST_F(PeerConnectionUsageHistogramTest, - AddRemoteCandidatesFromRemoteDescription) { - // We construct the following data-channel-only scenario. The caller collects - // private local candidates and appends them in the Offer as in non-trickled - // sessions. The callee collects mDNS candidates. Only Offer is signaled to - // the callee and we expect a connection with prflx candidates. - auto caller = CreatePeerConnectionWithPrivateLocalAddresses(); - auto callee = CreatePeerConnectionWithMdns(RTCConfiguration()); - caller->CreateDataChannel("test_channel"); - ASSERT_TRUE(caller->SetLocalDescription(caller->CreateOffer())); - // Wait until the gathering completes (or at least having gathered one - // candidate) so that the session description would have contained ICE - // candidates. - EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceGatheringComplete, - caller->ice_gathering_state(), kDefaultTimeout); - // Get the current offer that contains candidates and pass it to the callee. - // - // Note that we cannot use CloneSessionDescription on |cur_offer| to obtain an - // SDP with candidates. The method above does not strictly copy everything, in - // particular, not copying the ICE candidates. - // TODO(qingsi): Technically, this is a bug. Fix it. - auto cur_offer = caller->pc()->local_description(); - ASSERT_TRUE(cur_offer); - std::string sdp_with_candidates_str; - cur_offer->ToString(&sdp_with_candidates_str); - auto offer = absl::make_unique(SdpType::kOffer); - ASSERT_TRUE(SdpDeserialize(sdp_with_candidates_str, offer.get(), - nullptr /* error */)); - ASSERT_TRUE(callee->SetRemoteDescription(std::move(offer))); - - auto answer = callee->CreateAnswer(); - callee->SetLocalDescription(CloneSessionDescription(answer.get())); - caller->SetRemoteDescription(std::move(answer)); - EXPECT_TRUE_WAIT(caller->IsConnected(), kDefaultTimeout); - EXPECT_TRUE_WAIT(callee->IsConnected(), kDefaultTimeout); - // The callee needs to process the open message to have the data channel open. - EXPECT_TRUE_WAIT(callee->observer()->last_datachannel_ != nullptr, - kDefaultTimeout); - caller->pc()->Close(); - callee->pc()->Close(); - - int expected_fingerprint_caller = MakeUsageFingerprint( - {PeerConnection::UsageEvent::DATA_ADDED, - PeerConnection::UsageEvent::SET_LOCAL_DESCRIPTION_CALLED, - PeerConnection::UsageEvent::SET_REMOTE_DESCRIPTION_CALLED, - PeerConnection::UsageEvent::CANDIDATE_COLLECTED, - PeerConnection::UsageEvent::PRIVATE_CANDIDATE_COLLECTED, - PeerConnection::UsageEvent::ICE_STATE_CONNECTED, - PeerConnection::UsageEvent::CLOSE_CALLED}); - - int expected_fingerprint_callee = MakeUsageFingerprint( - {PeerConnection::UsageEvent::DATA_ADDED, - PeerConnection::UsageEvent::SET_LOCAL_DESCRIPTION_CALLED, - PeerConnection::UsageEvent::SET_REMOTE_DESCRIPTION_CALLED, - PeerConnection::UsageEvent::CANDIDATE_COLLECTED, - PeerConnection::UsageEvent::MDNS_CANDIDATE_COLLECTED, - PeerConnection::UsageEvent::REMOTE_CANDIDATE_ADDED, - PeerConnection::UsageEvent::REMOTE_PRIVATE_CANDIDATE_ADDED, - PeerConnection::UsageEvent::ICE_STATE_CONNECTED, - PeerConnection::UsageEvent::CLOSE_CALLED}); - - EXPECT_EQ(2, webrtc::metrics::NumSamples(kUsagePatternMetric)); - EXPECT_EQ(1, webrtc::metrics::NumEvents(kUsagePatternMetric, - expected_fingerprint_caller)); - EXPECT_EQ(1, webrtc::metrics::NumEvents(kUsagePatternMetric, - expected_fingerprint_callee)); -} - TEST_F(PeerConnectionUsageHistogramTest, NotableUsageNoted) { auto caller = CreatePeerConnection(); caller->CreateDataChannel("foo");