From 34c15bc5110c1b20f30ff1e4eefbf58a71f8c2b0 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Fri, 31 Jan 2025 16:12:12 +0100 Subject: [PATCH] Restructure PeerConnectionBundleTest helper not to create PortAllocator Instead rely on PeerConnectionFactory to create it. Bug: webrtc:42232556 Change-Id: If3de8a2e311fcdca4371cca03d10bd383fbd3e01 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/375922 Reviewed-by: Harald Alvestrand Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#43835} --- pc/BUILD.gn | 85 +++++++++++-------------- pc/peer_connection_bundle_unittest.cc | 92 ++++++++++++--------------- 2 files changed, 78 insertions(+), 99 deletions(-) diff --git a/pc/BUILD.gn b/pc/BUILD.gn index fb07fae859..d46ff16d72 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -2349,7 +2349,6 @@ if (rtc_include_tests && !build_with_chromium) { ":audio_track", ":channel", ":channel_interface", - ":data_channel_controller_unittest", ":dtls_srtp_transport", ":dtls_transport", ":dtmf_sender", @@ -2362,6 +2361,7 @@ if (rtc_include_tests && !build_with_chromium) { ":media_protocol_names", ":media_session", ":media_stream", + ":pc_test_utils", ":peer_connection", ":peer_connection_factory", ":peer_connection_internal", @@ -2397,7 +2397,6 @@ if (rtc_include_tests && !build_with_chromium) { "../api:candidate", "../api:create_peerconnection_factory", "../api:dtls_transport_interface", - "../api:dtmf_sender_interface", "../api:enable_media", "../api:enable_media_with_defaults", "../api:fake_frame_decryptor", @@ -2419,6 +2418,8 @@ if (rtc_include_tests && !build_with_chromium) { "../api:ref_count", "../api:rtc_error", "../api:rtc_error_matchers", + "../api:rtc_stats_api", + "../api:rtp_parameters", "../api:rtp_sender_interface", "../api:rtp_transceiver_direction", "../api:scoped_refptr", @@ -2428,6 +2429,13 @@ if (rtc_include_tests && !build_with_chromium) { "../api/audio:audio_mixer_api", "../api/audio:audio_processing", "../api/audio:audio_processing_statistics", + "../api/audio_codecs:audio_codecs_api", + "../api/audio_codecs:builtin_audio_decoder_factory", + "../api/audio_codecs:builtin_audio_encoder_factory", + "../api/audio_codecs:opus_audio_decoder_factory", + "../api/audio_codecs:opus_audio_encoder_factory", + "../api/audio_codecs/L16:audio_decoder_L16", + "../api/audio_codecs/L16:audio_encoder_L16", "../api/crypto:frame_decryptor_interface", "../api/crypto:frame_encryptor_interface", "../api/crypto:options", @@ -2450,12 +2458,25 @@ if (rtc_include_tests && !build_with_chromium) { "../api/video:builtin_video_bitrate_allocator_factory", "../api/video:encoded_image", "../api/video:recordable_encoded_frame", - "../api/video:resolution", "../api/video:video_bitrate_allocator_factory", "../api/video:video_codec_constants", "../api/video:video_frame", "../api/video:video_rtp_headers", + "../api/video_codecs:builtin_video_decoder_factory", + "../api/video_codecs:builtin_video_encoder_factory", "../api/video_codecs:scalability_mode", + "../api/video_codecs:video_codecs_api", + "../api/video_codecs:video_decoder_factory_template", + "../api/video_codecs:video_decoder_factory_template_dav1d_adapter", + "../api/video_codecs:video_decoder_factory_template_libvpx_vp8_adapter", + "../api/video_codecs:video_decoder_factory_template_libvpx_vp9_adapter", + "../api/video_codecs:video_decoder_factory_template_open_h264_adapter", + "../api/video_codecs:video_encoder_factory_template", + "../api/video_codecs:video_encoder_factory_template_libaom_av1_adapter", + "../api/video_codecs:video_encoder_factory_template_libvpx_vp8_adapter", + "../api/video_codecs:video_encoder_factory_template_libvpx_vp9_adapter", + "../api/video_codecs:video_encoder_factory_template_open_h264_adapter", + "../call:call_interfaces", "../call/adaptation:resource_adaptation_test_utilities", "../common_video", "../logging:fake_rtc_event_log", @@ -2464,20 +2485,21 @@ if (rtc_include_tests && !build_with_chromium) { "../media:media_constants", "../media:media_engine", "../media:rid_description", + "../media:rtc_audio_video", "../media:rtc_data_sctp_transport_internal", "../media:rtc_media_config", + "../media:rtc_media_tests_utils", "../media:stream_params", "../modules/audio_processing:mocks", "../modules/rtp_rtcp:rtp_rtcp_format", "../p2p:basic_packet_socket_factory", "../p2p:basic_port_allocator", - "../p2p:connection", "../p2p:connection_info", "../p2p:dtls_transport_internal", "../p2p:fake_port_allocator", "../p2p:ice_transport_internal", "../p2p:p2p_constants", - "../p2p:p2p_server_utils", + "../p2p:p2p_test_utils", "../p2p:port", "../p2p:port_allocator", "../p2p:port_interface", @@ -2493,7 +2515,6 @@ if (rtc_include_tests && !build_with_chromium) { "../rtc_base:ip_address", "../rtc_base:logging", "../rtc_base:macromagic", - "../rtc_base:mdns_responder_interface", "../rtc_base:net_helper", "../rtc_base:network", "../rtc_base:network_constants", @@ -2503,6 +2524,7 @@ if (rtc_include_tests && !build_with_chromium) { "../rtc_base:rtc_base_tests_utils", "../rtc_base:rtc_certificate_generator", "../rtc_base:rtc_json", + "../rtc_base:safe_conversions", "../rtc_base:socket_address", "../rtc_base:socket_server", "../rtc_base:ssl", @@ -2517,16 +2539,23 @@ if (rtc_include_tests && !build_with_chromium) { "../rtc_base/third_party/base64", "../rtc_base/third_party/sigslot", "../system_wrappers:metrics", + "../test:audio_codec_mocks", "../test:rtc_expect_death", "../test:run_loop", - "../test:scoped_key_value_config", + "../test:test_support", "../test:wait_until", "../test/pc/sctp:fake_sctp_transport", - "//testing/gmock", - "//testing/gtest", "//third_party/abseil-cpp/absl/algorithm:container", "//third_party/abseil-cpp/absl/memory", "//third_party/abseil-cpp/absl/strings", + "//third_party/abseil-cpp/absl/strings:string_view", + ] + + # These deps are kept separately because they can't be automatically + # regenerated by gn_check_autofix tool + deps += [ + ":data_channel_controller_unittest", + "../test:test_main", ] if (is_android) { @@ -2540,44 +2569,6 @@ if (rtc_include_tests && !build_with_chromium) { ] shard_timeout = 900 } - - deps += [ - ":libjingle_peerconnection", - ":pc_test_utils", - ":rtc_pc", - "../api:rtc_event_log_output_file", - "../api:rtc_stats_api", - "../api:rtp_parameters", - "../api/audio_codecs:audio_codecs_api", - "../api/audio_codecs:builtin_audio_decoder_factory", - "../api/audio_codecs:builtin_audio_encoder_factory", - "../api/audio_codecs:opus_audio_decoder_factory", - "../api/audio_codecs:opus_audio_encoder_factory", - "../api/audio_codecs/L16:audio_decoder_L16", - "../api/audio_codecs/L16:audio_encoder_L16", - "../api/video_codecs:builtin_video_decoder_factory", - "../api/video_codecs:builtin_video_encoder_factory", - "../api/video_codecs:video_codecs_api", - "../api/video_codecs:video_decoder_factory_template", - "../api/video_codecs:video_decoder_factory_template_dav1d_adapter", - "../api/video_codecs:video_decoder_factory_template_libvpx_vp8_adapter", - "../api/video_codecs:video_decoder_factory_template_libvpx_vp9_adapter", - "../api/video_codecs:video_decoder_factory_template_open_h264_adapter", - "../api/video_codecs:video_encoder_factory_template", - "../api/video_codecs:video_encoder_factory_template_libaom_av1_adapter", - "../api/video_codecs:video_encoder_factory_template_libvpx_vp8_adapter", - "../api/video_codecs:video_encoder_factory_template_libvpx_vp9_adapter", - "../api/video_codecs:video_encoder_factory_template_open_h264_adapter", - "../call:call_interfaces", - "../media:rtc_audio_video", - "../media:rtc_media_tests_utils", - "../modules/audio_processing", - "../p2p:p2p_test_utils", - "../rtc_base:safe_conversions", - "../test:audio_codec_mocks", - "../test:test_main", - "../test:test_support", - ] } rtc_library("data_channel_controller_unittest") { diff --git a/pc/peer_connection_bundle_unittest.cc b/pc/peer_connection_bundle_unittest.cc index fb7b8ddf38..db828b49c3 100644 --- a/pc/peer_connection_bundle_unittest.cc +++ b/pc/peer_connection_bundle_unittest.cc @@ -18,10 +18,8 @@ #include #include "api/audio/audio_device.h" -#include "api/audio_codecs/builtin_audio_decoder_factory.h" -#include "api/audio_codecs/builtin_audio_encoder_factory.h" #include "api/candidate.h" -#include "api/create_peerconnection_factory.h" +#include "api/enable_media_with_defaults.h" #include "api/jsep.h" #include "api/media_types.h" #include "api/peer_connection_interface.h" @@ -43,11 +41,9 @@ #include "api/video_codecs/video_encoder_factory_template_libvpx_vp9_adapter.h" #include "api/video_codecs/video_encoder_factory_template_open_h264_adapter.h" #include "media/base/stream_params.h" -#include "p2p/base/basic_packet_socket_factory.h" #include "p2p/base/p2p_constants.h" #include "p2p/base/port_allocator.h" #include "p2p/base/transport_info.h" -#include "p2p/client/basic_port_allocator.h" #include "pc/channel.h" #include "pc/peer_connection.h" #include "pc/peer_connection_proxy.h" @@ -56,23 +52,24 @@ #include "pc/rtp_transport_internal.h" #include "pc/sdp_utils.h" #include "pc/session_description.h" +#include "pc/test/fake_audio_capture_module.h" #include "pc/test/integration_test_helpers.h" #include "pc/test/mock_peer_connection_observers.h" #include "rtc_base/checks.h" +#include "rtc_base/fake_network.h" #include "rtc_base/logging.h" #include "rtc_base/net_helper.h" #include "rtc_base/network.h" #include "rtc_base/socket_address.h" #include "rtc_base/thread.h" +#include "rtc_base/virtual_socket_server.h" +#include "test/gmock.h" #include "test/gtest.h" #include "test/wait_until.h" + #ifdef WEBRTC_ANDROID #include "pc/test/android_test_initializer.h" #endif -#include "pc/test/fake_audio_capture_module.h" -#include "rtc_base/fake_network.h" -#include "rtc_base/virtual_socket_server.h" -#include "test/gmock.h" namespace webrtc { @@ -205,24 +202,10 @@ class PeerConnectionBundleBaseTest : public ::testing::Test { typedef std::unique_ptr WrapperPtr; explicit PeerConnectionBundleBaseTest(SdpSemantics sdp_semantics) - : vss_(new rtc::VirtualSocketServer()), - socket_factory_(new rtc::BasicPacketSocketFactory(vss_.get())), - main_(vss_.get()), - sdp_semantics_(sdp_semantics) { + : main_(&vss_), sdp_semantics_(sdp_semantics) { #ifdef WEBRTC_ANDROID InitializeAndroidObjects(); #endif - pc_factory_ = CreatePeerConnectionFactory( - rtc::Thread::Current(), rtc::Thread::Current(), rtc::Thread::Current(), - rtc::scoped_refptr(FakeAudioCaptureModule::Create()), - CreateBuiltinAudioEncoderFactory(), CreateBuiltinAudioDecoderFactory(), - std::make_unique>(), - std::make_unique>(), - nullptr /* audio_mixer */, nullptr /* audio_processing */); } WrapperPtr CreatePeerConnection() { @@ -230,25 +213,46 @@ class PeerConnectionBundleBaseTest : public ::testing::Test { } WrapperPtr CreatePeerConnection(const RTCConfiguration& config) { - auto* fake_network = NewFakeNetwork(); - auto port_allocator = std::make_unique( - fake_network, socket_factory_.get()); - port_allocator->set_flags(cricket::PORTALLOCATOR_DISABLE_TCP | - cricket::PORTALLOCATOR_DISABLE_RELAY); - port_allocator->set_step_delay(cricket::kMinimumStepDelay); + // Each PeerConnection has its own `NetworkManager` which is injected into + // `PeerConnectionFactoryDependencies`, thus each PeerConnection in these + // tests is created with own PeerConnectionFactory. + PeerConnectionFactoryDependencies pcf_deps; + pcf_deps.network_thread = rtc::Thread::Current(); + pcf_deps.worker_thread = rtc::Thread::Current(); + pcf_deps.signaling_thread = rtc::Thread::Current(); + pcf_deps.socket_factory = &vss_; + auto network_manager = + std::make_unique(); + auto* fake_network = network_manager.get(); + pcf_deps.network_manager = std::move(network_manager); + pcf_deps.adm = FakeAudioCaptureModule::Create(); + pcf_deps.video_encoder_factory = + std::make_unique>(); + pcf_deps.video_decoder_factory = + std::make_unique>(); + EnableMediaWithDefaults(pcf_deps); + + scoped_refptr pc_factory = + CreateModularPeerConnectionFactory(std::move(pcf_deps)); + auto observer = std::make_unique(); RTCConfiguration modified_config = config; + modified_config.set_port_allocator_flags( + cricket::PORTALLOCATOR_DISABLE_TCP | + cricket::PORTALLOCATOR_DISABLE_RELAY); modified_config.sdp_semantics = sdp_semantics_; - PeerConnectionDependencies pc_dependencies(observer.get()); - pc_dependencies.allocator = std::move(port_allocator); - auto result = pc_factory_->CreatePeerConnectionOrError( - modified_config, std::move(pc_dependencies)); + auto result = pc_factory->CreatePeerConnectionOrError( + modified_config, PeerConnectionDependencies(observer.get())); if (!result.ok()) { return nullptr; } auto wrapper = std::make_unique( - pc_factory_, result.MoveValue(), std::move(observer)); + std::move(pc_factory), result.MoveValue(), std::move(observer)); wrapper->set_network(fake_network); return wrapper; } @@ -275,24 +279,8 @@ class PeerConnectionBundleBaseTest : public ::testing::Test { return candidate; } - rtc::FakeNetworkManager* NewFakeNetwork() { - // The PeerConnection's port allocator is tied to the PeerConnection's - // lifetime and expects the underlying NetworkManager to outlive it. If - // PeerConnectionWrapper owned the NetworkManager, it would be destroyed - // before the PeerConnection (since subclass members are destroyed before - // base class members). Therefore, the test fixture will own all the fake - // networks even though tests should access the fake network through the - // PeerConnectionWrapper. - auto* fake_network = new FakeNetworkManagerWithNoAnyNetwork(); - fake_networks_.emplace_back(fake_network); - return fake_network; - } - - std::unique_ptr vss_; - std::unique_ptr socket_factory_; + rtc::VirtualSocketServer vss_; rtc::AutoSocketServerThread main_; - rtc::scoped_refptr pc_factory_; - std::vector> fake_networks_; const SdpSemantics sdp_semantics_; };