From 4931512cb01c1932299db3cd7c822b08dd812125 Mon Sep 17 00:00:00 2001 From: Taiyo Mizuhashi Date: Mon, 18 Dec 2023 15:11:33 +0000 Subject: [PATCH] [CodeHealth][1/2] Remove expired histogram: WebRTC.PeerConnection.IceRestartState To remove histogram WebRTC.PeerConnection.IceRestartState expired in M77, this CL removes the supporting codebase for it. The removal of the histogram itself will be performed in crrev.com/c/5131874. This CL is a part of Chrome Code Health Rotation project. Bug: chromium:1507998 Change-Id: I0451b6fbe2ae46a9c9d9be7c0302f7f01117af97 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/331881 Reviewed-by: Per Kjellander Commit-Queue: Taiyo Mizuhashi Cr-Commit-Position: refs/heads/main@{#41429} --- p2p/base/p2p_transport_channel.cc | 14 ---- p2p/base/p2p_transport_channel.h | 4 - p2p/base/p2p_transport_channel_unittest.cc | 86 ---------------------- 3 files changed, 104 deletions(-) diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc index 0c869ff622..070a25b511 100644 --- a/p2p/base/p2p_transport_channel.cc +++ b/p2p/base/p2p_transport_channel.cc @@ -872,20 +872,6 @@ void P2PTransportChannel::MaybeStartGathering() { SendGatheringStateEvent(); } - if (!allocator_sessions_.empty()) { - IceRestartState state; - if (writable()) { - state = IceRestartState::CONNECTED; - } else if (IsGettingPorts()) { - state = IceRestartState::CONNECTING; - } else { - state = IceRestartState::DISCONNECTED; - } - RTC_HISTOGRAM_ENUMERATION("WebRTC.PeerConnection.IceRestartState", - static_cast(state), - static_cast(IceRestartState::MAX_VALUE)); - } - for (const auto& session : allocator_sessions_) { if (session->IsStopped()) { continue; diff --git a/p2p/base/p2p_transport_channel.h b/p2p/base/p2p_transport_channel.h index 84325b8bef..47f37c8c67 100644 --- a/p2p/base/p2p_transport_channel.h +++ b/p2p/base/p2p_transport_channel.h @@ -79,10 +79,6 @@ class RtcEventLog; namespace cricket { -// Enum for UMA metrics, used to record whether the channel is -// connected/connecting/disconnected when ICE restart happens. -enum class IceRestartState { CONNECTING, CONNECTED, DISCONNECTED, MAX_VALUE }; - static const int MIN_PINGS_AT_WEAK_PING_INTERVAL = 3; bool IceCredentialsChanged(absl::string_view old_ufrag, diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc index f4e66ff9ef..63aa126820 100644 --- a/p2p/base/p2p_transport_channel_unittest.cc +++ b/p2p/base/p2p_transport_channel_unittest.cc @@ -1478,92 +1478,6 @@ TEST_F(P2PTransportChannelTest, GetStatsSwitchConnection) { DestroyChannels(); } -// Tests that UMAs are recorded when ICE restarts while the channel -// is disconnected. -TEST_F(P2PTransportChannelTest, TestUMAIceRestartWhileDisconnected) { - rtc::ScopedFakeClock clock; - ConfigureEndpoints(OPEN, OPEN, kOnlyLocalPorts, kOnlyLocalPorts); - - CreateChannels(); - EXPECT_TRUE_SIMULATED_WAIT(CheckConnected(ep1_ch1(), ep2_ch1()), - kDefaultTimeout, clock); - - // Drop all packets so that both channels become not writable. - fw()->AddRule(false, rtc::FP_ANY, rtc::FD_ANY, kPublicAddrs[0]); - const int kWriteTimeoutDelay = 8000; - EXPECT_TRUE_SIMULATED_WAIT(!ep1_ch1()->writable() && !ep2_ch1()->writable(), - kWriteTimeoutDelay, clock); - - ep1_ch1()->SetIceParameters(kIceParams[2]); - ep1_ch1()->SetRemoteIceParameters(kIceParams[3]); - ep1_ch1()->MaybeStartGathering(); - EXPECT_METRIC_EQ(1, webrtc::metrics::NumEvents( - "WebRTC.PeerConnection.IceRestartState", - static_cast(IceRestartState::DISCONNECTED))); - - ep2_ch1()->SetIceParameters(kIceParams[3]); - ep2_ch1()->SetRemoteIceParameters(kIceParams[2]); - ep2_ch1()->MaybeStartGathering(); - EXPECT_METRIC_EQ(2, webrtc::metrics::NumEvents( - "WebRTC.PeerConnection.IceRestartState", - static_cast(IceRestartState::DISCONNECTED))); - - DestroyChannels(); -} - -// Tests that UMAs are recorded when ICE restarts while the channel -// is connected. -TEST_F(P2PTransportChannelTest, TestUMAIceRestartWhileConnected) { - rtc::ScopedFakeClock clock; - ConfigureEndpoints(OPEN, OPEN, kOnlyLocalPorts, kOnlyLocalPorts); - - CreateChannels(); - EXPECT_TRUE_SIMULATED_WAIT(CheckConnected(ep1_ch1(), ep2_ch1()), - kDefaultTimeout, clock); - - ep1_ch1()->SetIceParameters(kIceParams[2]); - ep1_ch1()->SetRemoteIceParameters(kIceParams[3]); - ep1_ch1()->MaybeStartGathering(); - EXPECT_METRIC_EQ(1, webrtc::metrics::NumEvents( - "WebRTC.PeerConnection.IceRestartState", - static_cast(IceRestartState::CONNECTED))); - - ep2_ch1()->SetIceParameters(kIceParams[3]); - ep2_ch1()->SetRemoteIceParameters(kIceParams[2]); - ep2_ch1()->MaybeStartGathering(); - EXPECT_METRIC_EQ(2, webrtc::metrics::NumEvents( - "WebRTC.PeerConnection.IceRestartState", - static_cast(IceRestartState::CONNECTED))); - - DestroyChannels(); -} - -// Tests that UMAs are recorded when ICE restarts while the channel -// is connecting. -TEST_F(P2PTransportChannelTest, TestUMAIceRestartWhileConnecting) { - rtc::ScopedFakeClock clock; - ConfigureEndpoints(OPEN, OPEN, kOnlyLocalPorts, kOnlyLocalPorts); - - // Create the channels without waiting for them to become connected. - CreateChannels(); - - ep1_ch1()->SetIceParameters(kIceParams[2]); - ep1_ch1()->SetRemoteIceParameters(kIceParams[3]); - ep1_ch1()->MaybeStartGathering(); - EXPECT_METRIC_EQ(1, webrtc::metrics::NumEvents( - "WebRTC.PeerConnection.IceRestartState", - static_cast(IceRestartState::CONNECTING))); - - ep2_ch1()->SetIceParameters(kIceParams[3]); - ep2_ch1()->SetRemoteIceParameters(kIceParams[2]); - ep2_ch1()->MaybeStartGathering(); - EXPECT_METRIC_EQ(2, webrtc::metrics::NumEvents( - "WebRTC.PeerConnection.IceRestartState", - static_cast(IceRestartState::CONNECTING))); - - DestroyChannels(); -} - // Tests that a UMA on ICE regathering is recorded when there is a network // change if and only if continual gathering is enabled. TEST_F(P2PTransportChannelTest,