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

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/test/simple_test_clock.h"
@@ skipping 87 matching lines @@
     // streams.
     DemuxerStream* null_pointer = NULL;
     EXPECT_CALL(*demuxer_, GetStream(_))
         .WillRepeatedly(Return(null_pointer));
 
     EXPECT_CALL(*demuxer_, GetStartTime())
         .WillRepeatedly(Return(base::TimeDelta()));
   }
 
   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:
   // Sets up expectations to allow the demuxer to initialize.
   typedef std::vector<MockDemuxerStream*> MockDemuxerStreamVector;
   void InitializeDemuxer(MockDemuxerStreamVector* streams,
                          const base::TimeDelta& duration) {
     EXPECT_CALL(callbacks_, OnDurationChange());
     EXPECT_CALL(*demuxer_, Initialize(_, _))
         .WillOnce(DoAll(SetDemuxerProperties(duration),
@@ skipping 138 matching lines @@
     pipeline_->Seek(seek_time,
                     base::Bind(&CallbackHelper::OnSeek,
                                base::Unretained(&callbacks_)));
 
     // We expect the time to be updated only after the seek has completed.
     EXPECT_NE(seek_time, pipeline_->GetMediaTime());
     message_loop_.RunUntilIdle();
     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_;
   base::MessageLoop message_loop_;
   scoped_ptr<Pipeline> pipeline_;
 
   scoped_ptr<FilterCollection> filter_collection_;
   scoped_ptr<MockDemuxer> demuxer_;
   MockVideoRenderer* video_renderer_;
   MockAudioRenderer* audio_renderer_;
@@ skipping 531 matching lines @@
   EXPECT_EQ(pipeline_->GetMediaTime(), seek_time);
 
   // Now that the seek is complete, verify that time updates advance the current
   // time.
   base::TimeDelta new_time = seek_time + base::TimeDelta::FromMilliseconds(100);
   audio_time_cb_.Run(new_time, new_time);
 
   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 {
     kInitDemuxer,
     kInitAudioRenderer,
     kInitVideoRenderer,
     kPausing,
     kFlushing,
     kSeeking,
     kPrerolling,
@@ skipping 303 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