Disabling periodic tasks on SSCC in unit tests.
Time triggered tasks in the SendSideCongestionController caused
flakyness in long running unit tests of SendSideCongestionController.
This CL lets the unit test code disable the periodic tasks so they are
only triggered on demand.
Bug: webrtc:9039
Change-Id: I934045d7e6eeaa765dd221cef87389f1d98b58a5
Reviewed-on: https://webrtc-review.googlesource.com/63265
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22536}diff --git a/modules/congestion_controller/rtp/include/send_side_congestion_controller.h b/modules/congestion_controller/rtp/include/send_side_congestion_controller.h
index 9ce0826..04c8e6d 100644
--- a/modules/congestion_controller/rtp/include/send_side_congestion_controller.h
+++ b/modules/congestion_controller/rtp/include/send_side_congestion_controller.h
@@ -130,6 +130,10 @@
protected:
// TODO(srte): The tests should be rewritten to not depend on internals and
// these functions should be removed.
+ // Since we can't control the timing of the internal task queue, this method
+ // is used in unit tests to stop the periodic tasks from running unless
+ // PostPeriodicTasksForTest is called.
+ void DisablePeriodicTasks();
// Post periodic tasks just once. This allows unit tests to trigger process
// updates immediately.
void PostPeriodicTasksForTest();
@@ -182,6 +186,7 @@
// TODO(srte): Remove atomic when feedback adapter runs on task queue.
std::atomic<size_t> transport_overhead_bytes_per_packet_;
bool network_available_ RTC_GUARDED_BY(task_queue_ptr_);
+ bool periodic_tasks_enabled_ RTC_GUARDED_BY(task_queue_ptr_);
// Protects access to last_packet_feedback_vector_ in feedback adapter.
// TODO(srte): Remove this checker when feedback adapter runs on task queue.
diff --git a/modules/congestion_controller/rtp/send_side_congestion_controller.cc b/modules/congestion_controller/rtp/send_side_congestion_controller.cc
index c6510b1..f8035e3 100644
--- a/modules/congestion_controller/rtp/send_side_congestion_controller.cc
+++ b/modules/congestion_controller/rtp/send_side_congestion_controller.cc
@@ -314,6 +314,7 @@
webrtc::field_trial::IsEnabled("WebRTC-SendSideBwe-WithOverhead")),
transport_overhead_bytes_per_packet_(0),
network_available_(false),
+ periodic_tasks_enabled_(true),
task_queue_(MakeUnique<rtc::TaskQueue>("SendSideCCQueue")) {
task_queue_ptr_ = task_queue_.get();
initial_config_.constraints =
@@ -543,6 +544,8 @@
}
void SendSideCongestionController::StartProcessPeriodicTasks() {
+ if (!periodic_tasks_enabled_)
+ return;
task_queue_ptr_->PostDelayedTask(
NewPeriodicTask(
rtc::Bind(
@@ -653,6 +656,13 @@
});
}
+void SendSideCongestionController::DisablePeriodicTasks() {
+ task_queue_->PostTask([this]() {
+ RTC_DCHECK_RUN_ON(task_queue_ptr_);
+ periodic_tasks_enabled_ = false;
+ });
+}
+
void SendSideCongestionController::OnReceivedEstimatedBitrate(
uint32_t bitrate) {
RemoteBitrateReport msg;
diff --git a/modules/congestion_controller/rtp/send_side_congestion_controller_unittest.cc b/modules/congestion_controller/rtp/send_side_congestion_controller_unittest.cc
index ae9d1c4..b002b6d 100644
--- a/modules/congestion_controller/rtp/send_side_congestion_controller_unittest.cc
+++ b/modules/congestion_controller/rtp/send_side_congestion_controller_unittest.cc
@@ -48,6 +48,7 @@
public:
using SendSideCongestionController::SendSideCongestionController;
~SendSideCongestionControllerForTest() {}
+ using SendSideCongestionController::DisablePeriodicTasks;
void WaitOnTasks() { SendSideCongestionController::WaitOnTasksForTest(); }
void Process() override {
SendSideCongestionController::PostPeriodicTasksForTest();
@@ -77,6 +78,7 @@
controller_.reset(new SendSideCongestionControllerForTest(
&clock_, &event_log_, pacer_.get(), kInitialBitrateBps, 0,
5 * kInitialBitrateBps));
+ controller_->DisablePeriodicTasks();
controller_->RegisterNetworkObserver(&observer_);
controller_->SignalNetworkState(NetworkState::kNetworkUp);
bandwidth_observer_ = controller_->GetBandwidthObserver();
@@ -95,6 +97,7 @@
controller_.reset(new SendSideCongestionControllerForTest(
&clock_, &event_log_, pacer_.get(), kInitialBitrateBps, 0,
5 * kInitialBitrateBps));
+ controller_->DisablePeriodicTasks();
controller_->RegisterNetworkObserver(&target_bitrate_observer_);
controller_->SignalNetworkState(NetworkState::kNetworkUp);
}
@@ -472,13 +475,7 @@
EXPECT_LT(*target_bitrate_bps_, bitrate_before_delay);
}
-// TODO(bugs.webrtc.org/9039): Enable when cause of flakyness found.
-#if defined(WEBRTC_WIN)
-#define MAYBE_PacerQueueEncodeRatePushback DISABLED_PacerQueueEncodeRatePushback
-#else
-#define MAYBE_PacerQueueEncodeRatePushback PacerQueueEncodeRatePushback
-#endif
-TEST_F(SendSideCongestionControllerTest, MAYBE_PacerQueueEncodeRatePushback) {
+TEST_F(SendSideCongestionControllerTest, PacerQueueEncodeRatePushback) {
::webrtc::test::ScopedFieldTrials pushback_field_trial(
"WebRTC-PacerPushbackExperiment/Enabled/");
SetUp();
