Make MediaFilter::Stop() asynchronous.

This is the second issue regarding making Stop() asynchronous.

All Stop() in subclasses of MediaFilter is made to accept a callback and
runs the callback at the end of its stop.

BUG=16059
TEST=media_unittest, unit_tests, test_shell_tests still passes

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

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

29 files changed, 160 insertions, 77 deletions

diff --git a/chrome/renderer/media/audio_renderer_impl_unittest.cc b/chrome/renderer/media/audio_renderer_impl_unittest.cc
index 12d0100..27584a1 100644
--- a/chrome/renderer/media/audio_renderer_impl_unittest.cc
+++ b/chrome/renderer/media/audio_renderer_impl_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -72,6 +72,7 @@ class AudioRendererImplTest : public ::testing::Test {
   base::SharedMemory shared_mem_;
   media::MockFilterHost host_;
   media::MockFilterCallback callback_;
+  media::MockFilterCallback stop_callback_;
   scoped_refptr<media::MockAudioDecoder> decoder_;
   scoped_refptr<AudioRendererImpl> renderer_;
   media::MediaFormat decoder_media_format_;
@@ -88,14 +89,16 @@ TEST_F(AudioRendererImplTest, SetPlaybackRate) {
   renderer_->SetPlaybackRate(1.0f);
   renderer_->SetPlaybackRate(0.0f);
 
-  renderer_->Stop();
+  EXPECT_CALL(stop_callback_, OnFilterCallback());
+  renderer_->Stop(stop_callback_.NewCallback());
   message_loop_->RunAllPending();
 }
 
 TEST_F(AudioRendererImplTest, SetVolume) {
   // Execute SetVolume() codepath to create an IPC message.
   renderer_->SetVolume(0.5f);
-  renderer_->Stop();
+  EXPECT_CALL(stop_callback_, OnFilterCallback());
+  renderer_->Stop(stop_callback_.NewCallback());
   message_loop_->RunAllPending();
 }
 
@@ -109,7 +112,8 @@ TEST_F(AudioRendererImplTest, Stop) {
       { ViewMsg_AudioStreamState_Params::kPaused };
 
   // Execute Stop() codepath to create an IPC message.
-  renderer_->Stop();
+  EXPECT_CALL(stop_callback_, OnFilterCallback());
+  renderer_->Stop(stop_callback_.NewCallback());
   message_loop_->RunAllPending();
 
   // Run AudioMessageFilter::Delegate methods, which can be executed after being
@@ -132,14 +136,16 @@ TEST_F(AudioRendererImplTest, DestroyedMessageLoop_SetPlaybackRate) {
   renderer_->SetPlaybackRate(0.0f);
   renderer_->SetPlaybackRate(1.0f);
   renderer_->SetPlaybackRate(0.0f);
-  renderer_->Stop();
+  EXPECT_CALL(stop_callback_, OnFilterCallback());
+  renderer_->Stop(stop_callback_.NewCallback());
 }
 
 TEST_F(AudioRendererImplTest, DestroyedMessageLoop_SetVolume) {
   // Kill the message loop and verify SetVolume() still works.
   message_loop_.reset();
   renderer_->SetVolume(0.5f);
-  renderer_->Stop();
+  EXPECT_CALL(stop_callback_, OnFilterCallback());
+  renderer_->Stop(stop_callback_.NewCallback());
 }
 
 TEST_F(AudioRendererImplTest, DestroyedMessageLoop_OnReadComplete) {
@@ -147,5 +153,6 @@ TEST_F(AudioRendererImplTest, DestroyedMessageLoop_OnReadComplete) {
   message_loop_.reset();
   scoped_refptr<media::Buffer> buffer = new media::DataBuffer(kSize);
   renderer_->OnReadComplete(buffer);
-  renderer_->Stop();
+  EXPECT_CALL(stop_callback_, OnFilterCallback());
+  renderer_->Stop(stop_callback_.NewCallback());
 }
diff --git a/media/base/filters.h b/media/base/filters.h
index 141a753..80a3bd2 100644
--- a/media/base/filters.h
+++ b/media/base/filters.h
@@ -91,7 +91,9 @@ class MediaFilter : public base::RefCountedThreadSafe<MediaFilter> {
 
   // The pipeline has resumed playback.  Filters can continue requesting reads.
   // Filters may implement this method if they need to respond to this call.
+  // TODO(boliu): Check that callback is not NULL in sublcasses.
   virtual void Play(FilterCallback* callback) {
+    DCHECK(callback);
     if (callback) {
       callback->Run();
       delete callback;
@@ -101,24 +103,20 @@ class MediaFilter : public base::RefCountedThreadSafe<MediaFilter> {
   // The pipeline has paused playback.  Filters should fulfill any existing read
   // requests and then idle.  Filters may implement this method if they need to
   // respond to this call.
+  // TODO(boliu): Check that callback is not NULL in sublcasses.
   virtual void Pause(FilterCallback* callback) {
+    DCHECK(callback);
     if (callback) {
       callback->Run();
       delete callback;
     }
   }
 
-  // TODO(boliu): Remove once Stop() is asynchronous in subclasses.
-  virtual void Stop() {}
-
   // The pipeline is being stopped either as a result of an error or because
   // the client called Stop().
-  // TODO(boliu): No implementation in subclasses yet.
+  // TODO(boliu): Check that callback is not NULL in sublcasses.
   virtual void Stop(FilterCallback* callback) {
-    // TODO(boliu): Call the synchronous version for now. Remove once
-    // all filters have asynchronous stop.
-    Stop();
-
+    DCHECK(callback);
     if (callback) {
       callback->Run();
       delete callback;
diff --git a/media/filters/audio_renderer_base.cc b/media/filters/audio_renderer_base.cc
index 7e530a8..f3f9294 100644
--- a/media/filters/audio_renderer_base.cc
+++ b/media/filters/audio_renderer_base.cc
@@ -48,11 +48,17 @@ void AudioRendererBase::Pause(FilterCallback* callback) {
   }
 }
 
-void AudioRendererBase::Stop() {
+void AudioRendererBase::Stop(FilterCallback* callback) {
   OnStop();
-  AutoLock auto_lock(lock_);
-  state_ = kStopped;
-  algorithm_.reset(NULL);
+  {
+    AutoLock auto_lock(lock_);
+    state_ = kStopped;
+    algorithm_.reset(NULL);
+  }
+  if (callback) {
+    callback->Run();
+    delete callback;
+  }
 }
 
 void AudioRendererBase::Seek(base::TimeDelta time, FilterCallback* callback) {
diff --git a/media/filters/audio_renderer_base.h b/media/filters/audio_renderer_base.h
index 3762679..279d59f 100644
--- a/media/filters/audio_renderer_base.h
+++ b/media/filters/audio_renderer_base.h
@@ -33,7 +33,7 @@ class AudioRendererBase : public AudioRenderer {
   // MediaFilter implementation.
   virtual void Play(FilterCallback* callback);
   virtual void Pause(FilterCallback* callback);
-  virtual void Stop();
+  virtual void Stop(FilterCallback* callback);
 
   virtual void Seek(base::TimeDelta time, FilterCallback* callback);
 
diff --git a/media/filters/audio_renderer_base_unittest.cc b/media/filters/audio_renderer_base_unittest.cc
index 064f7a6..20d2912 100644
--- a/media/filters/audio_renderer_base_unittest.cc
+++ b/media/filters/audio_renderer_base_unittest.cc
@@ -70,7 +70,9 @@ class AudioRendererBaseTest : public ::testing::Test {
   virtual ~AudioRendererBaseTest() {
     // Expect a call into the subclass.
     EXPECT_CALL(*renderer_, OnStop());
-    renderer_->Stop();
+    EXPECT_CALL(callback_, OnFilterCallback());
+    EXPECT_CALL(callback_, OnCallbackDestroyed());
+    renderer_->Stop(callback_.NewCallback());
   }
 
  protected:
diff --git a/media/filters/decoder_base.h b/media/filters/decoder_base.h
index cc0e3cb..82f767e 100644
--- a/media/filters/decoder_base.h
+++ b/media/filters/decoder_base.h
@@ -30,9 +30,9 @@ class DecoderBase : public Decoder {
   typedef CallbackRunner< Tuple1<Output*> > ReadCallback;
 
   // MediaFilter implementation.
-  virtual void Stop() {
+  virtual void Stop(FilterCallback* callback) {
     this->message_loop()->PostTask(FROM_HERE,
-        NewRunnableMethod(this, &DecoderBase::StopTask));
+        NewRunnableMethod(this, &DecoderBase::StopTask, callback));
   }
 
   virtual void Seek(base::TimeDelta time,
@@ -146,7 +146,7 @@ class DecoderBase : public Decoder {
         NewRunnableMethod(this, &DecoderBase::ReadCompleteTask, buffer_ref));
   }
 
-  void StopTask() {
+  void StopTask(FilterCallback* callback) {
     DCHECK_EQ(MessageLoop::current(), this->message_loop());
 
     // Delegate to the subclass first.
@@ -155,6 +155,11 @@ class DecoderBase : public Decoder {
     // Throw away all buffers in all queues.
     result_queue_.clear();
     state_ = kStopped;
+
+    if (callback) {
+      callback->Run();
+      delete callback;
+    }
   }
 
   void SeekTask(base::TimeDelta time, FilterCallback* callback) {
diff --git a/media/filters/decoder_base_unittest.cc b/media/filters/decoder_base_unittest.cc
index e771989..4bc0531 100644
--- a/media/filters/decoder_base_unittest.cc
+++ b/media/filters/decoder_base_unittest.cc
@@ -153,7 +153,9 @@ TEST(DecoderBaseTest, FlowControl) {
 
   // Stop.
   EXPECT_CALL(*decoder, DoStop());
-  decoder->Stop();
+  EXPECT_CALL(callback, OnFilterCallback());
+  EXPECT_CALL(callback, OnCallbackDestroyed());
+  decoder->Stop(callback.NewCallback());
   message_loop.RunAllPending();
 }
 
diff --git a/media/filters/ffmpeg_demuxer.cc b/media/filters/ffmpeg_demuxer.cc
index c472504..27d8878 100644
--- a/media/filters/ffmpeg_demuxer.cc
+++ b/media/filters/ffmpeg_demuxer.cc
@@ -265,10 +265,10 @@ void FFmpegDemuxer::PostDemuxTask() {
       NewRunnableMethod(this, &FFmpegDemuxer::DemuxTask));
 }
 
-void FFmpegDemuxer::Stop() {
+void FFmpegDemuxer::Stop(FilterCallback* callback) {
   // Post a task to notify the streams to stop as well.
   message_loop()->PostTask(FROM_HERE,
-      NewRunnableMethod(this, &FFmpegDemuxer::StopTask));
+      NewRunnableMethod(this, &FFmpegDemuxer::StopTask, callback));
 
   // Then wakes up the thread from reading.
   SignalReadCompleted(DataSource::kReadError);
@@ -561,12 +561,16 @@ void FFmpegDemuxer::DemuxTask() {
   }
 }
 
-void FFmpegDemuxer::StopTask() {
+void FFmpegDemuxer::StopTask(FilterCallback* callback) {
   DCHECK_EQ(MessageLoop::current(), message_loop());
   StreamVector::iterator iter;
   for (iter = streams_.begin(); iter != streams_.end(); ++iter) {
     (*iter)->Stop();
   }
+  if (callback) {
+    callback->Run();
+    delete callback;
+  }
 }
 
 void FFmpegDemuxer::DisableAudioStreamTask() {
diff --git a/media/filters/ffmpeg_demuxer.h b/media/filters/ffmpeg_demuxer.h
index bc5cc62..c43bd20 100644
--- a/media/filters/ffmpeg_demuxer.h
+++ b/media/filters/ffmpeg_demuxer.h
@@ -131,7 +131,7 @@ class FFmpegDemuxer : public Demuxer,
   virtual void PostDemuxTask();
 
   // MediaFilter implementation.
-  virtual void Stop();
+  virtual void Stop(FilterCallback* callback);
   virtual void Seek(base::TimeDelta time, FilterCallback* callback);
   virtual void OnAudioRendererDisabled();
 
@@ -166,7 +166,7 @@ class FFmpegDemuxer : public Demuxer,
   void DemuxTask();
 
   // Carries out stopping the demuxer streams on the demuxer thread.
-  void StopTask();
+  void StopTask(FilterCallback* callback);
 
   // Carries out disabling the audio stream on the demuxer thread.
   void DisableAudioStreamTask();
diff --git a/media/filters/ffmpeg_demuxer_unittest.cc b/media/filters/ffmpeg_demuxer_unittest.cc
index ec263a6..c6c8e74 100644
--- a/media/filters/ffmpeg_demuxer_unittest.cc
+++ b/media/filters/ffmpeg_demuxer_unittest.cc
@@ -117,7 +117,9 @@ class FFmpegDemuxerTest : public testing::Test {
 
   virtual ~FFmpegDemuxerTest() {
     // Call Stop() to shut down internal threads.
-    demuxer_->Stop();
+    EXPECT_CALL(callback_, OnFilterCallback());
+    EXPECT_CALL(callback_, OnCallbackDestroyed());
+    demuxer_->Stop(callback_.NewCallback());
 
     // Finish up any remaining tasks.
     message_loop_.RunAllPending();
@@ -623,7 +625,9 @@ TEST_F(FFmpegDemuxerTest, Stop) {
   ASSERT_TRUE(audio);
 
   // Stop the demuxer.
-  demuxer_->Stop();
+  EXPECT_CALL(callback_, OnFilterCallback());
+  EXPECT_CALL(callback_, OnCallbackDestroyed());
+  demuxer_->Stop(callback_.NewCallback());
 
   // Expect all calls in sequence.
   InSequence s;
@@ -732,9 +736,6 @@ TEST_F(FFmpegDemuxerTest, ProtocolRead) {
   EXPECT_CALL(*data_source_, GetSize(_))
       .WillOnce(DoAll(SetArgumentPointee<0>(1024), Return(true)));
 
-  // This read complete signal is generated when demuxer is stopped.
-  EXPECT_CALL(*demuxer, SignalReadCompleted(DataSource::kReadError));
-
   // First read.
   EXPECT_EQ(512, demuxer->Read(512, kBuffer));
   int64 position;
@@ -749,7 +750,12 @@ TEST_F(FFmpegDemuxerTest, ProtocolRead) {
   // Third read will get an end-of-file error.
   EXPECT_EQ(AVERROR_EOF, demuxer->Read(512, kBuffer));
 
-  demuxer->Stop();
+  // This read complete signal is generated when demuxer is stopped.
+  EXPECT_CALL(*demuxer, SignalReadCompleted(DataSource::kReadError));
+  EXPECT_CALL(callback_, OnFilterCallback());
+  EXPECT_CALL(callback_, OnCallbackDestroyed());
+  demuxer->Stop(callback_.NewCallback());
+  message_loop_.RunAllPending();
 }
 
 TEST_F(FFmpegDemuxerTest, ProtocolGetSetPosition) {
diff --git a/media/filters/file_data_source.cc b/media/filters/file_data_source.cc
index 901e107..181149d 100644
--- a/media/filters/file_data_source.cc
+++ b/media/filters/file_data_source.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -19,7 +19,7 @@ FileDataSource::FileDataSource()
 }
 
 FileDataSource::~FileDataSource() {
-  Stop();
+  DCHECK(!file_);
 }
 
 void FileDataSource::Initialize(const std::string& url,
@@ -48,13 +48,17 @@ void FileDataSource::Initialize(const std::string& url,
   callback->Run();
 }
 
-void FileDataSource::Stop() {
+void FileDataSource::Stop(FilterCallback* callback) {
   AutoLock l(lock_);
   if (file_) {
     file_util::CloseFile(file_);
     file_ = NULL;
     file_size_ = 0;
   }
+  if (callback) {
+    callback->Run();
+    delete callback;
+  }
 }
 
 const MediaFormat& FileDataSource::media_format() {
diff --git a/media/filters/file_data_source.h b/media/filters/file_data_source.h
index 164e74c..182d5be 100644
--- a/media/filters/file_data_source.h
+++ b/media/filters/file_data_source.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -23,7 +23,7 @@ class FileDataSource : public DataSource {
   }
 
   // Implementation of MediaFilter.
-  virtual void Stop();
+  virtual void Stop(FilterCallback* callback);
 
   // Implementation of DataSource.
   virtual void Initialize(const std::string& url, FilterCallback* callback);
diff --git a/media/filters/file_data_source_unittest.cc b/media/filters/file_data_source_unittest.cc
index 80df9b7..48d999e 100644
--- a/media/filters/file_data_source_unittest.cc
+++ b/media/filters/file_data_source_unittest.cc
@@ -65,6 +65,10 @@ TEST(FileDataSourceTest, OpenFile) {
   scoped_refptr<FileDataSource> filter = new FileDataSource();
   filter->set_host(&host);
   filter->Initialize(TestFileURL(), callback.NewCallback());
+
+  EXPECT_CALL(callback, OnFilterCallback());
+  EXPECT_CALL(callback, OnCallbackDestroyed());
+  filter->Stop(callback.NewCallback());
 }
 
 // Use the mock filter host to directly call the Read and GetPosition methods.
@@ -99,6 +103,10 @@ TEST(FileDataSourceTest, ReadData) {
   filter->Read(5, 10, ten_bytes,
                NewCallback(&handler, &ReadCallbackHandler::ReadCallback));
   EXPECT_EQ('5', ten_bytes[0]);
+
+  EXPECT_CALL(callback, OnFilterCallback());
+  EXPECT_CALL(callback, OnCallbackDestroyed());
+  filter->Stop(callback.NewCallback());
 }
 
 // Test that FileDataSource does nothing on Seek().
@@ -110,6 +118,10 @@ TEST(FileDataSourceTest, Seek) {
 
   scoped_refptr<FileDataSource> filter = new FileDataSource();
   filter->Seek(kZero, callback.NewCallback());
+
+  EXPECT_CALL(callback, OnFilterCallback());
+  EXPECT_CALL(callback, OnCallbackDestroyed());
+  filter->Stop(callback.NewCallback());
 }
 
 }  // namespace media
diff --git a/media/filters/null_audio_renderer.cc b/media/filters/null_audio_renderer.cc
index 544b588..62cbeee 100644
--- a/media/filters/null_audio_renderer.cc
+++ b/media/filters/null_audio_renderer.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -23,7 +23,7 @@ NullAudioRenderer::NullAudioRenderer()
 }
 
 NullAudioRenderer::~NullAudioRenderer() {
-  Stop();
+  DCHECK_EQ(kNullThreadHandle, thread_);
 }
 
 // static
diff --git a/media/filters/omx_video_decode_engine.h b/media/filters/omx_video_decode_engine.h
index 6cdfe73..d1c5619 100644
--- a/media/filters/omx_video_decode_engine.h
+++ b/media/filters/omx_video_decode_engine.h
@@ -53,6 +53,7 @@ class OmxVideoDecodeEngine :
   //
   // TODO(ajwong): Normalize this interface with Task like the others, and
   // promote to the abstract interface.
+  // TODO(boliu): Should the callback type be FilterCallback*?
   virtual void Stop(Callback0::Type* done_cb);
 
   // Subclass can provide a different value.
@@ -258,4 +259,3 @@ class OmxVideoDecodeEngine :
 }  // namespace media
 
 #endif  // MEDIA_FILTERS_OMX_VIDEO_DECODE_ENGINE_H_
-
diff --git a/media/filters/omx_video_decoder.cc b/media/filters/omx_video_decoder.cc
index d969cb6..48d2cb6 100644
--- a/media/filters/omx_video_decoder.cc
+++ b/media/filters/omx_video_decoder.cc
@@ -72,11 +72,8 @@ void OmxVideoDecoder::FillThisBuffer(scoped_refptr<VideoFrame> frame) {
                        &OmxVideoDecodeEngine::FillThisBuffer, frame));
 }
 
-void OmxVideoDecoder::Stop() {
-  // TODO(ajwong): This is a total hack. Make async.
-  base::WaitableEvent event(false, false);
-  omx_engine_->Stop(NewCallback(&event, &base::WaitableEvent::Signal));
-  event.Wait();
+void OmxVideoDecoder::Stop(FilterCallback* callback) {
+  omx_engine_->Stop(callback);
 }
 
 void OmxVideoDecoder::DoInitialize(DemuxerStream* demuxer_stream,
diff --git a/media/filters/omx_video_decoder.h b/media/filters/omx_video_decoder.h
index 18c8b6b..236130b 100644
--- a/media/filters/omx_video_decoder.h
+++ b/media/filters/omx_video_decoder.h
@@ -28,7 +28,7 @@ class OmxVideoDecoder : public VideoDecoder {
   virtual ~OmxVideoDecoder();
 
   virtual void Initialize(DemuxerStream* stream, FilterCallback* callback);
-  virtual void Stop();
+  virtual void Stop(FilterCallback* callback);
   virtual void FillThisBuffer(scoped_refptr<VideoFrame> frame);
   virtual const MediaFormat& media_format() { return media_format_; }
 
diff --git a/media/filters/video_decoder_impl_unittest.cc b/media/filters/video_decoder_impl_unittest.cc
index f2161ed..d84067c 100644
--- a/media/filters/video_decoder_impl_unittest.cc
+++ b/media/filters/video_decoder_impl_unittest.cc
@@ -139,7 +139,9 @@ class VideoDecoderImplTest : public testing::Test {
 
   virtual ~VideoDecoderImplTest() {
     // Call Stop() to shut down internal threads.
-    decoder_->Stop();
+    EXPECT_CALL(callback_, OnFilterCallback());
+    EXPECT_CALL(callback_, OnCallbackDestroyed());
+    decoder_->Stop(callback_.NewCallback());
 
     // Finish up any remaining tasks.
     message_loop_.RunAllPending();
diff --git a/media/filters/video_renderer_base.cc b/media/filters/video_renderer_base.cc
index d3c1daa..13b6dee 100644
--- a/media/filters/video_renderer_base.cc
+++ b/media/filters/video_renderer_base.cc
@@ -99,7 +99,7 @@ void VideoRendererBase::Pause(FilterCallback* callback) {
   }
 }
 
-void VideoRendererBase::Stop() {
+void VideoRendererBase::Stop(FilterCallback* callback) {
   AutoLock auto_lock(lock_);
   state_ = kStopped;
 
@@ -119,6 +119,11 @@ void VideoRendererBase::Stop() {
     }
     thread_ = kNullThreadHandle;
   }
+
+  if (callback) {
+    callback->Run();
+    delete callback;
+  }
 }
 
 void VideoRendererBase::SetPlaybackRate(float playback_rate) {
diff --git a/media/filters/video_renderer_base.h b/media/filters/video_renderer_base.h
index 551d234..ba30f52 100644
--- a/media/filters/video_renderer_base.h
+++ b/media/filters/video_renderer_base.h
@@ -41,7 +41,7 @@ class VideoRendererBase : public VideoRenderer,
   // MediaFilter implementation.
   virtual void Play(FilterCallback* callback);
   virtual void Pause(FilterCallback* callback);
-  virtual void Stop();
+  virtual void Stop(FilterCallback* callback);
   virtual void SetPlaybackRate(float playback_rate);
   virtual void Seek(base::TimeDelta time, FilterCallback* callback);
 
diff --git a/media/filters/video_renderer_base_unittest.cc b/media/filters/video_renderer_base_unittest.cc
index 111df56..81af7d7 100644
--- a/media/filters/video_renderer_base_unittest.cc
+++ b/media/filters/video_renderer_base_unittest.cc
@@ -65,7 +65,9 @@ class VideoRendererBaseTest : public ::testing::Test {
 
     // Expect a call into the subclass.
     EXPECT_CALL(*renderer_, OnStop());
-    renderer_->Stop();
+    EXPECT_CALL(callback_, OnFilterCallback());
+    EXPECT_CALL(callback_, OnCallbackDestroyed());
+    renderer_->Stop(callback_.NewCallback());
   }
 
  protected:
diff --git a/webkit/glue/media/buffered_data_source.cc b/webkit/glue/media/buffered_data_source.cc
index bf91da0..64e09b4 100644
--- a/webkit/glue/media/buffered_data_source.cc
+++ b/webkit/glue/media/buffered_data_source.cc
@@ -592,13 +592,17 @@ void BufferedDataSource::Initialize(const std::string& url,
       NewRunnableMethod(this, &BufferedDataSource::InitializeTask));
 }
 
-void BufferedDataSource::Stop() {
+void BufferedDataSource::Stop(media::FilterCallback* callback) {
   {
     AutoLock auto_lock(lock_);
     stop_signal_received_ = true;
   }
+  if (callback) {
+    callback->Run();
+    delete callback;
+  }
   render_loop_->PostTask(FROM_HERE,
-      NewRunnableMethod(this, &BufferedDataSource::StopTask));
+      NewRunnableMethod(this, &BufferedDataSource::CleanupTask));
 }
 
 /////////////////////////////////////////////////////////////////////////////
@@ -664,7 +668,7 @@ void BufferedDataSource::ReadTask(
      media::DataSource::ReadCallback* read_callback) {
   DCHECK(MessageLoop::current() == render_loop_);
 
-  // If StopTask() was executed we should return immediately. We check this
+  // If CleanupTask() was executed we should return immediately. We check this
   // variable to prevent doing any actual work after clean up was done. We do
   // not check |stop_signal_received_| because anything use of it has to be
   // within |lock_| which is not desirable.
@@ -686,7 +690,7 @@ void BufferedDataSource::ReadTask(
   ReadInternal();
 }
 
-void BufferedDataSource::StopTask() {
+void BufferedDataSource::CleanupTask() {
   DCHECK(MessageLoop::current() == render_loop_);
   DCHECK(!stopped_on_render_loop_);
 
@@ -712,11 +716,11 @@ void BufferedDataSource::StopTask() {
 void BufferedDataSource::RestartLoadingTask() {
   DCHECK(MessageLoop::current() == render_loop_);
 
-  // This variable is set in StopTask(). We check this and do an early return.
-  // The sequence of actions which enable this conditions is:
+  // This variable is set in CleanupTask(). We check this and do an early
+  // return. The sequence of actions which enable this conditions is:
   // 1. Stop() is called from the pipeline.
   // 2. ReadCallback() is called from the resource loader.
-  // 3. StopTask() is executed.
+  // 3. CleanupTask() is executed.
   // 4. RestartLoadingTask() is executed.
   if (stopped_on_render_loop_)
     return;
diff --git a/webkit/glue/media/buffered_data_source.h b/webkit/glue/media/buffered_data_source.h
index 74813a9..4440f2f 100644
--- a/webkit/glue/media/buffered_data_source.h
+++ b/webkit/glue/media/buffered_data_source.h
@@ -230,7 +230,7 @@ class BufferedDataSource : public media::DataSource {
   // media::MediaFilter implementation.
   virtual void Initialize(const std::string& url,
                           media::FilterCallback* callback);
-  virtual void Stop();
+  virtual void Stop(media::FilterCallback* callback);
 
   // media::DataSource implementation.
   // Called from demuxer thread.
@@ -275,15 +275,17 @@ class BufferedDataSource : public media::DataSource {
   void ReadTask(int64 position, int read_size, uint8* read_buffer,
                 media::DataSource::ReadCallback* read_callback);
 
-  // Task posted when Stop() is called.
-  void StopTask();
+  // Task posted when Stop() is called. Stops |watch_dog_timer_| and
+  // |loader_|, reset Read() variables, and set |stopped_on_render_loop_|
+  // to signal any remaining tasks to stop.
+  void CleanupTask();
 
   // Restart resource loading on render thread.
   void RestartLoadingTask();
 
   // This task monitors the current active read request. If the current read
   // request has timed out, this task will destroy the current loader and
-  // creates a new one to accomodate the read request.
+  // creates a new one to accommodate the read request.
   void WatchDogTask();
 
   // The method that performs actual read. This method can only be executed on
@@ -381,7 +383,7 @@ class BufferedDataSource : public media::DataSource {
   // thread and read from the render thread.
   bool stop_signal_received_;
 
-  // This variable is set by StopTask() that indicates this object is stopped
+  // This variable is set by CleanupTask() that indicates this object is stopped
   // on the render thread.
   bool stopped_on_render_loop_;
 
diff --git a/webkit/glue/media/buffered_data_source_unittest.cc b/webkit/glue/media/buffered_data_source_unittest.cc
index 13e40f9..14e125c 100644
--- a/webkit/glue/media/buffered_data_source_unittest.cc
+++ b/webkit/glue/media/buffered_data_source_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -534,7 +534,10 @@ class BufferedDataSourceTest : public testing::Test {
       EXPECT_CALL(*loader_, Stop());
     }
 
-    data_source_->Stop();
+    StrictMock<media::MockFilterCallback> callback;
+    EXPECT_CALL(callback, OnFilterCallback());
+    EXPECT_CALL(callback, OnCallbackDestroyed());
+    data_source_->Stop(callback.NewCallback());
     message_loop_->RunAllPending();
   }
 
diff --git a/webkit/glue/media/simple_data_source.cc b/webkit/glue/media/simple_data_source.cc
index 57b294a..20bf0af 100644
--- a/webkit/glue/media/simple_data_source.cc
+++ b/webkit/glue/media/simple_data_source.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -56,9 +56,13 @@ SimpleDataSource::~SimpleDataSource() {
   DCHECK(state_ == UNINITIALIZED || state_ == STOPPED);
 }
 
-void SimpleDataSource::Stop() {
+void SimpleDataSource::Stop(media::FilterCallback* callback) {
   AutoLock auto_lock(lock_);
   state_ = STOPPED;
+  if (callback) {
+    callback->Run();
+    delete callback;
+  }
 
   // Post a task to the render thread to cancel loading the resource.
   render_loop_->PostTask(FROM_HERE,
diff --git a/webkit/glue/media/simple_data_source.h b/webkit/glue/media/simple_data_source.h
index 10bbb7d..577d973 100644
--- a/webkit/glue/media/simple_data_source.h
+++ b/webkit/glue/media/simple_data_source.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -39,7 +39,7 @@ class SimpleDataSource : public media::DataSource,
       const media::MediaFormat& media_format);
 
   // MediaFilter implementation.
-  virtual void Stop();
+  virtual void Stop(media::FilterCallback* callback);
 
   // DataSource implementation.
   virtual void Initialize(const std::string& url,
diff --git a/webkit/glue/media/simple_data_source_unittest.cc b/webkit/glue/media/simple_data_source_unittest.cc
index c43cbc9..e6acba9 100644
--- a/webkit/glue/media/simple_data_source_unittest.cc
+++ b/webkit/glue/media/simple_data_source_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -132,7 +132,10 @@ class SimpleDataSourceTest : public testing::Test {
     EXPECT_CALL(*bridge_factory_, OnDestroy())
         .WillOnce(Invoke(this, &SimpleDataSourceTest::ReleaseBridgeFactory));
 
-    data_source_->Stop();
+    StrictMock<media::MockFilterCallback> callback;
+    EXPECT_CALL(callback, OnFilterCallback());
+    EXPECT_CALL(callback, OnCallbackDestroyed());
+    data_source_->Stop(callback.NewCallback());
     MessageLoop::current()->RunAllPending();
 
     data_source_ = NULL;
diff --git a/webkit/glue/webmediaplayer_impl.cc b/webkit/glue/webmediaplayer_impl.cc
index 66cf896..d6f24ed 100644
--- a/webkit/glue/webmediaplayer_impl.cc
+++ b/webkit/glue/webmediaplayer_impl.cc
@@ -193,7 +193,8 @@ WebMediaPlayerImpl::WebMediaPlayerImpl(WebKit::WebMediaPlayerClient* client,
       pipeline_thread_("PipelineThread"),
       paused_(true),
       playback_rate_(0.0f),
-      client_(client) {
+      client_(client),
+      pipeline_stopped_(false, false) {
   // Saves the current message loop.
   DCHECK(!main_loop_);
   main_loop_ = MessageLoop::current();
@@ -691,8 +692,10 @@ void WebMediaPlayerImpl::Destroy() {
   DCHECK(MessageLoop::current() == main_loop_);
 
   // Make sure to kill the pipeline so there's no more media threads running.
-  // TODO(hclam): stopping the pipeline might block for a long time.
-  pipeline_->Stop(NULL);
+  // Note: stopping the pipeline might block for a long time.
+  pipeline_->Stop(NewCallback(this,
+      &WebMediaPlayerImpl::PipelineStoppedCallback));
+  pipeline_stopped_.Wait();
   pipeline_thread_.Stop();
 
   // And then detach the proxy, it may live on the render thread for a little
@@ -703,6 +706,10 @@ void WebMediaPlayerImpl::Destroy() {
   }
 }
 
+void WebMediaPlayerImpl::PipelineStoppedCallback() {
+  pipeline_stopped_.Signal();
+}
+
 WebKit::WebMediaPlayerClient* WebMediaPlayerImpl::GetClient() {
   DCHECK(MessageLoop::current() == main_loop_);
   DCHECK(client_);
diff --git a/webkit/glue/webmediaplayer_impl.h b/webkit/glue/webmediaplayer_impl.h
index 3bce689..f561866e 100644
--- a/webkit/glue/webmediaplayer_impl.h
+++ b/webkit/glue/webmediaplayer_impl.h
@@ -58,6 +58,7 @@
 #include "base/lock.h"
 #include "base/message_loop.h"
 #include "base/ref_counted.h"
+#include "base/waitable_event.h"
 #include "gfx/rect.h"
 #include "gfx/size.h"
 #include "media/base/filters.h"
@@ -259,6 +260,10 @@ class WebMediaPlayerImpl : public WebKit::WebMediaPlayer,
   // Destroy resources held.
   void Destroy();
 
+  // Callback executed after |pipeline_| stops which signals Destroy()
+  // to continue.
+  void PipelineStoppedCallback();
+
   // Getter method to |client_|.
   WebKit::WebMediaPlayerClient* GetClient();
 
@@ -300,6 +305,9 @@ class WebMediaPlayerImpl : public WebKit::WebMediaPlayer,
 
   scoped_refptr<Proxy> proxy_;
 
+  // Used to block Destroy() until Pipeline::Stop() is completed.
+  base::WaitableEvent pipeline_stopped_;
+
 #if WEBKIT_USING_CG
   scoped_ptr<skia::PlatformCanvas> skia_canvas_;
 #endif