Extract media::Clock::IsPlaying() into media::Pipeline::ClockState enum.

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 e1f1e56cb6b42d953230597cdeae2d8bd9690123..45cc73ba36fa7b1a46c5f19aed717b78da5be1fe 100644
--- a/media/base/pipeline_unittest.cc
+++ b/media/base/pipeline_unittest.cc
@@ -596,6 +596,7 @@ TEST_F(PipelineTest, EndedCallback) {
   pipeline_->OnVideoRendererEnded();
   message_loop_.RunUntilIdle();
 
+  EXPECT_CALL(*audio_renderer_, StopRendering());
   EXPECT_CALL(callbacks_, OnEnded());
   text_stream()->SendEosNotification();
   message_loop_.RunUntilIdle();
@@ -645,6 +646,7 @@ TEST_F(PipelineTest, AudioStreamShorterThanVideo) {
   EXPECT_GT(pipeline_->GetMediaTime().ToInternalValue(), start_time);
 
   // Signal end of video stream and make sure OnEnded() callback occurs.
+  EXPECT_CALL(*audio_renderer_, StopRendering());
   EXPECT_CALL(callbacks_, OnEnded());
   pipeline_->OnVideoRendererEnded();
 }
@@ -858,7 +860,6 @@ class PipelineTeardownTest : public PipelineTest {
     kInitDemuxer,
     kInitAudioRenderer,
     kInitVideoRenderer,
-    kPausing,
     kFlushing,
     kSeeking,
     kPrerolling,
@@ -882,7 +883,6 @@ class PipelineTeardownTest : public PipelineTest {
         DoInitialize(state, stop_or_error);
         break;
 
-      case kPausing:
       case kFlushing:
       case kSeeking:
       case kPrerolling:
@@ -1035,19 +1035,6 @@ class PipelineTeardownTest : public PipelineTest {
     base::Closure stop_cb = base::Bind(
         &CallbackHelper::OnStop, base::Unretained(&callbacks_));
 
-    if (state == kPausing) {
-      if (stop_or_error == kStop) {
-        EXPECT_CALL(*audio_renderer_, StopRendering())
-            .WillOnce(Stop(pipeline_.get(), stop_cb));
-      } else {
-        status = PIPELINE_ERROR_READ;
-        EXPECT_CALL(*audio_renderer_, StopRendering())
-            .WillOnce(SetError(pipeline_.get(), status));
-      }
-
-      return status;
-    }
-
     EXPECT_CALL(*audio_renderer_, StopRendering());
 
     if (state == kFlushing) {
@@ -1152,7 +1139,6 @@ class PipelineTeardownTest : public PipelineTest {
 INSTANTIATE_TEARDOWN_TEST(Stop, InitDemuxer);
 INSTANTIATE_TEARDOWN_TEST(Stop, InitAudioRenderer);
 INSTANTIATE_TEARDOWN_TEST(Stop, InitVideoRenderer);
-INSTANTIATE_TEARDOWN_TEST(Stop, Pausing);

[scherkus (not reviewing), 2014/05/21 00:08:52]

these tests were removed because they don't make a lot of sense now that
StopRendering() is a synchronous call (they were hitting reentrancy issues since
MockAudioRenderer has expectations that call public Pipeline APIs ... which
isn't really the API contract Pipeline has)

[acolwell GONE FROM CHROMIUM, 2014/05/22 01:28:21]

Could this be causing other issues? I wonder if we should have versions of
RunClosure<> and RunCallback<> that actutally PostTask() so the pipeline runs in
the context it is expected to.

[scherkus (not reviewing), 2014/05/22 17:23:40]

It'd be a nice to have, but it's not really causing any issues w/ the existing
tests today as calling Pipeline::Stop/SetError() force-post tasks anyway.

The one little snag with these tests was that Stop() has DCHECK(IsRunning())
inside the method, where the implementation of IsRunning() tries to grab a lock
to read a boolean :\

(I had a CL somewhere that got rid of IsRunning() since it's kind of useless of
a check anyway...)

 INSTANTIATE_TEARDOWN_TEST(Stop, Flushing);
 INSTANTIATE_TEARDOWN_TEST(Stop, Seeking);
 INSTANTIATE_TEARDOWN_TEST(Stop, Prerolling);
@@ -1161,7 +1147,6 @@ INSTANTIATE_TEARDOWN_TEST(Stop, Playing);
 INSTANTIATE_TEARDOWN_TEST(Error, InitDemuxer);
 INSTANTIATE_TEARDOWN_TEST(Error, InitAudioRenderer);
 INSTANTIATE_TEARDOWN_TEST(Error, InitVideoRenderer);
-INSTANTIATE_TEARDOWN_TEST(Error, Pausing);
 INSTANTIATE_TEARDOWN_TEST(Error, Flushing);
 INSTANTIATE_TEARDOWN_TEST(Error, Seeking);
 INSTANTIATE_TEARDOWN_TEST(Error, Prerolling);