Surface CandidatePairChange event

In order to be able to detect and measure context around candidate pair changes.

Bug: webrtc:10419
Change-Id: Iab0d7e7c80d925d1aa44617fc35975fdc6bbc6b9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/147340
Commit-Queue: Alex Drake <alexdrake@google.com>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Qingsi Wang <qingsi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28779}

api/peer_connection_interface.h

@@ -1184,6 +1184,10 @@ class PeerConnectionObserver {
   // Called when the ICE connection receiving status changes.
   virtual void OnIceConnectionReceivingChange(bool receiving) {}
 
+  // Called when the selected candidate pair for the ICE connection changes.
+  virtual void OnIceSelectedCandidatePairChanged(
+      const cricket::CandidatePairChangeEvent& event) {}
+
   // This is called when a receiver and its track are created.
   // TODO(zhihuang): Make this pure virtual when all subclasses implement it.
   // Note: This is called with both Plan B and Unified Plan semantics. Unified

p2p/base/ice_transport_internal.h

@@ -285,6 +285,9 @@ class RTC_EXPORT IceTransportInternal : public rtc::PacketTransportInternal {
   // SignalNetworkRouteChanged.
   sigslot::signal2<IceTransportInternal*, const Candidate&> SignalRouteChange;
 
+  sigslot::signal1<const cricket::CandidatePairChangeEvent&>
+      SignalCandidatePairChanged;
+
   // Invoked when there is conflict in the ICE role between local and remote
   // agents.
   sigslot::signal1<IceTransportInternal*> SignalRoleConflict;

p2p/base/p2p_transport_channel.cc

@@ -284,7 +284,7 @@ bool P2PTransportChannel::MaybeSwitchSelectedConnection(
   if (ShouldSwitchSelectedConnection(new_connection,
                                      &missed_receiving_unchanged_threshold)) {
     RTC_LOG(LS_INFO) << "Switching selected connection due to: " << reason;
-    SwitchSelectedConnection(new_connection);
+    SwitchSelectedConnection(new_connection, reason);
     return true;
   }
   if (missed_receiving_unchanged_threshold &&
@@ -1917,7 +1917,8 @@ void P2PTransportChannel::PruneConnections() {
 }
 
 // Change the selected connection, and let listeners know.
-void P2PTransportChannel::SwitchSelectedConnection(Connection* conn) {
+void P2PTransportChannel::SwitchSelectedConnection(Connection* conn,
+                                                   const std::string& reason) {
   RTC_DCHECK_RUN_ON(network_thread_);
   // Note: if conn is NULL, the previous |selected_connection_| has been
   // destroyed, so don't use it.
@@ -1963,6 +1964,16 @@ void P2PTransportChannel::SwitchSelectedConnection(Connection* conn) {
     RTC_LOG(LS_INFO) << ToString() << ": No selected connection";
   }
 
+  // Create event for candidate pair change.
+  CandidatePairChangeEvent pair_change;
+  pair_change.reason = reason;
+  if (selected_connection_) {
+    pair_change.local_candidate = selected_connection_->local_candidate();
+    pair_change.remote_candidate = selected_connection_->remote_candidate();
+    pair_change.last_data_received_ms =
+        selected_connection_->last_data_received();
+  }
+  SignalCandidatePairChanged(pair_change);
   SignalNetworkRouteChanged(network_route_);
 }
 
@@ -2377,7 +2388,6 @@ void P2PTransportChannel::OnConnectionStateChange(Connection* connection) {
   if (strongly_connected && latest_generation) {
     MaybeStopPortAllocatorSessions();
   }
-
   // We have to unroll the stack before doing this because we may be changing
   // the state of connections while sorting.
   RequestSortAndStateUpdate("candidate pair state changed");
@@ -2409,8 +2419,9 @@ void P2PTransportChannel::OnConnectionDestroyed(Connection* connection) {
   // there was no selected connection.
   if (selected_connection_ == connection) {
     RTC_LOG(LS_INFO) << "Selected connection destroyed. Will choose a new one.";
-    SwitchSelectedConnection(nullptr);
-    RequestSortAndStateUpdate("selected candidate pair destroyed");
+    const std::string reason = "selected candidate pair destroyed";
+    SwitchSelectedConnection(nullptr, reason);
+    RequestSortAndStateUpdate(reason);
   } else {
     // If a non-selected connection was destroyed, we don't need to re-sort but
     // we do need to update state, because we could be switching to "failed" or

p2p/base/p2p_transport_channel.h

@@ -267,7 +267,7 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal {
   bool PresumedWritable(const cricket::Connection* conn) const;
 
   void SortConnectionsAndUpdateState(const std::string& reason_to_sort);
-  void SwitchSelectedConnection(Connection* conn);
+  void SwitchSelectedConnection(Connection* conn, const std::string& reason);
   void UpdateState();
   void HandleAllTimedOut();
   void MaybeStopPortAllocatorSessions();

p2p/base/p2p_transport_channel_unittest.cc

@@ -3147,6 +3147,8 @@ class P2PTransportChannelPingTest : public ::testing::Test,
                                   &P2PTransportChannelPingTest::OnReadyToSend);
     ch->SignalStateChanged.connect(
         this, &P2PTransportChannelPingTest::OnChannelStateChanged);
+    ch->SignalCandidatePairChanged.connect(
+        this, &P2PTransportChannelPingTest::OnCandidatePairChanged);
   }
 
   Connection* WaitForConnectionTo(
@@ -3280,6 +3282,9 @@ class P2PTransportChannelPingTest : public ::testing::Test,
   void OnChannelStateChanged(IceTransportInternal* channel) {
     channel_state_ = channel->GetState();
   }
+  void OnCandidatePairChanged(const CandidatePairChangeEvent& event) {
+    last_candidate_change_event_ = event;
+  }
 
   int last_sent_packet_id() { return last_sent_packet_id_; }
   bool channel_ready_to_send() { return channel_ready_to_send_; }
@@ -3303,12 +3308,27 @@ class P2PTransportChannelPingTest : public ::testing::Test,
     }
   }
 
+  bool ConnectionMatchesChangeEvent(Connection* conn, std::string reason) {
+    if (!conn) {
+      return !last_candidate_change_event_.has_value();
+    } else {
+      return last_candidate_change_event_->local_candidate.IsEquivalent(
+                 conn->local_candidate()) &&
+             last_candidate_change_event_->remote_candidate.IsEquivalent(
+                 conn->remote_candidate()) &&
+             last_candidate_change_event_->last_data_received_ms ==
+                 conn->last_data_received() &&
+             last_candidate_change_event_->reason == reason;
+    }
+  }
+
  private:
   std::unique_ptr<rtc::VirtualSocketServer> vss_;
   rtc::AutoSocketServerThread thread_;
   int selected_candidate_pair_switches_ = 0;
   int last_sent_packet_id_ = -1;
   bool channel_ready_to_send_ = false;
+  absl::optional<CandidatePairChangeEvent> last_candidate_change_event_;
   IceTransportState channel_state_ = IceTransportState::STATE_INIT;
   absl::optional<rtc::NetworkRoute> last_network_route_;
 };
@@ -3712,6 +3732,8 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) {
   conn1->ReceivedPingResponse(LOW_RTT, "id");
   EXPECT_EQ_WAIT(conn1, ch.selected_connection(), kDefaultTimeout);
   EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn1));
+  EXPECT_TRUE(ConnectionMatchesChangeEvent(
+      conn1, "remote candidate generation maybe changed"));
   EXPECT_EQ(len, SendData(&ch, data, len, ++last_packet_id));
 
   // When a higher priority candidate comes in, the new connection is chosen
@@ -3722,6 +3744,8 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) {
   conn2->ReceivedPingResponse(LOW_RTT, "id");
   EXPECT_EQ_WAIT(conn2, ch.selected_connection(), kDefaultTimeout);
   EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn2));
+  EXPECT_TRUE(
+      ConnectionMatchesChangeEvent(conn2, "candidate pair state changed"));
   EXPECT_TRUE(channel_ready_to_send());
   EXPECT_EQ(last_packet_id, last_sent_packet_id());
 
@@ -3740,6 +3764,8 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) {
   NominateConnection(conn3);
   EXPECT_EQ(conn3, ch.selected_connection());
   EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn3));
