From 3382c1c06e0962bd9b9b7965f02e7a4fbd10d067 Mon Sep 17 00:00:00 2001 From: Sameer Vijaykar Date: Thu, 2 Jun 2022 11:29:09 +0200 Subject: [PATCH] Reland "Refactor IceControllerEvent" 1/n This is a partial reland of commit 794e68cf3dbc3b16ee8b12b5615d8a1622154dbd which - adds a new enum - IceSwitchReason - adds a member for the new enum in IceControllerEvent - adds methods to IceControllerInterface accepting the new enum - adds conversion between the old and new enums for compatibility Original change's description: > Refactor IceControllerEvent > > This change is the first step in decoupling IceControllerEvent from > the ICE switch reason. Further cleanup is earmarked, and will be > landed after some internal cleanup. > > This change > - adds a new enum - IceSwitchReason > - adds a member for the new enum in IceControllerEvent > - uses the new enum within P2PTransportChannel > - adds methods to IceControllerInterface accepting the new enum > - deprecates usages of the old enum in IceControllerInterface > - adds conversion between the old and new enums for compatibility > > Bug: webrtc:14125, webrtc:14131 > Change-Id: I5b7201c3f631eb40db334dfeec842841a7e58174 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264140 > Reviewed-by: Jonas Oreland > Commit-Queue: Sameer Vijaykar > Cr-Commit-Position: refs/heads/main@{#37051} Bug: webrtc:14125, webrtc:14131 Change-Id: I2ccdd53c80e38dc139669aa3f438864befed3dc0 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264506 Commit-Queue: Sameer Vijaykar Reviewed-by: Jonas Oreland Cr-Commit-Position: refs/heads/main@{#37094} --- p2p/BUILD.gn | 2 + p2p/base/ice_controller_interface.cc | 95 ++++++++++++++++++---------- p2p/base/ice_controller_interface.h | 28 +++++++- p2p/base/ice_switch_reason.cc | 44 +++++++++++++ p2p/base/ice_switch_reason.h | 38 +++++++++++ 5 files changed, 170 insertions(+), 37 deletions(-) create mode 100644 p2p/base/ice_switch_reason.cc create mode 100644 p2p/base/ice_switch_reason.h diff --git a/p2p/BUILD.gn b/p2p/BUILD.gn index 3fdf607bf3..69a2bc4acc 100644 --- a/p2p/BUILD.gn +++ b/p2p/BUILD.gn @@ -43,6 +43,8 @@ rtc_library("rtc_p2p") { "base/ice_controller_interface.h", "base/ice_credentials_iterator.cc", "base/ice_credentials_iterator.h", + "base/ice_switch_reason.cc", + "base/ice_switch_reason.h", "base/ice_transport_internal.cc", "base/ice_transport_internal.h", "base/p2p_constants.cc", diff --git a/p2p/base/ice_controller_interface.cc b/p2p/base/ice_controller_interface.cc index 6d9bb85343..9eae922d2e 100644 --- a/p2p/base/ice_controller_interface.cc +++ b/p2p/base/ice_controller_interface.cc @@ -12,46 +12,71 @@ #include +#include "p2p/base/ice_switch_reason.h" + namespace cricket { std::string IceControllerEvent::ToString() const { - std::string reason; - switch (type) { - case REMOTE_CANDIDATE_GENERATION_CHANGE: - reason = "remote candidate generation maybe changed"; - break; - case NETWORK_PREFERENCE_CHANGE: - reason = "network preference changed"; - break; - case NEW_CONNECTION_FROM_LOCAL_CANDIDATE: - reason = "new candidate pairs created from a new local candidate"; - break; - case NEW_CONNECTION_FROM_REMOTE_CANDIDATE: - reason = "new candidate pairs created from a new remote candidate"; - break; - case NEW_CONNECTION_FROM_UNKNOWN_REMOTE_ADDRESS: - reason = "a new candidate pair created from an unknown remote address"; - break; - case NOMINATION_ON_CONTROLLED_SIDE: - reason = "nomination on the controlled side"; - break; - case DATA_RECEIVED: - reason = "data received"; - break; - case CONNECT_STATE_CHANGE: - reason = "candidate pair state changed"; - break; - case SELECTED_CONNECTION_DESTROYED: - reason = "selected candidate pair destroyed"; - break; - case ICE_CONTROLLER_RECHECK: - reason = "ice-controller-request-recheck"; - break; - } + std::string str = IceSwitchReasonToString(reason); if (recheck_delay_ms) { - reason += " (after delay: " + std::to_string(recheck_delay_ms) + ")"; + str += " (after delay: " + std::to_string(recheck_delay_ms) + ")"; + } + return str; +} + +// TODO(bugs.webrtc.org/14125) remove when Type is replaced with +// IceSwitchReason. +IceControllerEvent::Type IceControllerEvent::FromIceSwitchReason( + IceSwitchReason reason) { + switch (reason) { + case IceSwitchReason::REMOTE_CANDIDATE_GENERATION_CHANGE: + return IceControllerEvent::REMOTE_CANDIDATE_GENERATION_CHANGE; + case IceSwitchReason::NETWORK_PREFERENCE_CHANGE: + return IceControllerEvent::NETWORK_PREFERENCE_CHANGE; + case IceSwitchReason::NEW_CONNECTION_FROM_LOCAL_CANDIDATE: + return IceControllerEvent::NEW_CONNECTION_FROM_LOCAL_CANDIDATE; + case IceSwitchReason::NEW_CONNECTION_FROM_REMOTE_CANDIDATE: + return IceControllerEvent::NEW_CONNECTION_FROM_REMOTE_CANDIDATE; + case IceSwitchReason::NEW_CONNECTION_FROM_UNKNOWN_REMOTE_ADDRESS: + return IceControllerEvent::NEW_CONNECTION_FROM_UNKNOWN_REMOTE_ADDRESS; + case IceSwitchReason::NOMINATION_ON_CONTROLLED_SIDE: + return IceControllerEvent::NOMINATION_ON_CONTROLLED_SIDE; + case IceSwitchReason::DATA_RECEIVED: + return IceControllerEvent::DATA_RECEIVED; + case IceSwitchReason::CONNECT_STATE_CHANGE: + return IceControllerEvent::CONNECT_STATE_CHANGE; + case IceSwitchReason::SELECTED_CONNECTION_DESTROYED: + return IceControllerEvent::SELECTED_CONNECTION_DESTROYED; + case IceSwitchReason::ICE_CONTROLLER_RECHECK: + return IceControllerEvent::ICE_CONTROLLER_RECHECK; + } +} + +// TODO(bugs.webrtc.org/14125) remove when Type is replaced with +// IceSwitchReason. +IceSwitchReason IceControllerEvent::FromType(IceControllerEvent::Type type) { + switch (type) { + case IceControllerEvent::REMOTE_CANDIDATE_GENERATION_CHANGE: + return IceSwitchReason::REMOTE_CANDIDATE_GENERATION_CHANGE; + case IceControllerEvent::NETWORK_PREFERENCE_CHANGE: + return IceSwitchReason::NETWORK_PREFERENCE_CHANGE; + case IceControllerEvent::NEW_CONNECTION_FROM_LOCAL_CANDIDATE: + return IceSwitchReason::NEW_CONNECTION_FROM_LOCAL_CANDIDATE; + case IceControllerEvent::NEW_CONNECTION_FROM_REMOTE_CANDIDATE: + return IceSwitchReason::NEW_CONNECTION_FROM_REMOTE_CANDIDATE; + case IceControllerEvent::NEW_CONNECTION_FROM_UNKNOWN_REMOTE_ADDRESS: + return IceSwitchReason::NEW_CONNECTION_FROM_UNKNOWN_REMOTE_ADDRESS; + case IceControllerEvent::NOMINATION_ON_CONTROLLED_SIDE: + return IceSwitchReason::NOMINATION_ON_CONTROLLED_SIDE; + case IceControllerEvent::DATA_RECEIVED: + return IceSwitchReason::DATA_RECEIVED; + case IceControllerEvent::CONNECT_STATE_CHANGE: + return IceSwitchReason::CONNECT_STATE_CHANGE; + case IceControllerEvent::SELECTED_CONNECTION_DESTROYED: + return IceSwitchReason::SELECTED_CONNECTION_DESTROYED; + case IceControllerEvent::ICE_CONTROLLER_RECHECK: + return IceSwitchReason::ICE_CONTROLLER_RECHECK; } - return reason; } } // namespace cricket diff --git a/p2p/base/ice_controller_interface.h b/p2p/base/ice_controller_interface.h index a33315a338..52bbd5b873 100644 --- a/p2p/base/ice_controller_interface.h +++ b/p2p/base/ice_controller_interface.h @@ -15,7 +15,9 @@ #include #include +#include "absl/types/optional.h" #include "p2p/base/connection.h" +#include "p2p/base/ice_switch_reason.h" #include "p2p/base/ice_transport_internal.h" namespace cricket { @@ -23,6 +25,7 @@ namespace cricket { struct IceFieldTrials; // Forward declaration to avoid circular dependency. struct IceControllerEvent { + // TODO(bugs.webrtc.org/14125) replace with IceSwitchReason. enum Type { REMOTE_CANDIDATE_GENERATION_CHANGE, NETWORK_PREFERENCE_CHANGE, @@ -39,11 +42,23 @@ struct IceControllerEvent { ICE_CONTROLLER_RECHECK, }; + // TODO(bugs.webrtc.org/14125) remove. IceControllerEvent(const Type& _type) // NOLINT: runtime/explicit - : type(_type) {} + : type(_type), reason(FromType(_type)) {} + + IceControllerEvent(IceSwitchReason _reason, int _recheck_delay_ms) + : type(FromIceSwitchReason(_reason)), + reason(_reason), + recheck_delay_ms(_recheck_delay_ms) {} + + static Type FromIceSwitchReason(IceSwitchReason reason); + static IceSwitchReason FromType(Type type); + std::string ToString() const; + // TODO(bugs.webrtc.org/14125) replace usage with IceSwitchReason. Type type; + IceSwitchReason reason; int recheck_delay_ms = 0; }; @@ -134,13 +149,22 @@ class IceControllerInterface { virtual void MarkConnectionPinged(const Connection* con) = 0; // Check if we should switch to `connection`. - // This method is called for IceControllerEvent's that can switch directly + // This method is called for IceSwitchReasons that can switch directly // i.e without resorting. virtual SwitchResult ShouldSwitchConnection(IceControllerEvent reason, const Connection* connection) = 0; + virtual SwitchResult ShouldSwitchConnection(IceSwitchReason reason, + const Connection* connection) { + return ShouldSwitchConnection( + IceControllerEvent::FromIceSwitchReason(reason), connection); + } // Sort connections and check if we should switch. virtual SwitchResult SortAndSwitchConnection(IceControllerEvent reason) = 0; + virtual SwitchResult SortAndSwitchConnection(IceSwitchReason reason) { + return SortAndSwitchConnection( + IceControllerEvent::FromIceSwitchReason(reason)); + } // Prune connections. virtual std::vector PruneConnections() = 0; diff --git a/p2p/base/ice_switch_reason.cc b/p2p/base/ice_switch_reason.cc new file mode 100644 index 0000000000..61f0fa7d5b --- /dev/null +++ b/p2p/base/ice_switch_reason.cc @@ -0,0 +1,44 @@ +/* + * Copyright 2022 The WebRTC Project Authors. All rights reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include "p2p/base/ice_switch_reason.h" + +#include + +namespace cricket { + +std::string IceSwitchReasonToString(IceSwitchReason reason) { + switch (reason) { + case IceSwitchReason::REMOTE_CANDIDATE_GENERATION_CHANGE: + return "remote candidate generation maybe changed"; + case IceSwitchReason::NETWORK_PREFERENCE_CHANGE: + return "network preference changed"; + case IceSwitchReason::NEW_CONNECTION_FROM_LOCAL_CANDIDATE: + return "new candidate pairs created from a new local candidate"; + case IceSwitchReason::NEW_CONNECTION_FROM_REMOTE_CANDIDATE: + return "new candidate pairs created from a new remote candidate"; + case IceSwitchReason::NEW_CONNECTION_FROM_UNKNOWN_REMOTE_ADDRESS: + return "a new candidate pair created from an unknown remote address"; + case IceSwitchReason::NOMINATION_ON_CONTROLLED_SIDE: + return "nomination on the controlled side"; + case IceSwitchReason::DATA_RECEIVED: + return "data received"; + case IceSwitchReason::CONNECT_STATE_CHANGE: + return "candidate pair state changed"; + case IceSwitchReason::SELECTED_CONNECTION_DESTROYED: + return "selected candidate pair destroyed"; + case IceSwitchReason::ICE_CONTROLLER_RECHECK: + return "ice-controller-request-recheck"; + default: + return "unknown"; + } +} + +} // namespace cricket diff --git a/p2p/base/ice_switch_reason.h b/p2p/base/ice_switch_reason.h new file mode 100644 index 0000000000..2c4fe31884 --- /dev/null +++ b/p2p/base/ice_switch_reason.h @@ -0,0 +1,38 @@ +/* + * Copyright 2022 The WebRTC Project Authors. All rights reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#ifndef P2P_BASE_ICE_SWITCH_REASON_H_ +#define P2P_BASE_ICE_SWITCH_REASON_H_ + +#include + +namespace cricket { + +enum class IceSwitchReason { + REMOTE_CANDIDATE_GENERATION_CHANGE, + NETWORK_PREFERENCE_CHANGE, + NEW_CONNECTION_FROM_LOCAL_CANDIDATE, + NEW_CONNECTION_FROM_REMOTE_CANDIDATE, + NEW_CONNECTION_FROM_UNKNOWN_REMOTE_ADDRESS, + NOMINATION_ON_CONTROLLED_SIDE, + DATA_RECEIVED, + CONNECT_STATE_CHANGE, + SELECTED_CONNECTION_DESTROYED, + // The ICE_CONTROLLER_RECHECK enum value lets an IceController request + // P2PTransportChannel to recheck a switch periodically without an event + // taking place. + ICE_CONTROLLER_RECHECK, +}; + +std::string IceSwitchReasonToString(IceSwitchReason reason); + +} // namespace cricket + +#endif // P2P_BASE_ICE_SWITCH_REASON_H_