[remoting] Add unittests for WebrtcFrameSchedulerSimple

This adds unittests for the changes made in
crrev.com/a6a3f1e8

This requires a small refactoring to allow unittests to fake the
current time for the WebrtcFrameSchedulerSimple class.

Bug: 773894
Change-Id: I00e1204b1eab446e18e5d8796770dbd64623abfc
Reviewed-on: https://chromium-review.googlesource.com/729565
Commit-Queue: Lambros Lambrou <lambroslambrou@chromium.org>
Reviewed-by: Zijie He <zijiehe@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512914}

remoting/protocol/webrtc_frame_scheduler_simple.cc

@@ -151,7 +151,7 @@ void WebrtcFrameSchedulerSimple::OnChannelParameters(int packet_loss,
 
 void WebrtcFrameSchedulerSimple::OnTargetBitrateChanged(int bandwidth_kbps) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  base::TimeTicks now = base::TimeTicks::Now();
+  base::TimeTicks now = Now();
   processing_time_estimator_.SetBandwidthKbps(bandwidth_kbps);
   pacing_bucket_.UpdateRate(bandwidth_kbps * 1000 / 8, now);
   encoder_bitrate_.SetBandwidthEstimate(bandwidth_kbps, now);
@@ -183,7 +183,7 @@ bool WebrtcFrameSchedulerSimple::OnFrameCaptured(
     WebrtcVideoEncoder::FrameParams* params_out) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  base::TimeTicks now = base::TimeTicks::Now();
+  base::TimeTicks now = Now();
 
   // Null |frame| indicates a capturer error.
   if (!frame) {
@@ -259,7 +259,7 @@ void WebrtcFrameSchedulerSimple::OnFrameEncoded(
   DCHECK(frame_pending_);
   frame_pending_ = false;
 
-  base::TimeTicks now = base::TimeTicks::Now();
+  base::TimeTicks now = Now();
 
   if (frame_stats) {
     // Calculate |send_pending_delay| before refilling |pacing_bucket_|.
@@ -289,9 +289,13 @@ void WebrtcFrameSchedulerSimple::OnFrameEncoded(
   }
 }
 
+void WebrtcFrameSchedulerSimple::SetCurrentTimeForTest(base::TimeTicks now) {
+  fake_now_for_test_ = now;
+}
+
 void WebrtcFrameSchedulerSimple::ScheduleNextFrame() {
   DCHECK(thread_checker_.CalledOnValidThread());
-  base::TimeTicks now = base::TimeTicks::Now();
+  base::TimeTicks now = Now();
 
   if (!encoder_ready_ || paused_ || pacing_bucket_.rate() == 0 ||
       capture_callback_.is_null() || frame_pending_) {
@@ -332,11 +336,16 @@ void WebrtcFrameSchedulerSimple::ScheduleNextFrame() {
 void WebrtcFrameSchedulerSimple::CaptureNextFrame() {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!frame_pending_);
-  last_capture_started_time_ = base::TimeTicks::Now();
+  last_capture_started_time_ = Now();
   processing_time_estimator_.StartFrame();
   frame_pending_ = true;
   capture_callback_.Run();
 }
 
+base::TimeTicks WebrtcFrameSchedulerSimple::Now() {
+  return fake_now_for_test_.is_null() ? base::TimeTicks::Now()
+                                      : fake_now_for_test_;
+}
+
 }  // namespace protocol
 }  // namespace remoting

remoting/protocol/webrtc_frame_scheduler_simple.h

@@ -42,6 +42,9 @@ class WebrtcFrameSchedulerSimple : public VideoChannelStateObserver,
   void OnFrameEncoded(const WebrtcVideoEncoder::EncodedFrame* encoded_frame,
                       HostFrameStats* frame_stats) override;
 
+  // Allows unit-tests to fake the current time.
+  void SetCurrentTimeForTest(base::TimeTicks now);
+
  private:
   // Helper class used to calculate target encoder bitrate.
   class EncoderBitrateFilter {
@@ -66,6 +69,13 @@ class WebrtcFrameSchedulerSimple : public VideoChannelStateObserver,
   void ScheduleNextFrame();
   void CaptureNextFrame();
 
+  // Returns the current time according to base::TimeTicks::Now(),
+  // or a fake time provided by a unit-test.
+  base::TimeTicks Now();
+
+  // Non-null if a fake current time is set by unit-test.
+  base::TimeTicks fake_now_for_test_;
+
   base::Closure capture_callback_;
   bool paused_ = false;
 

remoting/protocol/webrtc_frame_scheduler_unittest.cc

@@ -9,6 +9,11 @@
 #include "remoting/protocol/webrtc_dummy_video_encoder.h"
 #include "remoting/protocol/webrtc_frame_scheduler_simple.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/webrtc/modules/desktop_capture/desktop_frame.h"
+
+using webrtc::BasicDesktopFrame;
+using webrtc::DesktopRect;
+using webrtc::DesktopSize;
 
 namespace remoting {
 namespace protocol {
@@ -17,9 +22,11 @@ class WebrtcFrameSchedulerTest : public ::testing::Test {
  public:
   WebrtcFrameSchedulerTest()
       : task_runner_(new base::TestSimpleTaskRunner()),
-        task_runner_handle_(task_runner_.get()) {
+        task_runner_handle_(task_runner_.get()),
+        now_(base::TimeTicks::Now()) {
     video_encoder_factory_.reset(new WebrtcDummyVideoEncoderFactory());
     scheduler_.reset(new WebrtcFrameSchedulerSimple());
+    scheduler_->SetCurrentTimeForTest(now_);
     scheduler_->Start(video_encoder_factory_.get(),
                       base::Bind(&WebrtcFrameSchedulerTest::CaptureCallback,
                                  base::Unretained(this)));
@@ -38,9 +45,11 @@ class WebrtcFrameSchedulerTest : public ::testing::Test {
   base::ThreadTaskRunnerHandle task_runner_handle_;
 
   std::unique_ptr<WebrtcDummyVideoEncoderFactory> video_encoder_factory_;
-  std::unique_ptr<WebrtcFrameScheduler> scheduler_;
+  std::unique_ptr<WebrtcFrameSchedulerSimple> scheduler_;
 
   bool capture_callback_called_ = false;
+
+  base::TimeTicks now_;
 };
 
 TEST_F(WebrtcFrameSchedulerTest, UpdateBitrateWhenPending) {
@@ -62,6 +71,48 @@ TEST_F(WebrtcFrameSchedulerTest, UpdateBitrateWhenPending) {
   EXPECT_FALSE(task_runner_->HasPendingTask());
 }
 
+TEST_F(WebrtcFrameSchedulerTest, EmptyFrameUpdate_ShouldNotBeSentImmediately) {
+  auto video_channel_observer =
+      video_encoder_factory_->get_video_channel_state_observer_for_tests();
+  // Needed to avoid DCHECK in OnFrameCaptured().
+  video_channel_observer->OnTargetBitrateChanged(100);
+
+  WebrtcVideoEncoder::FrameParams outParams;
+  BasicDesktopFrame frame(DesktopSize(1, 1));
+  // Initial capture, full frame.
+  frame.mutable_updated_region()->SetRect(DesktopRect::MakeWH(1, 1));
+  scheduler_->OnFrameCaptured(&frame, &outParams);
+  // Empty frame.
+  frame.mutable_updated_region()->Clear();
+  bool result = scheduler_->OnFrameCaptured(&frame, &outParams);
+
+  // Should not be sent, because of throttling of empty frames.
+  EXPECT_FALSE(result);
+};
+
+TEST_F(WebrtcFrameSchedulerTest, EmptyFrameUpdate_ShouldBeSentAfter200ms) {
+  // Identical to the previous test, except it waits a short amount of time
+  // before the empty frame update.
+  auto video_channel_observer =
+      video_encoder_factory_->get_video_channel_state_observer_for_tests();
+  video_channel_observer->OnTargetBitrateChanged(100);
+
+  WebrtcVideoEncoder::FrameParams outParams;
+  BasicDesktopFrame frame(DesktopSize(1, 1));
+  // Initial capture, full frame.
+  frame.mutable_updated_region()->SetRect(DesktopRect::MakeWH(1, 1));
+  scheduler_->OnFrameCaptured(&frame, &outParams);
+  // Wait more than 200ms.
+  scheduler_->SetCurrentTimeForTest(now_ +
+                                    base::TimeDelta::FromMilliseconds(300));
+  // Empty frame.
+  frame.mutable_updated_region()->Clear();
+  bool result = scheduler_->OnFrameCaptured(&frame, &outParams);
+
+  // Empty frames should be sent at the throttled rate.
+  EXPECT_TRUE(result);
+};
+
 // TODO(sergeyu): Add more unittests.
 
 }  // namespace protocol