Rewrite media::Pipeline state transition machinery and simplify shutdown.

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.h"
 #include "base/stl_util.h"
 #include "base/threading/simple_thread.h"
@@ skipping 95 matching lines @@
     EXPECT_CALL(*mocks_->demuxer(), GetStartTime())
         .WillRepeatedly(Return(base::TimeDelta()));
   }
 
   virtual ~PipelineTest() {
     // Shutdown sequence.
     if (pipeline_->IsRunning()) {
       EXPECT_CALL(*mocks_->demuxer(), Stop(_))
           .WillOnce(RunClosure());
 
-      // TODO(scherkus): Don't pause+flush on shutdown,
-      // see http://crbug.com/110228
       if (audio_stream_) {

[Ami GONE FROM CHROMIUM, 2012/09/02 20:29:29]

braces no longer necessary (here and below).

[scherkus (not reviewing), 2012/09/05 15:04:17]

Done.

-        EXPECT_CALL(*mocks_->audio_renderer(), Pause(_))
-            .WillOnce(RunClosure());
-        EXPECT_CALL(*mocks_->audio_renderer(), Flush(_))
-            .WillOnce(RunClosure());
-        EXPECT_CALL(*mocks_->audio_renderer(), Stop(_))
-            .WillOnce(RunClosure());
+        EXPECT_CALL(*mocks_->audio_renderer(), Stop(_)).WillOnce(RunClosure());
       }
 
       if (video_stream_) {
-        EXPECT_CALL(*mocks_->video_renderer(), Pause(_))
-            .WillOnce(RunClosure());
-        EXPECT_CALL(*mocks_->video_renderer(), Flush(_))
-            .WillOnce(RunClosure());
-        EXPECT_CALL(*mocks_->video_renderer(), Stop(_))
-            .WillOnce(RunClosure());
+        EXPECT_CALL(*mocks_->video_renderer(), Stop(_)).WillOnce(RunClosure());
       }
     }
 
     // Expect a stop callback if we were started.
     EXPECT_CALL(callbacks_, OnStop());
     pipeline_->Stop(base::Bind(&CallbackHelper::OnStop,
                                base::Unretained(&callbacks_)));
     message_loop_.RunAllPending();
 
     pipeline_ = NULL;
@@ skipping 115 matching lines @@
   }
 
   MockDemuxerStream* video_stream() {
     return video_stream_;
   }
 
   void ExpectSeek(const base::TimeDelta& seek_time) {
     // Every filter should receive a call to Seek().
     EXPECT_CALL(*mocks_->demuxer(), Seek(seek_time, _))
         .WillOnce(RunPipelineStatusCB());
+    EXPECT_CALL(*mocks_->demuxer(), SetPlaybackRate(_));
 
     if (audio_stream_) {
       EXPECT_CALL(*mocks_->audio_renderer(), Pause(_))
           .WillOnce(RunClosure());
       EXPECT_CALL(*mocks_->audio_renderer(), Flush(_))
           .WillOnce(RunClosure());
       EXPECT_CALL(*mocks_->audio_renderer(), Preroll(seek_time, _))
           .WillOnce(RunPipelineStatusCB());
+      EXPECT_CALL(*mocks_->audio_renderer(), SetPlaybackRate(_));
+      EXPECT_CALL(*mocks_->audio_renderer(), SetVolume(_));
       EXPECT_CALL(*mocks_->audio_renderer(), Play(_))
           .WillOnce(RunClosure());
     }
 
     if (video_stream_) {
       EXPECT_CALL(*mocks_->video_renderer(), Pause(_))
           .WillOnce(RunClosure());
       EXPECT_CALL(*mocks_->video_renderer(), Flush(_))
           .WillOnce(RunClosure());
       EXPECT_CALL(*mocks_->video_renderer(), Preroll(seek_time, _))
           .WillOnce(RunPipelineStatusCB());
+      EXPECT_CALL(*mocks_->video_renderer(), SetPlaybackRate(_));
       EXPECT_CALL(*mocks_->video_renderer(), Play(_))
           .WillOnce(RunClosure());
     }
 
     EXPECT_CALL(callbacks_, OnBufferingState(Pipeline::kPrerollCompleted));
 
     // We expect a successful seek callback.
     EXPECT_CALL(callbacks_, OnSeek(PIPELINE_OK));
   }
 
