From dc43cb24bd8ee85d6a8224c5928ceaf90de729b6 Mon Sep 17 00:00:00 2001
From: Harald Alvestrand <hta@webrtc.org>
Date: Fri, 19 Apr 2024 14:57:33 +0000
Subject: [PATCH] Convert P2PTransportChannel Candidate Pair Change to
 CallbackList

Earlier attempts have shown that this signal is multiply listened to.

Bug: webrtc:11943
Change-Id: If9130a7f4c70714b5afda5aca0469b66c8e2612f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/347981
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42124}
---
 p2p/base/ice_transport_internal.h          | 16 +++++++++-------
 p2p/base/p2p_transport_channel.cc          |  4 +---
 p2p/base/p2p_transport_channel_unittest.cc |  4 ++--
 pc/jsep_transport_controller.cc            |  4 ++--
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/p2p/base/ice_transport_internal.h b/p2p/base/ice_transport_internal.h
index 38b6bfe303..fecae48b12 100644
--- a/p2p/base/ice_transport_internal.h
+++ b/p2p/base/ice_transport_internal.h
@@ -338,11 +338,15 @@ class RTC_EXPORT IceTransportInternal : public rtc::PacketTransportInternal {
   // SignalNetworkRouteChanged.
   sigslot::signal2<IceTransportInternal*, const Candidate&> SignalRouteChange;
 
-  void SetCandidatePairChangeCallback(
+  void AddCandidatePairChangeCallback(
+      const void* removal_tag,
       absl::AnyInvocable<void(const cricket::CandidatePairChangeEvent&)>
           callback) {
-    RTC_DCHECK(!candidate_pair_change_callback_);
-    candidate_pair_change_callback_ = std::move(callback);
+    candidate_pair_change_callback_list_.AddReceiver(removal_tag,
+                                                     std::move(callback));
+  }
+  void RemoveCandidatePairChangeCallback(const void* removal_tag) {
+    candidate_pair_change_callback_list_.RemoveReceivers(removal_tag);
   }
 
   // Invoked when there is conflict in the ICE role between local and remote
@@ -385,7 +389,8 @@ class RTC_EXPORT IceTransportInternal : public rtc::PacketTransportInternal {
 
  protected:
   void SendGatheringStateEvent() { SignalGatheringState(this); }
-
+  webrtc::CallbackList<const cricket::CandidatePairChangeEvent&>
+      candidate_pair_change_callback_list_;
   webrtc::CallbackList<IceTransportInternal*,
                        const StunDictionaryView&,
                        rtc::ArrayView<uint16_t>>
@@ -401,9 +406,6 @@ class RTC_EXPORT IceTransportInternal : public rtc::PacketTransportInternal {
   absl::AnyInvocable<void(IceTransportInternal*, const Candidates&)>
       candidates_removed_callback_;
 
-  absl::AnyInvocable<void(const cricket::CandidatePairChangeEvent&)>
-      candidate_pair_change_callback_;
-
  private:
   // TODO(bugs.webrtc.org/11943): remove when removing Signal
   void SignalGatheringStateFired(IceTransportInternal* transport) {
diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc
index 241b423be8..068e0f392b 100644
--- a/p2p/base/p2p_transport_channel.cc
+++ b/p2p/base/p2p_transport_channel.cc
@@ -1838,9 +1838,7 @@ void P2PTransportChannel::SwitchSelectedConnectionInternal(
     } else {
       pair_change.estimated_disconnected_time_ms = 0;
     }
-    if (candidate_pair_change_callback_) {
-      candidate_pair_change_callback_(pair_change);
-    }
+    candidate_pair_change_callback_list_.Send(pair_change);
   }
 
   ++selected_candidate_pair_changes_;
diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc
index 79ca2a5f26..ad4b43bfd3 100644
--- a/p2p/base/p2p_transport_channel_unittest.cc
+++ b/p2p/base/p2p_transport_channel_unittest.cc
@@ -3326,8 +3326,8 @@ class P2PTransportChannelPingTest : public ::testing::Test,
                                   &P2PTransportChannelPingTest::OnReadyToSend);
     ch->SignalStateChanged.connect(
         this, &P2PTransportChannelPingTest::OnChannelStateChanged);
-    ch->SetCandidatePairChangeCallback(
-        [this](const cricket::CandidatePairChangeEvent& event) {
+    ch->AddCandidatePairChangeCallback(
+        this, [this](const cricket::CandidatePairChangeEvent& event) {
           OnCandidatePairChanged(event);
         });
   }
diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc
index dae7060658..268f5ffff7 100644
--- a/pc/jsep_transport_controller.cc
+++ b/pc/jsep_transport_controller.cc
@@ -464,8 +464,8 @@ JsepTransportController::CreateDtlsTransport(
       this, &JsepTransportController::OnTransportStateChanged_n);
   dtls->ice_transport()->SignalIceTransportStateChanged.connect(
       this, &JsepTransportController::OnTransportStateChanged_n);
-  dtls->ice_transport()->SetCandidatePairChangeCallback(
-      [this](const cricket::CandidatePairChangeEvent& event) {
+  dtls->ice_transport()->AddCandidatePairChangeCallback(
+      this, [this](const cricket::CandidatePairChangeEvent& event) {
         RTC_DCHECK_RUN_ON(network_thread_);
         OnTransportCandidatePairChanged_n(event);
       });