diff --git a/api/BUILD.gn b/api/BUILD.gn index 50a9c5a242..db0723dec8 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn @@ -185,7 +185,6 @@ rtc_library("libjingle_peerconnection_api") { "transport:bitrate_settings", "transport:enums", "transport:network_control", - "transport:sctp_transport_factory_interface", "transport:webrtc_key_value_config", "transport/rtp:rtp_source", "units:data_rate", diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h index 6cf6dbb299..3ada740f18 100644 --- a/api/peer_connection_interface.h +++ b/api/peer_connection_interface.h @@ -105,7 +105,6 @@ #include "api/transport/bitrate_settings.h" #include "api/transport/enums.h" #include "api/transport/network_control.h" -#include "api/transport/sctp_transport_factory_interface.h" #include "api/transport/webrtc_key_value_config.h" #include "api/turn_customizer.h" #include "media/base/media_config.h" @@ -1364,7 +1363,6 @@ struct RTC_EXPORT PeerConnectionFactoryDependencies final { // used. std::unique_ptr network_monitor_factory; std::unique_ptr neteq_factory; - std::unique_ptr sctp_factory; std::unique_ptr trials; }; diff --git a/api/transport/BUILD.gn b/api/transport/BUILD.gn index d2da4453bf..a4ada07108 100644 --- a/api/transport/BUILD.gn +++ b/api/transport/BUILD.gn @@ -93,11 +93,6 @@ rtc_library("goog_cc") { ] } -rtc_source_set("sctp_transport_factory_interface") { - visibility = [ "*" ] - sources = [ "sctp_transport_factory_interface.h" ] -} - rtc_source_set("stun_types") { visibility = [ "*" ] sources = [ diff --git a/api/transport/sctp_transport_factory_interface.h b/api/transport/sctp_transport_factory_interface.h deleted file mode 100644 index 912be3a374..0000000000 --- a/api/transport/sctp_transport_factory_interface.h +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Copyright 2020 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 API_TRANSPORT_SCTP_TRANSPORT_FACTORY_INTERFACE_H_ -#define API_TRANSPORT_SCTP_TRANSPORT_FACTORY_INTERFACE_H_ - -#include - -// These classes are not part of the API, and are treated as opaque pointers. -namespace cricket { -class SctpTransportInternal; -} // namespace cricket - -namespace rtc { -class PacketTransportInternal; -} // namespace rtc - -namespace webrtc { - -// Factory class which can be used to allow fake SctpTransports to be injected -// for testing. An application is not intended to implement this interface nor -// 'cricket::SctpTransportInternal' because SctpTransportInternal is not -// guaranteed to remain stable in future WebRTC versions. -class SctpTransportFactoryInterface { - public: - virtual ~SctpTransportFactoryInterface() = default; - - // Create an SCTP transport using |channel| for the underlying transport. - virtual std::unique_ptr CreateSctpTransport( - rtc::PacketTransportInternal* channel) = 0; -}; - -} // namespace webrtc - -#endif // API_TRANSPORT_SCTP_TRANSPORT_FACTORY_INTERFACE_H_ diff --git a/media/BUILD.gn b/media/BUILD.gn index 284cd45714..238b1d2f01 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -426,10 +426,7 @@ rtc_library("rtc_data") { } if (rtc_enable_sctp && rtc_build_usrsctp) { - deps += [ - "../api/transport:sctp_transport_factory_interface", - "//third_party/usrsctp", - ] + deps += [ "//third_party/usrsctp" ] } } diff --git a/media/sctp/sctp_transport.h b/media/sctp/sctp_transport.h index 54542af6b3..38029ffeb3 100644 --- a/media/sctp/sctp_transport.h +++ b/media/sctp/sctp_transport.h @@ -21,7 +21,6 @@ #include #include "absl/types/optional.h" -#include "api/transport/sctp_transport_factory_interface.h" #include "rtc_base/async_invoker.h" #include "rtc_base/buffer.h" #include "rtc_base/constructor_magic.h" @@ -284,7 +283,7 @@ class SctpTransport : public SctpTransportInternal, RTC_DISALLOW_COPY_AND_ASSIGN(SctpTransport); }; -class SctpTransportFactory : public webrtc::SctpTransportFactoryInterface { +class SctpTransportFactory : public SctpTransportInternalFactory { public: explicit SctpTransportFactory(rtc::Thread* network_thread) : network_thread_(network_thread) {} diff --git a/media/sctp/sctp_transport_internal.h b/media/sctp/sctp_transport_internal.h index dc8ac4558d..b0e0e0f7e6 100644 --- a/media/sctp/sctp_transport_internal.h +++ b/media/sctp/sctp_transport_internal.h @@ -142,6 +142,18 @@ class SctpTransportInternal { virtual void set_debug_name_for_testing(const char* debug_name) = 0; }; +// Factory class which can be used to allow fake SctpTransports to be injected +// for testing. Or, theoretically, SctpTransportInternal implementations that +// use something other than usrsctp. +class SctpTransportInternalFactory { + public: + virtual ~SctpTransportInternalFactory() {} + + // Create an SCTP transport using |channel| for the underlying transport. + virtual std::unique_ptr CreateSctpTransport( + rtc::PacketTransportInternal* channel) = 0; +}; + } // namespace cricket #endif // MEDIA_SCTP_SCTP_TRANSPORT_INTERNAL_H_ diff --git a/pc/BUILD.gn b/pc/BUILD.gn index 712449f1c2..9d04b4c787 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -446,6 +446,7 @@ if (rtc_include_tests) { "test/fake_periodic_video_source.h", "test/fake_periodic_video_track_source.h", "test/fake_rtc_certificate_generator.h", + "test/fake_sctp_transport.h", "test/fake_video_track_renderer.h", "test/fake_video_track_source.h", "test/frame_generator_capturer_video_track_source.h", @@ -607,7 +608,6 @@ if (rtc_include_tests) { "../test:field_trial", "../test:fileutils", "../test:rtp_test_utils", - "../test/pc/sctp:fake_sctp_transport", "./scenario_tests:pc_scenario_tests", "//third_party/abseil-cpp/absl/algorithm:container", "//third_party/abseil-cpp/absl/memory", diff --git a/pc/jsep_transport_controller.h b/pc/jsep_transport_controller.h index 190f740c7c..d95b475969 100644 --- a/pc/jsep_transport_controller.h +++ b/pc/jsep_transport_controller.h @@ -101,7 +101,7 @@ class JsepTransportController : public sigslot::has_slots<> { RtcEventLog* event_log = nullptr; // Factory for SCTP transports. - SctpTransportFactoryInterface* sctp_factory = nullptr; + cricket::SctpTransportInternalFactory* sctp_factory = nullptr; }; // The ICE related events are signaled on the |signaling_thread|. diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index ea0548d1d9..3665a82e03 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -1073,6 +1073,7 @@ PeerConnection::~PeerConnection() { RTC_LOG(LS_INFO) << "Session: " << session_id() << " is destroyed."; webrtc_session_desc_factory_.reset(); + sctp_factory_.reset(); transport_controller_.reset(); // port_allocator_ lives on the network thread and should be destroyed there. @@ -1261,6 +1262,8 @@ bool PeerConnection::Initialize( } } + sctp_factory_ = factory_->CreateSctpTransportInternalFactory(); + if (configuration.enable_rtp_data_channel) { // Enable creation of RTP data channels if the kEnableRtpDataChannels is // set. It takes precendence over the disable_sctp_data_channels @@ -1270,7 +1273,7 @@ bool PeerConnection::Initialize( // DTLS has to be enabled to use SCTP. if (!options.disable_sctp_data_channels && dtls_enabled_) { data_channel_controller_.set_data_channel_type(cricket::DCT_SCTP); - config.sctp_factory = factory_->sctp_transport_factory(); + config.sctp_factory = sctp_factory_.get(); } } diff --git a/pc/peer_connection.h b/pc/peer_connection.h index ed20f21ef9..d33c658e9d 100644 --- a/pc/peer_connection.h +++ b/pc/peer_connection.h @@ -1274,6 +1274,9 @@ class PeerConnection : public PeerConnectionInternal, std::unique_ptr transport_controller_; // TODO(bugs.webrtc.org/9987): Accessed on both // signaling and network thread. + std::unique_ptr + sctp_factory_; // TODO(bugs.webrtc.org/9987): Accessed on both + // signaling and network thread. // |sctp_mid_| is the content name (MID) in SDP. // Note: this is used as the data channel MID by both SCTP and data channel diff --git a/pc/peer_connection_data_channel_unittest.cc b/pc/peer_connection_data_channel_unittest.cc index 6c51f01594..0a674f462b 100644 --- a/pc/peer_connection_data_channel_unittest.cc +++ b/pc/peer_connection_data_channel_unittest.cc @@ -45,8 +45,8 @@ #ifdef WEBRTC_ANDROID #include "pc/test/android_test_initializer.h" #endif +#include "pc/test/fake_sctp_transport.h" #include "rtc_base/virtual_socket_server.h" -#include "test/pc/sctp/fake_sctp_transport.h" namespace webrtc { @@ -58,20 +58,46 @@ using ::testing::Values; namespace { -PeerConnectionFactoryDependencies CreatePeerConnectionFactoryDependencies() { +PeerConnectionFactoryDependencies CreatePeerConnectionFactoryDependencies( + rtc::Thread* network_thread, + rtc::Thread* worker_thread, + rtc::Thread* signaling_thread, + std::unique_ptr media_engine, + std::unique_ptr call_factory) { PeerConnectionFactoryDependencies deps; - deps.network_thread = rtc::Thread::Current(); - deps.worker_thread = rtc::Thread::Current(); - deps.signaling_thread = rtc::Thread::Current(); + deps.network_thread = network_thread; + deps.worker_thread = worker_thread; + deps.signaling_thread = signaling_thread; deps.task_queue_factory = CreateDefaultTaskQueueFactory(); - deps.media_engine = std::make_unique(); - deps.call_factory = CreateCallFactory(); - deps.sctp_factory = std::make_unique(); + deps.media_engine = std::move(media_engine); + deps.call_factory = std::move(call_factory); return deps; } } // namespace +class PeerConnectionFactoryForDataChannelTest + : public rtc::RefCountedObject { + public: + PeerConnectionFactoryForDataChannelTest() + : rtc::RefCountedObject( + CreatePeerConnectionFactoryDependencies( + rtc::Thread::Current(), + rtc::Thread::Current(), + rtc::Thread::Current(), + std::make_unique(), + CreateCallFactory())) {} + + std::unique_ptr + CreateSctpTransportInternalFactory() { + auto factory = std::make_unique(); + last_fake_sctp_transport_factory_ = factory.get(); + return factory; + } + + FakeSctpTransportFactory* last_fake_sctp_transport_factory_ = nullptr; +}; + class PeerConnectionWrapperForDataChannelTest : public PeerConnectionWrapper { public: using PeerConnectionWrapper::PeerConnectionWrapper; @@ -129,12 +155,10 @@ class PeerConnectionDataChannelBaseTest : public ::testing::Test { WrapperPtr CreatePeerConnection( const RTCConfiguration& config, const PeerConnectionFactoryInterface::Options factory_options) { - auto factory_deps = CreatePeerConnectionFactoryDependencies(); - FakeSctpTransportFactory* fake_sctp_transport_factory = - static_cast(factory_deps.sctp_factory.get()); - rtc::scoped_refptr pc_factory = - CreateModularPeerConnectionFactory(std::move(factory_deps)); + rtc::scoped_refptr pc_factory( + new PeerConnectionFactoryForDataChannelTest()); pc_factory->SetOptions(factory_options); + RTC_CHECK(pc_factory->Initialize()); auto observer = std::make_unique(); RTCConfiguration modified_config = config; modified_config.sdp_semantics = sdp_semantics_; @@ -147,7 +171,9 @@ class PeerConnectionDataChannelBaseTest : public ::testing::Test { observer->SetPeerConnectionInterface(pc.get()); auto wrapper = std::make_unique( pc_factory, pc, std::move(observer)); - wrapper->set_sctp_transport_factory(fake_sctp_transport_factory); + RTC_DCHECK(pc_factory->last_fake_sctp_transport_factory_); + wrapper->set_sctp_transport_factory( + pc_factory->last_fake_sctp_transport_factory_); return wrapper; } diff --git a/pc/peer_connection_factory.cc b/pc/peer_connection_factory.cc index 8c5e2723e2..d79e438152 100644 --- a/pc/peer_connection_factory.cc +++ b/pc/peer_connection_factory.cc @@ -67,18 +67,6 @@ CreateModularPeerConnectionFactory( pc_factory); } -std::unique_ptr MaybeCreateSctpFactory( - PeerConnectionFactoryDependencies* dependencies) { - if (dependencies->sctp_factory) { - return std::move(dependencies->sctp_factory); - } -#ifdef HAVE_SCTP - return std::make_unique( - dependencies->network_thread); -#endif - return nullptr; -} - PeerConnectionFactory::PeerConnectionFactory( PeerConnectionFactoryDependencies dependencies) : wraps_current_thread_(false), @@ -96,7 +84,6 @@ PeerConnectionFactory::PeerConnectionFactory( injected_network_controller_factory_( std::move(dependencies.network_controller_factory)), neteq_factory_(std::move(dependencies.neteq_factory)), - sctp_factory_(MaybeCreateSctpFactory(&dependencies)), trials_(dependencies.trials ? std::move(dependencies.trials) : std::make_unique()) { if (!network_thread_) { @@ -339,6 +326,15 @@ rtc::scoped_refptr PeerConnectionFactory::CreateAudioTrack( return AudioTrackProxy::Create(signaling_thread_, track); } +std::unique_ptr +PeerConnectionFactory::CreateSctpTransportInternalFactory() { +#ifdef HAVE_SCTP + return std::make_unique(network_thread()); +#else + return nullptr; +#endif +} + cricket::ChannelManager* PeerConnectionFactory::channel_manager() { return channel_manager_.get(); } diff --git a/pc/peer_connection_factory.h b/pc/peer_connection_factory.h index 6965a4968d..3932562d22 100644 --- a/pc/peer_connection_factory.h +++ b/pc/peer_connection_factory.h @@ -71,9 +71,8 @@ class PeerConnectionFactory : public PeerConnectionFactoryInterface { bool StartAecDump(FILE* file, int64_t max_size_bytes) override; void StopAecDump() override; - SctpTransportFactoryInterface* sctp_transport_factory() { - return sctp_factory_.get(); - } + virtual std::unique_ptr + CreateSctpTransportInternalFactory(); virtual cricket::ChannelManager* channel_manager(); @@ -126,7 +125,6 @@ class PeerConnectionFactory : public PeerConnectionFactoryInterface { std::unique_ptr injected_network_controller_factory_; std::unique_ptr neteq_factory_; - const std::unique_ptr sctp_factory_; const std::unique_ptr trials_; }; diff --git a/pc/peer_connection_jsep_unittest.cc b/pc/peer_connection_jsep_unittest.cc index 4d9884d615..b7c07598cf 100644 --- a/pc/peer_connection_jsep_unittest.cc +++ b/pc/peer_connection_jsep_unittest.cc @@ -21,10 +21,10 @@ #include "pc/test/android_test_initializer.h" #endif #include "pc/test/fake_audio_capture_module.h" +#include "pc/test/fake_sctp_transport.h" #include "rtc_base/gunit.h" #include "rtc_base/virtual_socket_server.h" #include "test/gmock.h" -#include "test/pc/sctp/fake_sctp_transport.h" // This file contains tests that ensure the PeerConnection's implementation of // CreateOffer/CreateAnswer/SetLocalDescription/SetRemoteDescription conform @@ -41,21 +41,30 @@ using ::testing::ElementsAre; using ::testing::UnorderedElementsAre; using ::testing::Values; -PeerConnectionFactoryDependencies CreatePeerConnectionFactoryDependencies() { - PeerConnectionFactoryDependencies dependencies; - dependencies.worker_thread = rtc::Thread::Current(); - dependencies.network_thread = rtc::Thread::Current(); - dependencies.signaling_thread = rtc::Thread::Current(); - dependencies.task_queue_factory = CreateDefaultTaskQueueFactory(); - cricket::MediaEngineDependencies media_deps; - media_deps.task_queue_factory = dependencies.task_queue_factory.get(); - media_deps.adm = FakeAudioCaptureModule::Create(); - SetMediaEngineDefaults(&media_deps); - dependencies.media_engine = cricket::CreateMediaEngine(std::move(media_deps)); - dependencies.call_factory = CreateCallFactory(); - dependencies.sctp_factory = std::make_unique(); - return dependencies; -} +class PeerConnectionFactoryForJsepTest : public PeerConnectionFactory { + public: + PeerConnectionFactoryForJsepTest() + : PeerConnectionFactory([] { + PeerConnectionFactoryDependencies dependencies; + dependencies.worker_thread = rtc::Thread::Current(); + dependencies.network_thread = rtc::Thread::Current(); + dependencies.signaling_thread = rtc::Thread::Current(); + dependencies.task_queue_factory = CreateDefaultTaskQueueFactory(); + cricket::MediaEngineDependencies media_deps; + media_deps.task_queue_factory = dependencies.task_queue_factory.get(); + media_deps.adm = FakeAudioCaptureModule::Create(); + SetMediaEngineDefaults(&media_deps); + dependencies.media_engine = + cricket::CreateMediaEngine(std::move(media_deps)); + dependencies.call_factory = CreateCallFactory(); + return dependencies; + }()) {} + + std::unique_ptr + CreateSctpTransportInternalFactory() { + return std::make_unique(); + } +}; class PeerConnectionJsepTest : public ::testing::Test { protected: @@ -75,9 +84,9 @@ class PeerConnectionJsepTest : public ::testing::Test { } WrapperPtr CreatePeerConnection(const RTCConfiguration& config) { - rtc::scoped_refptr pc_factory = - CreateModularPeerConnectionFactory( - CreatePeerConnectionFactoryDependencies()); + rtc::scoped_refptr pc_factory( + new rtc::RefCountedObject()); + RTC_CHECK(pc_factory->Initialize()); auto observer = std::make_unique(); auto pc = pc_factory->CreatePeerConnection(config, nullptr, nullptr, observer.get()); diff --git a/test/pc/sctp/fake_sctp_transport.h b/pc/test/fake_sctp_transport.h similarity index 91% rename from test/pc/sctp/fake_sctp_transport.h rename to pc/test/fake_sctp_transport.h index 5fdb3bbe42..50e59f1fc2 100644 --- a/test/pc/sctp/fake_sctp_transport.h +++ b/pc/test/fake_sctp_transport.h @@ -8,8 +8,8 @@ * be found in the AUTHORS file in the root of the source tree. */ -#ifndef TEST_PC_SCTP_FAKE_SCTP_TRANSPORT_H_ -#define TEST_PC_SCTP_FAKE_SCTP_TRANSPORT_H_ +#ifndef PC_TEST_FAKE_SCTP_TRANSPORT_H_ +#define PC_TEST_FAKE_SCTP_TRANSPORT_H_ #include @@ -49,7 +49,7 @@ class FakeSctpTransport : public cricket::SctpTransportInternal { int max_message_size_; }; -class FakeSctpTransportFactory : public webrtc::SctpTransportFactoryInterface { +class FakeSctpTransportFactory : public cricket::SctpTransportInternalFactory { public: std::unique_ptr CreateSctpTransport( rtc::PacketTransportInternal*) override { @@ -66,4 +66,4 @@ class FakeSctpTransportFactory : public webrtc::SctpTransportFactoryInterface { FakeSctpTransport* last_fake_sctp_transport_ = nullptr; }; -#endif // TEST_PC_SCTP_FAKE_SCTP_TRANSPORT_H_ +#endif // PC_TEST_FAKE_SCTP_TRANSPORT_H_ diff --git a/test/DEPS b/test/DEPS index 2cbb1d2dc3..170c4086d7 100644 --- a/test/DEPS +++ b/test/DEPS @@ -6,7 +6,6 @@ include_rules = [ "+common_video", "+logging/rtc_event_log", "+media/base", - "+media/sctp", "+media/engine", "+modules/audio_coding", "+modules/congestion_controller", diff --git a/test/pc/sctp/BUILD.gn b/test/pc/sctp/BUILD.gn deleted file mode 100644 index 93ae1bf59c..0000000000 --- a/test/pc/sctp/BUILD.gn +++ /dev/null @@ -1,15 +0,0 @@ -# Copyright (c) 2020 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. - -import("../../../webrtc.gni") - -rtc_source_set("fake_sctp_transport") { - visibility = [ "*" ] - sources = [ "fake_sctp_transport.h" ] - deps = [ "../../../media:rtc_data" ] -}