diff --git a/api/BUILD.gn b/api/BUILD.gn index f8a6532866..b736f93885 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn @@ -1223,3 +1223,10 @@ if (rtc_include_tests) { ] } } + +rtc_source_set("webrtc_key_value_config") { + visibility = [ "*" ] + sources = [ "webrtc_key_value_config.h" ] + deps = [ "../rtc_base/system:rtc_export" ] + absl_deps = [ "//third_party/abseil-cpp/absl/strings" ] +} diff --git a/api/transport/BUILD.gn b/api/transport/BUILD.gn index 30955273b0..fb0fcf7a64 100644 --- a/api/transport/BUILD.gn +++ b/api/transport/BUILD.gn @@ -48,7 +48,10 @@ rtc_library("network_control") { rtc_source_set("webrtc_key_value_config") { visibility = [ "*" ] sources = [ "webrtc_key_value_config.h" ] - deps = [ "../../rtc_base/system:rtc_export" ] + deps = [ + "../../api:webrtc_key_value_config", + "../../rtc_base/system:rtc_export", + ] absl_deps = [ "//third_party/abseil-cpp/absl/strings" ] } diff --git a/api/transport/webrtc_key_value_config.h b/api/transport/webrtc_key_value_config.h index 5666a82783..f666873cfe 100644 --- a/api/transport/webrtc_key_value_config.h +++ b/api/transport/webrtc_key_value_config.h @@ -10,24 +10,8 @@ #ifndef API_TRANSPORT_WEBRTC_KEY_VALUE_CONFIG_H_ #define API_TRANSPORT_WEBRTC_KEY_VALUE_CONFIG_H_ -#include - -#include "absl/strings/string_view.h" -#include "rtc_base/system/rtc_export.h" - -namespace webrtc { - -// An interface that provides a key-value mapping for configuring internal -// details of WebRTC. Note that there's no guarantess that the meaning of a -// particular key value mapping will be preserved over time and no announcements -// will be made if they are changed. It's up to the library user to ensure that -// the behavior does not break. -class RTC_EXPORT WebRtcKeyValueConfig { - public: - virtual ~WebRtcKeyValueConfig() = default; - // The configured value for the given key. Defaults to an empty string. - virtual std::string Lookup(absl::string_view key) const = 0; -}; -} // namespace webrtc +// TODO(bugs.webrtc.org/10335): Remove once all migrated to +// "api/webrtc_key_value_config.h". +#include "api/webrtc_key_value_config.h" #endif // API_TRANSPORT_WEBRTC_KEY_VALUE_CONFIG_H_ diff --git a/api/webrtc_key_value_config.h b/api/webrtc_key_value_config.h new file mode 100644 index 0000000000..32a2e47eed --- /dev/null +++ b/api/webrtc_key_value_config.h @@ -0,0 +1,41 @@ +/* + * 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 API_WEBRTC_KEY_VALUE_CONFIG_H_ +#define API_WEBRTC_KEY_VALUE_CONFIG_H_ + +#include + +#include "absl/strings/string_view.h" +#include "rtc_base/system/rtc_export.h" + +namespace webrtc { + +// An interface that provides a key-value mapping for configuring internal +// details of WebRTC. Note that there's no guarantess that the meaning of a +// particular key value mapping will be preserved over time and no announcements +// will be made if they are changed. It's up to the library user to ensure that +// the behavior does not break. +class RTC_EXPORT WebRtcKeyValueConfig { + public: + virtual ~WebRtcKeyValueConfig() = default; + // The configured value for the given key. Defaults to an empty string. + virtual std::string Lookup(absl::string_view key) const = 0; + + bool IsEnabled(absl::string_view key) const { + return Lookup(key).find("Enabled") == 0; + } + + bool IsDisabled(absl::string_view key) const { + return Lookup(key).find("Disabled") == 0; + } +}; +} // namespace webrtc + +#endif // API_WEBRTC_KEY_VALUE_CONFIG_H_ diff --git a/call/bitrate_allocator.cc b/call/bitrate_allocator.cc index 1693661ef5..2684a1650e 100644 --- a/call/bitrate_allocator.cc +++ b/call/bitrate_allocator.cc @@ -23,7 +23,6 @@ #include "rtc_base/logging.h" #include "rtc_base/numerics/safe_minmax.h" #include "system_wrappers/include/clock.h" -#include "system_wrappers/include/field_trial.h" #include "system_wrappers/include/metrics.h" namespace webrtc { diff --git a/call/call.cc b/call/call.cc index bb62365cd4..2638b22b31 100644 --- a/call/call.cc +++ b/call/call.cc @@ -61,7 +61,6 @@ #include "rtc_base/trace_event.h" #include "system_wrappers/include/clock.h" #include "system_wrappers/include/cpu_info.h" -#include "system_wrappers/include/field_trial.h" #include "system_wrappers/include/metrics.h" #include "video/call_stats2.h" #include "video/send_delay_stats.h" diff --git a/modules/congestion_controller/rtp/transport_feedback_adapter.cc b/modules/congestion_controller/rtp/transport_feedback_adapter.cc index 87691bf263..87e917e978 100644 --- a/modules/congestion_controller/rtp/transport_feedback_adapter.cc +++ b/modules/congestion_controller/rtp/transport_feedback_adapter.cc @@ -22,7 +22,6 @@ #include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" -#include "system_wrappers/include/field_trial.h" namespace webrtc { diff --git a/modules/pacing/paced_sender_unittest.cc b/modules/pacing/paced_sender_unittest.cc index 53cc1c42ed..7abd532417 100644 --- a/modules/pacing/paced_sender_unittest.cc +++ b/modules/pacing/paced_sender_unittest.cc @@ -19,7 +19,6 @@ #include "modules/pacing/packet_router.h" #include "modules/utility/include/mock/mock_process_thread.h" #include "system_wrappers/include/clock.h" -#include "system_wrappers/include/field_trial.h" #include "test/field_trial.h" #include "test/gmock.h" #include "test/gtest.h" diff --git a/modules/remote_bitrate_estimator/bwe_defines.cc b/modules/remote_bitrate_estimator/bwe_defines.cc index 6afbe133e2..22c605e82d 100644 --- a/modules/remote_bitrate_estimator/bwe_defines.cc +++ b/modules/remote_bitrate_estimator/bwe_defines.cc @@ -10,8 +10,6 @@ #include "modules/remote_bitrate_estimator/include/bwe_defines.h" -#include "system_wrappers/include/field_trial.h" - namespace webrtc { const char kBweTypeHistogram[] = "WebRTC.BWE.Types"; diff --git a/p2p/BUILD.gn b/p2p/BUILD.gn index a9c059bed3..66da247ade 100644 --- a/p2p/BUILD.gn +++ b/p2p/BUILD.gn @@ -91,6 +91,7 @@ rtc_library("rtc_p2p") { "../api:rtc_error", "../api:scoped_refptr", "../api:sequence_checker", + "../api:webrtc_key_value_config", "../api:wrapping_async_dns_resolver", "../api/crypto:options", "../api/rtc_event_log", @@ -240,6 +241,7 @@ if (rtc_include_tests) { "../api:mock_async_dns_resolver", "../api:packet_socket_factory", "../api:scoped_refptr", + "../api:webrtc_key_value_config", "../api/transport:stun_types", "../api/units:time_delta", "../rtc_base", @@ -259,6 +261,7 @@ if (rtc_include_tests) { "../system_wrappers:metrics", "../test:field_trial", "../test:rtc_expect_death", + "../test:scoped_key_value_config", "../test:test_support", "//testing/gtest", ] diff --git a/p2p/base/port.cc b/p2p/base/port.cc index 5d3552b7f6..f864150969 100644 --- a/p2p/base/port.cc +++ b/p2p/base/port.cc @@ -34,7 +34,6 @@ #include "rtc_base/strings/string_builder.h" #include "rtc_base/third_party/base64/base64.h" #include "rtc_base/trace_event.h" -#include "system_wrappers/include/field_trial.h" namespace { diff --git a/p2p/base/stun_request.cc b/p2p/base/stun_request.cc index 09a7a8345e..ed94ccb5fc 100644 --- a/p2p/base/stun_request.cc +++ b/p2p/base/stun_request.cc @@ -19,7 +19,6 @@ #include "rtc_base/logging.h" #include "rtc_base/string_encode.h" #include "rtc_base/time_utils.h" // For TimeMillis -#include "system_wrappers/include/field_trial.h" namespace cricket { diff --git a/p2p/base/transport_description_factory.cc b/p2p/base/transport_description_factory.cc index e46114ed83..18c4a28c2b 100644 --- a/p2p/base/transport_description_factory.cc +++ b/p2p/base/transport_description_factory.cc @@ -21,8 +21,9 @@ namespace cricket { -TransportDescriptionFactory::TransportDescriptionFactory() - : secure_(SEC_DISABLED) {} +TransportDescriptionFactory::TransportDescriptionFactory( + const webrtc::WebRtcKeyValueConfig& field_trials) + : secure_(SEC_DISABLED), field_trials_(field_trials) {} TransportDescriptionFactory::~TransportDescriptionFactory() = default; diff --git a/p2p/base/transport_description_factory.h b/p2p/base/transport_description_factory.h index 0be7f32929..46f1c2f9fa 100644 --- a/p2p/base/transport_description_factory.h +++ b/p2p/base/transport_description_factory.h @@ -13,6 +13,7 @@ #include +#include "api/webrtc_key_value_config.h" #include "p2p/base/ice_credentials_iterator.h" #include "p2p/base/transport_description.h" #include "rtc_base/rtc_certificate.h" @@ -37,7 +38,8 @@ struct TransportOptions { class TransportDescriptionFactory { public: // Default ctor; use methods below to set configuration. - TransportDescriptionFactory(); + explicit TransportDescriptionFactory( + const webrtc::WebRtcKeyValueConfig& field_trials); ~TransportDescriptionFactory(); SecurePolicy secure() const { return secure_; } @@ -73,12 +75,15 @@ class TransportDescriptionFactory { const TransportDescription* current_description, IceCredentialsIterator* ice_credentials) const; + const webrtc::WebRtcKeyValueConfig& trials() const { return field_trials_; } + private: bool SetSecurityInfo(TransportDescription* description, ConnectionRole role) const; SecurePolicy secure_; rtc::scoped_refptr certificate_; + const webrtc::WebRtcKeyValueConfig& field_trials_; }; } // namespace cricket diff --git a/p2p/base/transport_description_factory_unittest.cc b/p2p/base/transport_description_factory_unittest.cc index 77a56eff26..75e6e8ee09 100644 --- a/p2p/base/transport_description_factory_unittest.cc +++ b/p2p/base/transport_description_factory_unittest.cc @@ -25,6 +25,7 @@ #include "rtc_base/ssl_identity.h" #include "test/gmock.h" #include "test/gtest.h" +#include "test/scoped_key_value_config.h" using cricket::TransportDescription; using cricket::TransportDescriptionFactory; @@ -36,6 +37,8 @@ class TransportDescriptionFactoryTest : public ::testing::Test { public: TransportDescriptionFactoryTest() : ice_credentials_({}), + f1_(field_trials_), + f2_(field_trials_), cert1_(rtc::RTCCertificate::Create(std::unique_ptr( new rtc::FakeSSLIdentity("User1")))), cert2_(rtc::RTCCertificate::Create(std::unique_ptr( @@ -156,6 +159,7 @@ class TransportDescriptionFactoryTest : public ::testing::Test { } } + webrtc::test::ScopedKeyValueConfig field_trials_; cricket::IceCredentialsIterator ice_credentials_; TransportDescriptionFactory f1_; TransportDescriptionFactory f2_; diff --git a/pc/BUILD.gn b/pc/BUILD.gn index 32d114444f..f88226a51c 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -143,6 +143,7 @@ rtc_library("rtc_pc_base") { "../api:scoped_refptr", "../api:sequence_checker", "../api:video_track_source_constraints", + "../api:webrtc_key_value_config", "../api/crypto:options", "../api/rtc_event_log", "../api/task_queue", @@ -813,6 +814,7 @@ rtc_source_set("stats_collector") { "../api:rtp_parameters", "../api:scoped_refptr", "../api:sequence_checker", + "../api:webrtc_key_value_config", "../api/audio_codecs:audio_codecs_api", "../api/video:video_rtp_headers", "../call:call_interfaces", @@ -829,7 +831,6 @@ rtc_source_set("stats_collector") { "../rtc_base:stringutils", "../rtc_base:threading", "../rtc_base:timeutils", - "../system_wrappers:field_trial", ] absl_deps = [ "//third_party/abseil-cpp/absl/strings", @@ -906,6 +907,7 @@ rtc_source_set("webrtc_session_description_factory") { "webrtc_session_description_factory.h", ] deps = [ + ":connection_context", ":rtc_pc_base", ":sdp_state_provider", ":session_description", @@ -1581,7 +1583,8 @@ if (rtc_include_tests && !build_with_chromium) { "../rtc_base/task_utils:to_queued_task", "../rtc_base/third_party/sigslot", "../system_wrappers:metrics", - "../test:field_trial", + "../test:explicit_key_value_config", + "../test:scoped_key_value_config", "../test:test_common", "../test:test_main", "../test:test_support", @@ -1767,6 +1770,7 @@ if (rtc_include_tests && !build_with_chromium) { "../api:rtc_error", "../api:rtp_transceiver_direction", "../api:scoped_refptr", + "../api:webrtc_key_value_config", "../api/adaptation:resource_adaptation_api", "../api/audio:audio_mixer_api", "../api/crypto:frame_decryptor_interface", @@ -1817,9 +1821,10 @@ if (rtc_include_tests && !build_with_chromium) { "../rtc_base/third_party/sigslot", "../system_wrappers:field_trial", "../system_wrappers:metrics", - "../test:field_trial", + "../test:explicit_key_value_config", "../test:fileutils", "../test:rtp_test_utils", + "../test:scoped_key_value_config", "../test:test_common", "../test/pc/sctp:fake_sctp_transport", "./scenario_tests:pc_scenario_tests", @@ -1994,9 +1999,10 @@ if (rtc_include_tests && !build_with_chromium) { "../rtc_base/third_party/base64", "../rtc_base/third_party/sigslot", "../system_wrappers:metrics", - "../test:field_trial", + "../test:explicit_key_value_config", "../test:fileutils", "../test:rtp_test_utils", + "../test:scoped_key_value_config", "../test:test_support", "../test/pc/sctp:fake_sctp_transport", ] @@ -2054,10 +2060,12 @@ if (rtc_include_tests && !build_with_chromium) { "../api:rtc_stats_api", "../api:scoped_refptr", "../api:sequence_checker", + "../api:webrtc_key_value_config", "../api/audio:audio_mixer_api", "../api/audio_codecs:audio_codecs_api", "../api/task_queue", "../api/task_queue:default_task_queue_factory", + "../api/transport:webrtc_key_value_config", "../api/video:builtin_video_bitrate_allocator_factory", "../api/video:video_frame", "../api/video:video_rtp_headers", @@ -2084,6 +2092,7 @@ if (rtc_include_tests && !build_with_chromium) { "../rtc_base/synchronization:mutex", "../rtc_base/task_utils:repeating_task", "../rtc_base/third_party/sigslot", + "../test:scoped_key_value_config", "../test:test_support", "../test:video_test_common", ] diff --git a/pc/channel_manager_unittest.cc b/pc/channel_manager_unittest.cc index 9503243a09..7d7d3e475f 100644 --- a/pc/channel_manager_unittest.cc +++ b/pc/channel_manager_unittest.cc @@ -24,6 +24,7 @@ #include "rtc_base/location.h" #include "rtc_base/thread.h" #include "test/gtest.h" +#include "test/scoped_key_value_config.h" namespace cricket { namespace { @@ -88,6 +89,7 @@ class ChannelManagerTest : public ::testing::Test { video_bitrate_allocator_factory_; std::unique_ptr cm_; cricket::FakeCall fake_call_; + webrtc::test::ScopedKeyValueConfig field_trials_; }; TEST_F(ChannelManagerTest, SetVideoRtxEnabled) { @@ -123,7 +125,8 @@ TEST_F(ChannelManagerTest, CreateDestroyChannels) { "fake_dtls_transport", cricket::ICE_CANDIDATE_COMPONENT_RTP, network_.get()); auto dtls_srtp_transport = std::make_unique( - /*rtcp_mux_required=*/true); + /*rtcp_mux_required=*/true, field_trials_); + network_->Invoke( RTC_FROM_HERE, [&rtp_dtls_transport, &dtls_srtp_transport] { dtls_srtp_transport->SetDtlsTransports(rtp_dtls_transport.get(), diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc index 3bf9556403..463f60bf35 100644 --- a/pc/channel_unittest.cc +++ b/pc/channel_unittest.cc @@ -46,6 +46,7 @@ #include "rtc_base/task_utils/to_queued_task.h" #include "test/gmock.h" #include "test/gtest.h" +#include "test/scoped_key_value_config.h" using cricket::DtlsTransportInternal; using cricket::FakeVoiceMediaChannel; @@ -317,7 +318,7 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { cricket::DtlsTransportInternal* rtp_dtls_transport, cricket::DtlsTransportInternal* rtcp_dtls_transport) { auto dtls_srtp_transport = std::make_unique( - rtcp_dtls_transport == nullptr); + rtcp_dtls_transport == nullptr, field_trials_); network_thread_->Invoke( RTC_FROM_HERE, @@ -1441,6 +1442,7 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { rtc::Buffer rtcp_packet_; cricket::CandidatePairInterface* last_selected_candidate_pair_; rtc::UniqueRandomIdGenerator ssrc_generator_; + webrtc::test::ScopedKeyValueConfig field_trials_; }; template <> diff --git a/pc/connection_context.cc b/pc/connection_context.cc index d093ee3cf5..31810bca62 100644 --- a/pc/connection_context.cc +++ b/pc/connection_context.cc @@ -73,7 +73,8 @@ rtc::Thread* MaybeWrapThread(rtc::Thread* signaling_thread, std::unique_ptr MaybeCreateSctpFactory( std::unique_ptr factory, - rtc::Thread* network_thread) { + rtc::Thread* network_thread, + const WebRtcKeyValueConfig& field_trials) { if (factory) { return factory; } @@ -102,15 +103,15 @@ ConnectionContext::ConnectionContext( owned_worker_thread_)), signaling_thread_(MaybeWrapThread(dependencies->signaling_thread, wraps_current_thread_)), + trials_(dependencies->trials ? std::move(dependencies->trials) + : std::make_unique()), network_monitor_factory_( std::move(dependencies->network_monitor_factory)), call_factory_(std::move(dependencies->call_factory)), sctp_factory_( MaybeCreateSctpFactory(std::move(dependencies->sctp_factory), - network_thread())), - trials_(dependencies->trials - ? std::move(dependencies->trials) - : std::make_unique()) { + network_thread(), + *trials_.get())) { signaling_thread_->AllowInvokesToThread(worker_thread_); signaling_thread_->AllowInvokesToThread(network_thread_); worker_thread_->AllowInvokesToThread(network_thread_); diff --git a/pc/connection_context.h b/pc/connection_context.h index 2aaa840df1..05f838fac4 100644 --- a/pc/connection_context.h +++ b/pc/connection_context.h @@ -112,6 +112,10 @@ class ConnectionContext final rtc::Thread* const network_thread_; rtc::Thread* const worker_thread_; rtc::Thread* const signaling_thread_; + + // Accessed both on signaling thread and worker thread. + std::unique_ptr const trials_; + // channel_manager is accessed both on signaling thread and worker thread. std::unique_ptr channel_manager_; std::unique_ptr const network_monitor_factory_ @@ -124,8 +128,6 @@ class ConnectionContext final std::unique_ptr default_socket_factory_ RTC_GUARDED_BY(signaling_thread_); std::unique_ptr const sctp_factory_; - // Accessed both on signaling thread and worker thread. - std::unique_ptr const trials_; }; } // namespace webrtc diff --git a/pc/data_channel_integrationtest.cc b/pc/data_channel_integrationtest.cc index c76a4d97b0..6d96251ac1 100644 --- a/pc/data_channel_integrationtest.cc +++ b/pc/data_channel_integrationtest.cc @@ -39,7 +39,6 @@ #include "rtc_base/logging.h" #include "rtc_base/numerics/safe_conversions.h" #include "rtc_base/virtual_socket_server.h" -#include "system_wrappers/include/field_trial.h" #include "test/gmock.h" #include "test/gtest.h" @@ -845,7 +844,7 @@ TEST_P(DataChannelIntegrationTest, EXPECT_GT(202u, callee()->data_observer()->received_message_count()); EXPECT_LE(2u, callee()->data_observer()->received_message_count()); // Then, check that observed behavior (lose some messages) has not changed - if (!webrtc::field_trial::IsDisabled("WebRTC-DataChannel-Dcsctp")) { + if (!trials().IsDisabled("WebRTC-DataChannel-Dcsctp")) { // DcSctp loses all messages. This is correct. EXPECT_EQ(2u, callee()->data_observer()->received_message_count()); } else { diff --git a/pc/dtls_srtp_transport.cc b/pc/dtls_srtp_transport.cc index 9a4135bb2f..9ec14f530b 100644 --- a/pc/dtls_srtp_transport.cc +++ b/pc/dtls_srtp_transport.cc @@ -27,8 +27,9 @@ static const char kDtlsSrtpExporterLabel[] = "EXTRACTOR-dtls_srtp"; namespace webrtc { -DtlsSrtpTransport::DtlsSrtpTransport(bool rtcp_mux_enabled) - : SrtpTransport(rtcp_mux_enabled) {} +DtlsSrtpTransport::DtlsSrtpTransport(bool rtcp_mux_enabled, + const WebRtcKeyValueConfig& field_trials) + : SrtpTransport(rtcp_mux_enabled, field_trials) {} void DtlsSrtpTransport::SetDtlsTransports( cricket::DtlsTransportInternal* rtp_dtls_transport, diff --git a/pc/dtls_srtp_transport.h b/pc/dtls_srtp_transport.h index c2c51c22f2..2ee6e02e30 100644 --- a/pc/dtls_srtp_transport.h +++ b/pc/dtls_srtp_transport.h @@ -32,7 +32,8 @@ namespace webrtc { // configures the SrtpSessions in the base class. class DtlsSrtpTransport : public SrtpTransport { public: - explicit DtlsSrtpTransport(bool rtcp_mux_enabled); + DtlsSrtpTransport(bool rtcp_mux_enabled, + const WebRtcKeyValueConfig& field_trials); // Set P2P layer RTP/RTCP DtlsTransports. When using RTCP-muxing, // `rtcp_dtls_transport` is null. diff --git a/pc/dtls_srtp_transport_unittest.cc b/pc/dtls_srtp_transport_unittest.cc index 76d9c30c5e..30f09e740c 100644 --- a/pc/dtls_srtp_transport_unittest.cc +++ b/pc/dtls_srtp_transport_unittest.cc @@ -31,6 +31,7 @@ #include "rtc_base/ssl_identity.h" #include "rtc_base/third_party/sigslot/sigslot.h" #include "test/gtest.h" +#include "test/scoped_key_value_config.h" using cricket::FakeDtlsTransport; using cricket::FakeIceTransport; @@ -59,7 +60,7 @@ class DtlsSrtpTransportTest : public ::testing::Test, FakeDtlsTransport* rtcp_dtls, bool rtcp_mux_enabled) { auto dtls_srtp_transport = - std::make_unique(rtcp_mux_enabled); + std::make_unique(rtcp_mux_enabled, field_trials_); dtls_srtp_transport->SetDtlsTransports(rtp_dtls, rtcp_dtls); @@ -256,6 +257,7 @@ class DtlsSrtpTransportTest : public ::testing::Test, webrtc::TransportObserver transport_observer2_; int sequence_number_ = 0; + webrtc::test::ScopedKeyValueConfig field_trials_; }; // Tests that if RTCP muxing is enabled and transports are set after RTP diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index e63742aaee..6c69130a38 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -62,6 +62,7 @@ JsepTransportController::JsepTransportController( RTC_DCHECK(config_.rtcp_handler); RTC_DCHECK(config_.ice_transport_factory); RTC_DCHECK(config_.on_dtls_handshake_error_); + RTC_DCHECK(config_.field_trials); } JsepTransportController::~JsepTransportController() { @@ -477,8 +478,8 @@ JsepTransportController::CreateSdesTransport( cricket::DtlsTransportInternal* rtp_dtls_transport, cricket::DtlsTransportInternal* rtcp_dtls_transport) { RTC_DCHECK_RUN_ON(network_thread_); - auto srtp_transport = - std::make_unique(rtcp_dtls_transport == nullptr); + auto srtp_transport = std::make_unique( + rtcp_dtls_transport == nullptr, *config_.field_trials); RTC_DCHECK(rtp_dtls_transport); srtp_transport->SetRtpPacketTransport(rtp_dtls_transport); if (rtcp_dtls_transport) { @@ -497,7 +498,7 @@ JsepTransportController::CreateDtlsSrtpTransport( cricket::DtlsTransportInternal* rtcp_dtls_transport) { RTC_DCHECK_RUN_ON(network_thread_); auto dtls_srtp_transport = std::make_unique( - rtcp_dtls_transport == nullptr); + rtcp_dtls_transport == nullptr, *config_.field_trials); if (config_.enable_external_auth) { dtls_srtp_transport->EnableExternalAuth(); } diff --git a/pc/jsep_transport_controller.h b/pc/jsep_transport_controller.h index ed4d20ba84..44d5981684 100644 --- a/pc/jsep_transport_controller.h +++ b/pc/jsep_transport_controller.h @@ -137,6 +137,9 @@ class JsepTransportController : public sigslot::has_slots<> { // Factory for SCTP transports. SctpTransportFactoryInterface* sctp_factory = nullptr; std::function on_dtls_handshake_error_; + + // Field trials. + const webrtc::WebRtcKeyValueConfig* field_trials; }; // The ICE related events are fired on the `network_thread`. diff --git a/pc/jsep_transport_controller_unittest.cc b/pc/jsep_transport_controller_unittest.cc index 622a9b90e3..ee306f093a 100644 --- a/pc/jsep_transport_controller_unittest.cc +++ b/pc/jsep_transport_controller_unittest.cc @@ -33,6 +33,7 @@ #include "rtc_base/ssl_identity.h" #include "rtc_base/thread.h" #include "test/gtest.h" +#include "test/scoped_key_value_config.h" using cricket::Candidate; using cricket::Candidates; @@ -100,6 +101,7 @@ class JsepTransportControllerTest : public JsepTransportController::Observer, config.ice_transport_factory = fake_ice_transport_factory_.get(); config.dtls_transport_factory = fake_dtls_transport_factory_.get(); config.on_dtls_handshake_error_ = [](rtc::SSLHandshakeError s) {}; + config.field_trials = &field_trials_; transport_controller_ = std::make_unique( network_thread, port_allocator, nullptr /* async_resolver_factory */, config); @@ -378,6 +380,7 @@ class JsepTransportControllerTest : public JsepTransportController::Observer, // Transport controller needs to be destroyed first, because it may issue // callbacks that modify the changed_*_by_mid in the destructor. std::unique_ptr transport_controller_; + webrtc::test::ScopedKeyValueConfig field_trials_; }; TEST_F(JsepTransportControllerTest, GetRtpTransport) { diff --git a/pc/jsep_transport_unittest.cc b/pc/jsep_transport_unittest.cc index 2ec96d2af7..c63dc496db 100644 --- a/pc/jsep_transport_unittest.cc +++ b/pc/jsep_transport_unittest.cc @@ -37,6 +37,7 @@ #include "rtc_base/ssl_identity.h" #include "rtc_base/third_party/sigslot/sigslot.h" #include "test/gtest.h" +#include "test/scoped_key_value_config.h" namespace cricket { namespace { @@ -89,7 +90,7 @@ class JsepTransport2Test : public ::testing::Test, public sigslot::has_slots<> { rtc::PacketTransportInternal* rtp_packet_transport, rtc::PacketTransportInternal* rtcp_packet_transport) { auto srtp_transport = std::make_unique( - rtcp_packet_transport == nullptr); + rtcp_packet_transport == nullptr, field_trials_); srtp_transport->SetRtpPacketTransport(rtp_packet_transport); if (rtcp_packet_transport) { @@ -102,7 +103,7 @@ class JsepTransport2Test : public ::testing::Test, public sigslot::has_slots<> { cricket::DtlsTransportInternal* rtp_dtls_transport, cricket::DtlsTransportInternal* rtcp_dtls_transport) { auto dtls_srtp_transport = std::make_unique( - rtcp_dtls_transport == nullptr); + rtcp_dtls_transport == nullptr, field_trials_); dtls_srtp_transport->SetDtlsTransports(rtp_dtls_transport, rtcp_dtls_transport); return dtls_srtp_transport; @@ -192,6 +193,8 @@ class JsepTransport2Test : public ::testing::Test, public sigslot::has_slots<> { // The SrtpTransport is owned by `jsep_transport_`. Keep a raw pointer here // for testing. webrtc::SrtpTransport* sdes_transport_ = nullptr; + + webrtc::test::ScopedKeyValueConfig field_trials_; }; // The parameterized tests cover both cases when RTCP mux is enable and diff --git a/pc/media_session.cc b/pc/media_session.cc index 2a3a698874..9aeae708d7 100644 --- a/pc/media_session.cc +++ b/pc/media_session.cc @@ -39,7 +39,6 @@ #include "rtc_base/string_encode.h" #include "rtc_base/third_party/base64/base64.h" #include "rtc_base/unique_id_generator.h" -#include "system_wrappers/include/field_trial.h" namespace { @@ -301,7 +300,8 @@ static StreamParams CreateStreamParamsForNewSenderWithSsrcs( const std::string& rtcp_cname, bool include_rtx_streams, bool include_flexfec_stream, - UniqueRandomIdGenerator* ssrc_generator) { + UniqueRandomIdGenerator* ssrc_generator, + const webrtc::WebRtcKeyValueConfig& field_trials) { StreamParams result; result.id = sender.track_id; @@ -313,8 +313,7 @@ static StreamParams CreateStreamParamsForNewSenderWithSsrcs( "a single media streams. This session has multiple " "media streams however, so no FlexFEC SSRC will be generated."; } - if (include_flexfec_stream && - !webrtc::field_trial::IsEnabled("WebRTC-FlexFEC-03")) { + if (include_flexfec_stream && !field_trials.IsEnabled("WebRTC-FlexFEC-03")) { include_flexfec_stream = false; RTC_LOG(LS_WARNING) << "WebRTC-FlexFEC trial is not enabled, not sending FlexFEC"; @@ -396,12 +395,12 @@ static void AddSimulcastToMediaDescription( // content_description. // `current_params` - All currently known StreamParams of any media type. template -static bool AddStreamParams( - const std::vector& sender_options, - const std::string& rtcp_cname, - UniqueRandomIdGenerator* ssrc_generator, - StreamParamsVec* current_streams, - MediaContentDescriptionImpl* content_description) { +static bool AddStreamParams(const std::vector& sender_options, + const std::string& rtcp_cname, + UniqueRandomIdGenerator* ssrc_generator, + StreamParamsVec* current_streams, + MediaContentDescriptionImpl* content_description, + const webrtc::WebRtcKeyValueConfig& field_trials) { // SCTP streams are not negotiated using SDP/ContentDescriptions. if (IsSctpProtocol(content_description->protocol())) { return true; @@ -423,7 +422,7 @@ static bool AddStreamParams( // Signal SSRCs and legacy simulcast (if requested). CreateStreamParamsForNewSenderWithSsrcs( sender, rtcp_cname, include_rtx_streams, - include_flexfec_stream, ssrc_generator) + include_flexfec_stream, ssrc_generator, field_trials) : // Signal RIDs and spec-compliant simulcast (if requested). CreateStreamParamsForNewSenderWithRids(sender, rtcp_cname); @@ -711,11 +710,12 @@ static bool CreateMediaContentOffer( const RtpHeaderExtensions& rtp_extensions, UniqueRandomIdGenerator* ssrc_generator, StreamParamsVec* current_streams, - MediaContentDescriptionImpl* offer) { + MediaContentDescriptionImpl* offer, + const webrtc::WebRtcKeyValueConfig& field_trials) { offer->AddCodecs(codecs); if (!AddStreamParams(media_description_options.sender_options, session_options.rtcp_cname, ssrc_generator, - current_streams, offer)) { + current_streams, offer, field_trials)) { return false; } @@ -1351,7 +1351,8 @@ static bool SetCodecsInAnswer( const MediaSessionOptions& session_options, UniqueRandomIdGenerator* ssrc_generator, StreamParamsVec* current_streams, - MediaContentDescriptionImpl* answer) { + MediaContentDescriptionImpl* answer, + const webrtc::WebRtcKeyValueConfig& field_trials) { std::vector negotiated_codecs; NegotiateCodecs(local_codecs, offer->codecs(), &negotiated_codecs, media_description_options.codec_preferences.empty()); @@ -1359,7 +1360,7 @@ static bool SetCodecsInAnswer( answer->set_protocol(offer->protocol()); if (!AddStreamParams(media_description_options.sender_options, session_options.rtcp_cname, ssrc_generator, - current_streams, answer)) { + current_streams, answer, field_trials)) { return false; // Something went seriously wrong. } return true; @@ -2325,11 +2326,11 @@ bool MediaSessionDescriptionFactory::AddAudioContentForOffer( std::vector crypto_suites; GetSupportedAudioSdesCryptoSuiteNames(session_options.crypto_options, &crypto_suites); - if (!CreateMediaContentOffer(media_description_options, session_options, - filtered_codecs, sdes_policy, - GetCryptos(current_content), crypto_suites, - audio_rtp_extensions, ssrc_generator_, - current_streams, audio.get())) { + if (!CreateMediaContentOffer( + media_description_options, session_options, filtered_codecs, + sdes_policy, GetCryptos(current_content), crypto_suites, + audio_rtp_extensions, ssrc_generator_, current_streams, audio.get(), + transport_desc_factory_->trials())) { return false; } @@ -2434,11 +2435,11 @@ bool MediaSessionDescriptionFactory::AddVideoContentForOffer( std::vector crypto_suites; GetSupportedVideoSdesCryptoSuiteNames(session_options.crypto_options, &crypto_suites); - if (!CreateMediaContentOffer(media_description_options, session_options, - filtered_codecs, sdes_policy, - GetCryptos(current_content), crypto_suites, - video_rtp_extensions, ssrc_generator_, - current_streams, video.get())) { + if (!CreateMediaContentOffer( + media_description_options, session_options, filtered_codecs, + sdes_policy, GetCryptos(current_content), crypto_suites, + video_rtp_extensions, ssrc_generator_, current_streams, video.get(), + transport_desc_factory_->trials())) { return false; } @@ -2622,8 +2623,8 @@ bool MediaSessionDescriptionFactory::AddAudioContentForAnswer( audio_transport->secure() ? cricket::SEC_DISABLED : secure(); if (!SetCodecsInAnswer(offer_audio_description, filtered_codecs, media_description_options, session_options, - ssrc_generator_, current_streams, - audio_answer.get())) { + ssrc_generator_, current_streams, audio_answer.get(), + transport_desc_factory_->trials())) { return false; } if (!CreateMediaContentAnswer( @@ -2748,8 +2749,8 @@ bool MediaSessionDescriptionFactory::AddVideoContentForAnswer( video_transport->secure() ? cricket::SEC_DISABLED : secure(); if (!SetCodecsInAnswer(offer_video_description, filtered_codecs, media_description_options, session_options, - ssrc_generator_, current_streams, - video_answer.get())) { + ssrc_generator_, current_streams, video_answer.get(), + transport_desc_factory_->trials())) { return false; } if (!CreateMediaContentAnswer( diff --git a/pc/media_session.h b/pc/media_session.h index aa24f015de..d6c7474fe6 100644 --- a/pc/media_session.h +++ b/pc/media_session.h @@ -22,6 +22,7 @@ #include "api/media_types.h" #include "api/rtp_parameters.h" #include "api/rtp_transceiver_direction.h" +#include "api/webrtc_key_value_config.h" #include "media/base/media_constants.h" #include "media/base/rid_description.h" #include "media/base/stream_params.h" @@ -35,6 +36,13 @@ #include "pc/simulcast_description.h" #include "rtc_base/unique_id_generator.h" +namespace webrtc { + +// Forward declaration due to circular dependecy. +class ConnectionContext; + +} // namespace webrtc + namespace cricket { class ChannelManager; diff --git a/pc/media_session_unittest.cc b/pc/media_session_unittest.cc index 6bc842d222..f06ddc7586 100644 --- a/pc/media_session_unittest.cc +++ b/pc/media_session_unittest.cc @@ -43,9 +43,9 @@ #include "rtc_base/string_encode.h" #include "rtc_base/strings/string_builder.h" #include "rtc_base/unique_id_generator.h" -#include "test/field_trial.h" #include "test/gmock.h" #include "test/gtest.h" +#include "test/scoped_key_value_config.h" #define ASSERT_CRYPTO(cd, s, cs) \ ASSERT_EQ(s, cd->cryptos().size()); \ @@ -431,7 +431,10 @@ void PreferGcmCryptoParameters(CryptoParamsVec* cryptos) { class MediaSessionDescriptionFactoryTest : public ::testing::Test { public: MediaSessionDescriptionFactoryTest() - : f1_(&tdf1_, &ssrc_generator1), f2_(&tdf2_, &ssrc_generator2) { + : tdf1_(field_trials), + tdf2_(field_trials), + f1_(&tdf1_, &ssrc_generator1), + f2_(&tdf2_, &ssrc_generator2) { f1_.set_audio_codecs(MAKE_VECTOR(kAudioCodecs1), MAKE_VECTOR(kAudioCodecs1)); f1_.set_video_codecs(MAKE_VECTOR(kVideoCodecs1), @@ -791,12 +794,13 @@ class MediaSessionDescriptionFactoryTest : public ::testing::Test { } protected: + webrtc::test::ScopedKeyValueConfig field_trials; UniqueRandomIdGenerator ssrc_generator1; UniqueRandomIdGenerator ssrc_generator2; - MediaSessionDescriptionFactory f1_; - MediaSessionDescriptionFactory f2_; TransportDescriptionFactory tdf1_; TransportDescriptionFactory tdf2_; + MediaSessionDescriptionFactory f1_; + MediaSessionDescriptionFactory f2_; }; // Create a typical audio offer, and ensure it matches what we expect. @@ -3260,8 +3264,8 @@ TEST_F(MediaSessionDescriptionFactoryTest, SimSsrcsGenerateMultipleRtxSsrcs) { // Test that, when the FlexFEC codec is added, a FlexFEC ssrc is created // together with a FEC-FR grouping. Guarded by WebRTC-FlexFEC-03 trial. TEST_F(MediaSessionDescriptionFactoryTest, GenerateFlexfecSsrc) { - webrtc::test::ScopedFieldTrials override_field_trials( - "WebRTC-FlexFEC-03/Enabled/"); + webrtc::test::ScopedKeyValueConfig override_field_trials( + field_trials, "WebRTC-FlexFEC-03/Enabled/"); MediaSessionOptions opts; AddMediaDescriptionOptions(MEDIA_TYPE_VIDEO, "video", RtpTransceiverDirection::kSendRecv, kActive, @@ -3303,8 +3307,8 @@ TEST_F(MediaSessionDescriptionFactoryTest, GenerateFlexfecSsrc) { // TODO(brandtr): Remove this test when we support simulcast, either through // multiple FlexfecSenders, or through multistream protection. TEST_F(MediaSessionDescriptionFactoryTest, SimSsrcsGenerateNoFlexfecSsrcs) { - webrtc::test::ScopedFieldTrials override_field_trials( - "WebRTC-FlexFEC-03/Enabled/"); + webrtc::test::ScopedKeyValueConfig override_field_trials( + field_trials, "WebRTC-FlexFEC-03/Enabled/"); MediaSessionOptions opts; AddMediaDescriptionOptions(MEDIA_TYPE_VIDEO, "video", RtpTransceiverDirection::kSendRecv, kActive, @@ -4330,7 +4334,10 @@ TEST_F(MediaSessionDescriptionFactoryTest, class MediaProtocolTest : public ::testing::TestWithParam { public: MediaProtocolTest() - : f1_(&tdf1_, &ssrc_generator1), f2_(&tdf2_, &ssrc_generator2) { + : tdf1_(field_trials_), + tdf2_(field_trials_), + f1_(&tdf1_, &ssrc_generator1), + f2_(&tdf2_, &ssrc_generator2) { f1_.set_audio_codecs(MAKE_VECTOR(kAudioCodecs1), MAKE_VECTOR(kAudioCodecs1)); f1_.set_video_codecs(MAKE_VECTOR(kVideoCodecs1), @@ -4350,10 +4357,11 @@ class MediaProtocolTest : public ::testing::TestWithParam { } protected: - MediaSessionDescriptionFactory f1_; - MediaSessionDescriptionFactory f2_; + webrtc::test::ScopedKeyValueConfig field_trials_; TransportDescriptionFactory tdf1_; TransportDescriptionFactory tdf2_; + MediaSessionDescriptionFactory f1_; + MediaSessionDescriptionFactory f2_; UniqueRandomIdGenerator ssrc_generator1; UniqueRandomIdGenerator ssrc_generator2; }; @@ -4389,7 +4397,8 @@ INSTANTIATE_TEST_SUITE_P(MediaProtocolDtlsPatternTest, ::testing::ValuesIn(kMediaProtocolsDtls)); TEST_F(MediaSessionDescriptionFactoryTest, TestSetAudioCodecs) { - TransportDescriptionFactory tdf; + webrtc::test::ScopedKeyValueConfig field_trials; + TransportDescriptionFactory tdf(field_trials); UniqueRandomIdGenerator ssrc_generator; MediaSessionDescriptionFactory sf(&tdf, &ssrc_generator); std::vector send_codecs = MAKE_VECTOR(kAudioCodecs1); @@ -4459,7 +4468,8 @@ bool CodecsMatch(const std::vector& codecs1, } void TestAudioCodecsOffer(RtpTransceiverDirection direction) { - TransportDescriptionFactory tdf; + webrtc::test::ScopedKeyValueConfig field_trials; + TransportDescriptionFactory tdf(field_trials); UniqueRandomIdGenerator ssrc_generator; MediaSessionDescriptionFactory sf(&tdf, &ssrc_generator); const std::vector send_codecs = MAKE_VECTOR(kAudioCodecs1); @@ -4556,11 +4566,13 @@ std::vector VectorFromIndices(const T* array, const int (&indices)[IDXS]) { void TestAudioCodecsAnswer(RtpTransceiverDirection offer_direction, RtpTransceiverDirection answer_direction, bool add_legacy_stream) { - TransportDescriptionFactory offer_tdf; - TransportDescriptionFactory answer_tdf; + webrtc::test::ScopedKeyValueConfig field_trials; + TransportDescriptionFactory offer_tdf(field_trials); + TransportDescriptionFactory answer_tdf(field_trials); UniqueRandomIdGenerator ssrc_generator1, ssrc_generator2; MediaSessionDescriptionFactory offer_factory(&offer_tdf, &ssrc_generator1); MediaSessionDescriptionFactory answer_factory(&answer_tdf, &ssrc_generator2); + offer_factory.set_audio_codecs( VectorFromIndices(kOfferAnswerCodecs, kOfferSendCodecs), VectorFromIndices(kOfferAnswerCodecs, kOfferRecvCodecs)); diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index a315ac510f..ec6bef32f7 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -711,6 +711,8 @@ JsepTransportController* PeerConnection::InitializeTransportController_n( } }; + config.field_trials = &context_->trials(); + transport_controller_.reset( new JsepTransportController(network_thread(), port_allocator_.get(), async_dns_resolver_factory_.get(), config)); diff --git a/pc/peer_connection.h b/pc/peer_connection.h index cd4af9e420..a6e4510b47 100644 --- a/pc/peer_connection.h +++ b/pc/peer_connection.h @@ -451,6 +451,8 @@ class PeerConnection : public PeerConnectionInternal, } void RequestUsagePatternReportForTesting(); + const WebRtcKeyValueConfig& trials() override { return context_->trials(); } + protected: // Available for rtc::scoped_refptr creation PeerConnection(rtc::scoped_refptr context, diff --git a/pc/peer_connection_internal.h b/pc/peer_connection_internal.h index 16caade6c9..eaff20a0d3 100644 --- a/pc/peer_connection_internal.h +++ b/pc/peer_connection_internal.h @@ -178,6 +178,8 @@ class PeerConnectionInternal : public PeerConnectionInterface, virtual void NoteDataAddedEvent() {} // Handler for the "channel closed" signal virtual void OnSctpDataChannelClosed(DataChannelInterface* channel) {} + + virtual const WebRtcKeyValueConfig& trials() = 0; }; } // namespace webrtc diff --git a/pc/rtp_sender_receiver_unittest.cc b/pc/rtp_sender_receiver_unittest.cc index c0b09e39c3..a7b9c9ab68 100644 --- a/pc/rtp_sender_receiver_unittest.cc +++ b/pc/rtp_sender_receiver_unittest.cc @@ -70,6 +70,7 @@ #include "test/gmock.h" #include "test/gtest.h" #include "test/run_loop.h" +#include "test/scoped_key_value_config.h" using ::testing::_; using ::testing::ContainerEq; @@ -187,7 +188,7 @@ class RtpSenderReceiverTest std::unique_ptr CreateDtlsSrtpTransport() { auto dtls_srtp_transport = std::make_unique( - /*rtcp_mux_required=*/true); + /*rtcp_mux_required=*/true, field_trials_); dtls_srtp_transport->SetDtlsTransports(rtp_dtls_transport_.get(), /*rtcp_dtls_transport=*/nullptr); return dtls_srtp_transport; @@ -544,6 +545,7 @@ class RtpSenderReceiverTest rtc::scoped_refptr video_track_; rtc::scoped_refptr audio_track_; bool audio_sender_destroyed_signal_fired_ = false; + webrtc::test::ScopedKeyValueConfig field_trials_; }; // Test that `voice_channel_` is updated when an audio track is associated diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index 69588350ab..a9501caa5e 100644 --- a/pc/sdp_offer_answer.cc +++ b/pc/sdp_offer_answer.cc @@ -63,7 +63,6 @@ #include "rtc_base/string_encode.h" #include "rtc_base/strings/string_builder.h" #include "rtc_base/trace_event.h" -#include "system_wrappers/include/field_trial.h" #include "system_wrappers/include/metrics.h" using cricket::ContentInfo; @@ -1186,13 +1185,14 @@ std::unique_ptr SdpOfferAnswerHandler::Create( PeerConnectionDependencies& dependencies, ConnectionContext* context) { auto handler = absl::WrapUnique(new SdpOfferAnswerHandler(pc, context)); - handler->Initialize(configuration, dependencies); + handler->Initialize(configuration, dependencies, context); return handler; } void SdpOfferAnswerHandler::Initialize( const PeerConnectionInterface::RTCConfiguration& configuration, - PeerConnectionDependencies& dependencies) { + PeerConnectionDependencies& dependencies, + ConnectionContext* context) { RTC_DCHECK_RUN_ON(signaling_thread()); video_options_.screencast_min_bitrate_kbps = configuration.screencast_min_bitrate; @@ -1222,9 +1222,8 @@ void SdpOfferAnswerHandler::Initialize( webrtc_session_desc_factory_ = std::make_unique( - signaling_thread(), channel_manager(), this, pc_->session_id(), - pc_->dtls_enabled(), std::move(dependencies.cert_generator), - certificate, + context, this, pc_->session_id(), pc_->dtls_enabled(), + std::move(dependencies.cert_generator), certificate, [this](const rtc::scoped_refptr& certificate) { RTC_DCHECK_RUN_ON(signaling_thread()); transport_controller_s()->SetLocalCertificate(certificate); diff --git a/pc/sdp_offer_answer.h b/pc/sdp_offer_answer.h index 67ead47242..a32ece930b 100644 --- a/pc/sdp_offer_answer.h +++ b/pc/sdp_offer_answer.h @@ -220,7 +220,8 @@ class SdpOfferAnswerHandler : public SdpStateProvider, // once. Modifies dependencies. void Initialize( const PeerConnectionInterface::RTCConfiguration& configuration, - PeerConnectionDependencies& dependencies); + PeerConnectionDependencies& dependencies, + ConnectionContext* context); rtc::Thread* signaling_thread() const; rtc::Thread* network_thread() const; diff --git a/pc/srtp_session.cc b/pc/srtp_session.cc index a81f2415a5..b5b244265c 100644 --- a/pc/srtp_session.cc +++ b/pc/srtp_session.cc @@ -18,6 +18,7 @@ #include "absl/base/attributes.h" #include "absl/base/const_init.h" #include "api/array_view.h" +#include "api/webrtc_key_value_config.h" #include "modules/rtp_rtcp/source/rtp_util.h" #include "pc/external_hmac.h" #include "rtc_base/byte_order.h" @@ -26,7 +27,6 @@ #include "rtc_base/ssl_stream_adapter.h" #include "rtc_base/string_encode.h" #include "rtc_base/time_utils.h" -#include "system_wrappers/include/field_trial.h" #include "system_wrappers/include/metrics.h" #include "third_party/libsrtp/include/srtp.h" #include "third_party/libsrtp/include/srtp_priv.h" @@ -40,8 +40,10 @@ using ::webrtc::ParseRtpSequenceNumber; // in srtp.h. constexpr int kSrtpErrorCodeBoundary = 28; -SrtpSession::SrtpSession() { - dump_plain_rtp_ = webrtc::field_trial::IsEnabled("WebRTC-Debugging-RtpDump"); +SrtpSession::SrtpSession() {} + +SrtpSession::SrtpSession(const webrtc::WebRtcKeyValueConfig& field_trials) { + dump_plain_rtp_ = field_trials.IsEnabled("WebRTC-Debugging-RtpDump"); } SrtpSession::~SrtpSession() { diff --git a/pc/srtp_session.h b/pc/srtp_session.h index d88eaae319..c3979aae7a 100644 --- a/pc/srtp_session.h +++ b/pc/srtp_session.h @@ -18,6 +18,7 @@ #include "api/scoped_refptr.h" #include "api/sequence_checker.h" +#include "api/webrtc_key_value_config.h" #include "rtc_base/synchronization/mutex.h" // Forward declaration to avoid pulling in libsrtp headers here @@ -35,6 +36,7 @@ void ProhibitLibsrtpInitialization(); class SrtpSession { public: SrtpSession(); + explicit SrtpSession(const webrtc::WebRtcKeyValueConfig& field_trials); ~SrtpSession(); SrtpSession(const SrtpSession&) = delete; diff --git a/pc/srtp_session_unittest.cc b/pc/srtp_session_unittest.cc index dc08c2e908..16a840a307 100644 --- a/pc/srtp_session_unittest.cc +++ b/pc/srtp_session_unittest.cc @@ -21,6 +21,7 @@ #include "system_wrappers/include/metrics.h" #include "test/gmock.h" #include "test/gtest.h" +#include "test/scoped_key_value_config.h" #include "third_party/libsrtp/include/srtp.h" using ::testing::ElementsAre; @@ -32,7 +33,9 @@ std::vector kEncryptedHeaderExtensionIds; class SrtpSessionTest : public ::testing::Test { public: - SrtpSessionTest() { webrtc::metrics::Reset(); } + SrtpSessionTest() : s1_(field_trials_), s2_(field_trials_) { + webrtc::metrics::Reset(); + } protected: virtual void SetUp() { @@ -69,6 +72,7 @@ class SrtpSessionTest : public ::testing::Test { EXPECT_EQ(expected_len, out_len); EXPECT_EQ(0, memcmp(rtcp_packet_, kRtcpReport, out_len)); } + webrtc::test::ScopedKeyValueConfig field_trials_; cricket::SrtpSession s1_; cricket::SrtpSession s2_; char rtp_packet_[sizeof(kPcmuFrame) + 10]; diff --git a/pc/srtp_transport.cc b/pc/srtp_transport.cc index 230c1a347b..b59fc1d1d2 100644 --- a/pc/srtp_transport.cc +++ b/pc/srtp_transport.cc @@ -34,8 +34,9 @@ namespace webrtc { -SrtpTransport::SrtpTransport(bool rtcp_mux_enabled) - : RtpTransport(rtcp_mux_enabled) {} +SrtpTransport::SrtpTransport(bool rtcp_mux_enabled, + const WebRtcKeyValueConfig& field_trials) + : RtpTransport(rtcp_mux_enabled), field_trials_(field_trials) {} RTCError SrtpTransport::SetSrtpSendKey(const cricket::CryptoParams& params) { if (send_params_) { @@ -324,13 +325,13 @@ bool SrtpTransport::SetRtcpParams(int send_cs, return false; } - send_rtcp_session_.reset(new cricket::SrtpSession()); + send_rtcp_session_.reset(new cricket::SrtpSession(field_trials_)); if (!send_rtcp_session_->SetSend(send_cs, send_key, send_key_len, send_extension_ids)) { return false; } - recv_rtcp_session_.reset(new cricket::SrtpSession()); + recv_rtcp_session_.reset(new cricket::SrtpSession(field_trials_)); if (!recv_rtcp_session_->SetRecv(recv_cs, recv_key, recv_key_len, recv_extension_ids)) { return false; @@ -361,8 +362,8 @@ void SrtpTransport::ResetParams() { } void SrtpTransport::CreateSrtpSessions() { - send_session_.reset(new cricket::SrtpSession()); - recv_session_.reset(new cricket::SrtpSession()); + send_session_.reset(new cricket::SrtpSession(field_trials_)); + recv_session_.reset(new cricket::SrtpSession(field_trials_)); if (external_auth_enabled_) { send_session_->EnableExternalAuth(); } diff --git a/pc/srtp_transport.h b/pc/srtp_transport.h index 4bc028d68e..2fec29eb01 100644 --- a/pc/srtp_transport.h +++ b/pc/srtp_transport.h @@ -21,6 +21,7 @@ #include "absl/types/optional.h" #include "api/crypto_params.h" #include "api/rtc_error.h" +#include "api/webrtc_key_value_config.h" #include "p2p/base/packet_transport_internal.h" #include "pc/rtp_transport.h" #include "pc/srtp_session.h" @@ -36,7 +37,8 @@ namespace webrtc { // parameters for the SrtpSession underneath. class SrtpTransport : public RtpTransport { public: - explicit SrtpTransport(bool rtcp_mux_enabled); + SrtpTransport(bool rtcp_mux_enabled, + const WebRtcKeyValueConfig& field_trials); virtual ~SrtpTransport() = default; @@ -167,6 +169,8 @@ class SrtpTransport : public RtpTransport { int rtp_abs_sendtime_extn_id_ = -1; int decryption_failure_count_ = 0; + + const WebRtcKeyValueConfig& field_trials_; }; } // namespace webrtc diff --git a/pc/srtp_transport_unittest.cc b/pc/srtp_transport_unittest.cc index 980ebca08a..4e643935a9 100644 --- a/pc/srtp_transport_unittest.cc +++ b/pc/srtp_transport_unittest.cc @@ -27,6 +27,7 @@ #include "rtc_base/ssl_stream_adapter.h" #include "rtc_base/third_party/sigslot/sigslot.h" #include "test/gtest.h" +#include "test/scoped_key_value_config.h" using rtc::kSrtpAeadAes128Gcm; using rtc::kTestKey1; @@ -57,8 +58,10 @@ class SrtpTransportTest : public ::testing::Test, public sigslot::has_slots<> { rtp_packet_transport1_->SetDestination(rtp_packet_transport2_.get(), asymmetric); - srtp_transport1_ = std::make_unique(rtcp_mux_enabled); - srtp_transport2_ = std::make_unique(rtcp_mux_enabled); + srtp_transport1_ = + std::make_unique(rtcp_mux_enabled, field_trials_); + srtp_transport2_ = + std::make_unique(rtcp_mux_enabled, field_trials_); srtp_transport1_->SetRtpPacketTransport(rtp_packet_transport1_.get()); srtp_transport2_->SetRtpPacketTransport(rtp_packet_transport2_.get()); @@ -333,6 +336,7 @@ class SrtpTransportTest : public ::testing::Test, public sigslot::has_slots<> { TransportObserver rtp_sink2_; int sequence_number_ = 0; + webrtc::test::ScopedKeyValueConfig field_trials_; }; class SrtpTransportTestWithExternalAuth diff --git a/pc/stats_collector.cc b/pc/stats_collector.cc index 6b1cda3ad5..927e993203 100644 --- a/pc/stats_collector.cc +++ b/pc/stats_collector.cc @@ -31,6 +31,7 @@ #include "api/sequence_checker.h" #include "api/video/video_content_type.h" #include "api/video/video_timing.h" +#include "api/webrtc_key_value_config.h" #include "call/call.h" #include "media/base/media_channel.h" #include "modules/audio_processing/include/audio_processing_statistics.h" @@ -55,7 +56,6 @@ #include "rtc_base/thread.h" #include "rtc_base/time_utils.h" #include "rtc_base/trace_event.h" -#include "system_wrappers/include/field_trial.h" namespace webrtc { namespace { @@ -543,7 +543,7 @@ StatsCollector::StatsCollector(PeerConnectionInternal* pc) : pc_(pc), stats_gathering_started_(0), use_standard_bytes_stats_( - webrtc::field_trial::IsEnabled(kUseStandardBytesStats)) { + pc->trials().IsEnabled(kUseStandardBytesStats)) { RTC_DCHECK(pc_); } diff --git a/pc/stats_collector.h b/pc/stats_collector.h index 751a2de09c..794437e9eb 100644 --- a/pc/stats_collector.h +++ b/pc/stats_collector.h @@ -30,6 +30,7 @@ #include "api/peer_connection_interface.h" #include "api/scoped_refptr.h" #include "api/stats_types.h" +#include "api/webrtc_key_value_config.h" #include "p2p/base/connection_info.h" #include "p2p/base/port.h" #include "pc/peer_connection_internal.h" diff --git a/pc/test/fake_peer_connection_base.h b/pc/test/fake_peer_connection_base.h index 7d64ab8180..2b4dd73913 100644 --- a/pc/test/fake_peer_connection_base.h +++ b/pc/test/fake_peer_connection_base.h @@ -18,7 +18,9 @@ #include #include "api/sctp_transport_interface.h" +#include "api/webrtc_key_value_config.h" #include "pc/peer_connection_internal.h" +#include "test/scoped_key_value_config.h" namespace webrtc { @@ -358,7 +360,10 @@ class FakePeerConnectionBase : public PeerConnectionInternal { void SetSctpDataMid(const std::string& mid) override {} void ResetSctpDataMid() override {} + const WebRtcKeyValueConfig& trials() override { return field_trials_; } + protected: + webrtc::test::ScopedKeyValueConfig field_trials_; sigslot::signal1 SignalSctpDataChannelCreated_; }; diff --git a/pc/test/integration_test_helpers.h b/pc/test/integration_test_helpers.h index 99ad8af4b5..00417d96a5 100644 --- a/pc/test/integration_test_helpers.h +++ b/pc/test/integration_test_helpers.h @@ -120,8 +120,8 @@ #include "rtc_base/time_utils.h" #include "rtc_base/virtual_socket_server.h" #include "system_wrappers/include/metrics.h" -#include "test/field_trial.h" #include "test/gmock.h" +#include "test/scoped_key_value_config.h" namespace webrtc { @@ -1366,9 +1366,9 @@ class PeerConnectionIntegrationBaseTest : public ::testing::Test { fss_(new rtc::FirewallSocketServer(ss_.get())), network_thread_(new rtc::Thread(fss_.get())), worker_thread_(rtc::Thread::Create()), - field_trials_(field_trials.has_value() - ? new test::ScopedFieldTrials(*field_trials) - : nullptr) { + // TODO(bugs.webrtc.org/10335): Pass optional ScopedKeyValueConfig. + field_trials_(new test::ScopedKeyValueConfig( + field_trials.has_value() ? *field_trials : "")) { network_thread_->SetName("PCNetworkThread", this); worker_thread_->SetName("PCWorkerThread", this); RTC_CHECK(network_thread_->Start()); @@ -1855,6 +1855,8 @@ class PeerConnectionIntegrationBaseTest : public ::testing::Test { expected_cipher_suite); } + const WebRtcKeyValueConfig& trials() const { return *field_trials_.get(); } + protected: SdpSemantics sdp_semantics_; @@ -1874,7 +1876,7 @@ class PeerConnectionIntegrationBaseTest : public ::testing::Test { std::vector> turn_customizers_; std::unique_ptr caller_; std::unique_ptr callee_; - std::unique_ptr field_trials_; + std::unique_ptr field_trials_; }; } // namespace webrtc diff --git a/pc/webrtc_session_description_factory.cc b/pc/webrtc_session_description_factory.cc index 82ba849544..09f2ee2f00 100644 --- a/pc/webrtc_session_description_factory.cc +++ b/pc/webrtc_session_description_factory.cc @@ -24,6 +24,7 @@ #include "api/jsep_session_description.h" #include "api/rtc_error.h" #include "api/sequence_checker.h" +#include "pc/connection_context.h" #include "pc/sdp_state_provider.h" #include "pc/session_description.h" #include "rtc_base/checks.h" @@ -127,8 +128,7 @@ void WebRtcSessionDescriptionFactory::CopyCandidatesFromSessionDescription( } WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory( - rtc::Thread* signaling_thread, - cricket::ChannelManager* channel_manager, + ConnectionContext* context, const SdpStateProvider* sdp_info, const std::string& session_id, bool dtls_enabled, @@ -136,8 +136,10 @@ WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory( const rtc::scoped_refptr& certificate, std::function&)> on_certificate_ready) - : signaling_thread_(signaling_thread), - session_desc_factory_(channel_manager, &transport_desc_factory_), + : signaling_thread_(context->signaling_thread()), + transport_desc_factory_(context->trials()), + session_desc_factory_(context->channel_manager(), + &transport_desc_factory_), // RFC 4566 suggested a Network Time Protocol (NTP) format timestamp // as the session id and session version. To simplify, it should be fine // to just use a random number as session id and start version from diff --git a/pc/webrtc_session_description_factory.h b/pc/webrtc_session_description_factory.h index 6a6e8efa51..79171f797b 100644 --- a/pc/webrtc_session_description_factory.h +++ b/pc/webrtc_session_description_factory.h @@ -79,8 +79,7 @@ class WebRtcSessionDescriptionFactory : public rtc::MessageHandler, // asynchronously. If a certificate is given, will use that for identifying // over DTLS. If neither is specified, DTLS is disabled. WebRtcSessionDescriptionFactory( - rtc::Thread* signaling_thread, - cricket::ChannelManager* channel_manager, + ConnectionContext* context, const SdpStateProvider* sdp_info, const std::string& session_id, bool dtls_enabled, diff --git a/test/BUILD.gn b/test/BUILD.gn index f49dbd485a..82d909d1ae 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -214,7 +214,6 @@ rtc_library("rtp_test_utils") { rtc_library("field_trial") { visibility = [ "*" ] - testonly = true sources = [ "field_trial.cc", "field_trial.h", @@ -233,9 +232,23 @@ rtc_library("explicit_key_value_config") { ] deps = [ - "../api/transport:webrtc_key_value_config", + ":field_trial", + "../api:webrtc_key_value_config", + "../rtc_base:checks", + ] + absl_deps = [ "//third_party/abseil-cpp/absl/strings:strings" ] +} + +rtc_library("scoped_key_value_config") { + sources = [ + "scoped_key_value_config.cc", + "scoped_key_value_config.h", + ] + + deps = [ + ":field_trial", + "../api:webrtc_key_value_config", "../rtc_base:checks", - "../system_wrappers:field_trial", ] absl_deps = [ "//third_party/abseil-cpp/absl/strings:strings" ] } diff --git a/test/explicit_key_value_config.cc b/test/explicit_key_value_config.cc index 69f725a9e2..6a561fa209 100644 --- a/test/explicit_key_value_config.cc +++ b/test/explicit_key_value_config.cc @@ -10,9 +10,8 @@ #include "test/explicit_key_value_config.h" -#include "api/transport/webrtc_key_value_config.h" +#include "api/webrtc_key_value_config.h" #include "rtc_base/checks.h" -#include "system_wrappers/include/field_trial.h" namespace webrtc { namespace test { diff --git a/test/explicit_key_value_config.h b/test/explicit_key_value_config.h index 9a3bc84f60..0a728ab536 100644 --- a/test/explicit_key_value_config.h +++ b/test/explicit_key_value_config.h @@ -15,7 +15,7 @@ #include #include "absl/strings/string_view.h" -#include "api/transport/webrtc_key_value_config.h" +#include "api/webrtc_key_value_config.h" namespace webrtc { namespace test { diff --git a/test/peer_scenario/BUILD.gn b/test/peer_scenario/BUILD.gn index a08b4acbeb..d3ac0da606 100644 --- a/test/peer_scenario/BUILD.gn +++ b/test/peer_scenario/BUILD.gn @@ -49,6 +49,8 @@ if (rtc_include_tests) { "../../rtc_base", "../../rtc_base:null_socket_server", "../../rtc_base:stringutils", + "../../test:explicit_key_value_config", + "../../test:scoped_key_value_config", "../logging:log_writer", "../network:emulated_network", "../scenario", diff --git a/test/peer_scenario/scenario_connection.cc b/test/peer_scenario/scenario_connection.cc index dac21d5586..07da453b8a 100644 --- a/test/peer_scenario/scenario_connection.cc +++ b/test/peer_scenario/scenario_connection.cc @@ -132,6 +132,7 @@ JsepTransportController::Config ScenarioIceConnectionImpl::CreateJsepConfig() { RTC_DCHECK_RUN_ON(network_thread_); observer_->OnPacketReceived(packet); }; + config.field_trials = &field_trials; return config; } diff --git a/test/peer_scenario/scenario_connection.h b/test/peer_scenario/scenario_connection.h index f43b3d39d4..e8cef527c5 100644 --- a/test/peer_scenario/scenario_connection.h +++ b/test/peer_scenario/scenario_connection.h @@ -19,6 +19,7 @@ #include "api/jsep.h" #include "p2p/base/transport_description.h" #include "test/network/network_emulation_manager.h" +#include "test/scoped_key_value_config.h" namespace webrtc { @@ -56,6 +57,8 @@ class ScenarioIceConnection { virtual EmulatedEndpoint* endpoint() = 0; virtual const cricket::TransportDescription& transport_description() const = 0; + + webrtc::test::ScopedKeyValueConfig field_trials; }; } // namespace webrtc diff --git a/test/scoped_key_value_config.cc b/test/scoped_key_value_config.cc new file mode 100644 index 0000000000..fe3f42635d --- /dev/null +++ b/test/scoped_key_value_config.cc @@ -0,0 +1,116 @@ +/* + * 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. + */ + +#include "test/scoped_key_value_config.h" + +#include "api/webrtc_key_value_config.h" +#include "rtc_base/checks.h" +#include "test/field_trial.h" + +namespace { + +// This part is copied from system_wrappers/field_trial.cc. +void InsertIntoMap(std::map& key_value_map, + const std::string& s) { + std::string::size_type field_start = 0; + while (field_start < s.size()) { + std::string::size_type separator_pos = s.find('/', field_start); + RTC_CHECK_NE(separator_pos, std::string::npos) + << "Missing separator '/' after field trial key."; + RTC_CHECK_GT(separator_pos, field_start) + << "Field trial key cannot be empty."; + std::string key = s.substr(field_start, separator_pos - field_start); + field_start = separator_pos + 1; + + RTC_CHECK_LT(field_start, s.size()) + << "Missing value after field trial key. String ended."; + separator_pos = s.find('/', field_start); + RTC_CHECK_NE(separator_pos, std::string::npos) + << "Missing terminating '/' in field trial string."; + RTC_CHECK_GT(separator_pos, field_start) + << "Field trial value cannot be empty."; + std::string value = s.substr(field_start, separator_pos - field_start); + field_start = separator_pos + 1; + + key_value_map[key] = value; + } + // This check is technically redundant due to earlier checks. + // We nevertheless keep the check to make it clear that the entire + // string has been processed, and without indexing past the end. + RTC_CHECK_EQ(field_start, s.size()); +} + +} // namespace + +namespace webrtc { +namespace test { + +ScopedKeyValueConfig::ScopedKeyValueConfig() + : ScopedKeyValueConfig(nullptr, "") {} + +ScopedKeyValueConfig::ScopedKeyValueConfig(const std::string& s) + : ScopedKeyValueConfig(nullptr, s) {} + +ScopedKeyValueConfig::ScopedKeyValueConfig(ScopedKeyValueConfig& parent, + const std::string& s) + : ScopedKeyValueConfig(&parent, s) {} + +ScopedKeyValueConfig::ScopedKeyValueConfig(ScopedKeyValueConfig* parent, + const std::string& s) + : parent_(parent), leaf_(nullptr) { + InsertIntoMap(key_value_map_, s); + // Also store field trials in global string (until we get rid of it). + scoped_field_trials_ = std::make_unique(s); + + if (parent == nullptr) { + // We are root, set leaf_. + leaf_ = this; + } else { + // Link root to new leaf. + GetRoot(parent)->leaf_ = this; + RTC_DCHECK(leaf_ == nullptr); + } +} + +ScopedKeyValueConfig::~ScopedKeyValueConfig() { + if (parent_) { + GetRoot(parent_)->leaf_ = parent_; + } +} + +ScopedKeyValueConfig* ScopedKeyValueConfig::GetRoot(ScopedKeyValueConfig* n) { + while (n->parent_ != nullptr) { + n = n->parent_; + } + return n; +} + +std::string ScopedKeyValueConfig::Lookup(absl::string_view key) const { + if (parent_ == nullptr) { + return leaf_->LookupRecurse(key); + } else { + return LookupRecurse(key); + } +} + +std::string ScopedKeyValueConfig::LookupRecurse(absl::string_view key) const { + auto it = key_value_map_.find(std::string(key)); + if (it != key_value_map_.end()) + return it->second; + + if (parent_) { + return parent_->LookupRecurse(key); + } + + return ""; +} + +} // namespace test +} // namespace webrtc diff --git a/test/scoped_key_value_config.h b/test/scoped_key_value_config.h new file mode 100644 index 0000000000..a00f61ac60 --- /dev/null +++ b/test/scoped_key_value_config.h @@ -0,0 +1,52 @@ +/* + * 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. + */ + +#ifndef TEST_SCOPED_KEY_VALUE_CONFIG_H_ +#define TEST_SCOPED_KEY_VALUE_CONFIG_H_ + +#include +#include +#include + +#include "absl/strings/string_view.h" +#include "api/webrtc_key_value_config.h" +#include "test/field_trial.h" + +namespace webrtc { +namespace test { + +class ScopedKeyValueConfig : public WebRtcKeyValueConfig { + public: + virtual ~ScopedKeyValueConfig(); + ScopedKeyValueConfig(); + explicit ScopedKeyValueConfig(const std::string& s); + ScopedKeyValueConfig(ScopedKeyValueConfig& parent, const std::string& s); + + std::string Lookup(absl::string_view key) const override; + + private: + ScopedKeyValueConfig(ScopedKeyValueConfig* parent, const std::string& s); + ScopedKeyValueConfig* GetRoot(ScopedKeyValueConfig* n); + std::string LookupRecurse(absl::string_view key) const; + + ScopedKeyValueConfig* const parent_; + + // The leaf in a list of stacked ScopedKeyValueConfig. + // Only set on root (e.g with parent_ == nullptr). + const ScopedKeyValueConfig* leaf_; + + std::map key_value_map_; + std::unique_ptr scoped_field_trials_; +}; + +} // namespace test +} // namespace webrtc + +#endif // TEST_SCOPED_KEY_VALUE_CONFIG_H_ diff --git a/video/encoder_bitrate_adjuster.cc b/video/encoder_bitrate_adjuster.cc index 9f4d8551c4..8ed16a7565 100644 --- a/video/encoder_bitrate_adjuster.cc +++ b/video/encoder_bitrate_adjuster.cc @@ -17,7 +17,6 @@ #include "rtc_base/experiments/rate_control_settings.h" #include "rtc_base/logging.h" #include "rtc_base/time_utils.h" -#include "system_wrappers/include/field_trial.h" namespace webrtc { namespace {