Pass Clock through Environment when constructing Call

while cleaning up Call factory function,

- pick rtp_transport_controller_send_factory based on presence in the config instead of based on the call site thus removing one extra factory function.

- when Call is created through test helper TimeControllerBasedFactory use original media factory instead of direct factory, thus allow to configure degraded call through field trials in tests, and ensure difference with production code path stay minimal in the future.

Bug: webrtc:15656
Change-Id: If9c2a9fc871e139502db2bec0a241d8d64c53720
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/330061
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41329}

5 files changed, 43 insertions, 59 deletions

diff --git a/api/BUILD.gn b/api/BUILD.gn
index ac97da52ad..3413b93b46 100644
--- a/api/BUILD.gn
+++ b/api/BUILD.gn
@@ -1391,12 +1391,12 @@ if (rtc_include_tests) {
       ":time_controller",
       "../call",
       "../call:call_interfaces",
-      "../call:rtp_interfaces",
       "../pc:media_factory",
       "../rtc_base:checks",
       "../system_wrappers",
       "../test/time_controller",
       "environment",
+      "environment:environment_factory",
     ]
     absl_deps = [ "//third_party/abseil-cpp/absl/base:nullability" ]
   }
diff --git a/api/test/create_time_controller.cc b/api/test/create_time_controller.cc
index 7523e05208..cbf1f09aa1 100644
--- a/api/test/create_time_controller.cc
+++ b/api/test/create_time_controller.cc
@@ -16,10 +16,10 @@
 #include "absl/base/nullability.h"
 #include "api/enable_media_with_defaults.h"
 #include "api/environment/environment.h"
+#include "api/environment/environment_factory.h"
 #include "api/peer_connection_interface.h"
 #include "call/call.h"
-#include "call/rtp_transport_config.h"
-#include "call/rtp_transport_controller_send_factory_interface.h"
+#include "call/call_config.h"
 #include "pc/media_factory.h"
 #include "rtc_base/checks.h"
 #include "system_wrappers/include/clock.h"
