Add a lot of Pipeline tests to cover stopping/error handling while in a variety of states.

media/base/pipeline_unittest.cc

Index: media/base/pipeline_unittest.cc
diff --git a/media/base/pipeline_unittest.cc b/media/base/pipeline_unittest.cc
index bd00361c47959848dbf2c66e8ff21d59480b0ed5..27f0ecca3a3e56db2c0fcd20353c73fa371a854d 100644
--- a/media/base/pipeline_unittest.cc
+++ b/media/base/pipeline_unittest.cc
@@ -19,6 +19,7 @@
 using ::testing::_;
 using ::testing::DeleteArg;
 using ::testing::DoAll;
+// TODO(scherkus): Remove InSequence after refactoring Pipeline.
 using ::testing::InSequence;
 using ::testing::Invoke;
 using ::testing::InvokeWithoutArgs;
@@ -41,6 +42,14 @@ ACTION_P(SetDemuxerProperties, duration) {
   arg0->SetDuration(duration);
 }
 
+ACTION_P2(Stop, pipeline, stop_cb) {
+  pipeline->Stop(stop_cb);
+}
+
+ACTION_P2(SetError, pipeline, status) {
+  pipeline->SetErrorForTesting(status);
+}
+
 ACTION(RunPipelineStatusCB1) {

[Ami GONE FROM CHROMIUM, 2012/08/08 23:43:55]

s/1//

[scherkus (not reviewing), 2012/08/09 01:18:19]

I swore we had other versions around but I guess I deleted them all

   arg1.Run(PIPELINE_OK);
 }
