commit 5bf60e446deaaa46b961a2d3589e0658274dab3d
author Henrik Boström <hbos@webrtc.org> Wed May 13 13:20:25 2020
committer Commit Bot <commit-bot@chromium.org> Wed May 13 15:23:25 2020
tree efae5d1c145b1d8d1af750d23d8f293b4dc3d616
parent 611fba451752b00d401d415c6ed57e93eb8f3a45

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

diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc
index 5123d45..e585a41 100644
--- a/video/video_stream_encoder_unittest.cc
+++ b/video/video_stream_encoder_unittest.cc
@@ -156,18 +156,27 @@
     : 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 @@
   // 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();
   }