diff --git a/api/ice_transport_factory.h b/api/ice_transport_factory.h index d981e0b277..a9fd04ee7b 100644 --- a/api/ice_transport_factory.h +++ b/api/ice_transport_factory.h @@ -11,48 +11,16 @@ #ifndef API_ICE_TRANSPORT_FACTORY_H_ #define API_ICE_TRANSPORT_FACTORY_H_ -#include "api/async_resolver_factory.h" #include "api/ice_transport_interface.h" -#include "api/rtc_event_log/rtc_event_log.h" #include "api/scoped_refptr.h" #include "rtc_base/system/rtc_export.h" namespace cricket { class PortAllocator; -} +} // namespace cricket namespace webrtc { -struct IceTransportInit final { - public: - IceTransportInit() = default; - IceTransportInit(const IceTransportInit&) = delete; - IceTransportInit(IceTransportInit&&) = default; - IceTransportInit& operator=(const IceTransportInit&) = delete; - IceTransportInit& operator=(IceTransportInit&&) = default; - - cricket::PortAllocator* port_allocator() { return port_allocator_; } - void set_port_allocator(cricket::PortAllocator* port_allocator) { - port_allocator_ = port_allocator; - } - - AsyncResolverFactory* async_resolver_factory() { - return async_resolver_factory_; - } - void set_async_resolver_factory( - AsyncResolverFactory* async_resolver_factory) { - async_resolver_factory_ = async_resolver_factory; - } - - RtcEventLog* event_log() { return event_log_; } - void set_event_log(RtcEventLog* event_log) { event_log_ = event_log; } - - private: - cricket::PortAllocator* port_allocator_ = nullptr; - AsyncResolverFactory* async_resolver_factory_ = nullptr; - RtcEventLog* event_log_ = nullptr; -}; - // Static factory for an IceTransport object that can be created // without using a webrtc::PeerConnection. // The returned object must be accessed and destroyed on the thread that diff --git a/api/ice_transport_interface.h b/api/ice_transport_interface.h index 6e630453c4..d2f1edc012 100644 --- a/api/ice_transport_interface.h +++ b/api/ice_transport_interface.h @@ -11,12 +11,17 @@ #ifndef API_ICE_TRANSPORT_INTERFACE_H_ #define API_ICE_TRANSPORT_INTERFACE_H_ +#include + +#include "api/async_resolver_factory.h" #include "api/rtc_error.h" +#include "api/rtc_event_log/rtc_event_log.h" #include "api/scoped_refptr.h" #include "rtc_base/ref_count.h" namespace cricket { class IceTransportInternal; +class PortAllocator; } // namespace cricket namespace webrtc { @@ -34,5 +39,57 @@ class IceTransportInterface : public rtc::RefCountInterface { virtual cricket::IceTransportInternal* internal() = 0; }; +struct IceTransportInit final { + public: + IceTransportInit() = default; + IceTransportInit(const IceTransportInit&) = delete; + IceTransportInit(IceTransportInit&&) = default; + IceTransportInit& operator=(const IceTransportInit&) = delete; + IceTransportInit& operator=(IceTransportInit&&) = default; + + cricket::PortAllocator* port_allocator() { return port_allocator_; } + void set_port_allocator(cricket::PortAllocator* port_allocator) { + port_allocator_ = port_allocator; + } + + AsyncResolverFactory* async_resolver_factory() { + return async_resolver_factory_; + } + void set_async_resolver_factory( + AsyncResolverFactory* async_resolver_factory) { + async_resolver_factory_ = async_resolver_factory; + } + + RtcEventLog* event_log() { return event_log_; } + void set_event_log(RtcEventLog* event_log) { event_log_ = event_log; } + + private: + cricket::PortAllocator* port_allocator_ = nullptr; + AsyncResolverFactory* async_resolver_factory_ = nullptr; + RtcEventLog* event_log_ = nullptr; +}; + +// TODO(qingsi): The factory interface is defined in this file instead of its +// namesake file ice_transport_factory.h to avoid the extra dependency on p2p/ +// introduced there by the p2p/-dependent factory methods. Move the factory +// methods to a different file or rename it. +class IceTransportFactory { + public: + virtual ~IceTransportFactory() = default; + // As a refcounted object, the returned ICE transport may outlive the host + // construct into which its reference is given, e.g. a peer connection. As a + // result, the returned ICE transport should not hold references to any object + // that the transport does not own and that has a lifetime bound to the host + // construct. Also, assumptions on the thread safety of the returned transport + // should be clarified by implementations. For example, a peer connection + // requires the returned transport to be constructed and destroyed on the + // network thread and an ICE transport factory that intends to work with a + // peer connection should offer transports compatible with these assumptions. + virtual rtc::scoped_refptr CreateIceTransport( + const std::string& transport_name, + int component, + IceTransportInit init) = 0; +}; + } // namespace webrtc #endif // API_ICE_TRANSPORT_INTERFACE_H_ diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h index a1280de551..f2ef336742 100644 --- a/api/peer_connection_interface.h +++ b/api/peer_connection_interface.h @@ -83,6 +83,7 @@ #include "api/data_channel_interface.h" #include "api/dtls_transport_interface.h" #include "api/fec_controller.h" +#include "api/ice_transport_interface.h" #include "api/jsep.h" #include "api/media_stream_interface.h" #include "api/neteq/neteq_factory.h" @@ -1288,6 +1289,7 @@ struct RTC_EXPORT PeerConnectionDependencies final { std::unique_ptr allocator; std::unique_ptr packet_socket_factory; std::unique_ptr async_resolver_factory; + std::unique_ptr ice_transport_factory; std::unique_ptr cert_generator; std::unique_ptr tls_cert_verifier; std::unique_ptr diff --git a/p2p/BUILD.gn b/p2p/BUILD.gn index 8c4b6f636b..42afcffdc0 100644 --- a/p2p/BUILD.gn +++ b/p2p/BUILD.gn @@ -41,8 +41,11 @@ rtc_library("rtc_p2p") { "base/connection.h", "base/connection_info.cc", "base/connection_info.h", + "base/default_ice_transport_factory.cc", + "base/default_ice_transport_factory.h", "base/dtls_transport.cc", "base/dtls_transport.h", + "base/dtls_transport_factory.h", "base/dtls_transport_internal.cc", "base/dtls_transport_internal.h", "base/ice_credentials_iterator.cc", @@ -80,7 +83,6 @@ rtc_library("rtc_p2p") { "base/transport_description.h", "base/transport_description_factory.cc", "base/transport_description_factory.h", - "base/transport_factory_interface.h", "base/transport_info.h", "base/turn_port.cc", "base/turn_port.h", @@ -132,6 +134,7 @@ if (rtc_include_tests) { ] deps = [ ":rtc_p2p", + "../api:libjingle_peerconnection_api", "../rtc_base", "../rtc_base:rtc_base_approved", "//third_party/abseil-cpp/absl/algorithm:container", diff --git a/p2p/base/default_ice_transport_factory.cc b/p2p/base/default_ice_transport_factory.cc new file mode 100644 index 0000000000..4430525500 --- /dev/null +++ b/p2p/base/default_ice_transport_factory.cc @@ -0,0 +1,36 @@ +/* + * 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. + */ + +#include "p2p/base/default_ice_transport_factory.h" + +#include + +namespace webrtc { + +DefaultIceTransport::DefaultIceTransport( + std::unique_ptr internal) + : internal_(std::move(internal)) {} + +DefaultIceTransport::~DefaultIceTransport() { + RTC_DCHECK_RUN_ON(&thread_checker_); +} + +rtc::scoped_refptr +DefaultIceTransportFactory::CreateIceTransport( + const std::string& transport_name, + int component, + IceTransportInit init) { + return new rtc::RefCountedObject( + std::make_unique( + transport_name, component, init.port_allocator(), + init.async_resolver_factory(), init.event_log())); +} + +} // namespace webrtc diff --git a/p2p/base/default_ice_transport_factory.h b/p2p/base/default_ice_transport_factory.h new file mode 100644 index 0000000000..4834c9ada7 --- /dev/null +++ b/p2p/base/default_ice_transport_factory.h @@ -0,0 +1,58 @@ +/* + * 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_DEFAULT_ICE_TRANSPORT_FACTORY_H_ +#define P2P_BASE_DEFAULT_ICE_TRANSPORT_FACTORY_H_ + +#include +#include + +#include "api/ice_transport_interface.h" +#include "p2p/base/p2p_transport_channel.h" +#include "rtc_base/thread.h" + +namespace webrtc { + +// The default ICE transport wraps the implementation of IceTransportInternal +// provided by P2PTransportChannel. This default transport is not thread safe +// and must be constructed, used and destroyed on the same network thread on +// which the internal P2PTransportChannel lives. +class DefaultIceTransport : public IceTransportInterface { + public: + explicit DefaultIceTransport( + std::unique_ptr internal); + ~DefaultIceTransport(); + + cricket::IceTransportInternal* internal() override { + RTC_DCHECK_RUN_ON(&thread_checker_); + return internal_.get(); + } + + private: + const rtc::ThreadChecker thread_checker_{}; + std::unique_ptr internal_ + RTC_GUARDED_BY(thread_checker_); +}; + +class DefaultIceTransportFactory : public IceTransportFactory { + public: + DefaultIceTransportFactory() = default; + ~DefaultIceTransportFactory() = default; + + // Must be called on the network thread and returns a DefaultIceTransport. + rtc::scoped_refptr CreateIceTransport( + const std::string& transport_name, + int component, + IceTransportInit init) override; +}; + +} // namespace webrtc + +#endif // P2P_BASE_DEFAULT_ICE_TRANSPORT_FACTORY_H_ diff --git a/p2p/base/transport_factory_interface.h b/p2p/base/dtls_transport_factory.h similarity index 52% rename from p2p/base/transport_factory_interface.h rename to p2p/base/dtls_transport_factory.h index e7eead7827..9ad78a7cc2 100644 --- a/p2p/base/transport_factory_interface.h +++ b/p2p/base/dtls_transport_factory.h @@ -8,8 +8,8 @@ * be found in the AUTHORS file in the root of the source tree. */ -#ifndef P2P_BASE_TRANSPORT_FACTORY_INTERFACE_H_ -#define P2P_BASE_TRANSPORT_FACTORY_INTERFACE_H_ +#ifndef P2P_BASE_DTLS_TRANSPORT_FACTORY_H_ +#define P2P_BASE_DTLS_TRANSPORT_FACTORY_H_ #include #include @@ -19,18 +19,15 @@ namespace cricket { -// This interface is used to create DTLS/ICE transports. The external transports -// can be injected into the JsepTransportController through it. For example, the -// FakeIceTransport/FakeDtlsTransport can be injected by setting a -// FakeTransportFactory which implements this interface to the -// JsepTransportController. -class TransportFactoryInterface { +// This interface is used to create DTLS transports. The external transports +// can be injected into the JsepTransportController through it. +// +// TODO(qingsi): Remove this factory in favor of one that produces +// DtlsTransportInterface given by the public API if this is going to be +// injectable. +class DtlsTransportFactory { public: - virtual ~TransportFactoryInterface() {} - - virtual std::unique_ptr CreateIceTransport( - const std::string& transport_name, - int component) = 0; + virtual ~DtlsTransportFactory() = default; virtual std::unique_ptr CreateDtlsTransport( IceTransportInternal* ice, @@ -39,4 +36,4 @@ class TransportFactoryInterface { } // namespace cricket -#endif // P2P_BASE_TRANSPORT_FACTORY_INTERFACE_H_ +#endif // P2P_BASE_DTLS_TRANSPORT_FACTORY_H_ diff --git a/p2p/base/fake_ice_transport.h b/p2p/base/fake_ice_transport.h index b1a83b8ced..d0fa1ea8cc 100644 --- a/p2p/base/fake_ice_transport.h +++ b/p2p/base/fake_ice_transport.h @@ -12,11 +12,13 @@ #define P2P_BASE_FAKE_ICE_TRANSPORT_H_ #include +#include #include #include #include "absl/algorithm/container.h" #include "absl/types/optional.h" +#include "api/ice_transport_interface.h" #include "p2p/base/ice_transport_internal.h" #include "rtc_base/async_invoker.h" #include "rtc_base/copy_on_write_buffer.h" @@ -330,6 +332,18 @@ class FakeIceTransport : public IceTransportInternal { rtc::Thread* const network_thread_; }; +class FakeIceTransportWrapper : public webrtc::IceTransportInterface { + public: + explicit FakeIceTransportWrapper( + std::unique_ptr internal) + : internal_(std::move(internal)) {} + + cricket::IceTransportInternal* internal() override { return internal_.get(); } + + private: + std::unique_ptr internal_; +}; + } // namespace cricket #endif // P2P_BASE_FAKE_ICE_TRANSPORT_H_ diff --git a/pc/BUILD.gn b/pc/BUILD.gn index 2dcbd91363..40f3b502f7 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -87,6 +87,7 @@ rtc_library("rtc_pc_base") { "../api:array_view", "../api:audio_options_api", "../api:call_api", + "../api:ice_transport_factory", "../api:libjingle_peerconnection_api", "../api:rtc_error", "../api:rtp_headers", @@ -227,6 +228,7 @@ rtc_library("peerconnection") { "../api:audio_options_api", "../api:call_api", "../api:fec_controller_api", + "../api:ice_transport_factory", "../api:libjingle_peerconnection_api", "../api:media_stream_interface", "../api:network_state_predictor_api", @@ -577,6 +579,7 @@ if (rtc_include_tests) { "../modules/audio_device:audio_device_api", "../modules/audio_processing:audio_processing_statistics", "../modules/rtp_rtcp:rtp_rtcp_format", + "../p2p:fake_ice_transport", "../p2p:fake_port_allocator", "../rtc_base:checks", "../rtc_base:gunit_helpers", diff --git a/pc/jsep_transport.cc b/pc/jsep_transport.cc index 13618c7a6c..79b933c9e5 100644 --- a/pc/jsep_transport.cc +++ b/pc/jsep_transport.cc @@ -102,8 +102,8 @@ JsepTransportDescription& JsepTransportDescription::operator=( JsepTransport::JsepTransport( const std::string& mid, const rtc::scoped_refptr& local_certificate, - std::unique_ptr ice_transport, - std::unique_ptr rtcp_ice_transport, + rtc::scoped_refptr ice_transport, + rtc::scoped_refptr rtcp_ice_transport, std::unique_ptr unencrypted_rtp_transport, std::unique_ptr sdes_transport, std::unique_ptr dtls_srtp_transport, diff --git a/pc/jsep_transport.h b/pc/jsep_transport.h index 3c63c47ba4..658e8e7b72 100644 --- a/pc/jsep_transport.h +++ b/pc/jsep_transport.h @@ -18,6 +18,7 @@ #include "absl/types/optional.h" #include "api/candidate.h" +#include "api/ice_transport_interface.h" #include "api/jsep.h" #include "api/transport/datagram_transport_interface.h" #include "api/transport/media/media_transport_interface.h" @@ -101,8 +102,8 @@ class JsepTransport : public sigslot::has_slots<>, JsepTransport( const std::string& mid, const rtc::scoped_refptr& local_certificate, - std::unique_ptr ice_transport, - std::unique_ptr rtcp_ice_transport, + rtc::scoped_refptr ice_transport, + rtc::scoped_refptr rtcp_ice_transport, std::unique_ptr unencrypted_rtp_transport, std::unique_ptr sdes_transport, std::unique_ptr dtls_srtp_transport, @@ -377,8 +378,8 @@ class JsepTransport : public sigslot::has_slots<>, // Ice transport which may be used by any of upper-layer transports (below). // Owned by JsepTransport and guaranteed to outlive the transports below. - const std::unique_ptr ice_transport_; - const std::unique_ptr rtcp_ice_transport_; + const rtc::scoped_refptr ice_transport_; + const rtc::scoped_refptr rtcp_ice_transport_; // To avoid downcasting and make it type safe, keep three unique pointers for // different SRTP mode and only one of these is non-nullptr. diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index d83b16e9b3..41907c8f93 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -14,6 +14,7 @@ #include #include "absl/algorithm/container.h" +#include "api/ice_transport_factory.h" #include "api/transport/datagram_transport_interface.h" #include "api/transport/media/media_transport_interface.h" #include "p2p/base/ice_transport_internal.h" @@ -90,6 +91,7 @@ JsepTransportController::JsepTransportController( // The |transport_observer| is assumed to be non-null. RTC_DCHECK(config_.transport_observer); RTC_DCHECK(config_.rtcp_handler); + RTC_DCHECK(config_.ice_transport_factory); } JsepTransportController::~JsepTransportController() { @@ -481,20 +483,18 @@ void JsepTransportController::RollbackTransportForMid(const std::string& mid) { MaybeDestroyJsepTransport(mid); } -std::unique_ptr -JsepTransportController::CreateIceTransport(const std::string transport_name, +rtc::scoped_refptr +JsepTransportController::CreateIceTransport(const std::string& transport_name, bool rtcp) { int component = rtcp ? cricket::ICE_CANDIDATE_COMPONENT_RTCP : cricket::ICE_CANDIDATE_COMPONENT_RTP; - if (config_.external_transport_factory) { - return config_.external_transport_factory->CreateIceTransport( - transport_name, component); - } else { - return std::make_unique( - transport_name, component, port_allocator_, async_resolver_factory_, - config_.event_log); - } + IceTransportInit init; + init.set_port_allocator(port_allocator_); + init.set_async_resolver_factory(async_resolver_factory_); + init.set_event_log(config_.event_log); + return config_.ice_transport_factory->CreateIceTransport( + transport_name, component, std::move(init)); } std::unique_ptr @@ -517,8 +517,8 @@ JsepTransportController::CreateDtlsTransport( // Otherwise, DTLS is still created. dtls = std::make_unique(ice, config_.crypto_options); - } else if (config_.external_transport_factory) { - dtls = config_.external_transport_factory->CreateDtlsTransport( + } else if (config_.dtls_transport_factory) { + dtls = config_.dtls_transport_factory->CreateDtlsTransport( ice, config_.crypto_options); } else { dtls = std::make_unique(ice, config_.crypto_options, @@ -1250,24 +1250,25 @@ RTCError JsepTransportController::MaybeCreateJsepTransport( "SDES and DTLS-SRTP cannot be enabled at the same time."); } - std::unique_ptr ice = + rtc::scoped_refptr ice = CreateIceTransport(content_info.name, /*rtcp=*/false); + RTC_DCHECK(ice); std::unique_ptr media_transport = MaybeCreateMediaTransport(content_info, description, local); if (media_transport) { media_transport_created_once_ = true; - media_transport->Connect(ice.get()); + media_transport->Connect(ice->internal()); } std::unique_ptr datagram_transport = MaybeCreateDatagramTransport(content_info, description, local); if (datagram_transport) { - datagram_transport->Connect(ice.get()); + datagram_transport->Connect(ice->internal()); } std::unique_ptr rtp_dtls_transport = - CreateDtlsTransport(content_info, ice.get(), nullptr); + CreateDtlsTransport(content_info, ice->internal(), nullptr); std::unique_ptr rtcp_dtls_transport; std::unique_ptr unencrypted_rtp_transport; @@ -1275,15 +1276,16 @@ RTCError JsepTransportController::MaybeCreateJsepTransport( std::unique_ptr dtls_srtp_transport; std::unique_ptr datagram_rtp_transport; - std::unique_ptr rtcp_ice; + rtc::scoped_refptr rtcp_ice; if (config_.rtcp_mux_policy != PeerConnectionInterface::kRtcpMuxPolicyRequire && content_info.type == cricket::MediaProtocolType::kRtp) { RTC_DCHECK(media_transport == nullptr); RTC_DCHECK(datagram_transport == nullptr); rtcp_ice = CreateIceTransport(content_info.name, /*rtcp=*/true); - rtcp_dtls_transport = CreateDtlsTransport(content_info, rtcp_ice.get(), - /*datagram_transport=*/nullptr); + rtcp_dtls_transport = + CreateDtlsTransport(content_info, rtcp_ice->internal(), + /*datagram_transport=*/nullptr); } // Only create a datagram RTP transport if the datagram transport should be @@ -1300,8 +1302,8 @@ RTCError JsepTransportController::MaybeCreateJsepTransport( "transport is used."; RTC_DCHECK(!rtcp_dtls_transport); datagram_rtp_transport = std::make_unique( - content_info.media_description()->rtp_header_extensions(), ice.get(), - datagram_transport.get()); + content_info.media_description()->rtp_header_extensions(), + ice->internal(), datagram_transport.get()); } if (config_.disable_encryption) { diff --git a/pc/jsep_transport_controller.h b/pc/jsep_transport_controller.h index bcaeed539f..b07783c894 100644 --- a/pc/jsep_transport_controller.h +++ b/pc/jsep_transport_controller.h @@ -19,14 +19,15 @@ #include "api/candidate.h" #include "api/crypto/crypto_options.h" +#include "api/ice_transport_factory.h" #include "api/peer_connection_interface.h" #include "api/rtc_event_log/rtc_event_log.h" #include "api/transport/media/media_transport_config.h" #include "api/transport/media/media_transport_interface.h" #include "media/sctp/sctp_transport_internal.h" #include "p2p/base/dtls_transport.h" +#include "p2p/base/dtls_transport_factory.h" #include "p2p/base/p2p_transport_channel.h" -#include "p2p/base/transport_factory_interface.h" #include "pc/channel.h" #include "pc/dtls_srtp_transport.h" #include "pc/dtls_transport.h" @@ -91,7 +92,8 @@ class JsepTransportController : public sigslot::has_slots<> { bool disable_encryption = false; bool enable_external_auth = false; // Used to inject the ICE/DTLS transports created externally. - cricket::TransportFactoryInterface* external_transport_factory = nullptr; + webrtc::IceTransportFactory* ice_transport_factory = nullptr; + cricket::DtlsTransportFactory* dtls_transport_factory = nullptr; Observer* transport_observer = nullptr; // Must be provided and valid for the lifetime of the // JsepTransportController instance. @@ -404,8 +406,8 @@ class JsepTransportController : public sigslot::has_slots<> { const cricket::ContentInfo& content_info, cricket::IceTransportInternal* ice, DatagramTransportInterface* datagram_transport); - std::unique_ptr CreateIceTransport( - const std::string transport_name, + rtc::scoped_refptr CreateIceTransport( + const std::string& transport_name, bool rtcp); std::unique_ptr CreateUnencryptedRtpTransport( diff --git a/pc/jsep_transport_controller_unittest.cc b/pc/jsep_transport_controller_unittest.cc index 408cb019b8..b96a999375 100644 --- a/pc/jsep_transport_controller_unittest.cc +++ b/pc/jsep_transport_controller_unittest.cc @@ -16,10 +16,10 @@ #include "api/test/fake_media_transport.h" #include "api/test/loopback_media_transport.h" #include "api/transport/media/media_transport_interface.h" +#include "p2p/base/dtls_transport_factory.h" #include "p2p/base/fake_dtls_transport.h" #include "p2p/base/fake_ice_transport.h" #include "p2p/base/no_op_dtls_transport.h" -#include "p2p/base/transport_factory_interface.h" #include "p2p/base/transport_info.h" #include "rtc_base/gunit.h" #include "rtc_base/thread.h" @@ -59,15 +59,20 @@ void AddCryptoSettings(cricket::SessionDescription* description) { } // namespace -class FakeTransportFactory : public cricket::TransportFactoryInterface { +class FakeIceTransportFactory : public webrtc::IceTransportFactory { public: - std::unique_ptr CreateIceTransport( + ~FakeIceTransportFactory() override = default; + rtc::scoped_refptr CreateIceTransport( const std::string& transport_name, - int component) override { - return std::make_unique(transport_name, - component); + int component, + IceTransportInit init) override { + return new rtc::RefCountedObject( + std::make_unique(transport_name, component)); } +}; +class FakeDtlsTransportFactory : public cricket::DtlsTransportFactory { + public: std::unique_ptr CreateDtlsTransport( cricket::IceTransportInternal* ice, const webrtc::CryptoOptions& crypto_options) override { @@ -81,7 +86,8 @@ class JsepTransportControllerTest : public JsepTransportController::Observer, public sigslot::has_slots<> { public: JsepTransportControllerTest() : signaling_thread_(rtc::Thread::Current()) { - fake_transport_factory_ = std::make_unique(); + fake_ice_transport_factory_ = std::make_unique(); + fake_dtls_transport_factory_ = std::make_unique(); } void CreateJsepTransportController( @@ -92,8 +98,8 @@ class JsepTransportControllerTest : public JsepTransportController::Observer, config.transport_observer = this; config.rtcp_handler = [](const rtc::CopyOnWriteBuffer& packet, int64_t packet_time_us) { RTC_NOTREACHED(); }; - // The tests only works with |fake_transport_factory|; - config.external_transport_factory = fake_transport_factory_.get(); + config.ice_transport_factory = fake_ice_transport_factory_.get(); + config.dtls_transport_factory = fake_dtls_transport_factory_.get(); // TODO(zstein): Provide an AsyncResolverFactory once it is required. transport_controller_ = std::make_unique( signaling_thread, network_thread, port_allocator, nullptr, config); @@ -358,7 +364,8 @@ class JsepTransportControllerTest : public JsepTransportController::Observer, // |network_thread_| should be destroyed after |transport_controller_| std::unique_ptr network_thread_; - std::unique_ptr fake_transport_factory_; + std::unique_ptr fake_ice_transport_factory_; + std::unique_ptr fake_dtls_transport_factory_; rtc::Thread* const signaling_thread_ = nullptr; bool signaled_on_non_signaling_thread_ = false; // Used to verify the SignalRtpTransportChanged/SignalDtlsTransportChanged are diff --git a/pc/jsep_transport_unittest.cc b/pc/jsep_transport_unittest.cc index 00f58f64a0..87d6e87212 100644 --- a/pc/jsep_transport_unittest.cc +++ b/pc/jsep_transport_unittest.cc @@ -14,12 +14,14 @@ #include #include +#include "api/ice_transport_factory.h" #include "media/base/fake_rtp.h" #include "p2p/base/fake_dtls_transport.h" #include "p2p/base/fake_ice_transport.h" #include "rtc_base/gunit.h" namespace cricket { +namespace { using webrtc::SdpType; static const char kIceUfrag1[] = "U001"; @@ -40,6 +42,16 @@ struct NegotiateRoleParams { SdpType remote_type; }; +rtc::scoped_refptr CreateIceTransport( + std::unique_ptr internal) { + if (!internal) { + return nullptr; + } + + return new rtc::RefCountedObject( + std::move(internal)); +} + class JsepTransport2Test : public ::testing::Test, public sigslot::has_slots<> { protected: std::unique_ptr CreateSdesTransport( @@ -69,17 +81,21 @@ class JsepTransport2Test : public ::testing::Test, public sigslot::has_slots<> { // FakeIceTransport. std::unique_ptr CreateJsepTransport2(bool rtcp_mux_enabled, SrtpMode srtp_mode) { - auto ice = std::make_unique(kTransportName, - ICE_CANDIDATE_COMPONENT_RTP); - auto rtp_dtls_transport = std::make_unique(ice.get()); + auto ice_internal = std::make_unique( + kTransportName, ICE_CANDIDATE_COMPONENT_RTP); + auto rtp_dtls_transport = + std::make_unique(ice_internal.get()); + auto ice = CreateIceTransport(std::move(ice_internal)); - std::unique_ptr rtcp_ice; + std::unique_ptr rtcp_ice_internal; std::unique_ptr rtcp_dtls_transport; if (!rtcp_mux_enabled) { - rtcp_ice = std::make_unique( + rtcp_ice_internal = std::make_unique( kTransportName, ICE_CANDIDATE_COMPONENT_RTCP); - rtcp_dtls_transport = std::make_unique(rtcp_ice.get()); + rtcp_dtls_transport = + std::make_unique(rtcp_ice_internal.get()); } + auto rtcp_ice = CreateIceTransport(std::move(rtcp_ice_internal)); std::unique_ptr unencrypted_rtp_transport; std::unique_ptr sdes_transport; @@ -1246,5 +1262,5 @@ INSTANTIATE_TEST_SUITE_P( std::make_tuple(Scenario::kDtlsBeforeCallerSendOffer, false), std::make_tuple(Scenario::kDtlsBeforeCallerSetAnswer, false), std::make_tuple(Scenario::kDtlsAfterCallerSetAnswer, false))); - +} // namespace } // namespace cricket diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index ea3d8e2651..1d7b4ea667 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -1139,6 +1139,7 @@ bool PeerConnection::Initialize( observer_ = dependencies.observer; async_resolver_factory_ = std::move(dependencies.async_resolver_factory); port_allocator_ = std::move(dependencies.allocator); + ice_transport_factory_ = std::move(dependencies.ice_transport_factory); tls_cert_verifier_ = std::move(dependencies.tls_cert_verifier); cricket::ServerAddresses stun_servers; @@ -1334,6 +1335,8 @@ bool PeerConnection::Initialize( } } + config.ice_transport_factory = ice_transport_factory_.get(); + transport_controller_.reset(new JsepTransportController( signaling_thread(), network_thread(), port_allocator_.get(), async_resolver_factory_.get(), config)); diff --git a/pc/peer_connection.h b/pc/peer_connection.h index 302ff3bfd4..cbff7e7050 100644 --- a/pc/peer_connection.h +++ b/pc/peer_connection.h @@ -1295,6 +1295,12 @@ class PeerConnection : public PeerConnectionInternal, std::unique_ptr port_allocator_; // TODO(bugs.webrtc.org/9987): Accessed on both // signaling and network thread. + std::unique_ptr + ice_transport_factory_; // TODO(bugs.webrtc.org/9987): Accessed on the + // signaling thread but the underlying raw + // pointer is given to + // |jsep_transport_controller_| and used on the + // network thread. std::unique_ptr tls_cert_verifier_; // TODO(bugs.webrtc.org/9987): Accessed on both // signaling and network thread. diff --git a/pc/peer_connection_factory.cc b/pc/peer_connection_factory.cc index a1a9f04eb0..8909ba9bd3 100644 --- a/pc/peer_connection_factory.cc +++ b/pc/peer_connection_factory.cc @@ -28,6 +28,7 @@ #include "media/base/rtp_data_engine.h" #include "media/sctp/sctp_transport.h" #include "p2p/base/basic_packet_socket_factory.h" +#include "p2p/base/default_ice_transport_factory.h" #include "p2p/client/basic_port_allocator.h" #include "pc/audio_track.h" #include "pc/local_audio_source.h" @@ -267,6 +268,11 @@ PeerConnectionFactory::CreatePeerConnection( }); } + if (!dependencies.ice_transport_factory) { + dependencies.ice_transport_factory = + std::make_unique(); + } + // TODO(zstein): Once chromium injects its own AsyncResolverFactory, set // |dependencies.async_resolver_factory| to a new // |rtc::BasicAsyncResolverFactory| if no factory is provided. diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc index 536ad01ebd..e59ce9a2a8 100644 --- a/pc/peer_connection_integrationtest.cc +++ b/pc/peer_connection_integrationtest.cc @@ -36,6 +36,7 @@ #include "media/engine/fake_webrtc_video_engine.h" #include "media/engine/webrtc_media_engine.h" #include "media/engine/webrtc_media_engine_defaults.h" +#include "p2p/base/fake_ice_transport.h" #include "p2p/base/mock_async_resolver.h" #include "p2p/base/p2p_constants.h" #include "p2p/base/port_interface.h" @@ -1166,6 +1167,34 @@ class MediaExpectations { int callee_video_frames_expected_ = 0; }; +class MockIceTransport : public webrtc::IceTransportInterface { + public: + MockIceTransport(const std::string& name, int component) + : internal_(std::make_unique( + name, + component, + nullptr /* network_thread */)) {} + ~MockIceTransport() = default; + cricket::IceTransportInternal* internal() { return internal_.get(); } + + private: + std::unique_ptr internal_; +}; + +class MockIceTransportFactory : public IceTransportFactory { + public: + ~MockIceTransportFactory() override = default; + rtc::scoped_refptr CreateIceTransport( + const std::string& transport_name, + int component, + IceTransportInit init) { + RecordIceTransportCreated(); + return new rtc::RefCountedObject(transport_name, + component); + } + MOCK_METHOD0(RecordIceTransportCreated, void()); +}; + // Tests two PeerConnections connecting to each other end-to-end, using a // virtual network, fake A/V capture and fake encoder/decoders. The // PeerConnections share the threads/socket servers, but use separate versions @@ -5529,6 +5558,25 @@ TEST_P(PeerConnectionIntegrationTest, EXPECT_GT(client_2_cert_verifier->call_count_, 0u); } +// Test that the injected ICE transport factory is used to create ICE transports +// for WebRTC connections. +TEST_P(PeerConnectionIntegrationTest, IceTransportFactoryUsedForConnections) { + PeerConnectionInterface::RTCConfiguration default_config; + PeerConnectionDependencies dependencies(nullptr); + auto ice_transport_factory = std::make_unique(); + EXPECT_CALL(*ice_transport_factory, RecordIceTransportCreated()).Times(1); + dependencies.ice_transport_factory = std::move(ice_transport_factory); + auto wrapper = + CreatePeerConnectionWrapper("Caller", nullptr, &default_config, + std::move(dependencies), nullptr, nullptr); + ASSERT_TRUE(wrapper); + wrapper->CreateDataChannel(); + rtc::scoped_refptr observer( + new rtc::RefCountedObject()); + wrapper->pc()->SetLocalDescription(observer, + wrapper->CreateOfferAndWait().release()); +} + // Test that audio and video flow end-to-end when codec names don't use the // expected casing, given that they're supposed to be case insensitive. To test // this, all but one codec is removed from each media description, and its