@@ -98,6 +107,8 @@ class PipelineTest : public ::testing::Test {
       EXPECT_CALL(*mocks_->demuxer(), Stop(_))
           .WillOnce(RunClosure());
 
+      // TODO(scherkus): Don't pause+flush on shutdown,
+      // see http://crbug.com/110228
       if (audio_stream_) {
         EXPECT_CALL(*mocks_->audio_renderer(), Pause(_))
             .WillOnce(RunClosure());
@@ -283,6 +294,124 @@ class PipelineTest : public ::testing::Test {
     EXPECT_EQ(seek_time, pipeline_->GetMediaTime());
   }
 
+  // Initializes a pipeline containing a single audio stream.
+  void InitializeAudioPipeline() {
+    CreateAudioStream();
+    MockDemuxerStreamVector streams;
+    streams.push_back(audio_stream());
+
+    InitializeDemuxer(&streams, base::TimeDelta::FromSeconds(3000));
+    InitializeAudioDecoder(audio_stream());
+    InitializeAudioRenderer();
+    InitializePipeline(PIPELINE_OK);
+  }
+
+  // TODO(scherkus): Why separate stop/error enums and helper functions? We do
+  // radically different things whether teardown is invoked via stop vs error.
+  // The teardown path should be the same, see http://crbug.com/110228
+
+  enum StopOnSeekState {

[Ami GONE FROM CHROMIUM, 2012/08/08 23:43:55]

I read your TODO above, but AFAICT these are only used to signal when you want
the Stop to be triggered.  So in fact they can be unified into
enum SeekDancePhase { kPause, kFlush, kSeek };
and then the methods below turn into
void TestStopOrErrorDuringSeekDance(SeekDancePhase, StopOrError);
with an
enum StopOrError { kStop, kError };

Once you do that, then it's unclear to me why not just fully parameterize the
tests; this test helper then just takes the state of the pipeline (playing/not)
and the phase in which to trigger stop, and goes to town.

[scherkus (not reviewing), 2012/08/09 01:18:19]

Took a stab but I strongly dislike how gtest parameterized tests look on the
console so I manually instantiated them myself

[Ami GONE FROM CHROMIUM, 2012/08/09 04:41:43]

Yeah, I didn't mean TEST_P necessarily.

+    kStopOnPause,
+    kStopOnFlush,
+    kStopOnSeek,
+  };
+
+  enum ErrorOnSeekState {

[Ami GONE FROM CHROMIUM, 2012/08/08 23:43:55]

Ditto.

+    kErrorOnPause,
+    kErrorOnFlush,
+    kErrorOnSeek,
+  };
+
+  void ExpectAudioPipelineSeekStop(StopOnSeekState action) {
+    base::Closure stop_cb = base::Bind(
+        &CallbackHelper::OnStop, base::Unretained(&callbacks_));
+
+    if (action == kStopOnPause) {
+      EXPECT_CALL(*mocks_->audio_renderer(), Pause(_))
+          .WillOnce(DoAll(Stop(pipeline_, stop_cb), RunClosure()));
+    } else {
+      EXPECT_CALL(*mocks_->audio_renderer(), Pause(_)).WillOnce(RunClosure());
+    }
+
+    if (action == kStopOnFlush) {
+      EXPECT_CALL(*mocks_->audio_renderer(), Flush(_))
+          .WillOnce(DoAll(Stop(pipeline_, stop_cb), RunClosure()));
+    } else {
+      EXPECT_CALL(*mocks_->audio_renderer(), Flush(_)).WillOnce(RunClosure());
+    }
+
+    if (action == kStopOnSeek) {
+      EXPECT_CALL(*mocks_->demuxer(), Seek(_, _))
+          .WillOnce(DoAll(Stop(pipeline_, stop_cb),
+                          RunPipelineStatusCB1WithStatus(PIPELINE_OK)));

[Ami GONE FROM CHROMIUM, 2012/08/08 23:43:55]

Isn't this just RunPipelineStatusCB1()?

[scherkus (not reviewing), 2012/08/09 01:18:19]

Done.

+    } else {
+      EXPECT_CALL(*mocks_->demuxer(), Seek(_, _))
+          .WillOnce(RunPipelineStatusCB1());
+    }
+
+    EXPECT_CALL(*mocks_->audio_renderer(), Preroll(_, _))
+        .WillOnce(RunPipelineStatusCB1());
+    EXPECT_CALL(*mocks_->audio_renderer(), Play(_)).WillOnce(RunClosure());
+    EXPECT_CALL(callbacks_, OnSeek(PIPELINE_OK));
+  }
+
+  void ExpectAudioPipelineSeekError(ErrorOnSeekState action) {
+    if (action == kErrorOnPause) {
+      EXPECT_CALL(*mocks_->audio_renderer(), Pause(_))
+          .WillOnce(DoAll(SetError(pipeline_, PIPELINE_ERROR_READ),
+                          RunClosure()));
+      EXPECT_CALL(callbacks_, OnSeek(PIPELINE_ERROR_READ));
+      return;
+    }
+
+    EXPECT_CALL(*mocks_->audio_renderer(), Pause(_)).WillOnce(RunClosure());
+
+    if (action == kErrorOnFlush) {
+      EXPECT_CALL(*mocks_->audio_renderer(), Flush(_))
+          .WillOnce(DoAll(SetError(pipeline_, PIPELINE_ERROR_READ),
+                          RunClosure()));
+      EXPECT_CALL(callbacks_, OnSeek(PIPELINE_ERROR_READ));
+      return;
+    }
+
+    EXPECT_CALL(*mocks_->audio_renderer(), Flush(_)).WillOnce(RunClosure());
+
+    if (action == kErrorOnSeek) {
+      EXPECT_CALL(*mocks_->demuxer(), Seek(_, _))
+          .WillOnce(RunPipelineStatusCB1WithStatus(PIPELINE_ERROR_READ));
+      EXPECT_CALL(callbacks_, OnSeek(PIPELINE_ERROR_READ));
+      return;
+    }
+
+    NOTREACHED();
+  }
+
+  void ExpectAudioPipelineStop() {
+    // TODO(scherkus): Don't pause+flush, see http://crbug.com/110228
+    EXPECT_CALL(*mocks_->audio_renderer(), Pause(_)).WillOnce(RunClosure());
+    EXPECT_CALL(*mocks_->audio_renderer(), Flush(_)).WillOnce(RunClosure());
+    EXPECT_CALL(*mocks_->demuxer(), Stop(_)).WillOnce(RunClosure());
+    EXPECT_CALL(*mocks_->audio_renderer(), Stop(_)).WillOnce(RunClosure());
+    EXPECT_CALL(callbacks_, OnStop());
+  }
+
+  void ExpectAudioPipelineError() {
+    EXPECT_CALL(*mocks_->demuxer(), Stop(_)).WillOnce(RunClosure());
+    EXPECT_CALL(*mocks_->audio_renderer(), Stop(_)).WillOnce(RunClosure());
+  }
+
+  void DoAudioPipelineSeek() {
+    pipeline_->Seek(base::TimeDelta::FromSeconds(10), base::Bind(
+        &CallbackHelper::OnSeek, base::Unretained(&callbacks_)));
+    message_loop_.RunAllPending();
+  }
+
+  void DoAudioPipelineStop() {
+    pipeline_->Stop(base::Bind(
+        &CallbackHelper::OnStop, base::Unretained(&callbacks_)));
+    message_loop_.RunAllPending();
+  }
+
   // Fixture members.
   StrictMock<CallbackHelper> callbacks_;
   MessageLoop message_loop_;
@@ -464,6 +593,140 @@ TEST_F(PipelineTest, Seek) {
   DoSeek(expected);
 }
 
+TEST_F(PipelineTest, Stop_DuringStart) {
+  base::Closure stop_cb = base::Bind(
+      &CallbackHelper::OnStop, base::Unretained(&callbacks_));
+
+  EXPECT_CALL(*mocks_->demuxer(), Initialize(_, _))
+      .WillOnce(DoAll(Stop(pipeline_, stop_cb), RunPipelineStatusCB1()));
+
+  // Stop sequence.
+  EXPECT_CALL(*mocks_->demuxer(), Stop(_))
+      .WillOnce(RunClosure());
+  EXPECT_CALL(callbacks_, OnStop());
+
+  InitializePipeline(PIPELINE_OK);
+}
+
+TEST_F(PipelineTest, Stop_DuringPause) {
+  InitializeAudioPipeline();
+
+  InSequence s;
+  ExpectAudioPipelineSeekStop(kStopOnPause);
+  ExpectAudioPipelineStop();
+
+  DoAudioPipelineSeek();
+}
+
+TEST_F(PipelineTest, Stop_DuringFlush) {
+  InitializeAudioPipeline();
+
+  InSequence s;
+  ExpectAudioPipelineSeekStop(kStopOnFlush);
+  ExpectAudioPipelineStop();
+
+  DoAudioPipelineSeek();
+}
+
+TEST_F(PipelineTest, Stop_DuringSeek) {
+  InitializeAudioPipeline();
+
+  InSequence s;
+  ExpectAudioPipelineSeekStop(kStopOnSeek);
+  ExpectAudioPipelineStop();
+
+  DoAudioPipelineSeek();
+}
+
+TEST_F(PipelineTest, Stop_DuringPlayback) {
+  InitializeAudioPipeline();
+
+  InSequence s;
+  ExpectAudioPipelineStop();
+
+  DoAudioPipelineStop();
+}
+
+TEST_F(PipelineTest, Error_DuringStart) {
+  EXPECT_CALL(*mocks_->demuxer(), Initialize(_, _))
+      .WillOnce(RunPipelineStatusCB1WithStatus(PIPELINE_ERROR_READ));
+
+  // Stop sequence.
+  EXPECT_CALL(*mocks_->demuxer(), Stop(_))
+      .WillOnce(RunClosure());
+
+  // Start callback should fire with error but nothing else.
+  InitializePipeline(PIPELINE_ERROR_READ);
+}
+
+TEST_F(PipelineTest, Error_DuringPause) {
+  InitializeAudioPipeline();
+
+  InSequence s;
+  ExpectAudioPipelineSeekError(kErrorOnPause);
+  ExpectAudioPipelineError();
+
+  DoAudioPipelineSeek();
+}
+
+TEST_F(PipelineTest, Error_DuringFlush) {
+  InitializeAudioPipeline();
+
+  InSequence s;
+  ExpectAudioPipelineSeekError(kErrorOnFlush);
+  ExpectAudioPipelineError();
+
+  DoAudioPipelineSeek();
+}
+
+TEST_F(PipelineTest, Error_DuringSeek) {
+  InitializeAudioPipeline();
+
+  InSequence s;
+  ExpectAudioPipelineSeekError(kErrorOnSeek);
+  ExpectAudioPipelineError();
+
+  DoAudioPipelineSeek();
+}
+
+TEST_F(PipelineTest, Error_DuringPlayback) {
+  InitializeAudioPipeline();
+
+  InSequence s;
+
+  // Curiously enough, errors during playback invokes the pause+flush shutdown
+  // path and not the error path.
+  //
+  // TODO(scherkus): Don't pause+flush, see http://crbug.com/110228
+  EXPECT_CALL(*mocks_->audio_renderer(), Pause(_)).WillOnce(RunClosure());
+  EXPECT_CALL(*mocks_->audio_renderer(), Flush(_)).WillOnce(RunClosure());
+  EXPECT_CALL(*mocks_->demuxer(), Stop(_)).WillOnce(RunClosure());
+  EXPECT_CALL(*mocks_->audio_renderer(), Stop(_)).WillOnce(RunClosure());
+
+  EXPECT_CALL(callbacks_, OnError(PIPELINE_ERROR_READ));
+
+  pipeline_->SetErrorForTesting(PIPELINE_ERROR_READ);
+  message_loop_.RunAllPending();
+}
+
+TEST_F(PipelineTest, Error_DuringStop) {
+  InitializeAudioPipeline();
+
+  InSequence s;
+
+  // TODO(scherkus): Don't pause+flush on shutdown, see http://crbug.com/110228
+  EXPECT_CALL(*mocks_->audio_renderer(), Pause(_)).WillOnce(RunClosure());
+  EXPECT_CALL(*mocks_->audio_renderer(), Flush(_)).WillOnce(RunClosure());
+  EXPECT_CALL(*mocks_->demuxer(), Stop(_))
+      .WillOnce(DoAll(SetError(pipeline_, PIPELINE_ERROR_READ), RunClosure()));
+  EXPECT_CALL(*mocks_->audio_renderer(), Stop(_)).WillOnce(RunClosure());
+
+  // No error callback should fire.
+  EXPECT_CALL(callbacks_, OnStop());
+
+  DoAudioPipelineStop();
+}
+
 TEST_F(PipelineTest, SetVolume) {
   CreateAudioStream();
   MockDemuxerStreamVector streams;