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

media/base/pipeline_unittest.cc

 // Copyright (c) 2012 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 <vector>
 
 #include "base/bind.h"
 #include "base/message_loop/message_loop.h"
 #include "base/stl_util.h"
 #include "base/test/simple_test_tick_clock.h"
@@ skipping 578 matching lines @@
 
   AddTextStream();
 
   // The ended callback shouldn't run until all renderers have ended.
   pipeline_->OnAudioRendererEnded();
   message_loop_.RunUntilIdle();
 
   pipeline_->OnVideoRendererEnded();
   message_loop_.RunUntilIdle();
 
+  EXPECT_CALL(*audio_renderer_, StopRendering());
   EXPECT_CALL(callbacks_, OnEnded());
   text_stream()->SendEosNotification();
   message_loop_.RunUntilIdle();
 }
 
 TEST_F(PipelineTest, AudioStreamShorterThanVideo) {
   base::TimeDelta duration = base::TimeDelta::FromSeconds(10);
 
   CreateAudioStream();
   CreateVideoStream();
@@ skipping 29 matching lines @@
   // Signal end of audio stream.
   pipeline_->OnAudioRendererEnded();
   message_loop_.RunUntilIdle();
 
   // Verify that the clock advances.
   start_time = pipeline_->GetMediaTime().ToInternalValue();
   test_tick_clock_.Advance(base::TimeDelta::FromMilliseconds(100));
   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();
 }
 
 TEST_F(PipelineTest, ErrorDuringSeek) {
   CreateAudioStream();
   MockDemuxerStreamVector streams;
   streams.push_back(audio_stream());
 
   InitializeDemuxer(&streams);
@@ skipping 193 matching lines @@
   pipeline->Stop(base::Bind(&DeletePipeline, base::Passed(&pipeline_)));
   message_loop_.RunUntilIdle();
 }
 
 class PipelineTeardownTest : public PipelineTest {
  public:
   enum TeardownState {
     kInitDemuxer,
     kInitAudioRenderer,
     kInitVideoRenderer,
-    kPausing,
     kFlushing,
     kSeeking,
     kPrerolling,
     kPlaying,
   };
 
   enum StopOrError {
     kStop,
     kError,
     kErrorAndStop,
   };
 
   PipelineTeardownTest() {}
   virtual ~PipelineTeardownTest() {}
 
   void RunTest(TeardownState state, StopOrError stop_or_error) {
     switch (state) {
       case kInitDemuxer:
       case kInitAudioRenderer:
       case kInitVideoRenderer:
         DoInitialize(state, stop_or_error);
         break;
 
-      case kPausing:
       case kFlushing:
       case kSeeking:
       case kPrerolling:
         DoInitialize(state, stop_or_error);
         DoSeek(state, stop_or_error);
         break;
 
       case kPlaying:
         DoInitialize(state, stop_or_error);
         DoStopOrError(stop_or_error);
@@ skipping 132 matching lines @@
         &CallbackHelper::OnSeek, base::Unretained(&callbacks_)));
     message_loop_.RunUntilIdle();
   }
 
   PipelineStatus SetSeekExpectations(TeardownState state,
                                      StopOrError stop_or_error) {
     PipelineStatus status = PIPELINE_OK;
     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) {
       if (stop_or_error == kStop) {
         EXPECT_CALL(*audio_renderer_, Flush(_))
             .WillOnce(DoAll(Stop(pipeline_.get(), stop_cb), RunClosure<0>()));
       } else {
         status = PIPELINE_ERROR_READ;
         EXPECT_CALL(*audio_renderer_, Flush(_)).WillOnce(
             DoAll(SetError(pipeline_.get(), status), RunClosure<0>()));
@@ skipping 84 matching lines @@
 };
 
 #define INSTANTIATE_TEARDOWN_TEST(stop_or_error, state) \
     TEST_F(PipelineTeardownTest, stop_or_error##_##state) { \
       RunTest(k##state, k##stop_or_error); \
     }
 
 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);
 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);
 INSTANTIATE_TEARDOWN_TEST(Error, Playing);
 
 INSTANTIATE_TEARDOWN_TEST(ErrorAndStop, Playing);
 
 }  // namespace media