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}
diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc
index 82d77bb..6cc82b8 100644
--- a/p2p/base/p2p_transport_channel.cc
+++ b/p2p/base/p2p_transport_channel.cc
@@ -324,6 +324,13 @@
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 @@
if (ice_field_trials_.send_ping_on_nomination_ice_controlled &&
conn != nullptr) {
- PingConnection(conn);
- MarkConnectionPinged(conn);
+ SendPingRequestInternal(conn);
}
// TODO(qingsi): RequestSortAndStateUpdate will eventually call
@@ -1747,6 +1753,14 @@
}
}
+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 @@
// 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 ||
- (selected_connection_ && selected_connection_->nominated())) {
+ if (AllowedToPruneConnections()) {
PruneConnections();
}
@@ -1812,7 +1825,7 @@
}
// Update the state of this channel.
- UpdateState();
+ UpdateTransportState();
// Also possibly start pinging.
// We could start pinging if:
@@ -1822,13 +1835,51 @@
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,9 +1900,17 @@
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,
- IceSwitchReason reason) {
+void P2PTransportChannel::SwitchSelectedConnectionInternal(
+ Connection* conn,
+ IceSwitchReason reason) {
RTC_DCHECK_RUN_ON(network_thread_);
// Note: if conn is NULL, the previous `selected_connection_` has been
// destroyed, so don't use it.
@@ -1891,8 +1950,7 @@
((ice_field_trials_.send_ping_on_switch_ice_controlling &&
old_selected_connection != nullptr) ||
ice_field_trials_.send_ping_on_selected_ice_controlling)) {
- PingConnection(conn);
- MarkConnectionPinged(conn);
+ SendPingRequestInternal(conn);
}
SignalNetworkRouteChanged(network_route_);
@@ -1931,13 +1989,14 @@
return (now_ms - last_data_or_old_ping);
}
-// Warning: UpdateState should eventually be called whenever a connection
-// is added, deleted, or the write state of any connection changes so that the
-// transport controller will get the up-to-date channel state. However it
-// should not be called too often; in the case that multiple connection states
-// change, it should be called after all the connection states have changed. For
-// example, we call this at the end of SortConnectionsAndUpdateState.
-void P2PTransportChannel::UpdateState() {
+// Warning: UpdateTransportState should eventually be called whenever a
+// connection is added, deleted, or the write state of any connection changes
+// so that the transport controller will get the up-to-date channel state.
+// However it should not be called too often; in the case that multiple
+// connection states change, it should be called after all the connection
+// states have changed. For example, we call this at the end of
+// 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 @@
<< 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
- // states from the standard; the difference is covered by
+ // P2PTransportChannel does not (yet) implement a direct mapping of the
+ // ICE states from the standard; the difference is covered by
// TransportController and PeerConnection.
switch (state_) {
case IceTransportState::STATE_INIT:
@@ -2031,7 +2090,7 @@
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 @@
// 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
- // which ones are pingable).
+ // Make sure the states of the connections are up-to-date (since this
+ // 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);
- PingConnection(conn);
- MarkConnectionPinged(conn);
+ SendPingRequest(result.connection.value());
}
network_thread_->PostDelayedTask(
@@ -2096,8 +2153,25 @@
}
}
+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 @@
// 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
- // changing from (writable, receiving) to (writable, not receiving).
+ // that the connection becomes weakly connected because the connection may
+ // 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 @@
// 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,
- // we can just set selected to nullptr and re-choose a best assuming that
- // there was no selected connection.
+ // between fairly similar ones. Since this connection is no longer an
+ // option, we can just set selected to nullptr and re-choose a best assuming
+ // 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
- // we do need to update state, because we could be switching to "failed" or
- // "completed".
- UpdateState();
+ // 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 "completed".
+ UpdateTransportState();
}
}
@@ -2231,9 +2305,9 @@
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
- // if this is not the last session because an ICE restart would have signaled
- // the remote side to remove all candidates in previous sessions.
+ // Do not signal candidate removals if continual gathering is not enabled,
+ // or if this is not the last session because an ICE restart would have
+ // signaled the remote side to remove all candidates in previous sessions.
if (!config_.gather_continually() || session != allocator_session()) {
return;
}
@@ -2255,7 +2329,8 @@
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 @@
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 @@
// 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
- // if this is the controlled side.
+ // May need to switch the sending connection based on the receiving media
+ // path if this is the controlled side.
if (ice_role_ == ICEROLE_CONTROLLED) {
MaybeSwitchSelectedConnection(connection, IceSwitchReason::DATA_RECEIVED);
}
diff --git a/p2p/base/p2p_transport_channel.h b/p2p/base/p2p_transport_channel.h
index 5e56df6..5592a4b 100644
--- a/p2p/base/p2p_transport_channel.h
+++ b/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 @@
// 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 @@
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 @@
// 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 UpdateState();
+ void SwitchSelectedConnectionInternal(Connection* conn,
+ IceSwitchReason reason);
+ void UpdateTransportState();
void HandleAllTimedOut();
void MaybeStopPortAllocatorSessions();
void OnSelectedConnectionDestroyed() RTC_RUN_ON(network_thread_);
@@ -349,6 +362,7 @@
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
