WebRTC-DeprecateGlobalFieldTrialString/Enabled/ - part 16/inf

This cl/ adds the feature actually injecting a FieldTrialsView into
PeerConnectionFactory, or into a PeerConnection or both.

The field trials used for a PeerConnection is those specified in
PeerConnectionDependencies. Otherwise will those from
PeerConnectionFactoryDependencies be used (and until we're finished with
this conversion, the global string fallback is used as last resort).

Note that it is currently not possible to create 2 FieldTrials
objects concurrently...due to global string,
so this cl/ is mostly (but entirely) for show, i.e one _can_
realistically inject them into a PeerConnectionFactory.


Bug: webrtc:10335
Change-Id: Id2e60525f48a1f8293c1dd0be771e3ed03790963
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/258134
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36578}

api/create_peerconnection_factory.cc

@@ -38,7 +38,8 @@ rtc::scoped_refptr<PeerConnectionFactoryInterface> CreatePeerConnectionFactory(
     std::unique_ptr<VideoDecoderFactory> video_decoder_factory,
     rtc::scoped_refptr<AudioMixer> audio_mixer,
     rtc::scoped_refptr<AudioProcessing> audio_processing,
-    AudioFrameProcessor* audio_frame_processor) {
+    AudioFrameProcessor* audio_frame_processor,
+    std::unique_ptr<FieldTrialsView> field_trials) {
   PeerConnectionFactoryDependencies dependencies;
   dependencies.network_thread = network_thread;
   dependencies.worker_thread = worker_thread;
@@ -47,7 +48,11 @@ rtc::scoped_refptr<PeerConnectionFactoryInterface> CreatePeerConnectionFactory(
   dependencies.call_factory = CreateCallFactory();
   dependencies.event_log_factory = std::make_unique<RtcEventLogFactory>(
       dependencies.task_queue_factory.get());
-  dependencies.trials = std::make_unique<webrtc::FieldTrialBasedConfig>();
+  if (field_trials) {
+    dependencies.trials = std::move(field_trials);
+  } else {
+    dependencies.trials = std::make_unique<webrtc::FieldTrialBasedConfig>();
+  }
 
   if (network_thread) {
     // TODO(bugs.webrtc.org/13145): Add an rtc::SocketFactory* argument.

api/create_peerconnection_factory.h

@@ -49,7 +49,8 @@ CreatePeerConnectionFactory(
     std::unique_ptr<VideoDecoderFactory> video_decoder_factory,
     rtc::scoped_refptr<AudioMixer> audio_mixer,
     rtc::scoped_refptr<AudioProcessing> audio_processing,
-    AudioFrameProcessor* audio_frame_processor = nullptr);
+    AudioFrameProcessor* audio_frame_processor = nullptr,
+    std::unique_ptr<FieldTrialsView> field_trials = nullptr);
 
 }  // namespace webrtc
 

api/peer_connection_interface.h

@@ -1398,6 +1398,9 @@ struct RTC_EXPORT PeerConnectionDependencies final {
   std::unique_ptr<rtc::SSLCertificateVerifier> tls_cert_verifier;
   std::unique_ptr<webrtc::VideoBitrateAllocatorFactory>
       video_bitrate_allocator_factory;
+  // Optional field trials to use.
+  // Overrides those from PeerConnectionFactoryDependencies.
+  std::unique_ptr<FieldTrialsView> trials;
 };
 
 // PeerConnectionFactoryDependencies holds all of the PeerConnectionFactory

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<rtc::BasicNetworkManager>(
-      network_monitor_factory_.get(), socket_factory, &trials());
+      network_monitor_factory_.get(), socket_factory, &field_trials());
 
   default_socket_factory_ =
       std::make_unique<rtc::BasicPacketSocketFactory>(socket_factory);

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() {

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<RtcEventLogOutput> output,
 bool PeerConnection::StartRtcEventLog(
     std::unique_ptr<RtcEventLogOutput> 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) {

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<ConnectionContext> 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<const FieldTrialsView, FieldTrialBasedConfig>
+      trials_;
   const PeerConnectionFactoryInterface::Options options_;
   PeerConnectionObserver* observer_ RTC_GUARDED_BY(signaling_thread()) =
       nullptr;

pc/peer_connection_factory.cc

@@ -243,9 +243,12 @@ PeerConnectionFactory::CreatePeerConnectionOrError(
       worker_thread()->Invoke<std::unique_ptr<RtcEventLog>>(
           RTC_FROM_HERE, [this] { return CreateRtcEventLog_w(); });
 
+  const FieldTrialsView* trials =
+      dependencies.trials ? dependencies.trials.get() : &field_trials();
   std::unique_ptr<Call> call = worker_thread()->Invoke<std::unique_ptr<Call>>(
-      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<RtcEventLog> PeerConnectionFactory::CreateRtcEventLog_w() {
 }
 
 std::unique_ptr<Call> 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<Call> PeerConnectionFactory::CreateCall_w(
   FieldTrialParameter<DataRate> 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<int>(min_bandwidth->bps());
@@ -347,7 +351,7 @@ std::unique_ptr<Call> 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<Call> 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

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<RtcEventLog> CreateRtcEventLog_w();
-  std::unique_ptr<Call> CreateCall_w(RtcEventLog* event_log);
+  std::unique_ptr<Call> CreateCall_w(RtcEventLog* event_log,
+                                     const FieldTrialsView& field_trials);
 
   rtc::scoped_refptr<ConnectionContext> context_;
   PeerConnectionFactoryInterface::Options options_

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

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<rtc::RTCCertificate>& 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);

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_;

pc/webrtc_session_description_factory.cc

@@ -135,9 +135,10 @@ WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory(
     std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator,
     const rtc::scoped_refptr<rtc::RTCCertificate>& certificate,
     std::function<void(const rtc::scoped_refptr<rtc::RTCCertificate>&)>
-        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

pc/webrtc_session_description_factory.h

@@ -86,7 +86,8 @@ class WebRtcSessionDescriptionFactory : public rtc::MessageHandler,
       std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator,
       const rtc::scoped_refptr<rtc::RTCCertificate>& certificate,
       std::function<void(const rtc::scoped_refptr<rtc::RTCCertificate>&)>
-          on_certificate_ready);
+          on_certificate_ready,
+      const FieldTrialsView& field_trials);
   virtual ~WebRtcSessionDescriptionFactory();
 
   WebRtcSessionDescriptionFactory(const WebRtcSessionDescriptionFactory&) =

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<Interface>&& instance, Interface* pointer)
+      : owned_instance_(
+            instance
+                ? std::move(instance)
+                : (pointer == nullptr ? std::make_unique<Default>() : 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 <typename... Args>
+  AlwaysValidPointer(std::unique_ptr<Interface>&& instance,
+                     Interface* pointer,
+                     Args... args)
+      : owned_instance_(
+            instance ? std::move(instance)
+                     : (pointer == nullptr
+                            ? std::make_unique<Default>(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_; }

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<std::string> str2 = std::make_unique<std::string>("kent");
+  AlwaysValidPointer<std::string> ptr(std::move(str2), &str);
+  EXPECT_EQ(*ptr, "kent");
+  EXPECT_EQ(str2, nullptr);
+}
+
+TEST(AlwaysValidPointerTest, TakeOverOwnershipFallbackOnPointer) {
+  std::string str("keso");
+  std::unique_ptr<std::string> str2;
+  AlwaysValidPointer<std::string> ptr(std::move(str2), &str);
+  EXPECT_EQ(*ptr, "keso");
+}
+
+TEST(AlwaysValidPointerTest, TakeOverOwnershipFallbackOnDefault) {
+  std::unique_ptr<std::string> str;
+  std::string* str_ptr = nullptr;
+  AlwaysValidPointer<std::string> ptr(std::move(str), str_ptr);
+  EXPECT_EQ(*ptr, "");
+}
+
+TEST(AlwaysValidPointerTest,
+     TakeOverOwnershipFallbackOnDefaultWithForwardedArgument) {
+  std::unique_ptr<std::string> str2;
+  AlwaysValidPointer<std::string> ptr(std::move(str2), nullptr, "keso");
+  EXPECT_EQ(*ptr, "keso");
+}
+
+TEST(AlwaysValidPointerTest, TakeOverOwnershipDoesNotForwardDefaultArguments) {
+  std::unique_ptr<std::string> str = std::make_unique<std::string>("kalle");
+  std::unique_ptr<std::string> str2 = std::make_unique<std::string>("anka");
+  AlwaysValidPointer<std::string> ptr(std::move(str), nullptr, *str2);
+  EXPECT_EQ(*ptr, "kalle");
+  EXPECT_TRUE(!str);
+  EXPECT_EQ(*str2, "anka");
+}
+
 }  // namespace webrtc