Fix crash on media teardown when triggered by pipeline error

This CL prohibits the media pipeline from posting non-teardown related tasks
onto its message loop during teardown. The CL includes a unit test to test
for this.

BUG=96849
TEST=media_unittests

Review URL: http://codereview.chromium.org/8116019

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@103974 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 62 insertions, 5 deletions

diff --git a/media/base/pipeline_impl.cc b/media/base/pipeline_impl.cc
index 5f9fb25..dd7f625 100644
--- a/media/base/pipeline_impl.cc
+++ b/media/base/pipeline_impl.cc
@@ -196,7 +196,7 @@ void PipelineImpl::SetPlaybackRate(float playback_rate) {
 
   base::AutoLock auto_lock(lock_);
   playback_rate_ = playback_rate;
-  if (running_) {
+  if (running_ && !tearing_down_) {
     message_loop_->PostTask(FROM_HERE, base::Bind(
         &PipelineImpl::PlaybackRateChangedTask, this, playback_rate));
   }
@@ -214,7 +214,7 @@ void PipelineImpl::SetVolume(float volume) {
 
   base::AutoLock auto_lock(lock_);
   volume_ = volume;
-  if (running_) {
+  if (running_ && !tearing_down_) {
     message_loop_->PostTask(FROM_HERE, base::Bind(
         &PipelineImpl::VolumeChangedTask, this, volume));
   }
@@ -228,7 +228,7 @@ Preload PipelineImpl::GetPreload() const {
 void PipelineImpl::SetPreload(Preload preload) {
   base::AutoLock auto_lock(lock_);
   preload_ = preload;
-  if (running_) {
+  if (running_ && !tearing_down_) {
     message_loop_->PostTask(FROM_HERE, base::Bind(
         &PipelineImpl::PreloadChangedTask, this, preload));
   }
@@ -818,6 +818,9 @@ void PipelineImpl::ErrorChangedTask(PipelineStatus error) {
 void PipelineImpl::PlaybackRateChangedTask(float playback_rate) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
 
+  if (!running_ || tearing_down_)
+    return;
+
   // Suppress rate change until after seeking.
   if (IsPipelineSeeking()) {
     pending_playback_rate_ = playback_rate;
@@ -842,14 +845,18 @@ void PipelineImpl::PlaybackRateChangedTask(float playback_rate) {
 
 void PipelineImpl::VolumeChangedTask(float volume) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
+  if (!running_ || tearing_down_)
+    return;
 
-  if (audio_renderer_) {
+  if (audio_renderer_)
     audio_renderer_->SetVolume(volume);
-  }
 }
 
 void PipelineImpl::PreloadChangedTask(Preload preload) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
+  if (!running_ || tearing_down_)
+    return;
+
   if (demuxer_)
     demuxer_->SetPreload(preload);
 }
diff --git a/media/base/pipeline_impl_unittest.cc b/media/base/pipeline_impl_unittest.cc
index 0abd9c4..4f7fb51 100644
--- a/media/base/pipeline_impl_unittest.cc
+++ b/media/base/pipeline_impl_unittest.cc
@@ -781,6 +781,56 @@ TEST_F(PipelineImplTest, ErrorDuringSeek) {
   message_loop_.RunAllPending();
 }
 
+// Invoked function OnError. This asserts that the pipeline does not enqueue
+// non-teardown related tasks while tearing down.
+static void TestNoCallsAfterError(
+    PipelineImpl* pipeline, MessageLoop* message_loop,
+    PipelineStatus /* status */) {
+  CHECK(pipeline);
+  CHECK(message_loop);
+
+  // When we get to this stage, the message loop should be empty.
+  message_loop->AssertIdle();
+
+  // Make calls on pipeline after error has occurred.
+  pipeline->SetPlaybackRate(0.5f);
+  pipeline->SetVolume(0.5f);
+  pipeline->SetPreload(AUTO);
+
+  // No additional tasks should be queued as a result of these calls.
+  message_loop->AssertIdle();
+}
+
+TEST_F(PipelineImplTest, NoMessageDuringTearDownFromError) {
+  CreateAudioStream();
+  MockDemuxerStreamVector streams;
+  streams.push_back(audio_stream());
+
+  InitializeDemuxer(&streams, base::TimeDelta::FromSeconds(10));
+  InitializeAudioDecoder(audio_stream());
+  InitializeAudioRenderer();
+  InitializePipeline(PIPELINE_OK);
+
+  // Trigger additional requests on the pipeline during tear down from error.
+  base::Callback<void(PipelineStatus)> cb = base::Bind(
+      &TestNoCallsAfterError, pipeline_, &message_loop_);
+  ON_CALL(callbacks_, OnError(_))
+      .WillByDefault(Invoke(&cb, &base::Callback<void(PipelineStatus)>::Run));
+
+  InSequence s;
+
+  base::TimeDelta seek_time = base::TimeDelta::FromSeconds(5);
+
+  EXPECT_CALL(*mocks_->demuxer(), Seek(seek_time, _))
+      .WillOnce(Invoke(&SendReadErrorToCB));
+
+  pipeline_->Seek(seek_time,base::Bind(&CallbackHelper::OnSeek,
+                                       base::Unretained(&callbacks_)));
+  EXPECT_CALL(callbacks_, OnSeek(PIPELINE_ERROR_READ));
+  EXPECT_CALL(callbacks_, OnError(PIPELINE_ERROR_READ));
+  message_loop_.RunAllPending();
+}
+
 TEST_F(PipelineImplTest, StartTimeIsZero) {
   CreateVideoStream();
   MockDemuxerStreamVector streams;