From 2f74d5f7931d7ba0d77ebac0690196331d325f32 Mon Sep 17 00:00:00 2001 From: Jonas Oreland Date: Fri, 22 Nov 2019 07:53:22 +0100 Subject: [PATCH] Make IceController injectable This patch is a follow up on - https://webrtc-review.googlesource.com/c/src/+/158820 - https://webrtc-review.googlesource.com/c/src/+/158205 And makes the IceController injectable into P2PTransportChannel. This is useful so that one can only modify the behaviour of the the controller and still use the rest of the functionality of P2PTransportChannel. Bug: chromium:1024965 Change-Id: I36a1bc5cb4a60da46935ce8e4ce43e3bbbfeaf6d Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/160188 Reviewed-by: Qingsi Wang Commit-Queue: Jonas Oreland Cr-Commit-Position: refs/heads/master@{#29882} --- p2p/BUILD.gn | 1 + p2p/base/basic_ice_controller.cc | 14 +++----- p2p/base/basic_ice_controller.h | 7 ++-- p2p/base/default_ice_transport_factory.cc | 19 ++++++++++- p2p/base/ice_controller_factory_interface.h | 38 +++++++++++++++++++++ p2p/base/p2p_transport_channel.cc | 22 +++++++++--- p2p/base/p2p_transport_channel.h | 13 ++++--- p2p/base/p2p_transport_channel_unittest.cc | 25 ++++++++++++++ 8 files changed, 114 insertions(+), 25 deletions(-) create mode 100644 p2p/base/ice_controller_factory_interface.h diff --git a/p2p/BUILD.gn b/p2p/BUILD.gn index 312ade59ee..01bc47d567 100644 --- a/p2p/BUILD.gn +++ b/p2p/BUILD.gn @@ -38,6 +38,7 @@ rtc_library("rtc_p2p") { "base/dtls_transport_factory.h", "base/dtls_transport_internal.cc", "base/dtls_transport_internal.h", + "base/ice_controller_factory_interface.h", "base/ice_controller_interface.cc", "base/ice_controller_interface.h", "base/ice_credentials_iterator.cc", diff --git a/p2p/base/basic_ice_controller.cc b/p2p/base/basic_ice_controller.cc index d348ae92d4..e2d5eef22b 100644 --- a/p2p/base/basic_ice_controller.cc +++ b/p2p/base/basic_ice_controller.cc @@ -57,15 +57,11 @@ int CompareCandidatePairsByNetworkPreference( namespace cricket { -BasicIceController::BasicIceController( - std::function ice_transport_state_func, - std::function ice_role_func, - std::function is_connection_pruned_func, - const IceFieldTrials* field_trials) - : ice_transport_state_func_(ice_transport_state_func), - ice_role_func_(ice_role_func), - is_connection_pruned_func_(is_connection_pruned_func), - field_trials_(field_trials) {} +BasicIceController::BasicIceController(const IceControllerFactoryArgs& args) + : ice_transport_state_func_(args.ice_transport_state_func), + ice_role_func_(args.ice_role_func), + is_connection_pruned_func_(args.is_connection_pruned_func), + field_trials_(args.ice_field_trials) {} BasicIceController::~BasicIceController() {} diff --git a/p2p/base/basic_ice_controller.h b/p2p/base/basic_ice_controller.h index 5335c0077c..a0917e7e51 100644 --- a/p2p/base/basic_ice_controller.h +++ b/p2p/base/basic_ice_controller.h @@ -17,6 +17,7 @@ #include #include +#include "p2p/base/ice_controller_factory_interface.h" #include "p2p/base/ice_controller_interface.h" #include "p2p/base/p2p_transport_channel.h" @@ -24,11 +25,7 @@ namespace cricket { class BasicIceController : public IceControllerInterface { public: - BasicIceController( - std::function ice_transport_state_func, - std::function ice_role_func, - std::function is_candidated_pruned_func, - const IceFieldTrials*); + explicit BasicIceController(const IceControllerFactoryArgs& args); virtual ~BasicIceController(); void SetIceConfig(const IceConfig& config) override; diff --git a/p2p/base/default_ice_transport_factory.cc b/p2p/base/default_ice_transport_factory.cc index 4430525500..f4b182efdf 100644 --- a/p2p/base/default_ice_transport_factory.cc +++ b/p2p/base/default_ice_transport_factory.cc @@ -12,6 +12,22 @@ #include +#include "p2p/base/basic_ice_controller.h" +#include "p2p/base/ice_controller_factory_interface.h" + +namespace { + +class BasicIceControllerFactory + : public cricket::IceControllerFactoryInterface { + public: + std::unique_ptr Create( + const cricket::IceControllerFactoryArgs& args) override { + return std::make_unique(args); + } +}; + +} // namespace + namespace webrtc { DefaultIceTransport::DefaultIceTransport( @@ -27,10 +43,11 @@ DefaultIceTransportFactory::CreateIceTransport( const std::string& transport_name, int component, IceTransportInit init) { + BasicIceControllerFactory factory; return new rtc::RefCountedObject( std::make_unique( transport_name, component, init.port_allocator(), - init.async_resolver_factory(), init.event_log())); + init.async_resolver_factory(), init.event_log(), &factory)); } } // namespace webrtc diff --git a/p2p/base/ice_controller_factory_interface.h b/p2p/base/ice_controller_factory_interface.h new file mode 100644 index 0000000000..a859c07be9 --- /dev/null +++ b/p2p/base/ice_controller_factory_interface.h @@ -0,0 +1,38 @@ +/* + * Copyright 2019 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_CONTROLLER_FACTORY_INTERFACE_H_ +#define P2P_BASE_ICE_CONTROLLER_FACTORY_INTERFACE_H_ + +#include + +#include "p2p/base/ice_controller_interface.h" +#include "p2p/base/ice_transport_internal.h" + +namespace cricket { + +// struct with arguments to IceControllerFactoryInterface::Create +struct IceControllerFactoryArgs { + std::function ice_transport_state_func; + std::function ice_role_func; + std::function is_connection_pruned_func; + const IceFieldTrials* ice_field_trials; +}; + +class IceControllerFactoryInterface { + public: + virtual ~IceControllerFactoryInterface() = default; + virtual std::unique_ptr Create( + const IceControllerFactoryArgs& args) = 0; +}; + +} // namespace cricket + +#endif // P2P_BASE_ICE_CONTROLLER_FACTORY_INTERFACE_H_ diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc index b5f57dc32f..093c35d7ab 100644 --- a/p2p/base/p2p_transport_channel.cc +++ b/p2p/base/p2p_transport_channel.cc @@ -78,14 +78,19 @@ bool IceCredentialsChanged(const std::string& old_ufrag, P2PTransportChannel::P2PTransportChannel(const std::string& transport_name, int component, PortAllocator* allocator) - : P2PTransportChannel(transport_name, component, allocator, nullptr) {} + : P2PTransportChannel(transport_name, + component, + allocator, + nullptr, + nullptr) {} P2PTransportChannel::P2PTransportChannel( const std::string& transport_name, int component, PortAllocator* allocator, webrtc::AsyncResolverFactory* async_resolver_factory, - webrtc::RtcEventLog* event_log) + webrtc::RtcEventLog* event_log, + IceControllerFactoryInterface* ice_controller_factory) : transport_name_(transport_name), component_(component), allocator_(allocator), @@ -123,13 +128,20 @@ P2PTransportChannel::P2PTransportChannel( this, &P2PTransportChannel::OnCandidateFilterChanged); ice_event_log_.set_event_log(event_log); - ice_controller_ = std::make_unique( - [this] { return GetState(); }, [this] { return GetIceRole(); }, + IceControllerFactoryArgs args{ + [this] { return GetState(); }, + [this] { return GetIceRole(); }, [this](const Connection* connection) { return IsPortPruned(connection->port()) || IsRemoteCandidatePruned(connection->remote_candidate()); }, - &field_trials_); + &field_trials_, + }; + if (ice_controller_factory != nullptr) { + ice_controller_ = ice_controller_factory->Create(args); + } else { + ice_controller_ = std::make_unique(args); + } } P2PTransportChannel::~P2PTransportChannel() { diff --git a/p2p/base/p2p_transport_channel.h b/p2p/base/p2p_transport_channel.h index fdc5dd202e..3d6c86f031 100644 --- a/p2p/base/p2p_transport_channel.h +++ b/p2p/base/p2p_transport_channel.h @@ -33,6 +33,7 @@ #include "logging/rtc_event_log/events/rtc_event_ice_candidate_pair_config.h" #include "logging/rtc_event_log/ice_logger.h" #include "p2p/base/candidate_pair_interface.h" +#include "p2p/base/ice_controller_factory_interface.h" #include "p2p/base/ice_controller_interface.h" #include "p2p/base/ice_transport_internal.h" #include "p2p/base/p2p_constants.h" @@ -86,11 +87,13 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal { P2PTransportChannel(const std::string& transport_name, int component, PortAllocator* allocator); - P2PTransportChannel(const std::string& transport_name, - int component, - PortAllocator* allocator, - webrtc::AsyncResolverFactory* async_resolver_factory, - webrtc::RtcEventLog* event_log = nullptr); + P2PTransportChannel( + const std::string& transport_name, + int component, + PortAllocator* allocator, + webrtc::AsyncResolverFactory* async_resolver_factory, + webrtc::RtcEventLog* event_log = nullptr, + IceControllerFactoryInterface* ice_controller_factory = nullptr); ~P2PTransportChannel() override; // From TransportChannelImpl: diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc index f9060003dd..042110bfc6 100644 --- a/p2p/base/p2p_transport_channel_unittest.cc +++ b/p2p/base/p2p_transport_channel_unittest.cc @@ -14,6 +14,7 @@ #include #include +#include "p2p/base/basic_ice_controller.h" #include "p2p/base/connection.h" #include "p2p/base/fake_port_allocator.h" #include "p2p/base/ice_transport_internal.h" @@ -173,6 +174,19 @@ cricket::BasicPortAllocator* CreateBasicPortAllocator( allocator->SetConfiguration(stun_servers, turn_servers, 0, webrtc::NO_PRUNE); return allocator; } + +class MockIceControllerFactory : public cricket::IceControllerFactoryInterface { + public: + ~MockIceControllerFactory() = default; + std::unique_ptr Create( + const cricket::IceControllerFactoryArgs& args) { + RecordIceControllerCreated(); + return std::make_unique(args); + } + + MOCK_METHOD0(RecordIceControllerCreated, void()); +}; + } // namespace namespace cricket { @@ -5590,4 +5604,15 @@ TEST_F(P2PTransportChannelPingTest, TestInitialSelectDampeningBoth) { EXPECT_EQ_SIMULATED_WAIT(conn1, ch.selected_connection(), 2 * kMargin, clock); } +TEST(P2PTransportChannel, InjectIceController) { + MockIceControllerFactory factory; + FakePortAllocator pa(rtc::Thread::Current(), nullptr); + EXPECT_CALL(factory, RecordIceControllerCreated()).Times(1); + std::make_unique( + "transport_name", + /* component= */ 77, &pa, + /* async_resolver_factory = */ nullptr, + /* event_log = */ nullptr, &factory); +} + } // namespace cricket