Refactor VideoRendererImpl to use VideoFrameScheduler.

media/filters/video_renderer_impl_unittest.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include <utility>
-
 #include "base/bind.h"
-#include "base/callback.h"
-#include "base/callback_helpers.h"
 #include "base/debug/stack_trace.h"
 #include "base/message_loop/message_loop.h"
-#include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/stringprintf.h"
-#include "base/synchronization/lock.h"
-#include "base/timer/timer.h"
-#include "media/base/data_buffer.h"
+#include "base/test/simple_test_tick_clock.h"
 #include "media/base/gmock_callback_support.h"
-#include "media/base/limits.h"
 #include "media/base/mock_filters.h"
 #include "media/base/test_helpers.h"
-#include "media/base/video_frame.h"
+#include "media/filters/test_video_frame_scheduler.h"
 #include "media/filters/video_renderer_impl.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using ::testing::_;
 using ::testing::AnyNumber;
-using ::testing::InSequence;
 using ::testing::Invoke;
 using ::testing::NiceMock;
-using ::testing::NotNull;
-using ::testing::Return;
-using ::testing::StrictMock;
 
 namespace media {
 
-MATCHER_P(HasTimestamp, ms, "") {
-  *result_listener << "has timestamp " << arg->timestamp().InMilliseconds();
-  return arg->timestamp().InMilliseconds() == ms;
+static void AssignValue(bool* b, bool value) {
+  *b = value;
 }
 
-// Arbitrary value. Has to be larger to cover any timestamp value used in tests.
-static const int kVideoDurationInMs = 1000;
-
 class VideoRendererImplTest : public ::testing::Test {
  public:
   VideoRendererImplTest()
       : decoder_(new MockVideoDecoder()),
-        demuxer_stream_(DemuxerStream::VIDEO) {
+        scheduler_(new TestVideoFrameScheduler()),
+        tick_clock_(new base::SimpleTestTickClock()),
+        demuxer_stream_(DemuxerStream::VIDEO),
+        max_time_(kNoTimestamp()),
+        decoded_frames_(0),
+        dropped_frames_(0),
+        ended_cb_run_(false) {
     ScopedVector<VideoDecoder> decoders;
     decoders.push_back(decoder_);
 
     renderer_.reset(
         new VideoRendererImpl(message_loop_.message_loop_proxy(),
+                              scoped_ptr<VideoFrameScheduler>(scheduler_),
                               decoders.Pass(),
-                              media::SetDecryptorReadyCB(),
-                              base::Bind(&StrictMock<MockDisplayCB>::Display,
-                                         base::Unretained(&mock_display_cb_)),
-                              true));
+                              media::SetDecryptorReadyCB()));
+    renderer_->SetTickClockForTesting(scoped_ptr<base::TickClock>(tick_clock_));
 
     demuxer_stream_.set_video_decoder_config(TestVideoConfig::Normal());
 
     // We expect these to be called but we don't care how/when.
     EXPECT_CALL(demuxer_stream_, Read(_))
         .WillRepeatedly(RunCallback<0>(DemuxerStream::kOk,
                                        DecoderBuffer::CreateEOSBuffer()));
     EXPECT_CALL(*decoder_, Stop())
         .WillRepeatedly(Invoke(this, &VideoRendererImplTest::StopRequested));
-    EXPECT_CALL(statistics_cb_object_, OnStatistics(_))
-        .Times(AnyNumber());
-    EXPECT_CALL(*this, OnTimeUpdate(_))
-        .Times(AnyNumber());
   }
 
   virtual ~VideoRendererImplTest() {}
 
-  // Callbacks passed into Initialize().
-  MOCK_METHOD1(OnTimeUpdate, void(base::TimeDelta));
-
   void Initialize() {
     // Monitor decodes from the decoder.
     EXPECT_CALL(*decoder_, Decode(_, _))
         .WillRepeatedly(Invoke(this, &VideoRendererImplTest::FrameRequested));
 
     EXPECT_CALL(*decoder_, Reset(_))
         .WillRepeatedly(Invoke(this, &VideoRendererImplTest::FlushRequested));
 
-    InSequence s;
-
     EXPECT_CALL(*decoder_, Initialize(_, _))
         .WillOnce(RunCallback<1>(PIPELINE_OK));
 
     // Set playback rate before anything else happens.
     renderer_->SetPlaybackRate(1.0f);
 
     // Initialize, we shouldn't have any reads.
     InitializeRenderer(PIPELINE_OK);
   }
 
   void InitializeRenderer(PipelineStatus expected) {
     SCOPED_TRACE(base::StringPrintf("InitializeRenderer(%d)", expected));
     WaitableMessageLoopEvent event;
     CallInitialize(event.GetPipelineStatusCB());
     event.RunAndWaitForStatus(expected);
   }
 
   void CallInitialize(const PipelineStatusCB& status_cb) {
     renderer_->Initialize(
         &demuxer_stream_,
         status_cb,
-        base::Bind(&MockStatisticsCB::OnStatistics,
-                   base::Unretained(&statistics_cb_object_)),
-        base::Bind(&VideoRendererImplTest::OnTimeUpdate,
+        base::Bind(&VideoRendererImplTest::OnStatisticsUpdate,
                    base::Unretained(this)),
-        ended_event_.GetClosure(),
+        base::Bind(&VideoRendererImplTest::OnMaxTimeUpdate,
+                   base::Unretained(this)),
+        base::Bind(&AssignValue, &ended_cb_run_, true),
         error_event_.GetPipelineStatusCB(),
-        base::Bind(&VideoRendererImplTest::GetTime, base::Unretained(this)),
-        base::Bind(&VideoRendererImplTest::GetDuration,
-                   base::Unretained(this)));
+        base::Bind(&VideoRendererImplTest::time, base::Unretained(this)),
+        base::Bind(&VideoRendererImplTest::duration, base::Unretained(this)));
   }
 
   void Play() {
     SCOPED_TRACE("Play()");
     WaitableMessageLoopEvent event;
     renderer_->Play(event.GetClosure());
     event.RunAndWait();
   }
 
   void Preroll(int timestamp_ms, PipelineStatus expected) {
@@ skipping 83 matching lines @@
 
       CHECK(false) << "Unrecognized decoder buffer token: " << tokens[i];
     }
   }
 
   void WaitForError(PipelineStatus expected) {
     SCOPED_TRACE(base::StringPrintf("WaitForError(%d)", expected));
     error_event_.RunAndWaitForStatus(expected);
   }
 
-  void WaitForEnded() {
-    SCOPED_TRACE("WaitForEnded()");
-    ended_event_.RunAndWait();
-  }
-
-  void WaitForPendingRead() {
-    SCOPED_TRACE("WaitForPendingRead()");
-    if (!read_cb_.is_null())
-      return;
-
-    DCHECK(wait_for_pending_read_cb_.is_null());
-
-    WaitableMessageLoopEvent event;
-    wait_for_pending_read_cb_ = event.GetClosure();
-    event.RunAndWait();
-
-    DCHECK(!read_cb_.is_null());
-    DCHECK(wait_for_pending_read_cb_.is_null());
-  }
-
   void SatisfyPendingRead() {
     CHECK(!read_cb_.is_null());
     CHECK(!decode_results_.empty());
 
     base::Closure closure = base::Bind(
         read_cb_, decode_results_.front().first,
         decode_results_.front().second);
 
     read_cb_.Reset();
     decode_results_.pop_front();
 
     message_loop_.PostTask(FROM_HERE, closure);
   }
 
-  void AdvanceTimeInMs(int time_ms) {
-    DCHECK_EQ(&message_loop_, base::MessageLoop::current());
-    base::AutoLock l(lock_);
-    time_ += base::TimeDelta::FromMilliseconds(time_ms);
-    DCHECK_LE(time_.InMicroseconds(), GetDuration().InMicroseconds());
+  bool pending_read() { return !read_cb_.is_null(); }
+  base::TimeDelta time() { return time_; }
+  void set_time(base::TimeDelta time) { time_ = time; }
+  base::TimeDelta max_time() { return max_time_; }
+  int decoded_frames() { return decoded_frames_; }
+  int dropped_frames() { return dropped_frames_; }
+  bool ended_cb_run() { return ended_cb_run_; }

[xhwang, 2014/04/24 18:48:44]

nit: make getters const?

[scherkus (not reviewing), 2014/04/25 02:04:47]

Done.

+
+  base::TimeDelta duration() {
+    // Arbitrary value. Has to be larger to cover any timestamp used in tests.

[acolwell GONE FROM CHROMIUM, 2014/04/24 16:43:59]

nit: s/larger/large/?

[scherkus (not reviewing), 2014/04/25 02:04:47]

Done.

+    return base::TimeDelta::FromMilliseconds(1000);
   }
 
  protected:
-  // Fixture members.
+  base::MessageLoop message_loop_;
+
   scoped_ptr<VideoRendererImpl> renderer_;
   MockVideoDecoder* decoder_;  // Owned by |renderer_|.
+  TestVideoFrameScheduler* scheduler_;     // Owned by |renderer_|.
+  base::SimpleTestTickClock* tick_clock_;  // Owned by |renderer_|.

[xhwang, 2014/04/24 18:48:44]

nit: if you align the comments, why not also align the comment on l.234?

[scherkus (not reviewing), 2014/04/25 02:04:47]

Done.

(I think clang-format did this)

   NiceMock<MockDemuxerStream> demuxer_stream_;
-  MockStatisticsCB statistics_cb_object_;
-
-  // Use StrictMock<T> to catch missing/extra display callbacks.
-  class MockDisplayCB {
-   public:
-    MOCK_METHOD1(Display, void(const scoped_refptr<VideoFrame>&));
-  };
-  StrictMock<MockDisplayCB> mock_display_cb_;
 
  private:
-  base::TimeDelta GetTime() {
-    base::AutoLock l(lock_);
-    return time_;
-  }
-
-  base::TimeDelta GetDuration() {
-    return base::TimeDelta::FromMilliseconds(kVideoDurationInMs);
-  }
-
   void FrameRequested(const scoped_refptr<DecoderBuffer>& buffer,
                       const VideoDecoder::DecodeCB& read_cb) {
     DCHECK_EQ(&message_loop_, base::MessageLoop::current());
     CHECK(read_cb_.is_null());
     read_cb_ = read_cb;
 
-    // Wake up WaitForPendingRead() if needed.
-    if (!wait_for_pending_read_cb_.is_null())
-      base::ResetAndReturn(&wait_for_pending_read_cb_).Run();
-
     if (decode_results_.empty())
       return;
 
     SatisfyPendingRead();
   }
 
   void FlushRequested(const base::Closure& callback) {
     DCHECK_EQ(&message_loop_, base::MessageLoop::current());
     decode_results_.clear();
     if (!read_cb_.is_null()) {
       QueueFrames("abort");
       SatisfyPendingRead();
     }
 
     message_loop_.PostTask(FROM_HERE, callback);
   }
 
   void StopRequested() {
     DCHECK_EQ(&message_loop_, base::MessageLoop::current());
     decode_results_.clear();
     if (!read_cb_.is_null()) {
       QueueFrames("abort");
       SatisfyPendingRead();
     }
   }
 
-  base::MessageLoop message_loop_;
+  void OnMaxTimeUpdate(base::TimeDelta max_time) { max_time_ = max_time; }
 
-  // Used to protect |time_|.
-  base::Lock lock_;
-  base::TimeDelta time_;
+  void OnStatisticsUpdate(const PipelineStatistics& stats) {
+    decoded_frames_ = stats.video_frames_decoded;
+    dropped_frames_ = stats.video_frames_dropped;
+  }
 
   // Used for satisfying reads.
   VideoDecoder::DecodeCB read_cb_;
   base::TimeDelta next_frame_timestamp_;
 
   WaitableMessageLoopEvent error_event_;
-  WaitableMessageLoopEvent ended_event_;
-
-  // Run during FrameRequested() to unblock WaitForPendingRead().
-  base::Closure wait_for_pending_read_cb_;
 
   std::deque<std::pair<
       VideoDecoder::Status, scoped_refptr<VideoFrame> > > decode_results_;
 
+  base::TimeDelta time_;
+  base::TimeDelta max_time_;
+  int decoded_frames_;
+  int dropped_frames_;
+  bool ended_cb_run_;
+
   DISALLOW_COPY_AND_ASSIGN(VideoRendererImplTest);
 };
 
 TEST_F(VideoRendererImplTest, DoNothing) {
   // Test that creation and deletion doesn't depend on calls to Initialize()
   // and/or Stop().
 }
 
 TEST_F(VideoRendererImplTest, StopWithoutInitialize) {
   Stop();
 }
 
 TEST_F(VideoRendererImplTest, Initialize) {
   Initialize();
   Shutdown();
 }
 
-TEST_F(VideoRendererImplTest, InitializeAndPreroll) {
-  Initialize();
-  QueueFrames("0 10 20 30");
-  EXPECT_CALL(mock_display_cb_, Display(HasTimestamp(0)));
-  Preroll(0, PIPELINE_OK);
-  Shutdown();
-}
-
 static void ExpectNotCalled(PipelineStatus) {
   base::debug::StackTrace stack;
   ADD_FAILURE() << "Expected callback not to be called\n" << stack.ToString();
 }
 
 TEST_F(VideoRendererImplTest, StopWhileInitializing) {
   EXPECT_CALL(*decoder_, Initialize(_, _))
       .WillOnce(RunCallback<1>(PIPELINE_OK));
   CallInitialize(base::Bind(&ExpectNotCalled));
   Stop();
 
   // ~VideoRendererImpl() will CHECK() if we left anything initialized.
 }
 
 TEST_F(VideoRendererImplTest, StopWhileFlushing) {
   Initialize();
   Pause();
   renderer_->Flush(base::Bind(&ExpectNotCalled, PIPELINE_OK));
   Stop();
 
   // ~VideoRendererImpl() will CHECK() if we left anything initialized.
 }
 
-TEST_F(VideoRendererImplTest, Play) {
-  Initialize();
-  QueueFrames("0 10 20 30");
-  EXPECT_CALL(mock_display_cb_, Display(HasTimestamp(0)));
-  Preroll(0, PIPELINE_OK);
-  Play();
-  Shutdown();
-}
-
-TEST_F(VideoRendererImplTest, EndOfStream_ClipDuration) {
-  Initialize();
-  QueueFrames("0 10 20 30");
-  EXPECT_CALL(mock_display_cb_, Display(HasTimestamp(0)));
-  Preroll(0, PIPELINE_OK);
-  Play();
-
-  // Next frame has timestamp way past duration. Its timestamp will be adjusted
+TEST_F(VideoRendererImplTest, PrerollSchedulesFirstFrame) {
+  Initialize();
+  QueueFrames("0 10 20 30");
+  Preroll(0, PIPELINE_OK);
+
+  EXPECT_EQ(1u, scheduler_->scheduled_frames().size());
+  EXPECT_EQ(base::TimeDelta(),
+            scheduler_->scheduled_frames().front().frame->timestamp());
+  EXPECT_EQ(base::TimeTicks(),

[xhwang, 2014/04/24 18:48:44]

Should base::TimeTicks() be tick_clock_->NowTicks()?

[scherkus (not reviewing), 2014/04/25 02:04:47]

Replaced with string format.

+            scheduler_->scheduled_frames().front().wall_ticks);
+
+  Shutdown();
+}
+
+TEST_F(VideoRendererImplTest, PlaySchedulesAllFrames) {
+  Initialize();
+  QueueFrames("0 10 20 30");
+  Preroll(0, PIPELINE_OK);
+  Play();
+
+  EXPECT_EQ(4u, scheduler_->scheduled_frames().size());
+  EXPECT_EQ(base::TimeDelta(),
+            scheduler_->scheduled_frames()[0].frame->timestamp());
+  EXPECT_EQ(base::TimeTicks(), scheduler_->scheduled_frames()[0].wall_ticks);

[acolwell GONE FROM CHROMIUM, 2014/04/24 16:43:59]

String format verification for these too? Either that or a helper function to
make this less cluttered looking.

[xhwang, 2014/04/24 18:48:44]

ditto about tick_clock_->NowTicks(). Agreed with acolwell that probably we
should have a function to use string format to verify these. In that function,
we could assert that tick_clock_->NowTicks() is always 0, so the scheduled time
should just be the timestamp of the frames.

[scherkus (not reviewing), 2014/04/25 02:04:47]

Done.

+  EXPECT_EQ(base::TimeDelta::FromMilliseconds(10),
+            scheduler_->scheduled_frames()[1].frame->timestamp());
+  EXPECT_EQ(base::TimeTicks::FromInternalValue(10000),
+            scheduler_->scheduled_frames()[1].wall_ticks);
+  EXPECT_EQ(base::TimeDelta::FromMilliseconds(20),
+            scheduler_->scheduled_frames()[2].frame->timestamp());
+  EXPECT_EQ(base::TimeTicks::FromInternalValue(20000),
+            scheduler_->scheduled_frames()[2].wall_ticks);
+  EXPECT_EQ(base::TimeDelta::FromMilliseconds(30),
+            scheduler_->scheduled_frames()[3].frame->timestamp());
+  EXPECT_EQ(base::TimeTicks::FromInternalValue(30000),
+            scheduler_->scheduled_frames()[3].wall_ticks);
+
+  Shutdown();
+}
+
+TEST_F(VideoRendererImplTest, PauseResetsScheduledFrames) {
+  Initialize();
+  QueueFrames("0 10 20 30");
+  Preroll(0, PIPELINE_OK);
+  Play();
+  Pause();
+
+  EXPECT_EQ(0u, scheduler_->scheduled_frames().size());

[xhwang, 2014/04/24 18:48:44]

Also check that "RESET" frames are pushed back to unscheduled_frames_?

[scherkus (not reviewing), 2014/04/25 02:04:47]

Isn't needed due to change in Reset() behaviour

+
+  Shutdown();
+}
+
+TEST_F(VideoRendererImplTest, PauseWaitsForResetFrames) {
+  Initialize();
+  QueueFrames("0 10 20 30");
+  Preroll(0, PIPELINE_OK);
+  Play();
+
+  scheduler_->set_should_reset(false);

[acolwell GONE FROM CHROMIUM, 2014/04/24 16:43:59]

This is for simulating the proxy VFS case right? Any reason not to simply use
that class for this test so it is clearer why the pause would get delayed?

[scherkus (not reviewing), 2014/04/25 02:04:47]

Removed this test case due to change in Reset() behaviour

+
+  bool pause_cb_run = false;
+  renderer_->Pause(base::Bind(&AssignValue, &pause_cb_run, true));
+  EXPECT_FALSE(pause_cb_run);
+
+  scheduler_->set_should_reset(true);
+  scheduler_->Reset();

[xhwang, 2014/04/24 18:48:44]

See comments above about calling scheduler_->Reset() twice. (Pause() calls
scheduler_->Reset()).

[scherkus (not reviewing), 2014/04/25 02:04:47]

Ditto

+  EXPECT_TRUE(pause_cb_run);
+
+  Shutdown();
+}
+
+TEST_F(VideoRendererImplTest, SetPlaybackRateReschedulesFrames) {
+  Initialize();
+  QueueFrames("0 10 20 30");
+  Preroll(0, PIPELINE_OK);
+  Play();
+
+  renderer_->SetPlaybackRate(2.0);
+

[xhwang, 2014/04/24 18:48:44]

hmm, Reset() and SetPlaybackRate() all finishes synchronously. I am a little
worried about reentrancy but if it works well than I am in favor of it!

[scherkus (not reviewing), 2014/04/25 02:04:47]

actually this test started failing once I made Reset() post tasks ... which led
me to re-think what Reset() should do!

+  // Wall times should be cut in half.
+  EXPECT_EQ(4u, scheduler_->scheduled_frames().size());

[acolwell GONE FROM CHROMIUM, 2014/04/24 16:43:59]

ditto. more concise expectations would be nice.

[scherkus (not reviewing), 2014/04/25 02:04:47]

Done.

+  EXPECT_EQ(base::TimeTicks(), scheduler_->scheduled_frames()[0].wall_ticks);

[xhwang, 2014/04/24 18:48:44]

So even the first frame is rescheduled because Reset() finishes synchronously,
but the timers fires asynchronously? It might be nice to note this somewhere.

Also, we can test the case where run the message loop first, then reschedule. I
guess the first frame will be displayed and not rescheduled?

[scherkus (not reviewing), 2014/04/25 02:04:47]

had to change this stuff

+  EXPECT_EQ(base::TimeDelta::FromMilliseconds(10),
+            scheduler_->scheduled_frames()[1].frame->timestamp());
+  EXPECT_EQ(base::TimeTicks::FromInternalValue(5000),
+            scheduler_->scheduled_frames()[1].wall_ticks);
+  EXPECT_EQ(base::TimeDelta::FromMilliseconds(20),
+            scheduler_->scheduled_frames()[2].frame->timestamp());
+  EXPECT_EQ(base::TimeTicks::FromInternalValue(10000),
+            scheduler_->scheduled_frames()[2].wall_ticks);
+  EXPECT_EQ(base::TimeDelta::FromMilliseconds(30),
+            scheduler_->scheduled_frames()[3].frame->timestamp());
+  EXPECT_EQ(base::TimeTicks::FromInternalValue(15000),
+            scheduler_->scheduled_frames()[3].wall_ticks);
+
+  Shutdown();
+}
+
+TEST_F(VideoRendererImplTest, MediaTimeUsedToScheduleFrames) {
+  Initialize();
+  QueueFrames("0 10 20 30");
+  Preroll(0, PIPELINE_OK);
+
+  // Introduce an offset in the media time before playing.
+  set_time(base::TimeDelta::FromMilliseconds(20));
+
+  Play();
+
+  // All newly scheduled frames should respect the media time.
+  EXPECT_EQ(4u, scheduler_->scheduled_frames().size());
+  EXPECT_EQ(base::TimeTicks(), scheduler_->scheduled_frames()[0].wall_ticks);

[xhwang, 2014/04/24 18:48:44]

So this is from the ScheduleFirstFrameForImmediateDisplay() call? Worth a
comment.

[scherkus (not reviewing), 2014/04/25 02:04:47]

Done.

+  EXPECT_EQ(base::TimeDelta::FromMilliseconds(10),
+            scheduler_->scheduled_frames()[1].frame->timestamp());
+  EXPECT_EQ(base::TimeTicks::FromInternalValue(-10000),
+            scheduler_->scheduled_frames()[1].wall_ticks);
+  EXPECT_EQ(base::TimeDelta::FromMilliseconds(20),
+            scheduler_->scheduled_frames()[2].frame->timestamp());
+  EXPECT_EQ(base::TimeTicks(), scheduler_->scheduled_frames()[2].wall_ticks);

[xhwang, 2014/04/24 18:48:44]

This make it harder to read, I'd rather see base::TimeDelta::FromMilliseconds(0)
:) But a function to check all these would be nice.

[scherkus (not reviewing), 2014/04/25 02:04:47]

Replaced with string format.

+  EXPECT_EQ(base::TimeDelta::FromMilliseconds(30),
+            scheduler_->scheduled_frames()[3].frame->timestamp());
+  EXPECT_EQ(base::TimeTicks::FromInternalValue(10000),
+            scheduler_->scheduled_frames()[3].wall_ticks);
+
+  Shutdown();
+}
+
+TEST_F(VideoRendererImplTest, EndedWaitsForLastFrame) {
+  Initialize();
+  QueueFrames("0 10 20 30 eos");
+  Preroll(0, PIPELINE_OK);
+  Play();
+
+  // Display up to last frame.
+  scheduler_->DisplayFrames(base::TimeTicks::FromInternalValue(20000));

[xhwang, 2014/04/24 18:48:44]

Write a helper function DisplayFrames(ms) so that we don't need to do the 20000
-> 20 conversion?

[scherkus (not reviewing), 2014/04/25 02:04:47]

Done.

+  message_loop_.RunUntilIdle();
+  EXPECT_FALSE(ended_cb_run());
+
+  // Display last frame.
+  scheduler_->DisplayFrames(base::TimeTicks::FromInternalValue(30000));
+  message_loop_.RunUntilIdle();
+  EXPECT_TRUE(ended_cb_run());
+
+  Shutdown();
+}
+
+TEST_F(VideoRendererImplTest, EndedWaitsForEndOfStream) {
+  Initialize();
+  QueueFrames("0 10 20 30");
+  Preroll(0, PIPELINE_OK);
+  Play();
+
+  // Display last frame.
+  scheduler_->DisplayFrames(base::TimeTicks::FromInternalValue(30000));
+  message_loop_.RunUntilIdle();
+  EXPECT_FALSE(ended_cb_run());
+
+  // Deliver end of stream.
+  QueueFrames("eos");
+  SatisfyPendingRead();
+  message_loop_.RunUntilIdle();
+  EXPECT_TRUE(ended_cb_run());
+
+  Shutdown();
+}
+
+TEST_F(VideoRendererImplTest, AdjustTimestampsPastDuration) {
+  Initialize();
+  QueueFrames("0 10 20 30");
+  Preroll(0, PIPELINE_OK);
+  Play();
+
+  // Last frame has timestamp way past duration. Its timestamp will be adjusted
   // to match the duration of the video.
-  QueueFrames(base::IntToString(kVideoDurationInMs + 1000));
-
-  // Queue the end of stream frame and wait for the last frame to be rendered.
+  QueueFrames(base::IntToString((duration() * 10).InMilliseconds()));
   QueueFrames("eos");
-  EXPECT_CALL(mock_display_cb_, Display(HasTimestamp(kVideoDurationInMs)));
-  AdvanceTimeInMs(kVideoDurationInMs);
-  WaitForEnded();
-
-  Shutdown();
-}
+
+  scheduler_->DisplayFrames(base::TimeTicks::FromInternalValue(30000));
+  message_loop_.RunUntilIdle();
+
+  EXPECT_EQ(1u, scheduler_->scheduled_frames().size());
+  EXPECT_EQ(duration(),
+            scheduler_->scheduled_frames().front().frame->timestamp());
+
+  Shutdown();
+}
+
+TEST_F(VideoRendererImplTest, IncomingFramesUpdateMaxTime) {
+  EXPECT_EQ(kNoTimestamp(), max_time());
+
+  Initialize();
+  QueueFrames("0 10 20 30");
+  Preroll(0, PIPELINE_OK);
+
+  EXPECT_EQ(base::TimeDelta::FromMilliseconds(30), max_time());
+
+  Shutdown();
+}
+
+TEST_F(VideoRendererImplTest, EndOfStreamSetsMaxTimeToDuration) {
+  EXPECT_EQ(kNoTimestamp(), max_time());
+
+  Initialize();
+  QueueFrames("0 10 20 eos");
+  Preroll(0, PIPELINE_OK);
+
+  EXPECT_EQ(duration(), max_time());
+
+  Shutdown();
+}
+
+TEST_F(VideoRendererImplTest, DisplayedFramesIncrementStatistics) {
+  Initialize();
+  QueueFrames("0 10 20 30");
+  Preroll(0, PIPELINE_OK);
+  Play();
+
+  EXPECT_EQ(0, decoded_frames());
+  EXPECT_EQ(0, dropped_frames());
+
+  scheduler_->DisplayFrames(base::TimeTicks());
+
+  EXPECT_EQ(1, decoded_frames());
+  EXPECT_EQ(0, dropped_frames());
+
+  Shutdown();
+}
+
+TEST_F(VideoRendererImplTest, DroppedFramesIncrementStatistics) {
+  Initialize();
+  QueueFrames("0 10 20 30");
+  Preroll(0, PIPELINE_OK);
+  Play();
+
+  EXPECT_EQ(0, decoded_frames());
+  EXPECT_EQ(0, dropped_frames());
+
+  scheduler_->DropFrames(base::TimeTicks());
+
+  EXPECT_EQ(1, decoded_frames());
+  EXPECT_EQ(1, dropped_frames());
+
+  Shutdown();
+}
 

