diff --git a/api/create_peerconnection_factory.cc b/api/create_peerconnection_factory.cc index c41b6d6fb2..4a01b2f3c9 100644 --- a/api/create_peerconnection_factory.cc +++ b/api/create_peerconnection_factory.cc @@ -38,7 +38,8 @@ rtc::scoped_refptr CreatePeerConnectionFactory( std::unique_ptr video_decoder_factory, rtc::scoped_refptr audio_mixer, rtc::scoped_refptr audio_processing, - AudioFrameProcessor* audio_frame_processor) { + AudioFrameProcessor* audio_frame_processor, + std::unique_ptr field_trials) { PeerConnectionFactoryDependencies dependencies; dependencies.network_thread = network_thread; dependencies.worker_thread = worker_thread; @@ -47,7 +48,11 @@ rtc::scoped_refptr CreatePeerConnectionFactory( dependencies.call_factory = CreateCallFactory(); dependencies.event_log_factory = std::make_unique( dependencies.task_queue_factory.get()); - dependencies.trials = std::make_unique(); + if (field_trials) { + dependencies.trials = std::move(field_trials); + } else { + dependencies.trials = std::make_unique(); + } if (network_thread) { // TODO(bugs.webrtc.org/13145): Add an rtc::SocketFactory* argument. diff --git a/api/create_peerconnection_factory.h b/api/create_peerconnection_factory.h index 4eb0a00e54..efebc5f3ea 100644 --- a/api/create_peerconnection_factory.h +++ b/api/create_peerconnection_factory.h @@ -49,7 +49,8 @@ CreatePeerConnectionFactory( std::unique_ptr video_decoder_factory, rtc::scoped_refptr audio_mixer, rtc::scoped_refptr audio_processing, - AudioFrameProcessor* audio_frame_processor = nullptr); + AudioFrameProcessor* audio_frame_processor = nullptr, + std::unique_ptr field_trials = nullptr); } // namespace webrtc diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h index 5f1f09937e..c326799edb 100644 --- a/api/peer_connection_interface.h +++ b/api/peer_connection_interface.h @@ -1398,6 +1398,9 @@ struct RTC_EXPORT PeerConnectionDependencies final { std::unique_ptr tls_cert_verifier; std::unique_ptr video_bitrate_allocator_factory; + // Optional field trials to use. + // Overrides those from PeerConnectionFactoryDependencies. + std::unique_ptr trials; }; // PeerConnectionFactoryDependencies holds all of the PeerConnectionFactory diff --git a/pc/connection_context.cc b/pc/connection_context.cc index 736a6d79b3..d7c80ebfda 100644 --- a/pc/connection_context.cc +++ b/pc/connection_context.cc @@ -145,7 +145,7 @@ ConnectionContext::ConnectionContext( // If network_monitor_factory_ is non-null, it will be used to create a // network monitor while on the network thread. default_network_manager_ = std::make_unique( - network_monitor_factory_.get(), socket_factory, &trials()); + network_monitor_factory_.get(), socket_factory, &field_trials()); default_socket_factory_ = std::make_unique(socket_factory); diff --git a/pc/connection_context.h b/pc/connection_context.h index 3b8ac07fc9..e4aa715985 100644 --- a/pc/connection_context.h +++ b/pc/connection_context.h @@ -75,7 +75,11 @@ class ConnectionContext final rtc::Thread* network_thread() { return network_thread_; } const rtc::Thread* network_thread() const { return network_thread_; } - const FieldTrialsView& trials() const { return *trials_.get(); } + // Field trials associated with the PeerConnectionFactory. + // Note: that there can be different field trials for different + // PeerConnections (but they are not supposed change after creating the + // PeerConnection). + const FieldTrialsView& field_trials() const { return *trials_.get(); } // Accessors only used from the PeerConnectionFactory class rtc::BasicNetworkManager* default_network_manager() { diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 0705c84a97..e7891796b5 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -509,6 +509,7 @@ PeerConnection::PeerConnection( PeerConnectionDependencies& dependencies, bool dtls_enabled) : context_(context), + trials_(std::move(dependencies.trials), &context->field_trials()), options_(options), observer_(dependencies.observer), is_unified_plan_(is_unified_plan), @@ -719,7 +720,7 @@ JsepTransportController* PeerConnection::InitializeTransportController_n( } }; - config.field_trials = &context_->trials(); + config.field_trials = trials_.get(); transport_controller_.reset( new JsepTransportController(network_thread(), port_allocator_.get(), @@ -1692,8 +1693,7 @@ bool PeerConnection::StartRtcEventLog(std::unique_ptr output, bool PeerConnection::StartRtcEventLog( std::unique_ptr output) { int64_t output_period_ms = webrtc::RtcEventLog::kImmediateOutput; - if (absl::StartsWith(context_->trials().Lookup("WebRTC-RtcEventLogNewFormat"), - "Enabled")) { + if (trials().IsEnabled("WebRTC-RtcEventLogNewFormat")) { output_period_ms = 5000; } return StartRtcEventLog(std::move(output), output_period_ms); @@ -2045,8 +2045,7 @@ PeerConnection::InitializePortAllocator_n( // by experiment. if (configuration.disable_ipv6) { port_allocator_flags &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6); - } else if (absl::StartsWith(context_->trials().Lookup("WebRTC-IPv6Default"), - "Disabled")) { + } else if (trials().IsDisabled("WebRTC-IPv6Default")) { port_allocator_flags &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6); } if (configuration.disable_ipv6_on_wifi) { diff --git a/pc/peer_connection.h b/pc/peer_connection.h index 919d377b2b..84a881083f 100644 --- a/pc/peer_connection.h +++ b/pc/peer_connection.h @@ -437,7 +437,7 @@ class PeerConnection : public PeerConnectionInternal, } void RequestUsagePatternReportForTesting(); - const FieldTrialsView& trials() override { return context_->trials(); } + const FieldTrialsView& trials() const override { return *trials_; } protected: // Available for rtc::scoped_refptr creation @@ -592,6 +592,12 @@ class PeerConnection : public PeerConnectionInternal, InitializeRtcpCallback(); const rtc::scoped_refptr context_; + // Field trials active for this PeerConnection is the first of: + // a) Specified in PeerConnectionDependencies (owned). + // b) Accessed via ConnectionContext (e.g PeerConnectionFactoryDependencies> + // c) Created as Default (FieldTrialBasedConfig). + const webrtc::AlwaysValidPointer + trials_; const PeerConnectionFactoryInterface::Options options_; PeerConnectionObserver* observer_ RTC_GUARDED_BY(signaling_thread()) = nullptr; diff --git a/pc/peer_connection_factory.cc b/pc/peer_connection_factory.cc index c598440201..aa9f328231 100644 --- a/pc/peer_connection_factory.cc +++ b/pc/peer_connection_factory.cc @@ -243,9 +243,12 @@ PeerConnectionFactory::CreatePeerConnectionOrError( worker_thread()->Invoke>( RTC_FROM_HERE, [this] { return CreateRtcEventLog_w(); }); + const FieldTrialsView* trials = + dependencies.trials ? dependencies.trials.get() : &field_trials(); std::unique_ptr call = worker_thread()->Invoke>( - RTC_FROM_HERE, - [this, &event_log] { return CreateCall_w(event_log.get()); }); + RTC_FROM_HERE, [this, &event_log, trials] { + return CreateCall_w(event_log.get(), *trials); + }); auto result = PeerConnection::Create(context_, options_, std::move(event_log), std::move(call), configuration, @@ -307,7 +310,8 @@ std::unique_ptr PeerConnectionFactory::CreateRtcEventLog_w() { } std::unique_ptr PeerConnectionFactory::CreateCall_w( - RtcEventLog* event_log) { + RtcEventLog* event_log, + const FieldTrialsView& field_trials) { RTC_DCHECK_RUN_ON(worker_thread()); webrtc::Call::Config call_config(event_log, network_thread()); @@ -324,7 +328,7 @@ std::unique_ptr PeerConnectionFactory::CreateCall_w( FieldTrialParameter max_bandwidth("max", DataRate::KilobitsPerSec(2000)); ParseFieldTrial({&min_bandwidth, &start_bandwidth, &max_bandwidth}, - trials().Lookup("WebRTC-PcFactoryDefaultBitrates")); + field_trials.Lookup("WebRTC-PcFactoryDefaultBitrates")); call_config.bitrate_config.min_bitrate_bps = rtc::saturated_cast(min_bandwidth->bps()); @@ -347,7 +351,7 @@ std::unique_ptr PeerConnectionFactory::CreateCall_w( RTC_LOG(LS_INFO) << "Using default network controller factory"; } - call_config.trials = &trials(); + call_config.trials = &field_trials; call_config.rtp_transport_controller_send_factory = transport_controller_send_factory_.get(); call_config.metronome = metronome_.get(); @@ -356,7 +360,7 @@ std::unique_ptr PeerConnectionFactory::CreateCall_w( } bool PeerConnectionFactory::IsTrialEnabled(absl::string_view key) const { - return absl::StartsWith(trials().Lookup(key), "Enabled"); + return absl::StartsWith(field_trials().Lookup(key), "Enabled"); } } // namespace webrtc diff --git a/pc/peer_connection_factory.h b/pc/peer_connection_factory.h index 4ee102a2fb..16695f234a 100644 --- a/pc/peer_connection_factory.h +++ b/pc/peer_connection_factory.h @@ -115,7 +115,9 @@ class PeerConnectionFactory : public PeerConnectionFactoryInterface { return options_; } - const FieldTrialsView& trials() const { return context_->trials(); } + const FieldTrialsView& field_trials() const { + return context_->field_trials(); + } protected: // Constructor used by the static Create() method. Modifies the dependencies. @@ -138,7 +140,8 @@ class PeerConnectionFactory : public PeerConnectionFactoryInterface { } std::unique_ptr CreateRtcEventLog_w(); - std::unique_ptr CreateCall_w(RtcEventLog* event_log); + std::unique_ptr CreateCall_w(RtcEventLog* event_log, + const FieldTrialsView& field_trials); rtc::scoped_refptr context_; PeerConnectionFactoryInterface::Options options_ diff --git a/pc/peer_connection_internal.h b/pc/peer_connection_internal.h index 66897ee758..762f9b1e08 100644 --- a/pc/peer_connection_internal.h +++ b/pc/peer_connection_internal.h @@ -123,6 +123,8 @@ class PeerConnectionSdpMethods { virtual void TeardownDataChannelTransport_n() = 0; virtual void SetSctpDataMid(const std::string& mid) = 0; virtual void ResetSctpDataMid() = 0; + + virtual const FieldTrialsView& trials() const = 0; }; // Functions defined in this class are called by other objects, @@ -178,8 +180,6 @@ class PeerConnectionInternal : public PeerConnectionInterface, virtual void NoteDataAddedEvent() {} // Handler for the "channel closed" signal virtual void OnSctpDataChannelClosed(DataChannelInterface* channel) {} - - virtual const FieldTrialsView& trials() = 0; }; } // namespace webrtc diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index d942548cbd..1ad793c841 100644 --- a/pc/sdp_offer_answer.cc +++ b/pc/sdp_offer_answer.cc @@ -1201,7 +1201,7 @@ void SdpOfferAnswerHandler::Initialize( // Use 100 kbps as the default minimum screencast bitrate unless this path is // kill-switched. if (!video_options_.screencast_min_bitrate_kbps.has_value() && - !context_->trials().IsEnabled(kDefaultScreencastMinBitrateKillSwitch)) { + !pc_->trials().IsEnabled(kDefaultScreencastMinBitrateKillSwitch)) { video_options_.screencast_min_bitrate_kbps = 100; } audio_options_.combined_audio_video_bwe = @@ -1235,7 +1235,8 @@ void SdpOfferAnswerHandler::Initialize( [this](const rtc::scoped_refptr& certificate) { RTC_DCHECK_RUN_ON(signaling_thread()); transport_controller_s()->SetLocalCertificate(certificate); - }); + }, + pc_->trials()); if (pc_->options()->disable_encryption) { webrtc_session_desc_factory_->SetSdesPolicy(cricket::SEC_DISABLED); diff --git a/pc/test/fake_peer_connection_base.h b/pc/test/fake_peer_connection_base.h index c8863059db..f629f04e6b 100644 --- a/pc/test/fake_peer_connection_base.h +++ b/pc/test/fake_peer_connection_base.h @@ -360,7 +360,7 @@ class FakePeerConnectionBase : public PeerConnectionInternal { void SetSctpDataMid(const std::string& mid) override {} void ResetSctpDataMid() override {} - const FieldTrialsView& trials() override { return field_trials_; } + const FieldTrialsView& trials() const override { return field_trials_; } protected: webrtc::test::ScopedKeyValueConfig field_trials_; diff --git a/pc/webrtc_session_description_factory.cc b/pc/webrtc_session_description_factory.cc index 09f2ee2f00..7ccc3bf252 100644 --- a/pc/webrtc_session_description_factory.cc +++ b/pc/webrtc_session_description_factory.cc @@ -135,9 +135,10 @@ WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory( std::unique_ptr cert_generator, const rtc::scoped_refptr& certificate, std::function&)> - on_certificate_ready) + on_certificate_ready, + const FieldTrialsView& field_trials) : signaling_thread_(context->signaling_thread()), - transport_desc_factory_(context->trials()), + transport_desc_factory_(field_trials), session_desc_factory_(context->channel_manager(), &transport_desc_factory_), // RFC 4566 suggested a Network Time Protocol (NTP) format timestamp diff --git a/pc/webrtc_session_description_factory.h b/pc/webrtc_session_description_factory.h index 79171f797b..3f1519082b 100644 --- a/pc/webrtc_session_description_factory.h +++ b/pc/webrtc_session_description_factory.h @@ -86,7 +86,8 @@ class WebRtcSessionDescriptionFactory : public rtc::MessageHandler, std::unique_ptr cert_generator, const rtc::scoped_refptr& certificate, std::function&)> - on_certificate_ready); + on_certificate_ready, + const FieldTrialsView& field_trials); virtual ~WebRtcSessionDescriptionFactory(); WebRtcSessionDescriptionFactory(const WebRtcSessionDescriptionFactory&) = diff --git a/rtc_base/memory/always_valid_pointer.h b/rtc_base/memory/always_valid_pointer.h index c6e0a70d6b..a878083fc0 100644 --- a/rtc_base/memory/always_valid_pointer.h +++ b/rtc_base/memory/always_valid_pointer.h @@ -37,6 +37,36 @@ class AlwaysValidPointer { RTC_DCHECK(pointer_); } + // Create a pointer by + // a) taking over ownership of |instance| + // b) or fallback to |pointer|, without taking ownership. + // c) or Default. + AlwaysValidPointer(std::unique_ptr&& instance, Interface* pointer) + : owned_instance_( + instance + ? std::move(instance) + : (pointer == nullptr ? std::make_unique() : nullptr)), + pointer_(owned_instance_ ? owned_instance_.get() : pointer) { + RTC_DCHECK(pointer_); + } + + // Create a pointer by + // a) taking over ownership of |instance| + // b) or fallback to |pointer|, without taking ownership. + // c) or Default (with forwarded args). + template + AlwaysValidPointer(std::unique_ptr&& instance, + Interface* pointer, + Args... args) + : owned_instance_( + instance ? std::move(instance) + : (pointer == nullptr + ? std::make_unique(std::move(args...)) + : nullptr)), + pointer_(owned_instance_ ? owned_instance_.get() : pointer) { + RTC_DCHECK(pointer_); + } + Interface* get() { return pointer_; } Interface* operator->() { return pointer_; } Interface& operator*() { return *pointer_; } diff --git a/rtc_base/memory/always_valid_pointer_unittest.cc b/rtc_base/memory/always_valid_pointer_unittest.cc index dbb0671e32..92cf56bffe 100644 --- a/rtc_base/memory/always_valid_pointer_unittest.cc +++ b/rtc_base/memory/always_valid_pointer_unittest.cc @@ -46,4 +46,42 @@ TEST(AlwaysValidPointerTest, NonDefaultValue) { EXPECT_EQ(*ptr, "keso"); } +TEST(AlwaysValidPointerTest, TakeOverOwnershipOfInstance) { + std::string str("keso"); + std::unique_ptr str2 = std::make_unique("kent"); + AlwaysValidPointer ptr(std::move(str2), &str); + EXPECT_EQ(*ptr, "kent"); + EXPECT_EQ(str2, nullptr); +} + +TEST(AlwaysValidPointerTest, TakeOverOwnershipFallbackOnPointer) { + std::string str("keso"); + std::unique_ptr str2; + AlwaysValidPointer ptr(std::move(str2), &str); + EXPECT_EQ(*ptr, "keso"); +} + +TEST(AlwaysValidPointerTest, TakeOverOwnershipFallbackOnDefault) { + std::unique_ptr str; + std::string* str_ptr = nullptr; + AlwaysValidPointer ptr(std::move(str), str_ptr); + EXPECT_EQ(*ptr, ""); +} + +TEST(AlwaysValidPointerTest, + TakeOverOwnershipFallbackOnDefaultWithForwardedArgument) { + std::unique_ptr str2; + AlwaysValidPointer ptr(std::move(str2), nullptr, "keso"); + EXPECT_EQ(*ptr, "keso"); +} + +TEST(AlwaysValidPointerTest, TakeOverOwnershipDoesNotForwardDefaultArguments) { + std::unique_ptr str = std::make_unique("kalle"); + std::unique_ptr str2 = std::make_unique("anka"); + AlwaysValidPointer ptr(std::move(str), nullptr, *str2); + EXPECT_EQ(*ptr, "kalle"); + EXPECT_TRUE(!str); + EXPECT_EQ(*str2, "anka"); +} + } // namespace webrtc