From b74a3e591feeed4c34a3a9970e03ee390e633969 Mon Sep 17 00:00:00 2001
From: Taiyo Mizuhashi <taiyo@chromium.org>
Date: Thu, 21 Dec 2023 12:42:54 +0000
Subject: [PATCH] [CodeHealth][1/2] Remove expired histogram:
 WebRTC.PeerConnection.IceRegatheringReason

To remove histogram WebRTC.PeerConnection.IceRegatheringReason expired
in M77, this CL removes the supporting codebase for it.

The removal of the histogram itself will be performed in crrev.com/c/5132272 .

This CL is a part of Chrome Code Health Rotation project.

Bug: chromium:1507997
Change-Id: I856e62920578e80970ca2453c86a588141bf120d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/331880
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Taiyo Mizuhashi <taiyo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#41436}
---
 p2p/base/p2p_transport_channel_unittest.cc  |  8 ++-----
 p2p/client/basic_port_allocator.cc          | 24 +++------------------
 p2p/client/basic_port_allocator.h           |  3 ---
 p2p/client/basic_port_allocator_unittest.cc | 18 ----------------
 4 files changed, 5 insertions(+), 48 deletions(-)

diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc
index 63aa126820..b30b2949fe 100644
--- a/p2p/base/p2p_transport_channel_unittest.cc
+++ b/p2p/base/p2p_transport_channel_unittest.cc
@@ -1478,7 +1478,7 @@ TEST_F(P2PTransportChannelTest, GetStatsSwitchConnection) {
   DestroyChannels();
 }
 
-// Tests that a UMA on ICE regathering is recorded when there is a network
+// Tests that an ICE regathering reason is recorded when there is a network
 // change if and only if continual gathering is enabled.
 TEST_F(P2PTransportChannelTest,
        TestIceRegatheringReasonContinualGatheringByNetworkChange) {
@@ -1514,7 +1514,7 @@ TEST_F(P2PTransportChannelTest,
   DestroyChannels();
 }
 
-// Tests that a UMA on ICE regathering is recorded when there is a network
+// Tests that an ICE regathering reason is recorded when there is a network
 // failure if and only if continual gathering is enabled.
 TEST_F(P2PTransportChannelTest,
        TestIceRegatheringReasonContinualGatheringByNetworkFailure) {
@@ -1537,10 +1537,6 @@ TEST_F(P2PTransportChannelTest,
   SIMULATED_WAIT(false, kNetworkFailureTimeout, clock);
   EXPECT_LE(1, GetEndpoint(0)->GetIceRegatheringCountForReason(
                    IceRegatheringReason::NETWORK_FAILURE));
-  EXPECT_METRIC_LE(
-      1, webrtc::metrics::NumEvents(
-             "WebRTC.PeerConnection.IceRegatheringReason",
-             static_cast<int>(IceRegatheringReason::NETWORK_FAILURE)));
   EXPECT_EQ(0, GetEndpoint(1)->GetIceRegatheringCountForReason(
                    IceRegatheringReason::NETWORK_FAILURE));
 
diff --git a/p2p/client/basic_port_allocator.cc b/p2p/client/basic_port_allocator.cc
index e8255f1fd5..a31dc7038b 100644
--- a/p2p/client/basic_port_allocator.cc
+++ b/p2p/client/basic_port_allocator.cc
@@ -199,21 +199,6 @@ BasicPortAllocator::BasicPortAllocator(
                    webrtc::NO_PRUNE, nullptr);
 }
 
-void BasicPortAllocator::OnIceRegathering(PortAllocatorSession* session,
-                                          IceRegatheringReason reason) {
-  // If the session has not been taken by an active channel, do not report the
-  // metric.
-  for (auto& allocator_session : pooled_sessions()) {
-    if (allocator_session.get() == session) {
-      return;
-    }
-  }
-
-  RTC_HISTOGRAM_ENUMERATION("WebRTC.PeerConnection.IceRegatheringReason",
-                            static_cast<int>(reason),
-                            static_cast<int>(IceRegatheringReason::MAX_VALUE));
-}
-
 BasicPortAllocator::~BasicPortAllocator() {
   CheckRunOnValidThreadIfInitialized();
   // Our created port allocator sessions depend on us, so destroy our remaining
@@ -251,12 +236,9 @@ PortAllocatorSession* BasicPortAllocator::CreateSessionInternal(
     absl::string_view ice_ufrag,
     absl::string_view ice_pwd) {
   CheckRunOnValidThreadAndInitialized();
-  PortAllocatorSession* session = new BasicPortAllocatorSession(
-      this, std::string(content_name), component, std::string(ice_ufrag),
-      std::string(ice_pwd));
-  session->SignalIceRegathering.connect(this,
-                                        &BasicPortAllocator::OnIceRegathering);
-  return session;
+  return new BasicPortAllocatorSession(this, std::string(content_name),
+                                       component, std::string(ice_ufrag),
+                                       std::string(ice_pwd));
 }
 
 void BasicPortAllocator::AddTurnServerForTesting(
diff --git a/p2p/client/basic_port_allocator.h b/p2p/client/basic_port_allocator.h
index 643904ab27..25201fd016 100644
--- a/p2p/client/basic_port_allocator.h
+++ b/p2p/client/basic_port_allocator.h
@@ -85,9 +85,6 @@ class RTC_EXPORT BasicPortAllocator : public PortAllocator {
   }
 
  private:
-  void OnIceRegathering(PortAllocatorSession* session,
-                        IceRegatheringReason reason);
-
   bool MdnsObfuscationEnabled() const override;
 
   webrtc::AlwaysValidPointer<const webrtc::FieldTrialsView,
diff --git a/p2p/client/basic_port_allocator_unittest.cc b/p2p/client/basic_port_allocator_unittest.cc
index 65f8e43895..43d552b971 100644
--- a/p2p/client/basic_port_allocator_unittest.cc
+++ b/p2p/client/basic_port_allocator_unittest.cc
@@ -2390,24 +2390,6 @@ TEST_F(BasicPortAllocatorTest,
                                             expected_stun_keepalive_interval);
 }
 
-TEST_F(BasicPortAllocatorTest, IceRegatheringMetricsLoggedWhenNetworkChanges) {
-  // Only test local ports to simplify test.
-  ResetWithNoServersOrNat();
-  AddInterface(kClientAddr, "test_net0");
-  ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
-  session_->StartGettingPorts();
-  EXPECT_TRUE_SIMULATED_WAIT(candidate_allocation_done_,
-                             kDefaultAllocationTimeout, fake_clock);
-  candidate_allocation_done_ = false;
-  AddInterface(kClientAddr2, "test_net1");
-  EXPECT_TRUE_SIMULATED_WAIT(candidate_allocation_done_,
-                             kDefaultAllocationTimeout, fake_clock);
-  EXPECT_METRIC_EQ(1,
-                   webrtc::metrics::NumEvents(
-                       "WebRTC.PeerConnection.IceRegatheringReason",
-                       static_cast<int>(IceRegatheringReason::NETWORK_CHANGE)));
-}
-
 // Test that when an mDNS responder is present, the local address of a host
 // candidate is concealed by an mDNS hostname and the related address of a srflx
 // candidate is set to 0.0.0.0 or ::0.