From 1cf342a321fcbd2bf293470f1b190d473393b64d Mon Sep 17 00:00:00 2001 From: Philipp Hancke Date: Wed, 4 Dec 2024 09:06:48 -0800 Subject: [PATCH] Add IceConfig getter to IceTransportInternal(Interface) and misc cleanup BUG=webrtc:367395350 No-Iwyu: remaining IWYU failure is deep inside gtest which is unrelated to the changes and needs to be investigated separately Change-Id: I5c2b7a6cc6b15fc5474c55eb98635cb9145b7373 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/370180 Reviewed-by: Jonas Oreland Reviewed-by: Harald Alvestrand Commit-Queue: Philipp Hancke Cr-Commit-Position: refs/heads/main@{#43498} --- p2p/BUILD.gn | 9 ++++++++ p2p/base/fake_ice_transport.h | 2 ++ p2p/base/ice_transport_internal.h | 9 +++++++- p2p/base/mock_ice_transport.h | 8 +++---- p2p/base/p2p_transport_channel.cc | 35 ++++++++++++++++++++++++++++--- p2p/base/p2p_transport_channel.h | 10 ++++----- pc/peer_connection.cc | 3 +-- 7 files changed, 59 insertions(+), 17 deletions(-) diff --git a/p2p/BUILD.gn b/p2p/BUILD.gn index f62331e3de..2c6a870e1e 100644 --- a/p2p/BUILD.gn +++ b/p2p/BUILD.gn @@ -511,19 +511,25 @@ rtc_library("ice_transport_internal") { "base/ice_transport_internal.h", ] deps = [ + ":candidate_pair_interface", ":connection", + ":connection_info", ":p2p_constants", ":packet_transport_internal", ":port", ":stun_dictionary", ":transport_description", + "../api:array_view", "../api:candidate", "../api:rtc_error", "../api/transport:enums", + "../rtc_base:callback_list", + "../rtc_base:checks", "../rtc_base:network_constants", "../rtc_base:timeutils", "../rtc_base/system:rtc_export", "../rtc_base/third_party/sigslot", + "//third_party/abseil-cpp/absl/functional:any_invocable", "//third_party/abseil-cpp/absl/strings:string_view", ] } @@ -543,6 +549,7 @@ rtc_library("p2p_transport_channel") { ] deps = [ ":active_ice_controller_factory_interface", + ":active_ice_controller_interface", ":basic_ice_controller", ":candidate_pair_interface", ":connection", @@ -581,6 +588,7 @@ rtc_library("p2p_transport_channel") { "../rtc_base:logging", "../rtc_base:macromagic", "../rtc_base:net_helper", + "../rtc_base:net_helpers", "../rtc_base:network", "../rtc_base:network_constants", "../rtc_base:network_route", @@ -590,6 +598,7 @@ rtc_library("p2p_transport_channel") { "../rtc_base:threading", "../rtc_base:timeutils", "../rtc_base/experiments:field_trial_parser", + "../rtc_base/network:received_packet", "../rtc_base/network:sent_packet", "../rtc_base/system:rtc_export", "../system_wrappers:metrics", diff --git a/p2p/base/fake_ice_transport.h b/p2p/base/fake_ice_transport.h index 34847dc2de..6590842857 100644 --- a/p2p/base/fake_ice_transport.h +++ b/p2p/base/fake_ice_transport.h @@ -257,6 +257,8 @@ class FakeIceTransport : public IceTransportInternal { ice_config_ = config; } + const IceConfig& config() const override { return ice_config_; } + void AddRemoteCandidate(const Candidate& candidate) override { RTC_DCHECK_RUN_ON(network_thread_); remote_candidates_.push_back(candidate); diff --git a/p2p/base/ice_transport_internal.h b/p2p/base/ice_transport_internal.h index fe09fa0cb6..707c28a255 100644 --- a/p2p/base/ice_transport_internal.h +++ b/p2p/base/ice_transport_internal.h @@ -13,24 +13,30 @@ #include +#include #include #include #include #include +#include "absl/functional/any_invocable.h" #include "absl/strings/string_view.h" +#include "api/array_view.h" #include "api/candidate.h" #include "api/rtc_error.h" #include "api/transport/enums.h" +#include "p2p/base/candidate_pair_interface.h" #include "p2p/base/connection.h" +#include "p2p/base/connection_info.h" #include "p2p/base/packet_transport_internal.h" #include "p2p/base/port.h" #include "p2p/base/stun_dictionary.h" #include "p2p/base/transport_description.h" +#include "rtc_base/callback_list.h" +#include "rtc_base/checks.h" #include "rtc_base/network_constants.h" #include "rtc_base/system/rtc_export.h" #include "rtc_base/third_party/sigslot/sigslot.h" -#include "rtc_base/time_utils.h" namespace cricket { @@ -276,6 +282,7 @@ class RTC_EXPORT IceTransportInternal : public rtc::PacketTransportInternal { virtual void SetRemoteIceMode(IceMode mode) = 0; virtual void SetIceConfig(const IceConfig& config) = 0; + virtual const IceConfig& config() const = 0; // Start gathering candidates if not already started, or if an ICE restart // occurred. diff --git a/p2p/base/mock_ice_transport.h b/p2p/base/mock_ice_transport.h index 0df0cba3a7..a8c7f3dd81 100644 --- a/p2p/base/mock_ice_transport.h +++ b/p2p/base/mock_ice_transport.h @@ -16,12 +16,8 @@ #include #include "p2p/base/ice_transport_internal.h" -#include "rtc_base/gunit.h" #include "test/gmock.h" -using ::testing::_; -using ::testing::Return; - namespace cricket { // Used in Chromium/remoting/protocol/channel_socket_adapter_unittest.cc @@ -62,7 +58,8 @@ class MockIceTransport : public IceTransportInternal { void SetIceParameters(const IceParameters& /* ice_params */) override {} void SetRemoteIceParameters(const IceParameters& /* ice_params */) override {} void SetRemoteIceMode(IceMode /* mode */) override {} - void SetIceConfig(const IceConfig& /* config */) override {} + void SetIceConfig(const IceConfig& config) override { ice_config_ = config; } + const IceConfig& config() const override { return ice_config_; } std::optional GetRttEstimate() override { return std::nullopt; } const Connection* selected_connection() const override { return nullptr; } std::optional GetSelectedCandidatePair() const override { @@ -81,6 +78,7 @@ class MockIceTransport : public IceTransportInternal { private: std::string transport_name_; + IceConfig ice_config_; }; } // namespace cricket diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc index 2071f3f1dd..78532332c0 100644 --- a/p2p/base/p2p_transport_channel.cc +++ b/p2p/base/p2p_transport_channel.cc @@ -14,36 +14,64 @@ #include #include +#include +#include #include #include +#include #include +#include +#include #include +#include #include "absl/algorithm/container.h" #include "absl/memory/memory.h" #include "absl/strings/match.h" #include "absl/strings/string_view.h" +#include "api/array_view.h" #include "api/async_dns_resolver.h" #include "api/candidate.h" #include "api/field_trials_view.h" +#include "api/ice_transport_interface.h" +#include "api/rtc_error.h" +#include "api/sequence_checker.h" +#include "api/transport/enums.h" +#include "api/transport/stun.h" #include "api/units/time_delta.h" +#include "logging/rtc_event_log/events/rtc_event_ice_candidate_pair_config.h" #include "logging/rtc_event_log/ice_logger.h" -#include "p2p/base/basic_ice_controller.h" +#include "p2p/base/active_ice_controller_factory_interface.h" +#include "p2p/base/candidate_pair_interface.h" #include "p2p/base/connection.h" #include "p2p/base/connection_info.h" +#include "p2p/base/ice_controller_factory_interface.h" +#include "p2p/base/ice_switch_reason.h" +#include "p2p/base/ice_transport_internal.h" +#include "p2p/base/p2p_constants.h" #include "p2p/base/port.h" +#include "p2p/base/port_allocator.h" +#include "p2p/base/port_interface.h" +#include "p2p/base/regathering_controller.h" +#include "p2p/base/transport_description.h" #include "p2p/base/wrapping_active_ice_controller.h" +#include "rtc_base/async_packet_socket.h" #include "rtc_base/checks.h" +#include "rtc_base/dscp.h" #include "rtc_base/experiments/struct_parameters_parser.h" #include "rtc_base/ip_address.h" #include "rtc_base/logging.h" #include "rtc_base/net_helper.h" +#include "rtc_base/net_helpers.h" #include "rtc_base/network.h" +#include "rtc_base/network/received_packet.h" +#include "rtc_base/network/sent_packet.h" #include "rtc_base/network_constants.h" -#include "rtc_base/string_encode.h" +#include "rtc_base/network_route.h" +#include "rtc_base/socket.h" +#include "rtc_base/socket_address.h" #include "rtc_base/time_utils.h" #include "rtc_base/trace_event.h" -#include "system_wrappers/include/metrics.h" namespace cricket { namespace { @@ -987,6 +1015,7 @@ void P2PTransportChannel::OnUnknownAddress(PortInterface* port, const std::string& remote_username, bool port_muxed) { RTC_DCHECK_RUN_ON(network_thread_); + RTC_DCHECK(stun_msg); // Port has received a valid stun packet from an address that no Connection // is currently available for. See if we already have a candidate with the diff --git a/p2p/base/p2p_transport_channel.h b/p2p/base/p2p_transport_channel.h index e1f5066c5d..71a9041d7b 100644 --- a/p2p/base/p2p_transport_channel.h +++ b/p2p/base/p2p_transport_channel.h @@ -23,16 +23,13 @@ #include #include -#include +#include #include #include #include -#include #include -#include #include -#include "absl/base/attributes.h" #include "absl/strings/string_view.h" #include "api/array_view.h" #include "api/async_dns_resolver.h" @@ -40,12 +37,12 @@ #include "api/ice_transport_interface.h" #include "api/rtc_error.h" #include "api/sequence_checker.h" -#include "api/task_queue/pending_task_safety_flag.h" #include "api/transport/enums.h" #include "api/transport/stun.h" #include "logging/rtc_event_log/events/rtc_event_ice_candidate_pair_config.h" #include "logging/rtc_event_log/ice_logger.h" #include "p2p/base/active_ice_controller_factory_interface.h" +#include "p2p/base/active_ice_controller_interface.h" #include "p2p/base/candidate_pair_interface.h" #include "p2p/base/connection.h" #include "p2p/base/ice_agent_interface.h" @@ -64,6 +61,7 @@ #include "rtc_base/async_packet_socket.h" #include "rtc_base/checks.h" #include "rtc_base/dscp.h" +#include "rtc_base/network/received_packet.h" #include "rtc_base/network/sent_packet.h" #include "rtc_base/network_route.h" #include "rtc_base/socket.h" @@ -146,7 +144,7 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal, // will not use it to update the respective parameter in `config_`. // TODO(deadbeef): Use std::optional instead of negative values. void SetIceConfig(const IceConfig& config) override; - const IceConfig& config() const; + const IceConfig& config() const override; static webrtc::RTCError ValidateIceConfig(const IceConfig& config); // From TransportChannel: diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index b0f0446c04..51a116414e 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -584,8 +584,7 @@ RTCErrorOr> PeerConnection::Create( << "PeerConnection constructed with legacy SDP semantics!"; } - RTCError config_error = cricket::P2PTransportChannel::ValidateIceConfig( - ParseIceConfig(configuration)); + RTCError config_error = ValidateConfiguration(configuration); if (!config_error.ok()) { RTC_LOG(LS_ERROR) << "Invalid ICE configuration: " << config_error.message();