@@ skipping 405 matching lines @@
   InitializePipeline(PIPELINE_OK);
 
   float playback_rate = 1.0f;
   EXPECT_CALL(*mocks_->demuxer(), SetPlaybackRate(playback_rate));
   EXPECT_CALL(*mocks_->audio_renderer(), SetPlaybackRate(playback_rate));
   pipeline_->SetPlaybackRate(playback_rate);
   message_loop_.RunAllPending();
 
   base::TimeDelta seek_time = base::TimeDelta::FromSeconds(5);
 
-  // Seek() isn't called as the demuxer errors out first.
+  // Preroll() isn't called as the demuxer errors out first.
   EXPECT_CALL(*mocks_->audio_renderer(), Pause(_))
       .WillOnce(RunClosure());
   EXPECT_CALL(*mocks_->audio_renderer(), Flush(_))
       .WillOnce(RunClosure());
   EXPECT_CALL(*mocks_->audio_renderer(), Stop(_))
       .WillOnce(RunClosure());
 
   EXPECT_CALL(*mocks_->demuxer(), Seek(seek_time, _))
       .WillOnce(RunPipelineStatusCBWithStatus(PIPELINE_ERROR_READ));
   EXPECT_CALL(*mocks_->demuxer(), Stop(_))
@@ skipping 135 matching lines @@
   EXPECT_CALL(*mocks_->demuxer(), Seek(seek_time, _))
       .WillOnce(DoAll(InvokeWithoutArgs(&closure, &base::Closure::Run),
                       RunPipelineStatusCB()));
 
   EXPECT_CALL(*mocks_->audio_renderer(), Pause(_))
       .WillOnce(RunClosure());
   EXPECT_CALL(*mocks_->audio_renderer(), Flush(_))
       .WillOnce(RunClosure());
   EXPECT_CALL(*mocks_->audio_renderer(), Preroll(seek_time, _))
       .WillOnce(RunPipelineStatusCB());
+  EXPECT_CALL(*mocks_->demuxer(), SetPlaybackRate(_));
+  EXPECT_CALL(*mocks_->audio_renderer(), SetPlaybackRate(_));
+  EXPECT_CALL(*mocks_->audio_renderer(), SetVolume(_));
   EXPECT_CALL(*mocks_->audio_renderer(), Play(_))
       .WillOnce(RunClosure());
 
   EXPECT_CALL(callbacks_, OnBufferingState(Pipeline::kPrerollCompleted));
   EXPECT_CALL(callbacks_, OnSeek(PIPELINE_OK));
   DoSeek(seek_time);
 
   EXPECT_EQ(pipeline_->GetMediaTime(), seek_time);
 
   // Now that the seek is complete, verify that time updates advance the current
@@ skipping 86 matching lines @@
       case kInitAudioDecoder:
       case kInitAudioRenderer:
       case kInitVideoRenderer:
         DoInitialize(state, stop_or_error);
         break;
 
       case kPausing:
       case kFlushing:
       case kSeeking:
       case kPrerolling:
-      case kStarting: {
+      case kStarting:
         DoInitialize(state, stop_or_error);
+        DoSeek(state, stop_or_error);
+        break;
 
-        InSequence s;
-        if (stop_or_error == kStop) {
-          ExpectSeekStop(state);
-        } else {
-          ExpectSeekError(state);
-        }
-        DoSeek();
+      case kPlaying:
+        DoInitialize(state, stop_or_error);
+        DoStopOrError(stop_or_error);
         break;
-      }
-
-      case kPlaying: {
-        DoInitialize(state, stop_or_error);
-
-        InSequence s;
-        if (stop_or_error == kStop) {
-          ExpectStop();
-          DoStop();
-        } else {
-          ExpectPlaybackError();
-          DoPlaybackError();
-        }
-        break;
-      }
     }
   }
 
  private:
   // TODO(scherkus): 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
   void DoInitialize(TeardownState state, StopOrError stop_or_error) {
     PipelineStatus expected_status =
         SetInitializeExpectations(state, stop_or_error);
@@ skipping 93 matching lines @@
       return status;
     }
 
     EXPECT_CALL(*mocks_->video_renderer(),
                 Initialize(_, _, _, _, _, _, _, _, _, _))
         .WillOnce(RunPipelineStatusCB2());
 
     EXPECT_CALL(callbacks_, OnBufferingState(Pipeline::kHaveMetadata));
 
     // If we get here it's a successful initialization.
-    EXPECT_CALL(*mocks_->demuxer(), SetPlaybackRate(0.0f));
-    EXPECT_CALL(*mocks_->audio_renderer(), SetPlaybackRate(0.0f));
-    EXPECT_CALL(*mocks_->video_renderer(), SetPlaybackRate(0.0f));
-
-    EXPECT_CALL(*mocks_->audio_renderer(), SetVolume(1.0f));
-
     EXPECT_CALL(*mocks_->audio_renderer(), Preroll(base::TimeDelta(), _))
         .WillOnce(RunPipelineStatusCB());
     EXPECT_CALL(*mocks_->video_renderer(), Preroll(base::TimeDelta(), _))
         .WillOnce(RunPipelineStatusCB());
 
+    EXPECT_CALL(*mocks_->demuxer(), SetPlaybackRate(0.0f));
+    EXPECT_CALL(*mocks_->audio_renderer(), SetPlaybackRate(0.0f));
+    EXPECT_CALL(*mocks_->video_renderer(), SetPlaybackRate(0.0f));
+    EXPECT_CALL(*mocks_->audio_renderer(), SetVolume(1.0f));
+
     EXPECT_CALL(*mocks_->audio_renderer(), Play(_))
         .WillOnce(RunClosure());
     EXPECT_CALL(*mocks_->video_renderer(), Play(_))
         .WillOnce(RunClosure());
 
     if (status == PIPELINE_OK)
       EXPECT_CALL(callbacks_, OnBufferingState(Pipeline::kPrerollCompleted));
 
     return status;
   }
 
-  void ExpectSeekStop(TeardownState state) {
+  void DoSeek(TeardownState state, StopOrError stop_or_error) {
+    InSequence s;
+    PipelineStatus status = SetSeekExpectations(state, stop_or_error);
+
+    EXPECT_CALL(*mocks_->demuxer(), Stop(_)).WillOnce(RunClosure());
+    EXPECT_CALL(*mocks_->audio_renderer(), Stop(_)).WillOnce(RunClosure());
+    EXPECT_CALL(*mocks_->video_renderer(), Stop(_)).WillOnce(RunClosure());
+    EXPECT_CALL(callbacks_, OnSeek(status));
+
+    if (status == PIPELINE_OK) {
+      EXPECT_CALL(callbacks_, OnStop());
+    }
+
+    pipeline_->Seek(base::TimeDelta::FromSeconds(10), base::Bind(
+        &CallbackHelper::OnSeek, base::Unretained(&callbacks_)));
+    message_loop_.RunAllPending();
+  }
+
+  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) {
-      EXPECT_CALL(*mocks_->audio_renderer(), Pause(_))
-          .WillOnce(DoAll(Stop(pipeline_, stop_cb), RunClosure()));
-    } else {
-      EXPECT_CALL(*mocks_->audio_renderer(), Pause(_)).WillOnce(RunClosure());
-    }
+      if (stop_or_error == kStop) {
+        EXPECT_CALL(*mocks_->audio_renderer(), Pause(_))
+            .WillOnce(DoAll(Stop(pipeline_, stop_cb), RunClosure()));
+      } else {
+        status = PIPELINE_ERROR_READ;
+        EXPECT_CALL(*mocks_->audio_renderer(), Pause(_))
+            .WillOnce(DoAll(SetError(pipeline_, status), RunClosure()));
+      }
 
-    EXPECT_CALL(*mocks_->video_renderer(), Pause(_)).WillOnce(RunClosure());
-
-    if (state == kFlushing) {
-      EXPECT_CALL(*mocks_->audio_renderer(), Flush(_))
-          .WillOnce(DoAll(Stop(pipeline_, stop_cb), RunClosure()));
-    } else {
-      EXPECT_CALL(*mocks_->audio_renderer(), Flush(_)).WillOnce(RunClosure());
-    }
-
-    EXPECT_CALL(*mocks_->video_renderer(), Flush(_)).WillOnce(RunClosure());
-
-    if (state == kSeeking) {
-      EXPECT_CALL(*mocks_->demuxer(), Seek(_, _))
-          .WillOnce(DoAll(Stop(pipeline_, stop_cb), RunPipelineStatusCB()));
-    } else {
-      EXPECT_CALL(*mocks_->demuxer(), Seek(_, _))
-          .WillOnce(RunPipelineStatusCB());
-    }
-
-    if (state == kPrerolling) {
-      EXPECT_CALL(*mocks_->audio_renderer(), Preroll(_, _))
-          .WillOnce(DoAll(Stop(pipeline_, stop_cb), RunPipelineStatusCB()));
-    } else {
-      EXPECT_CALL(*mocks_->audio_renderer(), Preroll(_, _))
-          .WillOnce(RunPipelineStatusCB());
-    }
-
-    EXPECT_CALL(*mocks_->video_renderer(), Preroll(_, _))
-        .WillOnce(RunPipelineStatusCB());
-
-    if (state == kStarting) {
-      EXPECT_CALL(*mocks_->audio_renderer(), Play(_))
-          .WillOnce(DoAll(Stop(pipeline_, stop_cb), RunClosure()));
-    } else {
-      EXPECT_CALL(*mocks_->audio_renderer(), Play(_)).WillOnce(RunClosure());
-    }
-
-    EXPECT_CALL(*mocks_->video_renderer(), Play(_)).WillOnce(RunClosure());
-
-    EXPECT_CALL(callbacks_, OnBufferingState(Pipeline::kPrerollCompleted));
-    EXPECT_CALL(callbacks_, OnSeek(PIPELINE_OK));
-    ExpectStop();
-  }
-
-  void ExpectSeekError(TeardownState state) {
-    SetSeekErrorExpectations(state);
-
-    // Executed after the error is raised.
-    EXPECT_CALL(callbacks_, OnSeek(PIPELINE_ERROR_READ));
-    EXPECT_CALL(*mocks_->demuxer(), Stop(_)).WillOnce(RunClosure());
-    EXPECT_CALL(*mocks_->audio_renderer(), Stop(_)).WillOnce(RunClosure());
-    EXPECT_CALL(*mocks_->video_renderer(), Stop(_)).WillOnce(RunClosure());
-  }
-
-  void SetSeekErrorExpectations(TeardownState state) {
-    if (state == kPausing) {
-      EXPECT_CALL(*mocks_->audio_renderer(), Pause(_))
-          .WillOnce(DoAll(SetError(pipeline_, PIPELINE_ERROR_READ),
-                          RunClosure()));
-      EXPECT_CALL(*mocks_->video_renderer(), Pause(_)).WillOnce(RunClosure());
-      return;
+      return status;
     }
 
     EXPECT_CALL(*mocks_->audio_renderer(), Pause(_)).WillOnce(RunClosure());
     EXPECT_CALL(*mocks_->video_renderer(), Pause(_)).WillOnce(RunClosure());
 
     if (state == kFlushing) {
-      EXPECT_CALL(*mocks_->audio_renderer(), Flush(_))
-          .WillOnce(DoAll(SetError(pipeline_, PIPELINE_ERROR_READ),
-                          RunClosure()));
-      EXPECT_CALL(*mocks_->video_renderer(), Flush(_)).WillOnce(RunClosure());
-      return;
+      if (stop_or_error == kStop) {
+        EXPECT_CALL(*mocks_->audio_renderer(), Flush(_))
+            .WillOnce(DoAll(Stop(pipeline_, stop_cb), RunClosure()));
+      } else {
+        status = PIPELINE_ERROR_READ;
+        EXPECT_CALL(*mocks_->audio_renderer(), Flush(_))
+            .WillOnce(DoAll(SetError(pipeline_, status), RunClosure()));
+      }
+
+      return status;
     }
 
     EXPECT_CALL(*mocks_->audio_renderer(), Flush(_)).WillOnce(RunClosure());
     EXPECT_CALL(*mocks_->video_renderer(), Flush(_)).WillOnce(RunClosure());
 
     if (state == kSeeking) {
-      EXPECT_CALL(*mocks_->demuxer(), Seek(_, _))
-          .WillOnce(RunPipelineStatusCBWithStatus(PIPELINE_ERROR_READ));
-      return;
+      if (stop_or_error == kStop) {
+        EXPECT_CALL(*mocks_->demuxer(), Seek(_, _))
+            .WillOnce(DoAll(Stop(pipeline_, stop_cb), RunPipelineStatusCB()));
+      } else {
+        status = PIPELINE_ERROR_READ;
+        EXPECT_CALL(*mocks_->demuxer(), Seek(_, _))
+            .WillOnce(RunPipelineStatusCBWithStatus(status));
+      }
+
+      return status;
     }
 
     EXPECT_CALL(*mocks_->demuxer(), Seek(_, _))
         .WillOnce(RunPipelineStatusCB());
 
     if (state == kPrerolling) {
-      EXPECT_CALL(*mocks_->audio_renderer(), Preroll(_, _))
-          .WillOnce(RunPipelineStatusCBWithStatus(PIPELINE_ERROR_READ));
-      return;
+      if (stop_or_error == kStop) {
+        EXPECT_CALL(*mocks_->audio_renderer(), Preroll(_, _))
+            .WillOnce(DoAll(Stop(pipeline_, stop_cb), RunPipelineStatusCB()));
+      } else {
+        status = PIPELINE_ERROR_READ;
+        EXPECT_CALL(*mocks_->audio_renderer(), Preroll(_, _))
+            .WillOnce(RunPipelineStatusCBWithStatus(status));
+      }
+
+      return status;
     }
 
     EXPECT_CALL(*mocks_->audio_renderer(), Preroll(_, _))
         .WillOnce(RunPipelineStatusCB());
     EXPECT_CALL(*mocks_->video_renderer(), Preroll(_, _))
         .WillOnce(RunPipelineStatusCB());
 
+    // Playback rate and volume are updated prior to starting.
+    EXPECT_CALL(*mocks_->demuxer(), SetPlaybackRate(0.0f));
+    EXPECT_CALL(*mocks_->audio_renderer(), SetPlaybackRate(0.0f));
+    EXPECT_CALL(*mocks_->video_renderer(), SetPlaybackRate(0.0f));
+    EXPECT_CALL(*mocks_->audio_renderer(), SetVolume(1.0f));
+
     if (state == kStarting) {
-      EXPECT_CALL(*mocks_->audio_renderer(), Play(_))
-          .WillOnce(DoAll(SetError(pipeline_, PIPELINE_ERROR_READ),
-                          RunClosure()));
-      EXPECT_CALL(*mocks_->video_renderer(), Play(_)).WillOnce(RunClosure());
-      return;
+      if (stop_or_error == kStop) {
+        EXPECT_CALL(*mocks_->audio_renderer(), Play(_))
+            .WillOnce(DoAll(Stop(pipeline_, stop_cb), RunClosure()));
+      } else {
+        status = PIPELINE_ERROR_READ;
+        EXPECT_CALL(*mocks_->audio_renderer(), Play(_))
+            .WillOnce(DoAll(SetError(pipeline_, status), RunClosure()));
+      }
+      return status;
     }
 
-    NOTREACHED() << "Unexpected TeardownState: " << state;
+    EXPECT_CALL(*mocks_->audio_renderer(), Play(_)).WillOnce(RunClosure());
+    EXPECT_CALL(*mocks_->video_renderer(), Play(_)).WillOnce(RunClosure());
+    NOTREACHED() << "WTF";

[acolwell GONE FROM CHROMIUM, 2012/09/05 13:22:22]

Remove WTF or replace with a more helpful comment.

[scherkus (not reviewing), 2012/09/05 15:04:17]

whoops haha

+    return status;
   }
 
-  void ExpectStop() {
-    // TODO(scherkus): Don't pause+flush, see http://crbug.com/110228
-    EXPECT_CALL(*mocks_->audio_renderer(), Pause(_)).WillOnce(RunClosure());
-    EXPECT_CALL(*mocks_->video_renderer(), Pause(_)).WillOnce(RunClosure());
-    EXPECT_CALL(*mocks_->audio_renderer(), Flush(_)).WillOnce(RunClosure());
-    EXPECT_CALL(*mocks_->video_renderer(), Flush(_)).WillOnce(RunClosure());
+  void DoStopOrError(StopOrError stop_or_error) {
+    InSequence s;
+
     EXPECT_CALL(*mocks_->demuxer(), Stop(_)).WillOnce(RunClosure());
     EXPECT_CALL(*mocks_->audio_renderer(), Stop(_)).WillOnce(RunClosure());
     EXPECT_CALL(*mocks_->video_renderer(), Stop(_)).WillOnce(RunClosure());
 
-    EXPECT_CALL(callbacks_, OnStop());
-  }
+    if (stop_or_error == kStop) {
+      EXPECT_CALL(callbacks_, OnStop());
+      pipeline_->Stop(base::Bind(
+          &CallbackHelper::OnStop, base::Unretained(&callbacks_)));
+    } else {
+      EXPECT_CALL(callbacks_, OnError(PIPELINE_ERROR_READ));
+      pipeline_->SetErrorForTesting(PIPELINE_ERROR_READ);
+    }
 
-  void ExpectPlaybackError() {
-    // TODO(scherkus): Don't pause+flush, see http://crbug.com/110228
-    EXPECT_CALL(*mocks_->audio_renderer(), Pause(_)).WillOnce(RunClosure());
-    EXPECT_CALL(*mocks_->video_renderer(), Pause(_)).WillOnce(RunClosure());
-    EXPECT_CALL(*mocks_->audio_renderer(), Flush(_)).WillOnce(RunClosure());
-    EXPECT_CALL(*mocks_->video_renderer(), Flush(_)).WillOnce(RunClosure());
-    EXPECT_CALL(*mocks_->demuxer(), Stop(_)).WillOnce(RunClosure());
-    EXPECT_CALL(*mocks_->audio_renderer(), Stop(_)).WillOnce(RunClosure());
-    EXPECT_CALL(*mocks_->video_renderer(), Stop(_)).WillOnce(RunClosure());
-
-    EXPECT_CALL(callbacks_, OnError(PIPELINE_ERROR_READ));
-  }
-
-  void DoSeek() {
-    pipeline_->Seek(base::TimeDelta::FromSeconds(10), base::Bind(
-        &CallbackHelper::OnSeek, base::Unretained(&callbacks_)));
     message_loop_.RunAllPending();
   }
 
-  void DoStop() {
-    pipeline_->Stop(base::Bind(
-        &CallbackHelper::OnStop, base::Unretained(&callbacks_)));
-    message_loop_.RunAllPending();
-  }
-
-  void DoPlaybackError() {
-    pipeline_->SetErrorForTesting(PIPELINE_ERROR_READ);
-    message_loop_.RunAllPending();
-  }
-
   DISALLOW_COPY_AND_ASSIGN(PipelineTeardownTest);
 };
 
 #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, InitAudioDecoder);
@@ skipping 11 matching lines @@
 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, Starting);
 INSTANTIATE_TEARDOWN_TEST(Error, Playing);
 
 }  // namespace media