Delete deprecated ReceiveSideCongestionController constructor

This makes Environment and thus field trials a required parameter, thus
RemoteBitrateEstimators member no longer need to fallback to the global field trial string.

Bug: webrtc:42220378
Change-Id: Ieb6ff442d5fde5fa9715573c758a7e078f0ceea4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/349922
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42314}

modules/congestion_controller/include/receive_side_congestion_controller.h

@@ -42,12 +42,6 @@ class ReceiveSideCongestionController : public CallStatsObserver {
       RembThrottler::RembSender remb_sender,
       absl::Nullable<NetworkStateEstimator*> network_state_estimator);
 
-  [[deprecated]] ReceiveSideCongestionController(
-      Clock* clock,
-      RemoteEstimatorProxy::TransportFeedbackSender feedback_sender,
-      RembThrottler::RembSender remb_sender,
-      NetworkStateEstimator* network_state_estimator);
-
   ~ReceiveSideCongestionController() override = default;
 
   void OnReceivedPacket(const RtpPacketReceived& packet, MediaType media_type);
@@ -80,7 +74,7 @@ class ReceiveSideCongestionController : public CallStatsObserver {
   void PickEstimator(bool has_absolute_send_time)
       RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
 
-  Clock& clock_;
+  const Environment env_;
   RembThrottler remb_throttler_;
   RemoteEstimatorProxy remote_estimator_proxy_;
 

modules/congestion_controller/receive_side_congestion_controller.cc

@@ -51,7 +51,7 @@ void ReceiveSideCongestionController::PickEstimator(
           << "WrappingBitrateEstimator: Switching to absolute send time RBE.";
       using_absolute_send_time_ = true;
       rbe_ = std::make_unique<RemoteBitrateEstimatorAbsSendTime>(
-          &remb_throttler_, &clock_);
+          env_, &remb_throttler_);
     }
     packets_since_absolute_send_time_ = 0;
   } else {
@@ -64,7 +64,7 @@ void ReceiveSideCongestionController::PickEstimator(
                "time offset RBE.";
         using_absolute_send_time_ = false;
         rbe_ = std::make_unique<RemoteBitrateEstimatorSingleStream>(
-            &remb_throttler_, &clock_);
+            env_, &remb_throttler_);
       }
     }
   }
@@ -75,26 +75,13 @@ ReceiveSideCongestionController::ReceiveSideCongestionController(
     RemoteEstimatorProxy::TransportFeedbackSender feedback_sender,
     RembThrottler::RembSender remb_sender,
     absl::Nullable<NetworkStateEstimator*> network_state_estimator)
-    : clock_(env.clock()),
+    : env_(env),
-      remb_throttler_(std::move(remb_sender), &clock_),
+      remb_throttler_(std::move(remb_sender), &env_.clock()),
       remote_estimator_proxy_(std::move(feedback_sender),
                               network_state_estimator),
-      rbe_(
+      rbe_(std::make_unique<RemoteBitrateEstimatorSingleStream>(
-          std::make_unique<RemoteBitrateEstimatorSingleStream>(&remb_throttler_,
+          env_,
-                                                               &clock_)),
+          &remb_throttler_)),
-      using_absolute_send_time_(false),
-      packets_since_absolute_send_time_(0) {}
-
-ReceiveSideCongestionController::ReceiveSideCongestionController(
-    Clock* clock,
-    RemoteEstimatorProxy::TransportFeedbackSender feedback_sender,
-    RembThrottler::RembSender remb_sender,
-    NetworkStateEstimator* network_state_estimator)
-    : clock_(*clock),
-      remb_throttler_(std::move(remb_sender), clock),
-      remote_estimator_proxy_(std::move(feedback_sender),
-                              network_state_estimator),
-      rbe_(new RemoteBitrateEstimatorSingleStream(&remb_throttler_, clock)),
       using_absolute_send_time_(false),
       packets_since_absolute_send_time_(0) {}
 
