Defer executing FFmpegVideoDecoder::Stop() callback until pending read has completed.

Previously CompositeFilter would protect against this case as it would defer calling Stop() on clients until the previous operation completed.

BUG=138583
TEST=media_unittests, stop seeing crashes in layout tests


Review URL: https://chromiumcodereview.appspot.com/10818018

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

3 files changed, 45 insertions, 1 deletions

diff --git a/media/filters/ffmpeg_video_decoder.cc b/media/filters/ffmpeg_video_decoder.cc
index 09cba03..7cd2c7a 100644
--- a/media/filters/ffmpeg_video_decoder.cc
+++ b/media/filters/ffmpeg_video_decoder.cc
@@ -233,9 +233,19 @@ void FFmpegVideoDecoder::Stop(const base::Closure& closure) {
     return;
   }
 
+  stop_cb_ = closure;
+
+  // Defer stopping if a read is pending.
+  if (!read_cb_.is_null())
+    return;
+
+  DoStop();
+}
+
+void FFmpegVideoDecoder::DoStop() {
   ReleaseFFmpegResources();
   state_ = kUninitialized;
-  closure.Run();
+  base::ResetAndReturn(&stop_cb_).Run();
 }
 
 const gfx::Size& FFmpegVideoDecoder::natural_size() {
@@ -299,6 +309,12 @@ void FFmpegVideoDecoder::DoDecryptOrDecodeBuffer(
   DCHECK_NE(state_, kDecodeFinished);
   DCHECK(!read_cb_.is_null());
 
+  if (!stop_cb_.is_null()) {
+    base::ResetAndReturn(&read_cb_).Run(kOk, NULL);
+    DoStop();
+    return;
+  }
+
   if (!reset_cb_.is_null()) {
     base::ResetAndReturn(&read_cb_).Run(kOk, NULL);
     DoReset();
diff --git a/media/filters/ffmpeg_video_decoder.h b/media/filters/ffmpeg_video_decoder.h
index 0ba475d..4d26380 100644
--- a/media/filters/ffmpeg_video_decoder.h
+++ b/media/filters/ffmpeg_video_decoder.h
@@ -87,6 +87,9 @@ class MEDIA_EXPORT FFmpegVideoDecoder : public VideoDecoder {
   // Reset decoder and call |reset_cb_|.
   void DoReset();
 
+  // Free decoder resources and call |stop_cb_|.
+  void DoStop();
+
   // This is !is_null() iff Initialize() hasn't been called.
   base::Callback<MessageLoop*()> message_loop_factory_cb_;
 
@@ -98,6 +101,7 @@ class MEDIA_EXPORT FFmpegVideoDecoder : public VideoDecoder {
 
   ReadCB read_cb_;
   base::Closure reset_cb_;
+  base::Closure stop_cb_;
 
   // FFmpeg structures owned by this object.
   AVCodecContext* codec_context_;
diff --git a/media/filters/ffmpeg_video_decoder_unittest.cc b/media/filters/ffmpeg_video_decoder_unittest.cc
index fb1b0c5..6a7fb4f 100644
--- a/media/filters/ffmpeg_video_decoder_unittest.cc
+++ b/media/filters/ffmpeg_video_decoder_unittest.cc
@@ -516,6 +516,30 @@ TEST_F(FFmpegVideoDecoderTest, Stop_EndOfStream) {
   Stop();
 }
 
+// Test stopping when there is a pending read on the demuxer.
+TEST_F(FFmpegVideoDecoderTest, Stop_DuringPendingRead) {
+  Initialize();
+
+  DemuxerStream::ReadCB read_cb;
+  EXPECT_CALL(*demuxer_, Read(_))
+      .WillOnce(SaveArg<0>(&read_cb));
+
+  decoder_->Read(read_cb_);
+  message_loop_.RunAllPending();
+
+  // Make sure the Read() on the decoder triggers a Read() on
+  // the demuxer.
+  EXPECT_FALSE(read_cb.is_null());
+
+  Stop();
+
+  EXPECT_CALL(*this, FrameReady(VideoDecoder::kOk, IsNull()));
+
+  read_cb.Run(DemuxerStream::kOk, i_frame_buffer_);
+  message_loop_.RunAllPending();
+}
+
+
 // Test aborted read on the demuxer stream.
 TEST_F(FFmpegVideoDecoderTest, AbortPendingRead) {
   Initialize();