From 7ddc7d548cf7f1a39b7efabd3a06f8162305baee Mon Sep 17 00:00:00 2001 From: Jonas Oreland Date: Wed, 16 Mar 2022 12:21:52 +0100 Subject: [PATCH] WebRTC-DeprecateGlobalFieldTrialString/Enabled/ - part 6/inf This patch just refactors creation of P2P transport channel, pushing down the IceTransportInit object rather than decomposing it going down. The IceTransportInit object will in subsequent patches be extended with a field trial container. Reason for splitting patch into this and subsequent is to allow changes to internal factories. Bug: webrtc:10335 Change-Id: Icc8b6e4142744b64d134bcb2d4a56777745db62b Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/255602 Reviewed-by: Mirko Bonadei Reviewed-by: Harald Alvestrand Commit-Queue: Jonas Oreland Cr-Commit-Position: refs/heads/main@{#36215} --- api/ice_transport_factory.cc | 15 ++------ api/ice_transport_interface.h | 10 ++++++ p2p/base/default_ice_transport_factory.cc | 6 ++-- p2p/base/p2p_transport_channel.cc | 41 +++++++--------------- p2p/base/p2p_transport_channel.h | 32 +++++++++++++++-- p2p/base/p2p_transport_channel_unittest.cc | 11 +++--- 6 files changed, 65 insertions(+), 50 deletions(-) diff --git a/api/ice_transport_factory.cc b/api/ice_transport_factory.cc index 26ef88bf1c..9e7e629a6f 100644 --- a/api/ice_transport_factory.cc +++ b/api/ice_transport_factory.cc @@ -58,18 +58,9 @@ rtc::scoped_refptr CreateIceTransport( rtc::scoped_refptr CreateIceTransport( IceTransportInit init) { - if (init.async_resolver_factory()) { - // Backwards compatibility mode - return rtc::make_ref_counted( - std::make_unique( - "", cricket::ICE_CANDIDATE_COMPONENT_RTP, init.port_allocator(), - init.async_resolver_factory(), init.event_log())); - } else { - return rtc::make_ref_counted( - cricket::P2PTransportChannel::Create( - "", cricket::ICE_CANDIDATE_COMPONENT_RTP, init.port_allocator(), - init.async_dns_resolver_factory(), init.event_log())); - } + return rtc::make_ref_counted( + cricket::P2PTransportChannel::Create( + "", cricket::ICE_CANDIDATE_COMPONENT_RTP, std::move(init))); } } // namespace webrtc diff --git a/api/ice_transport_interface.h b/api/ice_transport_interface.h index a3b364c87a..f202658acf 100644 --- a/api/ice_transport_interface.h +++ b/api/ice_transport_interface.h @@ -23,6 +23,7 @@ namespace cricket { class IceTransportInternal; class PortAllocator; +class IceControllerFactoryInterface; } // namespace cricket namespace webrtc { @@ -74,12 +75,21 @@ struct IceTransportInit final { RtcEventLog* event_log() { return event_log_; } void set_event_log(RtcEventLog* event_log) { event_log_ = event_log; } + void set_ice_controller_factory( + cricket::IceControllerFactoryInterface* ice_controller_factory) { + ice_controller_factory_ = ice_controller_factory; + } + cricket::IceControllerFactoryInterface* ice_controller_factory() { + return ice_controller_factory_; + } + private: cricket::PortAllocator* port_allocator_ = nullptr; AsyncDnsResolverFactoryInterface* async_dns_resolver_factory_ = nullptr; // For backwards compatibility. Only one resolver factory can be set. AsyncResolverFactory* async_resolver_factory_ = nullptr; RtcEventLog* event_log_ = nullptr; + cricket::IceControllerFactoryInterface* ice_controller_factory_ = nullptr; // TODO(https://crbug.com/webrtc/12657): Redesign to have const members. }; diff --git a/p2p/base/default_ice_transport_factory.cc b/p2p/base/default_ice_transport_factory.cc index 0a7175cfd8..8d87d66697 100644 --- a/p2p/base/default_ice_transport_factory.cc +++ b/p2p/base/default_ice_transport_factory.cc @@ -44,10 +44,10 @@ DefaultIceTransportFactory::CreateIceTransport( int component, IceTransportInit init) { BasicIceControllerFactory factory; + init.set_ice_controller_factory(&factory); return rtc::make_ref_counted( - cricket::P2PTransportChannel::Create( - transport_name, component, init.port_allocator(), - init.async_dns_resolver_factory(), init.event_log(), &factory)); + cricket::P2PTransportChannel::Create(transport_name, component, + std::move(init))); } } // namespace webrtc diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc index f95534b3e0..5d7f895b61 100644 --- a/p2p/base/p2p_transport_channel.cc +++ b/p2p/base/p2p_transport_channel.cc @@ -109,18 +109,22 @@ bool IceCredentialsChanged(const std::string& old_ufrag, return (old_ufrag != new_ufrag) || (old_pwd != new_pwd); } -// static std::unique_ptr P2PTransportChannel::Create( const std::string& transport_name, int component, - PortAllocator* allocator, - webrtc::AsyncDnsResolverFactoryInterface* async_dns_resolver_factory, - webrtc::RtcEventLog* event_log, - IceControllerFactoryInterface* ice_controller_factory) { - return absl::WrapUnique(new P2PTransportChannel( - transport_name, component, allocator, async_dns_resolver_factory, - /* owned_dns_resolver_factory= */ nullptr, event_log, - ice_controller_factory)); + webrtc::IceTransportInit init) { + if (init.async_resolver_factory()) { + return absl::WrapUnique(new P2PTransportChannel( + transport_name, component, init.port_allocator(), nullptr, + std::make_unique( + init.async_resolver_factory()), + init.event_log(), init.ice_controller_factory())); + } else { + return absl::WrapUnique(new P2PTransportChannel( + transport_name, component, init.port_allocator(), + init.async_dns_resolver_factory(), nullptr, init.event_log(), + init.ice_controller_factory())); + } } P2PTransportChannel::P2PTransportChannel(const std::string& transport_name, @@ -204,25 +208,6 @@ P2PTransportChannel::P2PTransportChannel( } } -// Public constructor, exposed for backwards compatibility. -// Deprecated. -P2PTransportChannel::P2PTransportChannel( - const std::string& transport_name, - int component, - PortAllocator* allocator, - webrtc::AsyncResolverFactory* async_resolver_factory, - webrtc::RtcEventLog* event_log, - IceControllerFactoryInterface* ice_controller_factory) - : P2PTransportChannel( - transport_name, - component, - allocator, - nullptr, - std::make_unique( - async_resolver_factory), - event_log, - ice_controller_factory) {} - P2PTransportChannel::~P2PTransportChannel() { TRACE_EVENT0("webrtc", "P2PTransportChannel::~P2PTransportChannel"); RTC_DCHECK_RUN_ON(network_thread_); diff --git a/p2p/base/p2p_transport_channel.h b/p2p/base/p2p_transport_channel.h index a1c2de04a7..58bd1fb978 100644 --- a/p2p/base/p2p_transport_channel.h +++ b/p2p/base/p2p_transport_channel.h @@ -28,6 +28,7 @@ #include #include #include +#include #include #include "absl/base/attributes.h" @@ -36,12 +37,14 @@ #include "api/async_dns_resolver.h" #include "api/async_resolver_factory.h" #include "api/candidate.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 "logging/rtc_event_log/events/rtc_event_ice_candidate_pair_config.h" #include "logging/rtc_event_log/ice_logger.h" +#include "p2p/base/basic_async_resolver_factory.h" #include "p2p/base/candidate_pair_interface.h" #include "p2p/base/connection.h" #include "p2p/base/ice_controller_factory_interface.h" @@ -101,18 +104,33 @@ class RemoteCandidate : public Candidate { // two P2P clients connected to each other. class RTC_EXPORT P2PTransportChannel : public IceTransportInternal { public: + static std::unique_ptr Create( + const std::string& transport_name, + int component, + webrtc::IceTransportInit init); + + // TODO(jonaso): This is deprecated and will be removed. static std::unique_ptr Create( const std::string& transport_name, int component, PortAllocator* allocator, webrtc::AsyncDnsResolverFactoryInterface* async_dns_resolver_factory, webrtc::RtcEventLog* event_log = nullptr, - IceControllerFactoryInterface* ice_controller_factory = nullptr); + IceControllerFactoryInterface* ice_controller_factory = nullptr) { + webrtc::IceTransportInit init; + init.set_port_allocator(allocator); + init.set_async_dns_resolver_factory(async_dns_resolver_factory); + init.set_event_log(event_log); + init.set_ice_controller_factory(ice_controller_factory); + return Create(transport_name, component, std::move(init)); + } + // For testing only. // TODO(zstein): Remove once AsyncDnsResolverFactory is required. P2PTransportChannel(const std::string& transport_name, int component, PortAllocator* allocator); + ABSL_DEPRECATED("bugs.webrtc.org/12598") P2PTransportChannel( const std::string& transport_name, @@ -120,7 +138,17 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal { PortAllocator* allocator, webrtc::AsyncResolverFactory* async_resolver_factory, webrtc::RtcEventLog* event_log = nullptr, - IceControllerFactoryInterface* ice_controller_factory = nullptr); + IceControllerFactoryInterface* ice_controller_factory = nullptr) + : P2PTransportChannel( + transport_name, + component, + allocator, + nullptr, + std::make_unique( + async_resolver_factory), + event_log, + ice_controller_factory) {} + ~P2PTransportChannel() override; P2PTransportChannel(const P2PTransportChannel&) = delete; diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc index 79bb2a340c..9acd7b1fa0 100644 --- a/p2p/base/p2p_transport_channel_unittest.cc +++ b/p2p/base/p2p_transport_channel_unittest.cc @@ -6074,11 +6074,12 @@ TEST(P2PTransportChannel, InjectIceController) { MockIceControllerFactory factory; FakePortAllocator pa(rtc::Thread::Current(), nullptr); EXPECT_CALL(factory, RecordIceControllerCreated()).Times(1); - auto dummy = std::make_unique( - "transport_name", - /* component= */ 77, &pa, - /* async_resolver_factory = */ nullptr, - /* event_log = */ nullptr, &factory); + webrtc::IceTransportInit init; + init.set_port_allocator(&pa); + init.set_ice_controller_factory(&factory); + auto dummy = + P2PTransportChannel::Create("transport_name", + /* component= */ 77, std::move(init)); } class ForgetLearnedStateController : public cricket::BasicIceController {