Fix peerconnection_unittests for force-test-env Update unittests in peerconnection_unittests to use test environment and test field trials instead of production ones, allowing them to pass with the --force-test-environment flag. Specifically, fix a lifetime issue in PeerConnectionTestWrapper where the field trials unique_ptr was deleted too early by moving it. Bug: webrtc:521534686 Change-Id: I36004a5574896fb0fa0c487d2417d8f22f080930 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/480881 Commit-Queue: Harald Alvestrand <hta@webrtc.org> Reviewed-by: Danil Chapovalov <danilchap@webrtc.org> Cr-Commit-Position: refs/heads/main@{#47975}
diff --git a/pc/BUILD.gn b/pc/BUILD.gn index fbe6d8d..40b9c44 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn
@@ -2682,9 +2682,11 @@ "../api:rtp_parameters", "../api:scoped_refptr", "../api:set_local_description_observer_interface", + "../api/environment", "../media:rtc_media_tests_utils", "../rtc_base:checks", "../rtc_base:threading", + "../test:create_test_environment", "../test:run_loop", "../test:test_support", ] @@ -2715,6 +2717,7 @@ "../api:rtp_sender_interface", "../api:rtp_transceiver_interface", "../api:scoped_refptr", + "../api/environment", "../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", @@ -2736,6 +2739,7 @@ "../rtc_base:rtc_base_tests_utils", "../rtc_base:socket_address", "../rtc_base:threading", + "../test:create_test_environment", "../test:run_loop", "../test:test_support", "../test:wait_until", @@ -2760,7 +2764,6 @@ "../api:scoped_refptr", "../api/audio_codecs:builtin_audio_decoder_factory", "../api/audio_codecs:builtin_audio_encoder_factory", - "../api/environment:environment_factory", "../api/units:time_delta", "../api/video_codecs:video_decoder_factory_template", "../api/video_codecs:video_decoder_factory_template_dav1d_adapter", @@ -2805,10 +2808,12 @@ "../api:rtp_parameters", "../api:scoped_refptr", "../api:sctp_transport_interface", + "../api/environment", "../rtc_base:checks", "../rtc_base:logging", "../rtc_base:rtc_base_tests_utils", "../rtc_base:threading", + "../test:create_test_environment", "../test:run_loop", "../test:test_support", "../test/pc/sctp:fake_sctp_transport", @@ -2955,6 +2960,7 @@ "../api:rtc_error", "../api:rtc_error_matchers", "../api:scoped_refptr", + "../api/environment", "../api/units:time_delta", "../rtc_base:checks", "../rtc_base:ip_address", @@ -2963,6 +2969,7 @@ "../rtc_base:socket_address", "../rtc_base:threading", "../system_wrappers:metrics", + "../test:create_test_environment", "../test:run_loop", "../test:test_support", "../test:wait_until", @@ -2997,7 +3004,7 @@ "../api:uma_metrics", "../api/audio_codecs:builtin_audio_decoder_factory", "../api/audio_codecs:builtin_audio_encoder_factory", - "../api/environment:environment_factory", + "../api/environment", "../api/units:time_delta", "../api/video_codecs:video_decoder_factory_template", "../api/video_codecs:video_decoder_factory_template_dav1d_adapter", @@ -3048,8 +3055,10 @@ ":session_description", ":stream_collection", ":video_track", + "../api:create_modular_peer_connection_factory", "../api:create_peerconnection_factory", "../api:data_channel_interface", + "../api:enable_media", "../api:enable_media_with_defaults", "../api:jsep", "../api:libjingle_logging_api", @@ -3068,7 +3077,7 @@ "../api/audio_codecs:builtin_audio_decoder_factory", "../api/audio_codecs:builtin_audio_encoder_factory", "../api/crypto:options", - "../api/environment:environment_factory", + "../api/environment", "../api/rtc_event_log", "../api/rtc_event_log:rtc_event_log_factory", "../api/transport:bitrate_settings", @@ -3167,7 +3176,6 @@ "../api:rtp_transceiver_interface", "../api:scoped_refptr", "../api/environment", - "../api/environment:environment_factory", "../api/rtc_event_log:rtc_event_log_factory", "../media:codec", "../media:media_channel", @@ -3475,7 +3483,6 @@ "../api/crypto:frame_encryptor_interface", "../api/crypto:options", "../api/environment", - "../api/environment:environment_factory", "../api/task_queue", "../api/video:builtin_video_bitrate_allocator_factory", "../api/video:video_bitrate_allocator_factory", @@ -4396,11 +4403,14 @@ "../api:call_api", "../api:candidate", "../api:create_frame_generator", + "../api:create_modular_peer_connection_factory", "../api:create_peerconnection_factory", "../api:data_channel_event_observer_interface", "../api:data_channel_interface", "../api:dtls_transport_interface", "../api:dtmf_sender_interface", + "../api:enable_media", + "../api:enable_media_with_defaults", "../api:field_trials_view", "../api:frame_transformer_interface", "../api:ice_transport_interface", @@ -4433,6 +4443,7 @@ "../api/crypto:options", "../api/environment", "../api/environment:environment_factory", + "../api/rtc_event_log:rtc_event_log_factory", "../api/task_queue", "../api/task_queue:default_task_queue_factory", "../api/task_queue:pending_task_safety_flag",
diff --git a/pc/peer_connection_audio_options_unittest.cc b/pc/peer_connection_audio_options_unittest.cc index 193b95b..c696fb1 100644 --- a/pc/peer_connection_audio_options_unittest.cc +++ b/pc/peer_connection_audio_options_unittest.cc
@@ -29,6 +29,7 @@ #include "pc/test/mock_peer_connection_observers.h" #include "rtc_base/checks.h" #include "rtc_base/thread.h" +#include "test/create_test_environment.h" #include "test/gmock.h" #include "test/gtest.h" #include "test/run_loop.h" @@ -92,6 +93,7 @@ dependencies.signaling_thread = Thread::Current(); EnableFakeMedia(dependencies, std::make_unique<FakeMediaEngine>()); + dependencies.env = CreateTestEnvironment(); pc_factory_ = CreateModularPeerConnectionFactory(std::move(dependencies)); } @@ -117,7 +119,6 @@ pc_.get()); return static_cast<PeerConnection*>(proxy->internal()); } - RunLoop loop_; std::unique_ptr<Thread> worker_thread_; std::unique_ptr<Thread> network_thread_;
diff --git a/pc/peer_connection_bundle_unittest.cc b/pc/peer_connection_bundle_unittest.cc index c1a4e2e..1d3b522 100644 --- a/pc/peer_connection_bundle_unittest.cc +++ b/pc/peer_connection_bundle_unittest.cc
@@ -19,6 +19,7 @@ #include "api/candidate.h" #include "api/create_modular_peer_connection_factory.h" #include "api/enable_media_with_defaults.h" +#include "api/environment/environment.h" #include "api/jsep.h" #include "api/media_types.h" #include "api/peer_connection_interface.h" @@ -60,6 +61,7 @@ #include "rtc_base/socket_address.h" #include "rtc_base/thread.h" #include "rtc_base/virtual_socket_server.h" +#include "test/create_test_environment.h" #include "test/gmock.h" #include "test/gtest.h" #include "test/run_loop.h" @@ -188,7 +190,9 @@ typedef std::unique_ptr<PeerConnectionWrapperForBundleTest> WrapperPtr; explicit PeerConnectionBundleBaseTest(SdpSemantics sdp_semantics) - : main_(&vss_), sdp_semantics_(sdp_semantics) { + : env_(CreateTestEnvironment()), + main_(&vss_), + sdp_semantics_(sdp_semantics) { #ifdef WEBRTC_ANDROID InitializeAndroidObjects(); #endif @@ -220,6 +224,7 @@ std::make_unique<VideoDecoderFactoryTemplate< LibvpxVp8DecoderTemplateAdapter, LibvpxVp9DecoderTemplateAdapter, OpenH264DecoderTemplateAdapter, Dav1dDecoderTemplateAdapter>>(); + pcf_deps.env = env_; EnableMediaWithDefaults(pcf_deps); scoped_refptr<PeerConnectionFactoryInterface> pc_factory = @@ -263,6 +268,7 @@ return candidate; } + const Environment env_; VirtualSocketServer vss_; test::RunLoop main_; const SdpSemantics sdp_semantics_;
diff --git a/pc/peer_connection_data_channel_unittest.cc b/pc/peer_connection_data_channel_unittest.cc index e086ce0..4e42034 100644 --- a/pc/peer_connection_data_channel_unittest.cc +++ b/pc/peer_connection_data_channel_unittest.cc
@@ -15,6 +15,7 @@ #include <vector> #include "api/create_modular_peer_connection_factory.h" +#include "api/environment/environment.h" #include "api/jsep.h" #include "api/media_types.h" #include "api/peer_connection_interface.h" @@ -31,6 +32,7 @@ #include "rtc_base/logging.h" #include "rtc_base/thread.h" #include "rtc_base/virtual_socket_server.h" +#include "test/create_test_environment.h" #include "test/gmock.h" #include "test/gtest.h" #include "test/pc/sctp/fake_sctp_transport.h" @@ -95,7 +97,8 @@ typedef std::unique_ptr<PeerConnectionWrapperForDataChannelTest> WrapperPtr; explicit PeerConnectionDataChannelBaseTest(SdpSemantics sdp_semantics) - : vss_(new VirtualSocketServer()), + : env_(CreateTestEnvironment()), + vss_(new VirtualSocketServer()), main_(vss_.get()), sdp_semantics_(sdp_semantics) { #ifdef WEBRTC_ANDROID @@ -118,6 +121,7 @@ auto factory_deps = CreatePeerConnectionFactoryDependencies(); FakeSctpTransportFactory* fake_sctp_transport_factory = static_cast<FakeSctpTransportFactory*>(factory_deps.sctp_factory.get()); + factory_deps.env = env_; scoped_refptr<PeerConnectionFactoryInterface> pc_factory = CreateModularPeerConnectionFactory(std::move(factory_deps)); pc_factory->SetOptions(factory_options); @@ -158,6 +162,7 @@ data_desc->set_port(port); } + const Environment env_; std::unique_ptr<VirtualSocketServer> vss_; test::RunLoop main_; const SdpSemantics sdp_semantics_;
diff --git a/pc/peer_connection_factory_unittest.cc b/pc/peer_connection_factory_unittest.cc index 8d0862a..77fc99b 100644 --- a/pc/peer_connection_factory_unittest.cc +++ b/pc/peer_connection_factory_unittest.cc
@@ -30,7 +30,6 @@ #include "api/enable_media_with_defaults.h" #include "api/environment/environment.h" #include "api/environment/environment_factory.h" -#include "api/field_trials.h" #include "api/field_trials_view.h" #include "api/jsep.h" #include "api/make_ref_counted.h" @@ -679,7 +678,7 @@ TEST(PeerConnectionFactoryDependenciesTest, CanInjectFieldTrialsWithEnvironment) { - std::unique_ptr<FieldTrialsView> field_trials = FieldTrials::Create(""); + std::unique_ptr<FieldTrialsView> field_trials = CreateTestFieldTrialsPtr(""); ASSERT_THAT(field_trials, NotNull()); FieldTrialsView* raw_field_trials = field_trials.get(); @@ -704,6 +703,7 @@ PeerConnectionFactoryDependencies pcf_dependencies; pcf_dependencies.network_manager = std::move(mock_network_manager); + pcf_dependencies.env = CreateTestEnvironment(); scoped_refptr<PeerConnectionFactoryInterface> pcf = CreateModularPeerConnectionFactory(std::move(pcf_dependencies)); @@ -733,6 +733,7 @@ PeerConnectionFactoryDependencies pcf_dependencies; pcf_dependencies.packet_socket_factory = std::move(mock_socket_factory); + pcf_dependencies.env = CreateTestEnvironment(); scoped_refptr<PeerConnectionFactoryInterface> pcf = CreateModularPeerConnectionFactory(std::move(pcf_dependencies)); @@ -766,6 +767,7 @@ PeerConnectionFactoryDependencies pcf_dependencies; pcf_dependencies.adm = FakeAudioCaptureModule::Create(); pcf_dependencies.audio_processing_builder = std::move(ap_factory); + pcf_dependencies.env = CreateTestEnvironment(); EnableMediaWithDefaults(pcf_dependencies); scoped_refptr<PeerConnectionFactoryInterface> pcf = @@ -787,6 +789,7 @@ pcf_dependencies.signaling_thread = Thread::Current(); pcf_dependencies.worker_thread = Thread::Current(); pcf_dependencies.network_thread = Thread::Current(); + pcf_dependencies.env = CreateTestEnvironment(); EnableMediaWithDefaults(pcf_dependencies); scoped_refptr<PeerConnectionFactoryInterface> pcf = @@ -825,6 +828,7 @@ pcf_dependencies.signaling_thread = Thread::Current(); pcf_dependencies.worker_thread = Thread::Current(); pcf_dependencies.network_thread = Thread::Current(); + pcf_dependencies.env = CreateTestEnvironment(); EnableMediaWithDefaults(pcf_dependencies); scoped_refptr<PeerConnectionFactoryInterface> pcf =
diff --git a/pc/peer_connection_histogram_unittest.cc b/pc/peer_connection_histogram_unittest.cc index 784fa7e..8dcdcb5 100644 --- a/pc/peer_connection_histogram_unittest.cc +++ b/pc/peer_connection_histogram_unittest.cc
@@ -20,6 +20,7 @@ #include "absl/strings/str_format.h" #include "absl/strings/string_view.h" #include "api/create_modular_peer_connection_factory.h" +#include "api/environment/environment.h" #include "api/jsep.h" #include "api/peer_connection_interface.h" #include "api/rtc_error.h" @@ -42,6 +43,7 @@ #include "rtc_base/thread.h" #include "rtc_base/virtual_socket_server.h" #include "system_wrappers/include/metrics.h" +#include "test/create_test_environment.h" #include "test/gmock.h" #include "test/gtest.h" #include "test/run_loop.h" @@ -230,7 +232,10 @@ typedef std::unique_ptr<PeerConnectionWrapperForUsageHistogramTest> WrapperPtr; - PeerConnectionUsageHistogramTest() : main_(&vss_) { metrics::Reset(); } + PeerConnectionUsageHistogramTest() + : env_(CreateTestEnvironment()), main_(&vss_) { + metrics::Reset(); + } WrapperPtr CreatePeerConnection() { RTCConfiguration config; @@ -310,6 +315,7 @@ pcf_deps.network_manager = std::move(fake_network); } EnableFakeMedia(pcf_deps); + pcf_deps.env = env_; auto pc_factory = CreateModularPeerConnectionFactory(std::move(pcf_deps)); pc_factory->SetOptions(factory_options); @@ -341,6 +347,7 @@ return kLocalAddrs[next_local_address_++]; } + const Environment env_; int next_local_address_ = 0; VirtualSocketServer vss_; test::RunLoop main_;
diff --git a/pc/peer_connection_ice_unittest.cc b/pc/peer_connection_ice_unittest.cc index 46e6985..dddcbc8 100644 --- a/pc/peer_connection_ice_unittest.cc +++ b/pc/peer_connection_ice_unittest.cc
@@ -20,6 +20,7 @@ #include "api/candidate.h" #include "api/create_modular_peer_connection_factory.h" #include "api/enable_media_with_defaults.h" +#include "api/environment/environment.h" #include "api/ice_transport_interface.h" #include "api/jsep.h" #include "api/make_ref_counted.h" @@ -158,7 +159,8 @@ typedef std::unique_ptr<PeerConnectionWrapperForIceTest> WrapperPtr; explicit PeerConnectionIceBaseTest(SdpSemantics sdp_semantics) - : network_thread_(new Thread(&vss_)), + : env_(CreateTestEnvironment()), + network_thread_(new Thread(&vss_)), worker_thread_(Thread::Create()), sdp_semantics_(sdp_semantics) { RTC_CHECK(network_thread_->Start()); @@ -191,6 +193,7 @@ std::make_unique<VideoDecoderFactoryTemplate< LibvpxVp8DecoderTemplateAdapter, LibvpxVp9DecoderTemplateAdapter, OpenH264DecoderTemplateAdapter, Dav1dDecoderTemplateAdapter>>(); + pcf_deps.env = env_; EnableMediaWithDefaults(pcf_deps); scoped_refptr<PeerConnectionFactoryInterface> pc_factory = CreateModularPeerConnectionFactory(std::move(pcf_deps)); @@ -332,6 +335,7 @@ return sdesc->AddCandidate(jsep_candidate.get()); } + const Environment env_; test::RunLoop main_; VirtualSocketServer vss_; std::unique_ptr<Thread> network_thread_;
diff --git a/pc/peer_connection_interface_unittest.cc b/pc/peer_connection_interface_unittest.cc index 70d5c32..e11b332 100644 --- a/pc/peer_connection_interface_unittest.cc +++ b/pc/peer_connection_interface_unittest.cc
@@ -24,10 +24,12 @@ #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/create_peerconnection_factory.h" +#include "api/create_modular_peer_connection_factory.h" #include "api/crypto/crypto_options.h" #include "api/data_channel_interface.h" +#include "api/enable_media.h" #include "api/enable_media_with_defaults.h" +#include "api/environment/environment.h" #include "api/jsep.h" #include "api/make_ref_counted.h" #include "api/media_stream_interface.h" @@ -640,7 +642,8 @@ class PeerConnectionInterfaceBaseTest : public ::testing::Test { protected: explicit PeerConnectionInterfaceBaseTest(SdpSemantics sdp_semantics) - : vss_(new VirtualSocketServer()), + : env_(CreateTestEnvironment()), + vss_(new VirtualSocketServer()), main_(vss_.get()), sdp_semantics_(sdp_semantics) { #ifdef WEBRTC_ANDROID @@ -659,17 +662,27 @@ worker_thread_->SetName("WorkerThread", nullptr); ASSERT_TRUE(worker_thread_->Start()); - pc_factory_ = CreatePeerConnectionFactory( - network_thread_.get(), worker_thread_.get(), Thread::Current(), - scoped_refptr<AudioDeviceModule>(fake_audio_capture_module_), - CreateBuiltinAudioEncoderFactory(), CreateBuiltinAudioDecoderFactory(), + PeerConnectionFactoryDependencies dependencies; + dependencies.network_thread = network_thread_.get(); + dependencies.worker_thread = worker_thread_.get(); + dependencies.signaling_thread = Thread::Current(); + dependencies.socket_factory = network_thread_->socketserver(); + dependencies.adm = + scoped_refptr<AudioDeviceModule>(fake_audio_capture_module_); + dependencies.audio_encoder_factory = CreateBuiltinAudioEncoderFactory(); + dependencies.audio_decoder_factory = CreateBuiltinAudioDecoderFactory(); + dependencies.video_encoder_factory = std::make_unique<VideoEncoderFactoryTemplate< LibvpxVp8EncoderTemplateAdapter, LibvpxVp9EncoderTemplateAdapter, - OpenH264EncoderTemplateAdapter, LibaomAv1EncoderTemplateAdapter>>(), + OpenH264EncoderTemplateAdapter, LibaomAv1EncoderTemplateAdapter>>(); + dependencies.video_decoder_factory = std::make_unique<VideoDecoderFactoryTemplate< LibvpxVp8DecoderTemplateAdapter, LibvpxVp9DecoderTemplateAdapter, - OpenH264DecoderTemplateAdapter, Dav1dDecoderTemplateAdapter>>(), - nullptr /* audio_mixer */, nullptr /* audio_processing */); + OpenH264DecoderTemplateAdapter, Dav1dDecoderTemplateAdapter>>(); + dependencies.event_log_factory = std::make_unique<RtcEventLogFactory>(); + dependencies.env = env_; + EnableMedia(dependencies); + pc_factory_ = CreateModularPeerConnectionFactory(std::move(dependencies)); ASSERT_TRUE(pc_factory_); } @@ -1285,6 +1298,7 @@ SocketServer* socket_server() const { return vss_.get(); } + const Environment env_; std::unique_ptr<VirtualSocketServer> vss_; test::RunLoop main_; std::unique_ptr<Thread> network_thread_;
diff --git a/pc/test/peer_connection_test_wrapper.cc b/pc/test/peer_connection_test_wrapper.cc index 2559c71..1ed6b11 100644 --- a/pc/test/peer_connection_test_wrapper.cc +++ b/pc/test/peer_connection_test_wrapper.cc
@@ -22,8 +22,9 @@ #include "api/audio_codecs/audio_decoder_factory.h" #include "api/audio_codecs/audio_encoder_factory.h" #include "api/audio_options.h" -#include "api/create_peerconnection_factory.h" +#include "api/create_modular_peer_connection_factory.h" #include "api/data_channel_interface.h" +#include "api/enable_media_with_defaults.h" #include "api/environment/environment.h" #include "api/environment/environment_factory.h" #include "api/field_trials_view.h" @@ -33,6 +34,7 @@ #include "api/media_types.h" #include "api/peer_connection_interface.h" #include "api/rtc_error.h" +#include "api/rtc_event_log/rtc_event_log_factory.h" #include "api/rtp_parameters.h" #include "api/rtp_receiver_interface.h" #include "api/scoped_refptr.h" @@ -180,7 +182,7 @@ std::unique_ptr<VideoDecoderFactory> video_decoder_factory, std::unique_ptr<FieldTrialsView> field_trials) { EnvironmentFactory env_factory(env_); - env_factory.Set(field_trials.get()); + env_factory.Set(std::move(field_trials)); Environment env = env_factory.Create(); auto port_allocator = std::make_unique<FakePortAllocator>(env, socket_server_, network_thread_); @@ -192,13 +194,22 @@ return false; } - peer_connection_factory_ = CreatePeerConnectionFactory( - network_thread_, worker_thread_, Thread::Current(), - scoped_refptr<AudioDeviceModule>(fake_audio_capture_module_), - audio_encoder_factory, audio_decoder_factory, - std::move(video_encoder_factory), std::move(video_decoder_factory), - nullptr /* audio_mixer */, nullptr /* audio_processing */, nullptr, - std::move(field_trials)); + PeerConnectionFactoryDependencies dependencies; + dependencies.network_thread = network_thread_; + dependencies.worker_thread = worker_thread_; + dependencies.signaling_thread = Thread::Current(); + dependencies.socket_factory = socket_server_; + dependencies.adm = + scoped_refptr<AudioDeviceModule>(fake_audio_capture_module_); + dependencies.audio_encoder_factory = audio_encoder_factory; + dependencies.audio_decoder_factory = audio_decoder_factory; + dependencies.video_encoder_factory = std::move(video_encoder_factory); + dependencies.video_decoder_factory = std::move(video_decoder_factory); + dependencies.event_log_factory = std::make_unique<RtcEventLogFactory>(); + dependencies.env = env; + EnableMediaWithDefaults(dependencies); + peer_connection_factory_ = + CreateModularPeerConnectionFactory(std::move(dependencies)); if (!peer_connection_factory_) { return false; }