Add RtcEventLogFactory::Create variant that uses Environment

With intent to delete previous versions of the Create functions.

Bug: webrtc:15656
Change-Id: I972377701becca21b8ecfe15d41a10a4248f87ec
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/328420
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41275}

api/rtc_event_log/BUILD.gn

@@ -22,8 +22,10 @@ rtc_library("rtc_event_log") {
     "..:libjingle_logging_api",
     "../../rtc_base:checks",
     "../../rtc_base:timeutils",
+    "../environment",
     "../task_queue",
   ]
+  absl_deps = [ "//third_party/abseil-cpp/absl/base:nullability" ]
 }
 
 rtc_library("rtc_event_log_factory") {
@@ -35,12 +37,16 @@ rtc_library("rtc_event_log_factory") {
 
   deps = [
     ":rtc_event_log",
+    "..:field_trials_view",
     "../../rtc_base:checks",
     "../../rtc_base/system:rtc_export",
     "../../system_wrappers:field_trial",
+    "../environment",
     "../task_queue",
   ]
 
+  absl_deps = [ "//third_party/abseil-cpp/absl/base:nullability" ]
+
   if (rtc_enable_protobuf) {
     defines = [ "WEBRTC_ENABLE_RTC_EVENT_LOG" ]
     deps += [ "../../logging:rtc_event_log_impl" ]

api/rtc_event_log/rtc_event_log_factory.cc

@@ -13,6 +13,7 @@
 #include <memory>
 #include <utility>
 
+#include "api/field_trials_view.h"
 #include "rtc_base/checks.h"
 #include "system_wrappers/include/field_trial.h"
 
@@ -27,9 +28,27 @@ RtcEventLogFactory::RtcEventLogFactory(TaskQueueFactory* task_queue_factory)
   RTC_DCHECK(task_queue_factory_);
 }
 
+absl::Nonnull<std::unique_ptr<RtcEventLog>> RtcEventLogFactory::Create(
+    const Environment& env) const {
+#ifndef WEBRTC_ENABLE_RTC_EVENT_LOG
+  return std::make_unique<RtcEventLogNull>();
+#else
+  if (env.field_trials().IsEnabled("WebRTC-RtcEventLogKillSwitch")) {
+    return std::make_unique<RtcEventLogNull>();
+  }
+  RtcEventLog::EncodingType encoding_type =
+      env.field_trials().IsDisabled("WebRTC-RtcEventLogNewFormat")
+          ? RtcEventLog::EncodingType::Legacy
+          : RtcEventLog::EncodingType::NewFormat;
+  return std::make_unique<RtcEventLogImpl>(
+      RtcEventLogImpl::CreateEncoder(encoding_type), &env.task_queue_factory());
+#endif
+}
+
 std::unique_ptr<RtcEventLog> RtcEventLogFactory::Create(
     RtcEventLog::EncodingType encoding_type) const {
 #ifdef WEBRTC_ENABLE_RTC_EVENT_LOG
+  RTC_DCHECK(task_queue_factory_);
   if (field_trial::IsEnabled("WebRTC-RtcEventLogKillSwitch")) {
     return std::make_unique<RtcEventLogNull>();
   }

api/rtc_event_log/rtc_event_log_factory.h

@@ -13,6 +13,8 @@
 
 #include <memory>
 
+#include "absl/base/nullability.h"
+#include "api/environment/environment.h"
 #include "api/rtc_event_log/rtc_event_log.h"
 #include "api/rtc_event_log/rtc_event_log_factory_interface.h"
 #include "api/task_queue/task_queue_factory.h"
@@ -22,8 +24,16 @@ namespace webrtc {
 
 class RTC_EXPORT RtcEventLogFactory : public RtcEventLogFactoryInterface {
  public:
+  RtcEventLogFactory() = default;
+
+  // TODO(bugs.webrtc.org/15656): deprecate and delete constructor taking
+  // task queue factory in favor of using task queue factory provided through
+  // the Environment parameter in Create function.
   explicit RtcEventLogFactory(TaskQueueFactory* task_queue_factory);
-  ~RtcEventLogFactory() override {}
+  ~RtcEventLogFactory() override = default;
+
+  absl::Nonnull<std::unique_ptr<RtcEventLog>> Create(
+      const Environment& env) const override;
 
   std::unique_ptr<RtcEventLog> Create(
       RtcEventLog::EncodingType encoding_type) const override;
@@ -31,7 +41,7 @@ class RTC_EXPORT RtcEventLogFactory : public RtcEventLogFactoryInterface {
       RtcEventLog::EncodingType encoding_type) override;
 
  private:
-  TaskQueueFactory* const task_queue_factory_;
+  TaskQueueFactory* const task_queue_factory_ = nullptr;
 };
 
 }  // namespace webrtc

api/rtc_event_log/rtc_event_log_factory_interface.h

@@ -13,6 +13,8 @@
 
 #include <memory>
 
+#include "absl/base/nullability.h"
+#include "api/environment/environment.h"
 #include "api/rtc_event_log/rtc_event_log.h"
 
 namespace webrtc {
@@ -24,6 +26,11 @@ class RtcEventLogFactoryInterface {
  public:
   virtual ~RtcEventLogFactoryInterface() = default;
 
+  virtual absl::Nonnull<std::unique_ptr<RtcEventLog>> Create(
+      const Environment& env) const = 0;
+
+  // TODO(bugs.webrtc.org/15656): Delete functions below when all usage is
+  // migrated to the Create(const Environment&) function above.
   virtual std::unique_ptr<RtcEventLog> Create(
       RtcEventLog::EncodingType encoding_type) const = 0;
   [[deprecated]] virtual std::unique_ptr<RtcEventLog> CreateRtcEventLog(

logging/BUILD.gn

@@ -482,10 +482,12 @@ rtc_library("fake_rtc_event_log") {
   ]
 
   deps = [
+    "../api/environment",
     "../api/rtc_event_log",
     "../rtc_base:macromagic",
     "../rtc_base/synchronization:mutex",
   ]
+  absl_deps = [ "//third_party/abseil-cpp/absl/base:nullability" ]
 }
 
 if (rtc_enable_protobuf) {
@@ -604,9 +606,10 @@ if (rtc_enable_protobuf) {
         "../api:rtc_event_log_output_file",
         "../api:rtp_headers",
         "../api:rtp_parameters",
+        "../api/environment",
+        "../api/environment:environment_factory",
         "../api/rtc_event_log",
         "../api/rtc_event_log:rtc_event_log_factory",
-        "../api/task_queue:default_task_queue_factory",
         "../api/units:time_delta",
         "../api/units:timestamp",
         "../call",
@@ -621,6 +624,7 @@ if (rtc_enable_protobuf) {
         "../rtc_base:timeutils",
         "../system_wrappers",
         "../system_wrappers:field_trial",
+        "../test:explicit_key_value_config",
         "../test:field_trial",
         "../test:fileutils",
         "../test:test_support",

logging/rtc_event_log/fake_rtc_event_log_factory.cc

@@ -17,17 +17,26 @@
 
 namespace webrtc {
 
-std::unique_ptr<RtcEventLog> FakeRtcEventLogFactory::Create(
+absl::Nonnull<std::unique_ptr<FakeRtcEventLog>>
-    RtcEventLog::EncodingType /*encoding_type*/) const {
+FakeRtcEventLogFactory::CreateFake() const {
   auto fake_event_log = std::make_unique<FakeRtcEventLog>();
-  const_cast<FakeRtcEventLogFactory*>(this)->last_log_created_ =
+  const_cast<FakeRtcEventLog*&>(last_log_created_) = fake_event_log.get();
-      fake_event_log.get();
   return fake_event_log;
 }
 
+std::unique_ptr<RtcEventLog> FakeRtcEventLogFactory::Create(
+    const Environment& /*env*/) const {
+  return CreateFake();
+}
+
+std::unique_ptr<RtcEventLog> FakeRtcEventLogFactory::Create(
+    RtcEventLog::EncodingType /*encoding_type*/) const {
+  return CreateFake();
+}
+
 std::unique_ptr<RtcEventLog> FakeRtcEventLogFactory::CreateRtcEventLog(
-    RtcEventLog::EncodingType encoding_type) {
+    RtcEventLog::EncodingType /*encoding_type*/) {
-  return Create(encoding_type);
+  return CreateFake();
 }
 
 }  // namespace webrtc

logging/rtc_event_log/fake_rtc_event_log_factory.h

@@ -13,6 +13,8 @@
 
 #include <memory>
 
+#include "absl/base/nullability.h"
+#include "api/environment/environment.h"
 #include "api/rtc_event_log/rtc_event_log_factory_interface.h"
 #include "logging/rtc_event_log/fake_rtc_event_log.h"
 
@@ -23,16 +25,21 @@ class FakeRtcEventLogFactory : public RtcEventLogFactoryInterface {
   FakeRtcEventLogFactory() = default;
   ~FakeRtcEventLogFactory() override = default;
 
+  absl::Nonnull<std::unique_ptr<RtcEventLog>> Create(
+      const Environment& env) const override;
+
   std::unique_ptr<RtcEventLog> Create(
       RtcEventLog::EncodingType encoding_type) const override;
 
   std::unique_ptr<RtcEventLog> CreateRtcEventLog(
       RtcEventLog::EncodingType encoding_type) override;
 
-  webrtc::FakeRtcEventLog* last_log_created() { return last_log_created_; }
+  FakeRtcEventLog* last_log_created() { return last_log_created_; }
 
  private:
-  webrtc::FakeRtcEventLog* last_log_created_;
+  absl::Nonnull<std::unique_ptr<FakeRtcEventLog>> CreateFake() const;
+
+  FakeRtcEventLog* last_log_created_ = nullptr;
 };
 
 }  // namespace webrtc

logging/rtc_event_log/rtc_event_log_unittest.cc

@@ -19,8 +19,9 @@
 #include <utility>
 #include <vector>
 
+#include "api/environment/environment.h"
+#include "api/environment/environment_factory.h"
 #include "api/rtc_event_log/rtc_event_log_factory.h"
-#include "api/task_queue/default_task_queue_factory.h"
 #include "logging/rtc_event_log/events/rtc_event_audio_network_adaptation.h"
 #include "logging/rtc_event_log/events/rtc_event_audio_playout.h"
 #include "logging/rtc_event_log/events/rtc_event_audio_receive_stream_config.h"
@@ -50,6 +51,7 @@
 #include "rtc_base/checks.h"
 #include "rtc_base/fake_clock.h"
 #include "rtc_base/random.h"
+#include "test/explicit_key_value_config.h"
 #include "test/gtest.h"
 #include "test/logging/memory_log_writer.h"
 #include "test/testsupport/file_utils.h"
@@ -58,6 +60,8 @@ namespace webrtc {
 
 namespace {
 
+using test::ExplicitKeyValueConfig;
+
 struct EventCounts {
   size_t audio_send_streams = 0;
   size_t audio_recv_streams = 0;
@@ -105,6 +109,21 @@ struct EventCounts {
   }
 };
 
+std::unique_ptr<FieldTrialsView> CreateFieldTrialsFor(
+    RtcEventLog::EncodingType encoding_type) {
+  switch (encoding_type) {
+    case RtcEventLog::EncodingType::Legacy:
+      return std::make_unique<ExplicitKeyValueConfig>(
+          "WebRTC-RtcEventLogNewFormat/Disabled/");
+    case RtcEventLog::EncodingType::NewFormat:
+      return std::make_unique<ExplicitKeyValueConfig>(
+          "WebRTC-RtcEventLogNewFormat/Enabled/");
+    case RtcEventLog::EncodingType::ProtoFree:
+      RTC_CHECK(false);
+      return nullptr;
+  }
+}
+
 class RtcEventLogSession
     : public ::testing::TestWithParam<
           std::tuple<uint64_t, int64_t, RtcEventLog::EncodingType>> {
@@ -336,14 +355,13 @@ void RtcEventLogSession::WriteVideoSendConfigs(size_t video_send_streams,
 
 void RtcEventLogSession::WriteLog(EventCounts count,
                                   size_t num_events_before_start) {
-  // TODO(terelius): Allow test to run with either a real or a fake clock_.
+  // TODO(terelius): Allow test to run with either a real or a fake clock_
-  // Maybe always use the ScopedFakeClock, but conditionally SleepMs()?
+  // e.g. by using clock and task_queue_factory from TimeController
+  // when RtcEventLogImpl switches to use injected clock from the environment.
 
-  auto task_queue_factory = CreateDefaultTaskQueueFactory();
-  RtcEventLogFactory rtc_event_log_factory(task_queue_factory.get());
   // The log will be flushed to output when the event_log goes out of scope.
-  std::unique_ptr<RtcEventLog> event_log =
+  std::unique_ptr<RtcEventLog> event_log = RtcEventLogFactory().Create(
-      rtc_event_log_factory.CreateRtcEventLog(encoding_type_);
+      CreateEnvironment(CreateFieldTrialsFor(encoding_type_)));
 
   // We can't send or receive packets without configured streams.
   RTC_CHECK_GE(count.video_recv_streams, 1);
@@ -934,12 +952,9 @@ TEST_P(RtcEventLogCircularBufferTest, KeepsMostRecentEvents) {
   int64_t start_time_us, utc_start_time_us, stop_time_us;
 
   {
-    auto task_queue_factory = CreateDefaultTaskQueueFactory();
+    // When `log` goes out of scope, the contents are flushed to the output.
-    RtcEventLogFactory rtc_event_log_factory(task_queue_factory.get());
+    std::unique_ptr<RtcEventLog> log = RtcEventLogFactory().Create(
-    // When `log` goes out of scope, the contents are flushed
+        CreateEnvironment(CreateFieldTrialsFor(encoding_type_)));
-    // to the output.
-    std::unique_ptr<RtcEventLog> log =
-        rtc_event_log_factory.CreateRtcEventLog(encoding_type_);
 
     for (size_t i = 0; i < kNumEvents; i++) {
       // The purpose of the test is to verify that the log can handle