Fix processing of multiple stream status changes by renderer

media/renderers/renderer_impl_unittest.cc

 // Copyright 2014 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 <stdint.h>
 
 #include <memory>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/macros.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/test/simple_test_tick_clock.h"
+#include "base/threading/thread_task_runner_handle.h"
 #include "media/base/gmock_callback_support.h"
 #include "media/base/mock_filters.h"
 #include "media/base/test_helpers.h"
 #include "media/renderers/renderer_impl.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using ::testing::_;
 using ::testing::DoAll;
 using ::testing::InSequence;
 using ::testing::Mock;
 using ::testing::Return;
 using ::testing::SaveArg;
 using ::testing::StrictMock;
+using ::testing::WithArg;
 
 namespace media {
 
 const int64_t kStartPlayingTimeInMs = 100;
 
 ACTION_P2(SetBufferingState, renderer_client, buffering_state) {
   (*renderer_client)->OnBufferingStateChange(buffering_state);
 }
 
 ACTION_P2(SetError, renderer_client, error) {
@@ skipping 708 matching lines @@
       .WillOnce(SaveArg<0>(&audio_stream_status_change_cb));
   EXPECT_CALL(*video_stream_, SetStreamStatusChangeCB(_))
       .WillOnce(SaveArg<0>(&video_stream_status_change_cb));
   SetAudioRendererInitializeExpectations(PIPELINE_OK);
   SetVideoRendererInitializeExpectations(PIPELINE_OK);
   InitializeAndExpect(PIPELINE_OK);
   Play();
 
   // Verify that DemuxerStream status changes cause the corresponding
   // audio/video renderer to be flushed and restarted.
   base::TimeDelta time0;

[xhwang, 2017/01/04 19:34:02]

What does time0 exactly mean? Maybe this should be something like dummy_time to
make it clear that the value doesn't matter.

[servolk, 2017/01/04 21:29:30]

There's no special meaning to this, it's just a dummy value indeed and is
slightly shorter to type than base::TimeDelta(). I guess we could just replace
it with base::TimeDelta() to avoid introducing a variable, hopefully that's less
confusing (and typing base::TimeDelta() is not that much easier than dummy_time
anyway).

   EXPECT_CALL(time_source_, StopTicking());
   EXPECT_CALL(*audio_renderer_, Flush(_)).WillOnce(RunClosure<0>());
   EXPECT_CALL(*audio_renderer_, StartPlaying()).Times(1);
   audio_stream_status_change_cb.Run(false, time0);
 
   EXPECT_CALL(*video_renderer_, Flush(_)).WillOnce(RunClosure<0>());
   EXPECT_CALL(*video_renderer_, StartPlayingFrom(_)).Times(1);
   video_stream_status_change_cb.Run(false, time0);
 }
 
+ACTION(PostCallback) {
+  base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, arg0);
+}
+
+ACTION(PostQuitWhenIdle) {
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::MessageLoop::QuitWhenIdleClosure());
+}
+
+// Stream status changes are handled asynchronously by the renderer and may take
+// some time to process. This test verifies that all status changes are
+// processed correctly by the renderer even if status changes of the stream
+// happen much faster than the renderer can process them. In that case the
+// renderer may postpone processing status changes, but still must process all
+// of them eventually.
+TEST_F(RendererImplTest, PostponedStreamStatusNotificationHandling) {
+  CreateAudioAndVideoStream();
+
+  DemuxerStream::StreamStatusChangeCB audio_stream_status_change_cb;
+  DemuxerStream::StreamStatusChangeCB video_stream_status_change_cb;
+  EXPECT_CALL(*audio_stream_, SetStreamStatusChangeCB(_))
+      .WillOnce(SaveArg<0>(&audio_stream_status_change_cb));
+  EXPECT_CALL(*video_stream_, SetStreamStatusChangeCB(_))
+      .WillOnce(SaveArg<0>(&video_stream_status_change_cb));
+  SetAudioRendererInitializeExpectations(PIPELINE_OK);
+  SetVideoRendererInitializeExpectations(PIPELINE_OK);
+  InitializeAndExpect(PIPELINE_OK);
+  Play();
+
+  EXPECT_CALL(callbacks_, OnBufferingStateChange(BUFFERING_HAVE_ENOUGH))
+      .Times(2);
+
+  base::TimeDelta time0;
+  EXPECT_CALL(time_source_, StopTicking()).Times(2);
+  EXPECT_CALL(time_source_, StartTicking()).Times(2);
+  EXPECT_CALL(*audio_renderer_, Flush(_))
+      .Times(2)
+      .WillRepeatedly(DoAll(
+          SetBufferingState(&audio_renderer_client_, BUFFERING_HAVE_NOTHING),
+          WithArg<0>(PostCallback())));
+  EXPECT_CALL(*audio_renderer_, StartPlaying())
+      .Times(2)
+      .WillOnce(
+          SetBufferingState(&audio_renderer_client_, BUFFERING_HAVE_ENOUGH))
+      .WillOnce(DoAll(
+          SetBufferingState(&audio_renderer_client_, BUFFERING_HAVE_ENOUGH),
+          PostQuitWhenIdle()));
+  // The first stream status change will be processed immediately. Each status
+  // change processing involves Flush + StartPlaying when the Flush is done. The
+  // Flush operation is async in this case, so the second status change will be
+  // postponed by renderer until after processing the first one is finished. But
+  // we must still get two pairs of Flush/StartPlaying calls eventually.
+  audio_stream_status_change_cb.Run(false, time0);
+  audio_stream_status_change_cb.Run(true, time0);
+  base::RunLoop().Run();
+
+  EXPECT_CALL(*video_renderer_, Flush(_))
+      .Times(2)
+      .WillRepeatedly(DoAll(
+          SetBufferingState(&video_renderer_client_, BUFFERING_HAVE_NOTHING),
+          WithArg<0>(PostCallback())));
+  EXPECT_CALL(*video_renderer_, StartPlayingFrom(time0))
+      .Times(2)
+      .WillOnce(
+          SetBufferingState(&video_renderer_client_, BUFFERING_HAVE_ENOUGH))
+      .WillOnce(DoAll(
+          SetBufferingState(&video_renderer_client_, BUFFERING_HAVE_ENOUGH),
+          PostQuitWhenIdle()));
+  // The first stream status change will be processed immediately. Each status
+  // change processing involves Flush + StartPlaying when the Flush is done. The
+  // Flush operation is async in this case, so the second status change will be
+  // postponed by renderer until after processing the first one is finished. But
+  // we must still get two pairs of Flush/StartPlaying calls eventually.
+  video_stream_status_change_cb.Run(false, time0);
+  video_stream_status_change_cb.Run(true, time0);
+  base::RunLoop().Run();
+}
+
 }  // namespace media