@@ -49,9 +49,12 @@ void EnableMediaWithDefaultsAndTimeController(
         : clock_(clock), media_factory_(std::move(media_factory)) {}
 
     std::unique_ptr<Call> CreateCall(const CallConfig& config) override {
-      return Call::Create(config, clock_,
-                          config.rtp_transport_controller_send_factory->Create(
-                              config.ExtractTransportConfig(), clock_));
+      EnvironmentFactory env_factory(config.env);
+      env_factory.Set(clock_);
+
+      CallConfig config_with_custom_clock = config;
+      config_with_custom_clock.env = env_factory.Create();
+      return media_factory_->CreateCall(config_with_custom_clock);
     }
 
     std::unique_ptr<cricket::MediaEngineInterface> CreateMediaEngine(
diff --git a/call/call.cc b/call/call.cc
index d9ea94a06a..7e6d57a37e 100644
--- a/call/call.cc
+++ b/call/call.cc
@@ -183,8 +183,7 @@ class Call final : public webrtc::Call,
                    public TargetTransferRateObserver,
                    public BitrateAllocator::LimitObserver {
  public:
-  Call(Clock* clock,
-       const CallConfig& config,
+  Call(const CallConfig& config,
        std::unique_ptr<RtpTransportControllerSendInterface> transport_send);
   ~Call() override;
 
@@ -344,9 +343,6 @@ class Call final : public webrtc::Call,
   // callbacks have been registered.
   void EnsureStarted() RTC_RUN_ON(worker_thread_);
 
-  // TODO: bugs.webrtc.org/15656 - Delete `clock_` when it would always be the
-  // same as &env_.clock()
-  Clock* const clock_;
   const Environment env_;
   TaskQueueBase* const worker_thread_;
   TaskQueueBase* const network_thread_;
@@ -471,19 +467,16 @@ std::string Call::Stats::ToString(int64_t time_ms) const {
 }
 
 std::unique_ptr<Call> Call::Create(const CallConfig& config) {
-  Clock* clock = &config.env.clock();
-  return Create(config, clock,
-                RtpTransportControllerSendFactory().Create(
-                    config.ExtractTransportConfig(), clock));
-}
+  std::unique_ptr<RtpTransportControllerSendInterface> transport_send;
+  if (config.rtp_transport_controller_send_factory != nullptr) {
+    transport_send = config.rtp_transport_controller_send_factory->Create(
+        config.ExtractTransportConfig(), &config.env.clock());
+  } else {
+    transport_send = RtpTransportControllerSendFactory().Create(
+        config.ExtractTransportConfig(), &config.env.clock());
+  }
 
-std::unique_ptr<Call> Call::Create(
-    const CallConfig& config,
-    Clock* clock,
-    std::unique_ptr<RtpTransportControllerSendInterface>
-        transportControllerSend) {
-  return std::make_unique<internal::Call>(clock, config,
-                                          std::move(transportControllerSend));
+  return std::make_unique<internal::Call>(config, std::move(transport_send));
 }
 
 // This method here to avoid subclasses has to implement this method.
@@ -649,11 +642,9 @@ void Call::SendStats::SetMinAllocatableRate(BitrateAllocationLimits limits) {
   min_allocated_send_bitrate_bps_ = limits.min_allocatable_rate.bps();
 }
 
-Call::Call(Clock* clock,
-           const CallConfig& config,
+Call::Call(const CallConfig& config,
            std::unique_ptr<RtpTransportControllerSendInterface> transport_send)
-    : clock_(clock),
-      env_(config.env),
+    : env_(config.env),
       worker_thread_(GetCurrentTaskQueueOrThread()),
       // If `network_task_queue_` was set to nullptr, network related calls
       // must be made on `worker_thread_` (i.e. they're one and the same).
@@ -661,20 +652,20 @@ Call::Call(Clock* clock,
                                                  : worker_thread_),
       decode_sync_(
           config.decode_metronome
-              ? std::make_unique<DecodeSynchronizer>(clock_,
+              ? std::make_unique<DecodeSynchronizer>(&env_.clock(),
                                                      config.decode_metronome,
                                                      worker_thread_)
               : nullptr),
       num_cpu_cores_(CpuInfo::DetectNumberOfCores()),
-      call_stats_(new CallStats(clock_, worker_thread_)),
+      call_stats_(new CallStats(&env_.clock(), worker_thread_)),
       bitrate_allocator_(new BitrateAllocator(this)),
       config_(config),
       audio_network_state_(kNetworkDown),
       video_network_state_(kNetworkDown),
       aggregate_network_up_(false),
-      receive_stats_(clock_),
-      send_stats_(clock_),
-      receive_side_cc_(clock,
+      receive_stats_(&env_.clock()),
+      send_stats_(&env_.clock()),
+      receive_side_cc_(&env_.clock(),
                        absl::bind_front(&PacketRouter::SendCombinedRtcpPacket,
                                         transport_send->packet_router()),
                        absl::bind_front(&PacketRouter::SendRemb,
@@ -682,8 +673,8 @@ Call::Call(Clock* clock,
                        /*network_state_estimator=*/nullptr),
       receive_time_calculator_(
           ReceiveTimeCalculator::CreateFromFieldTrial(env_.field_trials())),
-      video_send_delay_stats_(new SendDelayStats(clock_)),
-      start_of_call_(clock_->CurrentTime()),
+      video_send_delay_stats_(new SendDelayStats(&env_.clock())),
+      start_of_call_(env_.clock().CurrentTime()),
       transport_send_ptr_(transport_send.get()),
       transport_send_(std::move(transport_send)) {
   RTC_DCHECK(network_thread_);
@@ -703,7 +694,7 @@ Call::Call(Clock* clock,
   receive_side_cc_periodic_task_ = RepeatingTaskHandle::Start(
       worker_thread_,
       [receive_side_cc] { return receive_side_cc->MaybeProcess(); },
-      TaskQueueBase::DelayPrecision::kLow, clock_);
+      TaskQueueBase::DelayPrecision::kLow, &env_.clock());
 }
 
 Call::~Call() {
@@ -721,7 +712,7 @@ Call::~Call() {
 
   RTC_HISTOGRAM_COUNTS_100000(
       "WebRTC.Call.LifetimeInSeconds",
-      (clock_->CurrentTime() - start_of_call_).seconds());
+      (env_.clock().CurrentTime() - start_of_call_).seconds());
 }
 
 void Call::EnsureStarted() {
@@ -766,7 +757,7 @@ webrtc::AudioSendStream* Call::CreateAudioSendStream(
   }
 
   AudioSendStream* send_stream = new AudioSendStream(
-      clock_, config, config_.audio_state, &env_.task_queue_factory(),
+      &env_.clock(), config, config_.audio_state, &env_.task_queue_factory(),
       transport_send_.get(), bitrate_allocator_.get(), &env_.event_log(),
       call_stats_->AsRtcpRttStats(), suspended_rtp_state, trials());
   RTC_DCHECK(audio_send_ssrcs_.find(config.rtp.ssrc) ==
@@ -823,8 +814,8 @@ webrtc::AudioReceiveStreamInterface* Call::CreateAudioReceiveStream(
       CreateRtcLogStreamConfig(config)));
 
   AudioReceiveStreamImpl* receive_stream = new AudioReceiveStreamImpl(
-      clock_, transport_send_->packet_router(), config_.neteq_factory, config,
-      config_.audio_state, &env_.event_log());
+      &env_.clock(), transport_send_->packet_router(), config_.neteq_factory,
+      config, config_.audio_state, &env_.event_log());
   audio_receive_streams_.insert(receive_stream);
 
   // TODO(bugs.webrtc.org/11993): Make the registration on the network thread
@@ -896,8 +887,8 @@ webrtc::VideoSendStream* Call::CreateVideoSendStream(
   std::vector<uint32_t> ssrcs = config.rtp.ssrcs;
 
   VideoSendStream* send_stream = new VideoSendStream(
-      clock_, num_cpu_cores_, &env_.task_queue_factory(), network_thread_,
-      call_stats_->AsRtcpRttStats(), transport_send_.get(),
+      &env_.clock(), num_cpu_cores_, &env_.task_queue_factory(),
+      network_thread_, call_stats_->AsRtcpRttStats(), transport_send_.get(),
       bitrate_allocator_.get(), video_send_delay_stats_.get(),
       &env_.event_log(), std::move(config), std::move(encoder_config),
       suspended_video_send_ssrcs_, suspended_video_payload_states_,
@@ -930,7 +921,7 @@ webrtc::VideoSendStream* Call::CreateVideoSendStream(
   std::unique_ptr<FecController> fec_controller =
       config_.fec_controller_factory
           ? config_.fec_controller_factory->CreateFecController()
-          : std::make_unique<FecControllerDefault>(clock_);
+          : std::make_unique<FecControllerDefault>(&env_.clock());
   return CreateVideoSendStream(std::move(config), std::move(encoder_config),
                                std::move(fec_controller));
 }
@@ -997,7 +988,8 @@ webrtc::VideoReceiveStreamInterface* Call::CreateVideoReceiveStream(
   VideoReceiveStream2* receive_stream = new VideoReceiveStream2(
       &env_.task_queue_factory(), this, num_cpu_cores_,
       transport_send_->packet_router(), std::move(configuration),
-      call_stats_.get(), clock_, std::make_unique<VCMTiming>(clock_, trials()),
+      call_stats_.get(), &env_.clock(),
+      std::make_unique<VCMTiming>(&env_.clock(), trials()),
       &nack_periodic_processor_, decode_sync_.get(), &env_.event_log());
   // TODO(bugs.webrtc.org/11993): Set this up asynchronously on the network
   // thread.
@@ -1041,7 +1033,7 @@ FlexfecReceiveStream* Call::CreateFlexfecReceiveStream(
   // OnRtpPacket until the constructor is finished and the object is
   // in a valid state, since OnRtpPacket runs on the same thread.
   FlexfecReceiveStreamImpl* receive_stream = new FlexfecReceiveStreamImpl(
-      clock_, std::move(config), &video_receiver_controller_,
+      &env_.clock(), std::move(config), &video_receiver_controller_,
       call_stats_->AsRtcpRttStats());
 
   // TODO(bugs.webrtc.org/11993): Set this up asynchronously on the network
@@ -1245,7 +1237,7 @@ void Call::OnSentPacket(const rtc::SentPacket& sent_packet) {
   // on a ProcessThread. This is alright as is since we forward the call to
   // implementations that either just do a PostTask or use locking.
   video_send_delay_stats_->OnSentPacket(sent_packet.packet_id,
-                                        clock_->CurrentTime());
+                                        env_.clock().CurrentTime());
   transport_send_->OnSentPacket(sent_packet);
 }
 
@@ -1369,7 +1361,8 @@ void Call::DeliverRtpPacket(
     // Repair packet_time_us for clock resets by comparing a new read of
     // the same clock (TimeUTCMicros) to a monotonic clock reading.
     packet_time_us = receive_time_calculator_->ReconcileReceiveTimes(
-        packet_time_us, rtc::TimeUTCMicros(), clock_->TimeInMicroseconds());
+        packet_time_us, rtc::TimeUTCMicros(),
+        env_.clock().TimeInMicroseconds());
     packet.set_arrival_time(Timestamp::Micros(packet_time_us));
   }
 
diff --git a/call/call.h b/call/call.h
index 6f8e4cd6d7..a680335192 100644
--- a/call/call.h
+++ b/call/call.h
@@ -24,7 +24,6 @@
 #include "call/call_config.h"
 #include "call/flexfec_receive_stream.h"
 #include "call/packet_receiver.h"
-#include "call/rtp_transport_controller_send_interface.h"
 #include "call/video_receive_stream.h"
 #include "call/video_send_stream.h"
 #include "rtc_base/copy_on_write_buffer.h"
@@ -57,11 +56,6 @@ class Call {
   };
 
   static std::unique_ptr<Call> Create(const CallConfig& config);
-  static std::unique_ptr<Call> Create(
-      const CallConfig& config,
-      Clock* clock,
-      std::unique_ptr<RtpTransportControllerSendInterface>
-          transportControllerSend);
 
   virtual AudioSendStream* CreateAudioSendStream(
       const AudioSendStream::Config& config) = 0;
diff --git a/call/create_call.cc b/call/create_call.cc
index 44b204db5c..8b565745a8 100644
--- a/call/create_call.cc
+++ b/call/create_call.cc
@@ -22,7 +22,6 @@
 #include "api/units/time_delta.h"
 #include "call/call.h"
 #include "call/degraded_call.h"
-#include "call/rtp_transport_config.h"
 #include "rtc_base/checks.h"
 #include "rtc_base/experiments/field_trial_list.h"
 #include "rtc_base/experiments/field_trial_parser.h"
@@ -85,12 +84,7 @@ std::unique_ptr<Call> CreateCall(const CallConfig& config) {
       receive_degradation_configs =
           GetNetworkConfigs(config.env.field_trials(), /*send=*/false);
 
-  RtpTransportConfig transportConfig = config.ExtractTransportConfig();
-
-  std::unique_ptr<Call> call =
-      Call::Create(config, Clock::GetRealTimeClock(),
-                   config.rtp_transport_controller_send_factory->Create(
-                       transportConfig, Clock::GetRealTimeClock()));
+  std::unique_ptr<Call> call = Call::Create(config);
 
   if (!send_degradation_configs.empty() ||
       !receive_degradation_configs.empty()) {