[xhwang, 2014/04/24 18:48:44]

Also test that reset doesn't affect stats?

[scherkus (not reviewing), 2014/04/25 02:04:47]

Changed reset so it no longer calls back

 TEST_F(VideoRendererImplTest, DecodeError_Playing) {
   Initialize();
   QueueFrames("0 10 20 30");
-  EXPECT_CALL(mock_display_cb_, Display(HasTimestamp(0)));
   Preroll(0, PIPELINE_OK);
   Play();
 
+  scheduler_->DisplayFrames(base::TimeTicks::FromInternalValue(30000));
+  EXPECT_TRUE(pending_read());
+
   QueueFrames("error");
-  EXPECT_CALL(mock_display_cb_, Display(HasTimestamp(10)));
-  AdvanceTimeInMs(10);
+  SatisfyPendingRead();
   WaitForError(PIPELINE_ERROR_DECODE);
   Shutdown();
 }
 
 TEST_F(VideoRendererImplTest, DecodeError_DuringPreroll) {
   Initialize();
   QueueFrames("error");
   Preroll(0, PIPELINE_ERROR_DECODE);
   Shutdown();
 }
 
 TEST_F(VideoRendererImplTest, Preroll_Exact) {
   Initialize();
   QueueFrames("50 60 70 80 90");
-
-  EXPECT_CALL(mock_display_cb_, Display(HasTimestamp(60)));
   Preroll(60, PIPELINE_OK);
+  EXPECT_EQ(base::TimeDelta::FromMilliseconds(60),
+            scheduler_->scheduled_frames().front().frame->timestamp());
   Shutdown();
 }
 
 TEST_F(VideoRendererImplTest, Preroll_RightBefore) {
   Initialize();
   QueueFrames("50 60 70 80 90");
-
-  EXPECT_CALL(mock_display_cb_, Display(HasTimestamp(50)));
   Preroll(59, PIPELINE_OK);
+  EXPECT_EQ(base::TimeDelta::FromMilliseconds(50),
+            scheduler_->scheduled_frames().front().frame->timestamp());
   Shutdown();
 }
 
 TEST_F(VideoRendererImplTest, Preroll_RightAfter) {
   Initialize();
   QueueFrames("50 60 70 80 90");
-
-  EXPECT_CALL(mock_display_cb_, Display(HasTimestamp(60)));
   Preroll(61, PIPELINE_OK);
+  EXPECT_EQ(base::TimeDelta::FromMilliseconds(60),
+            scheduler_->scheduled_frames().front().frame->timestamp());
   Shutdown();
 }
 
 TEST_F(VideoRendererImplTest, PlayAfterPreroll) {
   Initialize();
   QueueFrames("0 10 20 30");
-  EXPECT_CALL(mock_display_cb_, Display(HasTimestamp(0)));
   Preroll(0, PIPELINE_OK);
   Play();
 
-  // Advance time past prerolled time to trigger a Read().
-  EXPECT_CALL(mock_display_cb_, Display(HasTimestamp(10)));
-  AdvanceTimeInMs(10);
-  WaitForPendingRead();
+  scheduler_->DisplayFrames(base::TimeTicks());
+  EXPECT_TRUE(pending_read());
+
   Shutdown();
 }
 
 TEST_F(VideoRendererImplTest, Rebuffer) {
   Initialize();
   QueueFrames("0 10 20 30");
-  EXPECT_CALL(mock_display_cb_, Display(HasTimestamp(0)));
   Preroll(0, PIPELINE_OK);
   Play();
 
-  // Advance time past prerolled time drain the ready frame queue.
-  AdvanceTimeInMs(50);
-  WaitForPendingRead();
+  // Display all frames to empty frame buffer.
+  scheduler_->DisplayFrames(base::TimeTicks::FromInternalValue(30000));
+  EXPECT_TRUE(pending_read());
 
   // Simulate a Pause/Preroll/Play rebuffer sequence.
   Pause();
 
   WaitableMessageLoopEvent event;
   renderer_->Preroll(kNoTimestamp(),
                      event.GetPipelineStatusCB());
 
   // Queue enough frames to satisfy preroll.
   QueueFrames("40 50 60 70");
   SatisfyPendingRead();
+  event.RunAndWaitForStatus(PIPELINE_OK);
 
   // TODO(scherkus): We shouldn't display the next ready frame in a rebuffer
   // situation, see http://crbug.com/365516
-  EXPECT_CALL(mock_display_cb_, Display(_));
-
-  event.RunAndWaitForStatus(PIPELINE_OK);
+  EXPECT_EQ(1u, scheduler_->scheduled_frames().size());
 
   Play();
 
   Shutdown();
 }
 
 TEST_F(VideoRendererImplTest, Rebuffer_AlreadyHaveEnoughFrames) {
   Initialize();
   QueueFrames("0 10 20 30");
-  EXPECT_CALL(mock_display_cb_, Display(HasTimestamp(0)));
   Preroll(0, PIPELINE_OK);
-
-  // Queue an extra frame so that we'll have enough frames to satisfy
-  // preroll even after the first frame is painted.
-  QueueFrames("40");
   Play();
 
   // Simulate a Pause/Preroll/Play rebuffer sequence.
   Pause();
 
-  // TODO(scherkus): We shouldn't display the next ready frame in a rebuffer
-  // situation, see http://crbug.com/365516
-  EXPECT_CALL(mock_display_cb_, Display(_));
-
   WaitableMessageLoopEvent event;
   renderer_->Preroll(kNoTimestamp(),
                      event.GetPipelineStatusCB());
+  event.RunAndWaitForStatus(PIPELINE_OK);
 
-  event.RunAndWaitForStatus(PIPELINE_OK);
+  // TODO(scherkus): We shouldn't display the next ready frame in a rebuffer
+  // situation, see http://crbug.com/365516
+  EXPECT_EQ(1u, scheduler_->scheduled_frames().size());
 
   Play();
 
   Shutdown();
 }
 
 // Verify that a late decoder response doesn't break invariants in the renderer.
 TEST_F(VideoRendererImplTest, StopDuringOutstandingRead) {
   Initialize();
   QueueFrames("0 10 20 30");
-  EXPECT_CALL(mock_display_cb_, Display(HasTimestamp(0)));
   Preroll(0, PIPELINE_OK);
   Play();
 
-  // Advance time a bit to trigger a Read().
-  EXPECT_CALL(mock_display_cb_, Display(HasTimestamp(10)));
-  AdvanceTimeInMs(10);
-  WaitForPendingRead();
+  scheduler_->DisplayFrames(base::TimeTicks());
+  EXPECT_TRUE(pending_read());
 
   WaitableMessageLoopEvent event;
   renderer_->Stop(event.GetClosure());
   event.RunAndWait();
 }
 
 TEST_F(VideoRendererImplTest, AbortPendingRead_Playing) {
   Initialize();
   QueueFrames("0 10 20 30");
-  EXPECT_CALL(mock_display_cb_, Display(HasTimestamp(0)));
   Preroll(0, PIPELINE_OK);
   Play();
 
-  // Advance time a bit to trigger a Read().
-  EXPECT_CALL(mock_display_cb_, Display(HasTimestamp(10)));
-  AdvanceTimeInMs(10);
-  WaitForPendingRead();
+  scheduler_->DisplayFrames(base::TimeTicks());
+  EXPECT_TRUE(pending_read());
   QueueFrames("abort");
   SatisfyPendingRead();
 
   Pause();
   Flush();
   QueueFrames("60 70 80 90");
-  EXPECT_CALL(mock_display_cb_, Display(HasTimestamp(60)));
   Preroll(60, PIPELINE_OK);
   Shutdown();
 }
 
 TEST_F(VideoRendererImplTest, AbortPendingRead_Flush) {
   Initialize();
   QueueFrames("0 10 20 30");
-  EXPECT_CALL(mock_display_cb_, Display(HasTimestamp(0)));
   Preroll(0, PIPELINE_OK);
   Play();
 
-  // Advance time a bit to trigger a Read().
-  EXPECT_CALL(mock_display_cb_, Display(HasTimestamp(10)));
-  AdvanceTimeInMs(10);
-  WaitForPendingRead();
+  scheduler_->DisplayFrames(base::TimeTicks());
+  EXPECT_TRUE(pending_read());
 
   Pause();
   Flush();
   Shutdown();
 }
 
 TEST_F(VideoRendererImplTest, AbortPendingRead_Preroll) {
   Initialize();
   QueueFrames("0 10 abort");
-  EXPECT_CALL(mock_display_cb_, Display(HasTimestamp(0)));
   Preroll(0, PIPELINE_OK);
   Shutdown();
 }
 
 TEST_F(VideoRendererImplTest, VideoDecoder_InitFailure) {
-  InSequence s;
-
   EXPECT_CALL(*decoder_, Initialize(_, _))
       .WillOnce(RunCallback<1>(DECODER_ERROR_NOT_SUPPORTED));
   InitializeRenderer(DECODER_ERROR_NOT_SUPPORTED);
 
   Stop();
 }
 
 }  // namespace media