+  EXPECT_TRUE(
+      ConnectionMatchesChangeEvent(conn3, "nomination on the controlled side"));
   EXPECT_EQ(last_packet_id, last_sent_packet_id());
   EXPECT_TRUE(channel_ready_to_send());
 
@@ -3761,6 +3787,8 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) {
   conn4->ReceivedPingResponse(LOW_RTT, "id");
   EXPECT_EQ_WAIT(conn4, ch.selected_connection(), kDefaultTimeout);
   EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn4));
+  EXPECT_TRUE(
+      ConnectionMatchesChangeEvent(conn4, "candidate pair state changed"));
   EXPECT_EQ(last_packet_id, last_sent_packet_id());
   // SignalReadyToSend is fired again because conn4 is writable.
   EXPECT_TRUE(channel_ready_to_send());

p2p/base/port.h

@@ -148,6 +148,13 @@ struct IceCandidateErrorEvent {
   std::string error_text;
 };
 
+struct CandidatePairChangeEvent {
+  Candidate local_candidate;
+  Candidate remote_candidate;
+  int64_t last_data_received_ms;
+  std::string reason;
+};
+
 typedef std::set<rtc::SocketAddress> ServerAddresses;
 
 // Represents a local communication mechanism that can be used to create

pc/jsep_transport_controller.cc

