Make P2PTransportChannel implement IceAgentInterface (#5/n)

This functionally no-op change adds the methods to allow an active ICE controller to manipulate the connection used by the ICE transport. Most methods reuse existing code, this will be explicitly marked for cleanup with a follow-up CL which adds active ICE controller support.

Non-trivial changes are needed for P2PTransportChannel unit tests to cover the new code, and these are also being added in a follow-up CL.

Bug: webrtc:14367, webrtc:14131
Change-Id: I4f012efcd8cb5766eb8c6f0872de50f8375f3a73
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/275301
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Commit-Queue: Sameer Vijaykar <samvi@google.com>
Cr-Commit-Position: refs/heads/main@{#38081}

p2p/base/p2p_transport_channel.cc

@@ -324,6 +324,13 @@ bool P2PTransportChannel::MaybeSwitchSelectedConnection(
   return result.connection.has_value();
 }
 
+void P2PTransportChannel::ForgetLearnedStateForConnections(
+    std::vector<const Connection*> connections) {
+  for (const Connection* con : connections) {
+    FromIceController(con)->ForgetLearnedState();
+  }
+}
+
 void P2PTransportChannel::SetIceRole(IceRole ice_role) {
   RTC_DCHECK_RUN_ON(network_thread_);
   if (ice_role_ != ice_role) {
@@ -1207,8 +1214,7 @@ void P2PTransportChannel::OnNominated(Connection* conn) {
 
   if (ice_field_trials_.send_ping_on_nomination_ice_controlled &&
       conn != nullptr) {
-    PingConnection(conn);
+    SendPingRequestInternal(conn);
-    MarkConnectionPinged(conn);
   }
 
   // TODO(qingsi): RequestSortAndStateUpdate will eventually call
@@ -1747,6 +1753,14 @@ void P2PTransportChannel::MaybeStartPinging() {
   }
 }
 
+void P2PTransportChannel::OnStartedPinging() {
+  RTC_DCHECK_RUN_ON(network_thread_);
+  RTC_LOG(LS_INFO) << ToString()
+                   << ": Have a pingable connection for the first time; "
+                      "starting to ping.";
+  regathering_controller_->Start();
+}
+
 bool P2PTransportChannel::IsPortPruned(const Port* port) const {
   RTC_DCHECK_RUN_ON(network_thread_);
   return !absl::c_linear_search(ports_, port);
@@ -1791,8 +1805,7 @@ void P2PTransportChannel::SortConnectionsAndUpdateState(
   // TODO(honghaiz): This is not enough to prevent a connection from being
   // pruned too early because with aggressive nomination, the controlling side
   // will nominate every connection until it becomes writable.
-  if (ice_role_ == ICEROLE_CONTROLLING ||
+  if (AllowedToPruneConnections()) {
-      (selected_connection_ && selected_connection_->nominated())) {
     PruneConnections();
   }
 
@@ -1812,7 +1825,7 @@ void P2PTransportChannel::SortConnectionsAndUpdateState(
   }
 
   // Update the state of this channel.
-  UpdateState();
+  UpdateTransportState();
 
   // Also possibly start pinging.
   // We could start pinging if:
@@ -1822,13 +1835,51 @@ void P2PTransportChannel::SortConnectionsAndUpdateState(
   MaybeStartPinging();
 }
 
+void P2PTransportChannel::UpdateState() {
+  // Check if all connections are timedout.
+  bool all_connections_timedout = true;
+  for (const Connection* conn : connections()) {
+    if (conn->write_state() != Connection::STATE_WRITE_TIMEOUT) {
+      all_connections_timedout = false;
+      break;
+    }
+  }
+
+  // Now update the writable state of the channel with the information we have
+  // so far.
+  if (all_connections_timedout) {
+    HandleAllTimedOut();
+  }
+
+  // Update the state of this channel.
+  UpdateTransportState();
+}
+
+bool P2PTransportChannel::AllowedToPruneConnections() const {
+  RTC_DCHECK_RUN_ON(network_thread_);
+  return ice_role_ == ICEROLE_CONTROLLING ||
+         (selected_connection_ && selected_connection_->nominated());
+}
+
+// TODO(bugs.webrtc.org/14367) remove once refactor lands.
 void P2PTransportChannel::PruneConnections() {
   RTC_DCHECK_RUN_ON(network_thread_);
   std::vector<const Connection*> connections_to_prune =
       ice_controller_->PruneConnections();
-  for (const Connection* conn : connections_to_prune) {
+  PruneConnections(connections_to_prune);
+}
+
+bool P2PTransportChannel::PruneConnections(
+    std::vector<const Connection*> connections) {
+  RTC_DCHECK_RUN_ON(network_thread_);
+  if (!AllowedToPruneConnections()) {
+    RTC_LOG(LS_WARNING) << "Not allowed to prune connections";
+    return false;
+  }
+  for (const Connection* conn : connections) {
     FromIceController(conn)->Prune();
   }
+  return true;
 }
 
 rtc::NetworkRoute P2PTransportChannel::ConfigureNetworkRoute(
@@ -1849,8 +1900,16 @@ rtc::NetworkRoute P2PTransportChannel::ConfigureNetworkRoute(
           GetProtocolOverhead(conn->local_candidate().protocol())};
 }
 
+void P2PTransportChannel::SwitchSelectedConnection(
+    const Connection* new_connection,
+    IceSwitchReason reason) {
+  RTC_DCHECK_RUN_ON(network_thread_);
+  SwitchSelectedConnectionInternal(FromIceController(new_connection), reason);
+}
+
 // Change the selected connection, and let listeners know.
-void P2PTransportChannel::SwitchSelectedConnection(Connection* conn,
+void P2PTransportChannel::SwitchSelectedConnectionInternal(
+    Connection* conn,
     IceSwitchReason reason) {
   RTC_DCHECK_RUN_ON(network_thread_);
   // Note: if conn is NULL, the previous `selected_connection_` has been
@@ -1891,8 +1950,7 @@ void P2PTransportChannel::SwitchSelectedConnection(Connection* conn,
       ((ice_field_trials_.send_ping_on_switch_ice_controlling &&
         old_selected_connection != nullptr) ||
        ice_field_trials_.send_ping_on_selected_ice_controlling)) {
-    PingConnection(conn);
+    SendPingRequestInternal(conn);
-    MarkConnectionPinged(conn);
   }
 
   SignalNetworkRouteChanged(network_route_);
@@ -1931,13 +1989,14 @@ int64_t P2PTransportChannel::ComputeEstimatedDisconnectedTimeMs(
   return (now_ms - last_data_or_old_ping);
 }
 
-// Warning: UpdateState should eventually be called whenever a connection
+// Warning: UpdateTransportState should eventually be called whenever a
-// is added, deleted, or the write state of any connection changes so that the
+// connection is added, deleted, or the write state of any connection changes
-// transport controller will get the up-to-date channel state. However it
+// so that the transport controller will get the up-to-date channel state.
-// should not be called too often; in the case that multiple connection states
+// However it should not be called too often; in the case that multiple
-// change, it should be called after all the connection states have changed. For
+// connection states change, it should be called after all the connection
-// example, we call this at the end of SortConnectionsAndUpdateState.
+// states have changed. For example, we call this at the end of
-void P2PTransportChannel::UpdateState() {
+// SortConnectionsAndUpdateState.
+void P2PTransportChannel::UpdateTransportState() {
   RTC_DCHECK_RUN_ON(network_thread_);
   // If our selected connection is "presumed writable" (TURN-TURN with no
   // CreatePermission required), act like we're already writable to the upper
@@ -1965,8 +2024,8 @@ void P2PTransportChannel::UpdateState() {
                      << static_cast<int>(state_) << " to "
                      << static_cast<int>(state);
     // Check that the requested transition is allowed. Note that
-    // P2PTransportChannel does not (yet) implement a direct mapping of the ICE
+    // P2PTransportChannel does not (yet) implement a direct mapping of the
-    // states from the standard; the difference is covered by
+    // ICE states from the standard; the difference is covered by
     // TransportController and PeerConnection.
     switch (state_) {
       case IceTransportState::STATE_INIT:
@@ -2031,7 +2090,7 @@ void P2PTransportChannel::OnSelectedConnectionDestroyed() {
   RTC_DCHECK_RUN_ON(network_thread_);
   RTC_LOG(LS_INFO) << "Selected connection destroyed. Will choose a new one.";
   IceSwitchReason reason = IceSwitchReason::SELECTED_CONNECTION_DESTROYED;
-  SwitchSelectedConnection(nullptr, reason);
+  SwitchSelectedConnectionInternal(nullptr, reason);
   RequestSortAndStateUpdate(reason);
 }
 
@@ -2068,17 +2127,15 @@ bool P2PTransportChannel::ReadyToSend(const Connection* connection) const {
 // Handle queued up check-and-ping request
 void P2PTransportChannel::CheckAndPing() {
   RTC_DCHECK_RUN_ON(network_thread_);
-  // Make sure the states of the connections are up-to-date (since this affects
+  // Make sure the states of the connections are up-to-date (since this
-  // which ones are pingable).
+  // affects which ones are pingable).
   UpdateConnectionStates();
 
   auto result = ice_controller_->SelectConnectionToPing(last_ping_sent_ms_);
   TimeDelta delay = TimeDelta::Millis(result.recheck_delay_ms);
 
   if (result.connection.value_or(nullptr)) {
-    Connection* conn = FromIceController(*result.connection);
+    SendPingRequest(result.connection.value());
-    PingConnection(conn);
-    MarkConnectionPinged(conn);
   }
 
   network_thread_->PostDelayedTask(
@@ -2096,8 +2153,25 @@ Connection* P2PTransportChannel::FindNextPingableConnection() {
   }
 }
 
+int64_t P2PTransportChannel::GetLastPingSentMs() const {
+  RTC_DCHECK_RUN_ON(network_thread_);
+  return last_ping_sent_ms_;
+}
+
+void P2PTransportChannel::SendPingRequest(const Connection* connection) {
+  RTC_DCHECK_RUN_ON(network_thread_);
+  SendPingRequestInternal(FromIceController(connection));
+}
+
+void P2PTransportChannel::SendPingRequestInternal(Connection* connection) {
+  RTC_DCHECK_RUN_ON(network_thread_);
+  PingConnection(connection);
+  MarkConnectionPinged(connection);
+}
+
 // A connection is considered a backup connection if the channel state
-// is completed, the connection is not the selected connection and it is active.
+// is completed, the connection is not the selected connection and it is
+// active.
 void P2PTransportChannel::MarkConnectionPinged(Connection* conn) {
   RTC_DCHECK_RUN_ON(network_thread_);
   ice_controller_->MarkConnectionPinged(conn);
@@ -2147,8 +2221,8 @@ void P2PTransportChannel::OnConnectionStateChange(Connection* connection) {
   // May stop the allocator session when at least one connection becomes
   // strongly connected after starting to get ports and the local candidate of
   // the connection is at the latest generation. It is not enough to check
-  // that the connection becomes weakly connected because the connection may be
+  // that the connection becomes weakly connected because the connection may
-  // changing from (writable, receiving) to (writable, not receiving).
+  // be changing from (writable, receiving) to (writable, not receiving).
   if (ice_field_trials_.stop_gather_on_strongly_connected) {
     bool strongly_connected = !connection->weak();
     bool latest_generation = connection->local_candidate().generation() >=
@@ -2181,16 +2255,16 @@ void P2PTransportChannel::OnConnectionDestroyed(Connection* connection) {
   // If this is currently the selected connection, then we need to pick a new
   // one. The call to SortConnectionsAndUpdateState will pick a new one. It
   // looks at the current selected connection in order to avoid switching
-  // between fairly similar ones. Since this connection is no longer an option,
+  // between fairly similar ones. Since this connection is no longer an
-  // we can just set selected to nullptr and re-choose a best assuming that
+  // option, we can just set selected to nullptr and re-choose a best assuming
-  // there was no selected connection.
+  // that there was no selected connection.
   if (selected_connection_ == connection) {
     OnSelectedConnectionDestroyed();
   } else {
-    // If a non-selected connection was destroyed, we don't need to re-sort but
+    // If a non-selected connection was destroyed, we don't need to re-sort
-    // we do need to update state, because we could be switching to "failed" or
+    // but we do need to update state, because we could be switching to
-    // "completed".
+    // "failed" or "completed".
-    UpdateState();
+    UpdateTransportState();
   }
 }
 
@@ -2231,9 +2305,9 @@ void P2PTransportChannel::OnCandidatesRemoved(
     PortAllocatorSession* session,
     const std::vector<Candidate>& candidates) {
   RTC_DCHECK_RUN_ON(network_thread_);
-  // Do not signal candidate removals if continual gathering is not enabled, or
+  // Do not signal candidate removals if continual gathering is not enabled,
-  // if this is not the last session because an ICE restart would have signaled
+  // or if this is not the last session because an ICE restart would have
-  // the remote side to remove all candidates in previous sessions.
+  // signaled the remote side to remove all candidates in previous sessions.
   if (!config_.gather_continually() || session != allocator_session()) {
     return;
   }
@@ -2255,7 +2329,8 @@ void P2PTransportChannel::PruneAllPorts() {
 bool P2PTransportChannel::PrunePort(PortInterface* port) {
   RTC_DCHECK_RUN_ON(network_thread_);
   auto it = absl::c_find(ports_, port);
-  // Don't need to do anything if the port has been deleted from the port list.
+  // Don't need to do anything if the port has been deleted from the port
+  // list.
   if (it == ports_.end()) {
     return false;
   }
@@ -2282,7 +2357,8 @@ void P2PTransportChannel::OnReadPacket(Connection* connection,
     return;
   }
 
-  // Do not deliver, if packet doesn't belong to the correct transport channel.
+  // Do not deliver, if packet doesn't belong to the correct transport
+  // channel.
   if (!FindConnection(connection))
     return;
 
@@ -2295,8 +2371,8 @@ void P2PTransportChannel::OnReadPacket(Connection* connection,
   // Let the client know of an incoming packet
   SignalReadPacket(this, data, len, packet_time_us, 0);
 
-  // May need to switch the sending connection based on the receiving media path
+  // May need to switch the sending connection based on the receiving media
-  // if this is the controlled side.
+  // path if this is the controlled side.
   if (ice_role_ == ICEROLE_CONTROLLED) {
     MaybeSwitchSelectedConnection(connection, IceSwitchReason::DATA_RECEIVED);
   }

p2p/base/p2p_transport_channel.h

@@ -49,6 +49,7 @@
 #include "p2p/base/basic_async_resolver_factory.h"
 #include "p2p/base/candidate_pair_interface.h"
 #include "p2p/base/connection.h"
+#include "p2p/base/ice_agent_interface.h"
 #include "p2p/base/ice_controller_factory_interface.h"
 #include "p2p/base/ice_controller_interface.h"
 #include "p2p/base/ice_switch_reason.h"
@@ -104,7 +105,8 @@ class RemoteCandidate : public Candidate {
 
 // P2PTransportChannel manages the candidates and connection process to keep
 // two P2P clients connected to each other.
-class RTC_EXPORT P2PTransportChannel : public IceTransportInternal {
+class RTC_EXPORT P2PTransportChannel : public IceTransportInternal,
+                                       public IceAgentInterface {
  public:
   static std::unique_ptr<P2PTransportChannel> Create(
       absl::string_view transport_name,
@@ -168,6 +170,18 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal {
   const Connection* selected_connection() const override;
   absl::optional<const CandidatePair> GetSelectedCandidatePair() const override;
 
+  // From IceAgentInterface
+  void OnStartedPinging() override;
+  int64_t GetLastPingSentMs() const override;
+  void UpdateConnectionStates() override;
+  void UpdateState() override;
+  void SendPingRequest(const Connection* connection) override;
+  void SwitchSelectedConnection(const Connection* connection,
+                                IceSwitchReason reason) override;
+  void ForgetLearnedStateForConnections(
+      std::vector<const Connection*> connections) override;
+  bool PruneConnections(std::vector<const Connection*> connections) override;
+
   // TODO(honghaiz): Remove this method once the reference of it in
   // Chromoting is removed.
   const Connection* best_connection() const {
@@ -260,18 +274,17 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal {
   // Returns true if it's possible to send packets on `connection`.
   bool ReadyToSend(const Connection* connection) const;
   bool PresumedWritable(const Connection* conn) const;
-  void UpdateConnectionStates();
   void RequestSortAndStateUpdate(IceSwitchReason reason_to_sort);
   // Start pinging if we haven't already started, and we now have a connection
   // that's pingable.
   void MaybeStartPinging();
+  void SendPingRequestInternal(Connection* connection);
 
   void SortConnectionsAndUpdateState(IceSwitchReason reason_to_sort);
-  void SortConnections();
-  void SortConnectionsIfNeeded();
   rtc::NetworkRoute ConfigureNetworkRoute(const Connection* conn);
-  void SwitchSelectedConnection(Connection* conn, IceSwitchReason reason);
+  void SwitchSelectedConnectionInternal(Connection* conn,
-  void UpdateState();
+                                        IceSwitchReason reason);
+  void UpdateTransportState();
   void HandleAllTimedOut();
   void MaybeStopPortAllocatorSessions();
   void OnSelectedConnectionDestroyed() RTC_RUN_ON(network_thread_);
@@ -349,6 +362,7 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal {
   bool MaybeSwitchSelectedConnection(
       IceSwitchReason reason,
       IceControllerInterface::SwitchResult result);
+  bool AllowedToPruneConnections() const;
   void PruneConnections();
 
   // Returns the latest remote ICE parameters or nullptr if there are no remote