From 5bf60e446deaaa46b961a2d3589e0658274dab3d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Henrik=20Bostr=C3=B6m?= <hbos@webrtc.org>
Date: Wed, 13 May 2020 15:20:25 +0200
Subject: [PATCH] VideoStreamEncoderTest: Wait for QP usage handled callback
 event.

(Misc cleanup associated with
https://webrtc-review.googlesource.com/c/src/+/174441.)

This test was previously assuming what when QP usage is handled it
first posts to the adaptation queue and then back with the result from
the encoder queue.

While the assumption is correct it is not an implementation detail that
the test was trying to assert, nor do we need such a test.

TriggerQualityScalerHighQpAndReturnIfQpSamplesShouldBeCleared() is
updated to wait for an event associated with QP having been handled,
which is all that the test really cares about.

Bug: webrtc:11542, webrtc:11520
Change-Id: I3286c3ab631f09c43abe0fd59f31c3997aedd9f8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/175004
Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Evan Shrubsole <eshr@google.com>
Cr-Commit-Position: refs/heads/master@{#31243}
---
 video/video_stream_encoder_unittest.cc | 28 ++++++++++++++------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc
index 5123d45bdd..e585a41cdc 100644
--- a/video/video_stream_encoder_unittest.cc
+++ b/video/video_stream_encoder_unittest.cc
@@ -156,18 +156,27 @@ class FakeQualityScalerQpUsageHandlerCallback
     : public QualityScalerQpUsageHandlerCallbackInterface {
  public:
   FakeQualityScalerQpUsageHandlerCallback()
-      : QualityScalerQpUsageHandlerCallbackInterface() {}
-  ~FakeQualityScalerQpUsageHandlerCallback() override {}
+      : QualityScalerQpUsageHandlerCallbackInterface(),
+        qp_usage_handled_event_(/*manual_reset=*/true,
+                                /*initially_signaled=*/false),
+        clear_qp_samples_result_(absl::nullopt) {}
+  ~FakeQualityScalerQpUsageHandlerCallback() override {
+    RTC_DCHECK(clear_qp_samples_result_.has_value());
+  }
 
   void OnQpUsageHandled(bool clear_qp_samples) override {
     clear_qp_samples_result_ = clear_qp_samples;
+    qp_usage_handled_event_.Set();
   }
 
+  bool WaitForQpUsageHandled() { return qp_usage_handled_event_.Wait(5000); }
+
   absl::optional<bool> clear_qp_samples_result() const {
     return clear_qp_samples_result_;
   }
 
  private:
+  rtc::Event qp_usage_handled_event_;
   absl::optional<bool> clear_qp_samples_result_;
 };
 
@@ -310,21 +319,14 @@ class VideoStreamEncoderUnderTest : public VideoStreamEncoder {
   // QualityScalerResource. Returns whether or not QP samples would have been
   // cleared if this had been a real signal from the QualityScaler.
   bool TriggerQualityScalerHighQpAndReturnIfQpSamplesShouldBeCleared() {
-    rtc::Event event;
     rtc::scoped_refptr<FakeQualityScalerQpUsageHandlerCallback> callback =
         new FakeQualityScalerQpUsageHandlerCallback();
-    encoder_queue()->PostTask([this, &event, callback] {
-      // This should post a usage measurement to the adaptation processor.
+    encoder_queue()->PostTask([this, callback] {
+      // This will cause a "ping" between adaptation task queue and encoder
+      // queue. When we have the result, the |callback| will be notified.
       quality_scaler_resource_for_testing()->OnReportQpUsageHigh(callback);
-      // Give the processor a chance to react and trigger adaptation on the
-      // adaptation queue.
-      resource_adaptation_queue()->PostTask([this, &event] {
-        // Finally, give the QualityScalerResource time to resolve the callback
-        // on the encoder queue.
-        encoder_queue()->PostTask([&event] { event.Set(); });
-      });
     });
-    EXPECT_TRUE(event.Wait(5000));
+    EXPECT_TRUE(callback->WaitForQpUsageHandled());
     EXPECT_TRUE(callback->clear_qp_samples_result().has_value());
     return callback->clear_qp_samples_result().value();
   }