@@ -125,7 +112,7 @@ void ReceiveSideCongestionController::OnBitrateChanged(int bitrate_bps) {
 }
 
 TimeDelta ReceiveSideCongestionController::MaybeProcess() {
-  Timestamp now = clock_.CurrentTime();
+  Timestamp now = env_.clock().CurrentTime();
   mutex_.Lock();
   TimeDelta time_until_rbe = rbe_->Process();
   mutex_.Unlock();

modules/remote_bitrate_estimator/BUILD.gn

@@ -36,7 +36,7 @@ rtc_library("remote_bitrate_estimator") {
     "../../api:field_trials_view",
     "../../api:network_state_predictor_api",
     "../../api:rtp_headers",
-    "../../api/transport:field_trial_based_config",
+    "../../api/environment",
     "../../api/transport:network_control",
     "../../api/units:data_rate",
     "../../api/units:data_size",
@@ -55,10 +55,10 @@ rtc_library("remote_bitrate_estimator") {
     "../../rtc_base/experiments:field_trial_parser",
     "../../rtc_base/synchronization:mutex",
     "../../system_wrappers",
-    "../../system_wrappers:field_trial",
     "../../system_wrappers:metrics",
   ]
   absl_deps = [
+    "//third_party/abseil-cpp/absl/base:nullability",
     "//third_party/abseil-cpp/absl/strings",
     "//third_party/abseil-cpp/absl/types:optional",
   ]
@@ -120,6 +120,7 @@ if (rtc_include_tests) {
     deps = [
       ":remote_bitrate_estimator",
       "..:module_api_public",
+      "../../api/environment:environment_factory",
       "../../api/transport:mock_network_control",
       "../../api/transport:network_control",
       "../../api/units:data_rate",

modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc

@@ -16,7 +16,8 @@
 #include <memory>
 #include <utility>
 
-#include "api/transport/field_trial_based_config.h"
+#include "absl/base/nullability.h"
+#include "api/environment/environment.h"
 #include "api/units/data_rate.h"
 #include "api/units/data_size.h"
 #include "api/units/time_delta.h"
@@ -88,11 +89,9 @@ void RemoteBitrateEstimatorAbsSendTime::MaybeAddCluster(
 }
 
 RemoteBitrateEstimatorAbsSendTime::RemoteBitrateEstimatorAbsSendTime(
-    RemoteBitrateObserver* observer,
+    const Environment& env,
-    Clock* clock)
+    absl::Nonnull<RemoteBitrateObserver*> observer)
-    : clock_(clock), observer_(observer), remote_rate_(field_trials_) {
+    : env_(env), observer_(observer), remote_rate_(env_.field_trials()) {
-  RTC_DCHECK(clock_);
-  RTC_DCHECK(observer_);
   RTC_LOG(LS_INFO) << "RemoteBitrateEstimatorAbsSendTime: Instantiating.";
 }
 
@@ -225,7 +224,7 @@ void RemoteBitrateEstimatorAbsSendTime::IncomingPacket(
   Timestamp send_time =
       Timestamp::Millis(static_cast<int64_t>(timestamp) * kTimestampToMs);
 
-  Timestamp now = clock_->CurrentTime();
+  Timestamp now = env_.clock().CurrentTime();
   // TODO(holmer): SSRCs are only needed for REMB, should be broken out from
   // here.
 

modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h

@@ -19,8 +19,9 @@
 #include <memory>
 #include <vector>
 
+#include "absl/base/nullability.h"
+#include "api/environment/environment.h"
 #include "api/rtp_headers.h"
-#include "api/transport/field_trial_based_config.h"
 #include "api/units/data_rate.h"
 #include "api/units/data_size.h"
 #include "api/units/time_delta.h"
@@ -32,14 +33,14 @@
 #include "modules/remote_bitrate_estimator/overuse_estimator.h"
 #include "rtc_base/bitrate_tracker.h"
 #include "rtc_base/checks.h"
-#include "system_wrappers/include/clock.h"
 
 namespace webrtc {
 
 class RemoteBitrateEstimatorAbsSendTime : public RemoteBitrateEstimator {
  public:
-  RemoteBitrateEstimatorAbsSendTime(RemoteBitrateObserver* observer,
+  RemoteBitrateEstimatorAbsSendTime(
-                                    Clock* clock);
+      const Environment& env,
+      absl::Nonnull<RemoteBitrateObserver*> observer);
 
   RemoteBitrateEstimatorAbsSendTime() = delete;
   RemoteBitrateEstimatorAbsSendTime(const RemoteBitrateEstimatorAbsSendTime&) =
@@ -98,9 +99,8 @@ class RemoteBitrateEstimatorAbsSendTime : public RemoteBitrateEstimator {
 
   void TimeoutStreams(Timestamp now);
 
-  Clock* const clock_;
+  const Environment env_;
-  const FieldTrialBasedConfig field_trials_;
+  const absl::Nonnull<RemoteBitrateObserver*> observer_;
-  RemoteBitrateObserver* const observer_;
   std::unique_ptr<InterArrival> inter_arrival_;
   std::unique_ptr<OveruseEstimator> estimator_;
   OveruseDetector detector_;

modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time_unittest.cc

@@ -10,6 +10,9 @@
 
 #include "modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h"
 
+#include <memory>
+
+#include "api/environment/environment_factory.h"
 #include "modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.h"
 #include "test/gtest.h"
 
@@ -25,12 +28,10 @@ class RemoteBitrateEstimatorAbsSendTimeTest
   RemoteBitrateEstimatorAbsSendTimeTest& operator=(
       const RemoteBitrateEstimatorAbsSendTimeTest&) = delete;
 
-  virtual void SetUp() {
+  void SetUp() override {
-    bitrate_estimator_.reset(new RemoteBitrateEstimatorAbsSendTime(
+    bitrate_estimator_ = std::make_unique<RemoteBitrateEstimatorAbsSendTime>(
-        bitrate_observer_.get(), &clock_));
+        CreateEnvironment(&clock_), bitrate_observer_.get());
   }
-
- protected:
 };
 
 TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, InitialBehavior) {

modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc

@@ -13,7 +13,9 @@
 #include <cstdint>
 #include <utility>
 
+#include "absl/base/nullability.h"
 #include "absl/types/optional.h"
+#include "api/environment/environment.h"
 #include "modules/remote_bitrate_estimator/aimd_rate_control.h"
 #include "modules/remote_bitrate_estimator/include/bwe_defines.h"
 #include "modules/remote_bitrate_estimator/inter_arrival.h"
@@ -39,13 +41,13 @@ RemoteBitrateEstimatorSingleStream::Detector::Detector()
       inter_arrival(90 * kTimestampGroupLengthMs, kTimestampToMs) {}
 
 RemoteBitrateEstimatorSingleStream::RemoteBitrateEstimatorSingleStream(
-    RemoteBitrateObserver* observer,
+    const Environment& env,
-    Clock* clock)
+    absl::Nonnull<RemoteBitrateObserver*> observer)
-    : clock_(clock),
+    : env_(env),
+      observer_(observer),
       incoming_bitrate_(kBitrateWindow),
       last_valid_incoming_bitrate_(DataRate::Zero()),
-      remote_rate_(field_trials_),
+      remote_rate_(env_.field_trials()),
-      observer_(observer),
       process_interval_(kProcessInterval),
       uma_recorded_(false) {
   RTC_LOG(LS_INFO) << "RemoteBitrateEstimatorSingleStream: Instantiating.";
@@ -68,7 +70,7 @@ void RemoteBitrateEstimatorSingleStream::IncomingPacket(
   uint32_t ssrc = rtp_packet.Ssrc();
   uint32_t rtp_timestamp =
       rtp_packet.Timestamp() + transmission_time_offset.value_or(0);
-  Timestamp now = clock_->CurrentTime();
+  Timestamp now = env_.clock().CurrentTime();
   Detector& estimator = overuse_detectors_[ssrc];
   estimator.last_packet_time = now;
 
@@ -114,7 +116,7 @@ void RemoteBitrateEstimatorSingleStream::IncomingPacket(
 }
 
 TimeDelta RemoteBitrateEstimatorSingleStream::Process() {
-  Timestamp now = clock_->CurrentTime();
+  Timestamp now = env_.clock().CurrentTime();
   Timestamp next_process_time = last_process_time_.has_value()
                                     ? *last_process_time_ + process_interval_
                                     : now;

modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h

@@ -17,8 +17,9 @@
 #include <map>
 #include <vector>
 
+#include "absl/base/nullability.h"
 #include "absl/types/optional.h"
-#include "api/transport/field_trial_based_config.h"
+#include "api/environment/environment.h"
 #include "api/units/data_rate.h"
 #include "api/units/time_delta.h"
 #include "api/units/timestamp.h"
@@ -31,13 +32,11 @@
 
 namespace webrtc {
 
-class Clock;
-struct RTPHeader;
-
 class RemoteBitrateEstimatorSingleStream : public RemoteBitrateEstimator {
  public:
-  RemoteBitrateEstimatorSingleStream(RemoteBitrateObserver* observer,
+  RemoteBitrateEstimatorSingleStream(
-                                     Clock* clock);
+      const Environment& env,
+      absl::Nonnull<RemoteBitrateObserver*> observer);
 
   RemoteBitrateEstimatorSingleStream() = delete;
   RemoteBitrateEstimatorSingleStream(
@@ -68,13 +67,12 @@ class RemoteBitrateEstimatorSingleStream : public RemoteBitrateEstimator {
 
   std::vector<uint32_t> GetSsrcs() const;
 
-  Clock* const clock_;
+  const Environment env_;
-  const FieldTrialBasedConfig field_trials_;
+  const absl::Nonnull<RemoteBitrateObserver*> observer_;
   std::map<uint32_t, Detector> overuse_detectors_;
   BitrateTracker incoming_bitrate_;
   DataRate last_valid_incoming_bitrate_;
   AimdRateControl remote_rate_;
-  RemoteBitrateObserver* const observer_;
   absl::optional<Timestamp> last_process_time_;
   TimeDelta process_interval_;
   bool uma_recorded_;

modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream_unittest.cc

@@ -10,6 +10,9 @@
 
 #include "modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h"
 
+#include <memory>
+
+#include "api/environment/environment_factory.h"
 #include "modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.h"
 #include "test/gtest.h"
 
@@ -24,12 +27,10 @@ class RemoteBitrateEstimatorSingleTest : public RemoteBitrateEstimatorTest {
   RemoteBitrateEstimatorSingleTest& operator=(
       const RemoteBitrateEstimatorSingleTest&) = delete;
 
-  virtual void SetUp() {
+  void SetUp() override {
-    bitrate_estimator_.reset(new RemoteBitrateEstimatorSingleStream(
+    bitrate_estimator_ = std::make_unique<RemoteBitrateEstimatorSingleStream>(
-        bitrate_observer_.get(), &clock_));
+        CreateEnvironment(&clock_), bitrate_observer_.get());
   }
-
- protected:
 };
 
 TEST_F(RemoteBitrateEstimatorSingleTest, InitialBehavior) {