From 5234a49a078b3cab76d571456396dcb8bd6aa8a5 Mon Sep 17 00:00:00 2001 From: Benjamin Wright Date: Tue, 29 May 2018 15:04:32 -0700 Subject: [PATCH] Create PeerConnectionFactoryDependencies to prevent new function overloads. To address this, this CL introduces a PeerConnectionFactoryDependencies structure to encapsulate all mandatory and optional dependencies (where a dependency is defined as non trivial executable code that an API user may want to provide to the native API). This allows adding a new injectable dependency by simply adding a new field to the struct, avoiding the hassle described above. Bug: webrtc:7913 Change-Id: Ice58fa72e8c578b250084a1629499fabda66dabf Reviewed-on: https://webrtc-review.googlesource.com/79720 Reviewed-by: Karl Wiberg Commit-Queue: Benjamin Wright Cr-Commit-Position: refs/heads/master@{#23480} --- api/peerconnectioninterface.h | 34 ++++++++++++++++++++++++++ pc/peerconnectionfactory.cc | 45 ++++++++++++++++++++++++++++------- pc/peerconnectionfactory.h | 6 +++++ 3 files changed, 77 insertions(+), 8 deletions(-) diff --git a/api/peerconnectioninterface.h b/api/peerconnectioninterface.h index fa940936ef..53979694a7 100644 --- a/api/peerconnectioninterface.h +++ b/api/peerconnectioninterface.h @@ -1205,6 +1205,36 @@ struct PeerConnectionDependencies final { std::unique_ptr tls_cert_verifier; }; +// PeerConnectionFactoryDependencies holds all of the PeerConnectionFactory +// dependencies. All new dependencies should be added here instead of +// overloading the function. This simplifies dependency injection and makes it +// clear which are mandatory and optional. If possible please allow the peer +// connection factory to take ownership of the dependency by adding a unique_ptr +// to this structure. +struct PeerConnectionFactoryDependencies final { + PeerConnectionFactoryDependencies() = default; + // This object is not copyable or assignable. + PeerConnectionFactoryDependencies(const PeerConnectionFactoryDependencies&) = + delete; + PeerConnectionFactoryDependencies& operator=( + const PeerConnectionFactoryDependencies&) = delete; + // This object is only moveable. + PeerConnectionFactoryDependencies(PeerConnectionFactoryDependencies&&) = + default; + PeerConnectionFactoryDependencies& operator=( + PeerConnectionFactoryDependencies&&) = default; + + // Optional dependencies + rtc::Thread* network_thread = nullptr; + rtc::Thread* worker_thread = nullptr; + rtc::Thread* signaling_thread = nullptr; + std::unique_ptr media_engine; + std::unique_ptr call_factory; + std::unique_ptr event_log_factory; + std::unique_ptr fec_controller_factory; + std::unique_ptr network_controller_factory; +}; + // PeerConnectionFactoryInterface is the factory interface used for creating // PeerConnection, MediaStream and MediaStreamTrack objects. // @@ -1555,6 +1585,10 @@ CreateModularPeerConnectionFactory( std::unique_ptr network_controller_factory = nullptr); +rtc::scoped_refptr +CreateModularPeerConnectionFactory( + PeerConnectionFactoryDependencies dependencies); + } // namespace webrtc #endif // API_PEERCONNECTIONINTERFACE_H_ diff --git a/pc/peerconnectionfactory.cc b/pc/peerconnectionfactory.cc index 0fc6af2557..c9fa223283 100644 --- a/pc/peerconnectionfactory.cc +++ b/pc/peerconnectionfactory.cc @@ -55,9 +55,14 @@ CreateModularPeerConnectionFactory( std::unique_ptr media_engine, std::unique_ptr call_factory, std::unique_ptr event_log_factory) { - return CreateModularPeerConnectionFactory( - network_thread, worker_thread, signaling_thread, std::move(media_engine), - std::move(call_factory), std::move(event_log_factory), nullptr, nullptr); + PeerConnectionFactoryDependencies dependencies; + dependencies.network_thread = network_thread; + dependencies.worker_thread = worker_thread; + dependencies.signaling_thread = signaling_thread; + dependencies.media_engine = std::move(media_engine); + dependencies.call_factory = std::move(call_factory); + dependencies.event_log_factory = std::move(event_log_factory); + return CreateModularPeerConnectionFactory(std::move(dependencies)); } rtc::scoped_refptr @@ -71,13 +76,25 @@ CreateModularPeerConnectionFactory( std::unique_ptr fec_controller_factory, std::unique_ptr network_controller_factory) { + PeerConnectionFactoryDependencies dependencies; + dependencies.network_thread = network_thread; + dependencies.worker_thread = worker_thread; + dependencies.signaling_thread = signaling_thread; + dependencies.media_engine = std::move(media_engine); + dependencies.call_factory = std::move(call_factory); + dependencies.event_log_factory = std::move(event_log_factory); + dependencies.fec_controller_factory = std::move(fec_controller_factory); + dependencies.network_controller_factory = + std::move(network_controller_factory); + return CreateModularPeerConnectionFactory(std::move(dependencies)); +} + +rtc::scoped_refptr +CreateModularPeerConnectionFactory( + PeerConnectionFactoryDependencies dependencies) { rtc::scoped_refptr pc_factory( new rtc::RefCountedObject( - network_thread, worker_thread, signaling_thread, - std::move(media_engine), std::move(call_factory), - std::move(event_log_factory), std::move(fec_controller_factory), - std::move(network_controller_factory))); - + std::move(dependencies))); // Call Initialize synchronously but make sure it is executed on // |signaling_thread|. MethodCall0 call( @@ -158,6 +175,18 @@ PeerConnectionFactory::PeerConnectionFactory( // RTC_DCHECK(default_adm != NULL); } +PeerConnectionFactory::PeerConnectionFactory( + PeerConnectionFactoryDependencies dependencies) + : PeerConnectionFactory( + dependencies.network_thread, + dependencies.worker_thread, + dependencies.signaling_thread, + std::move(dependencies.media_engine), + std::move(dependencies.call_factory), + std::move(dependencies.event_log_factory), + std::move(dependencies.fec_controller_factory), + std::move(dependencies.network_controller_factory)) {} + PeerConnectionFactory::~PeerConnectionFactory() { RTC_DCHECK(signaling_thread_->IsCurrent()); channel_manager_.reset(nullptr); diff --git a/pc/peerconnectionfactory.h b/pc/peerconnectionfactory.h index b566dd3e88..bce1e4c2e0 100644 --- a/pc/peerconnectionfactory.h +++ b/pc/peerconnectionfactory.h @@ -119,6 +119,12 @@ class PeerConnectionFactory : public PeerConnectionFactoryInterface { std::unique_ptr fec_controller_factory, std::unique_ptr network_controller_factory); + // Use this implementation for all future use. This structure allows simple + // management of all new dependencies being added to the + // PeerConnectionFactory. + explicit PeerConnectionFactory( + PeerConnectionFactoryDependencies dependencies); + virtual ~PeerConnectionFactory(); private: