Reland: Use a shared thread for media operations (round 3).

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 725695f966f272c490463027fa20e4f087e6aecf..5131e0fa61c43bf818babb6d2bc88f5d9fa98729 100644
--- a/media/base/pipeline_unittest.cc
+++ b/media/base/pipeline_unittest.cc
@@ -105,28 +105,16 @@ class PipelineTest : public ::testing::Test {
   }
 
   virtual ~PipelineTest() {
-    // Shutdown sequence.
-    if (pipeline_->IsRunning()) {
-      EXPECT_CALL(*demuxer_, Stop(_))
-          .WillOnce(RunClosure<0>());
+    if (!pipeline_ || !pipeline_->IsRunning())
+      return;
 
-      if (audio_stream_)
-        EXPECT_CALL(*audio_renderer_, Stop(_))
-            .WillOnce(RunClosure<0>());
-
-      if (video_stream_)
-        EXPECT_CALL(*video_renderer_, Stop(_))
-            .WillOnce(RunClosure<0>());
-    }
+    ExpectStop();
 
     // Expect a stop callback if we were started.
     EXPECT_CALL(callbacks_, OnStop());
     pipeline_->Stop(base::Bind(&CallbackHelper::OnStop,
                                base::Unretained(&callbacks_)));
     message_loop_.RunUntilIdle();
-
-    filter_collection_.reset();
-    pipeline_.reset();
   }
 
  protected:
@@ -285,6 +273,20 @@ class PipelineTest : public ::testing::Test {
     EXPECT_EQ(seek_time, pipeline_->GetMediaTime());
   }
 
+  void ExpectStop() {
+    if (demuxer_)

[acolwell GONE FROM CHROMIUM, 2013/06/05 01:09:05]

nit: Add { } here and below since these take up multiple lines.

[scherkus (not reviewing), 2013/06/05 18:40:43]

actually all of these can fit on a single line

+      EXPECT_CALL(*demuxer_, Stop(_))
+          .WillOnce(RunClosure<0>());
+
+    if (audio_stream_)
+      EXPECT_CALL(*audio_renderer_, Stop(_))
+          .WillOnce(RunClosure<0>());
+
+    if (video_stream_)
+      EXPECT_CALL(*video_renderer_, Stop(_))
+          .WillOnce(RunClosure<0>());
+  }
+
   // Fixture members.
   StrictMock<CallbackHelper> callbacks_;
   base::SimpleTestClock test_clock_;
@@ -836,6 +838,24 @@ TEST_F(PipelineTest, AudioTimeUpdateDuringSeek) {
   EXPECT_EQ(pipeline_->GetMediaTime(), new_time);
 }
 
+static void DeletePipeline(scoped_ptr<Pipeline> pipeline) {
+  // |pipeline| will go out of scope.
+}
+
+TEST_F(PipelineTest, DeleteAfterStop) {
+  CreateAudioStream();
+  MockDemuxerStreamVector streams;
+  streams.push_back(audio_stream());
+  InitializeDemuxer(&streams);
+  InitializeAudioRenderer(audio_stream(), false);
+  InitializePipeline(PIPELINE_OK);
+
+  ExpectStop();
+
+  pipeline_->Stop(base::Bind(&DeletePipeline, base::Passed(&pipeline_)));

[acolwell GONE FROM CHROMIUM, 2013/06/05 01:09:05]

Whoa... how is this not crashing? Isn't the Passed clearing pipeline_? I think
you need to make a local copy of the pipeline pointer to avoid undefined
compiler behavior.

[scherkus (not reviewing), 2013/06/05 18:40:43]

Turns out it _is_ crashing! (check out try bot results)

+  message_loop_.RunUntilIdle();
+}
+
 class PipelineTeardownTest : public PipelineTest {
  public:
   enum TeardownState {