@@ -535,6 +535,8 @@ JsepTransportController::CreateDtlsTransport(
       this, &JsepTransportController::OnTransportStateChanged_n);
   dtls->ice_transport()->SignalIceTransportStateChanged.connect(
       this, &JsepTransportController::OnTransportStateChanged_n);
+  dtls->ice_transport()->SignalCandidatePairChanged.connect(
+      this, &JsepTransportController::OnTransportCandidatePairChanged_n);
   return dtls;
 }
 
@@ -1401,6 +1403,12 @@ void JsepTransportController::OnTransportCandidatesRemoved_n(
       RTC_FROM_HERE, signaling_thread_,
       [this, candidates] { SignalIceCandidatesRemoved(candidates); });
 }
+void JsepTransportController::OnTransportCandidatePairChanged_n(
+    const cricket::CandidatePairChangeEvent& event) {
+  invoker_.AsyncInvoke<void>(RTC_FROM_HERE, signaling_thread_, [this, event] {
+    SignalIceCandidatePairChanged(event);
+  });
+}
 
 void JsepTransportController::OnTransportRoleConflict_n(
     cricket::IceTransportInternal* transport) {

pc/jsep_transport_controller.h

@@ -248,6 +248,9 @@ class JsepTransportController : public sigslot::has_slots<> {
   sigslot::signal1<const std::vector<cricket::Candidate>&>
       SignalIceCandidatesRemoved;
 
+  sigslot::signal1<const cricket::CandidatePairChangeEvent&>
+      SignalIceCandidatePairChanged;
+
   sigslot::signal1<rtc::SSLHandshakeError> SignalDtlsHandshakeError;
 
   sigslot::signal<> SignalMediaTransportStateChanged;
@@ -394,6 +397,8 @@ class JsepTransportController : public sigslot::has_slots<> {
   void OnTransportRoleConflict_n(cricket::IceTransportInternal* transport);
   void OnTransportStateChanged_n(cricket::IceTransportInternal* transport);
   void OnMediaTransportStateChanged_n();
+  void OnTransportCandidatePairChanged_n(
+      const cricket::CandidatePairChangeEvent& event);
 
   void UpdateAggregateStates_n();
 

pc/peer_connection.cc

@@ -1139,6 +1139,8 @@ bool PeerConnection::Initialize(
       this, &PeerConnection::OnTransportControllerCandidatesRemoved);
   transport_controller_->SignalDtlsHandshakeError.connect(
       this, &PeerConnection::OnTransportControllerDtlsHandshakeError);
+  transport_controller_->SignalIceCandidatePairChanged.connect(
+      this, &PeerConnection::OnTransportControllerCandidateChanged);
 
   sctp_factory_ = factory_->CreateSctpTransportInternalFactory();
 
@@ -4273,6 +4275,14 @@ void PeerConnection::OnIceCandidatesRemoved(
   Observer()->OnIceCandidatesRemoved(candidates);
 }
 
+void PeerConnection::OnSelectedCandidatePairChanged(
+    const cricket::CandidatePairChangeEvent& event) {
+  if (IsClosed()) {
+    return;
+  }
+  Observer()->OnIceSelectedCandidatePairChanged(event);
+}
+
 void PeerConnection::ChangeSignalingState(
     PeerConnectionInterface::SignalingState signaling_state) {
   if (signaling_state_ == signaling_state) {
@@ -6246,6 +6256,11 @@ void PeerConnection::OnTransportControllerCandidatesRemoved(
   OnIceCandidatesRemoved(candidates);
 }
 
+void PeerConnection::OnTransportControllerCandidateChanged(
+    const cricket::CandidatePairChangeEvent& event) {
+  OnSelectedCandidatePairChanged(event);
+}
+
 void PeerConnection::OnTransportControllerDtlsHandshakeError(
     rtc::SSLHandshakeError error) {
   RTC_HISTOGRAM_ENUMERATION(

pc/peer_connection.h

@@ -458,6 +458,10 @@ class PeerConnection : public PeerConnectionInternal,
   void OnIceCandidatesRemoved(const std::vector<cricket::Candidate>& candidates)
       RTC_RUN_ON(signaling_thread());
 
+  void OnSelectedCandidatePairChanged(
+      const cricket::CandidatePairChangeEvent& event)
+      RTC_RUN_ON(signaling_thread());
+
   // Update the state, signaling if necessary.
   void ChangeSignalingState(SignalingState signaling_state)
       RTC_RUN_ON(signaling_thread());
@@ -1041,6 +1045,9 @@ class PeerConnection : public PeerConnectionInternal,
   void OnTransportControllerCandidatesRemoved(
       const std::vector<cricket::Candidate>& candidates)
       RTC_RUN_ON(signaling_thread());
+  void OnTransportControllerCandidateChanged(
+      const cricket::CandidatePairChangeEvent& event)
+      RTC_RUN_ON(signaling_thread());
   void OnTransportControllerDtlsHandshakeError(rtc::SSLHandshakeError error);
 
   const char* SessionErrorToString(SessionError error) const;

pc/peer_connection_integrationtest.cc

@@ -298,6 +298,10 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver,
   ice_gathering_state_history() const {
     return ice_gathering_state_history_;
   }
+  std::vector<cricket::CandidatePairChangeEvent>
+  ice_candidate_pair_change_history() const {
+    return ice_candidate_pair_change_history_;
+  }
 
   void AddAudioVideoTracks() {
     AddAudioTrack();
@@ -931,6 +935,11 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver,
     EXPECT_EQ(pc()->ice_gathering_state(), new_state);
     ice_gathering_state_history_.push_back(new_state);
   }
+
+  void OnIceSelectedCandidatePairChanged(
+      const cricket::CandidatePairChangeEvent& event) {
+    ice_candidate_pair_change_history_.push_back(event);
+  }
   void OnIceCandidate(const webrtc::IceCandidateInterface* candidate) override {
     RTC_LOG(LS_INFO) << debug_name_ << ": OnIceCandidate";
 
@@ -1025,6 +1034,8 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver,
       peer_connection_state_history_;
   std::vector<PeerConnectionInterface::IceGatheringState>
       ice_gathering_state_history_;
+  std::vector<cricket::CandidatePairChangeEvent>
+      ice_candidate_pair_change_history_;
 
   webrtc::FakeRtcEventLogFactory* event_log_factory_;
 
@@ -4208,6 +4219,7 @@ TEST_P(PeerConnectionIntegrationTest, MediaContinuesFlowingAfterIceRestart) {
   std::string callee_ufrag_pre_restart =
       desc->transport_infos()[0].description.ice_ufrag;
 
+  EXPECT_EQ(caller()->ice_candidate_pair_change_history().size(), 1u);
   // Have the caller initiate an ICE restart.
   caller()->SetOfferAnswerOptions(IceRestartOfferAnswerOptions());
   caller()->CreateAndSetAndSignalOffer();
@@ -4239,6 +4251,7 @@ TEST_P(PeerConnectionIntegrationTest, MediaContinuesFlowingAfterIceRestart) {
   ASSERT_NE(callee_candidate_pre_restart, callee_candidate_post_restart);
   ASSERT_NE(caller_ufrag_pre_restart, caller_ufrag_post_restart);
   ASSERT_NE(callee_ufrag_pre_restart, callee_ufrag_post_restart);
+  EXPECT_GT(caller()->ice_candidate_pair_change_history().size(), 1u);
 
   // Ensure that additional frames are received after the ICE restart.
   MediaExpectations media_expectations;