Fix VideoRendererBase to allow the decoder to be flushed with a pending read.

media/filters/video_renderer_base.cc

Index: media/filters/video_renderer_base.cc
diff --git a/media/filters/video_renderer_base.cc b/media/filters/video_renderer_base.cc
index 30b299593b1c956b2ce54d1c86abd18d02114572..edfca94899ebcb0c2bbeb47ab868e6d1a20c77aa 100644
--- a/media/filters/video_renderer_base.cc
+++ b/media/filters/video_renderer_base.cc
@@ -54,9 +54,12 @@ void VideoRendererBase::Flush(const base::Closure& callback) {
   base::AutoLock auto_lock(lock_);
   DCHECK_EQ(state_, kPaused);
   flush_cb_ = callback;
-  state_ = kFlushing;
+  state_ = kFlushingDecoder;
 
-  AttemptFlush_Locked();
+  // We must unlock here because the callback might run within the Flush()
+  // call.

[scherkus (not reviewing), 2012/04/25 03:33:58]

should we have semantics similar to VideoDecoder::Read() that the cb must be
executed on a separate call stack?

[acolwell GONE FROM CHROMIUM, 2012/04/25 17:41:44]

Yes. I think that is a good idea. I created a bug for this work
(http://crbug.com/125020)

+  base::AutoUnlock auto_unlock(lock_);
+  decoder_->Flush(base::Bind(&VideoRendererBase::OnDecoderFlushDone, this));
 }
 
 void VideoRendererBase::Stop(const base::Closure& callback) {
@@ -342,9 +345,15 @@ void VideoRendererBase::PutCurrentFrame(scoped_refptr<VideoFrame> frame) {
   // frame is timed-out. We will wake up our main thread to advance the current
   // frame when this is true.
   frame_available_.Signal();
+  if (state_ == kFlushingDecoder)
+    return;
+
   if (state_ == kFlushing) {
     AttemptFlush_Locked();
-  } else if (state_ == kError || state_ == kStopped) {
+    return;
+  }
+
+  if (state_ == kError || state_ == kStopped) {
     DoStopOrError_Locked();
   }
 }
@@ -358,7 +367,8 @@ void VideoRendererBase::FrameReady(scoped_refptr<VideoFrame> frame) {
 
   // Already-queued Decoder ReadCB's can fire after various state transitions
   // have happened; in that case just drop those frames immediately.
-  if (state_ == kStopped || state_ == kError || state_ == kFlushed)
+  if (state_ == kStopped || state_ == kError || state_ == kFlushed ||
+      state_ == kFlushingDecoder)
     return;
 
   if (state_ == kFlushing) {
@@ -446,14 +456,25 @@ void VideoRendererBase::AttemptRead_Locked() {
 
   if (pending_read_ ||
       NumFrames_Locked() == limits::kMaxVideoFrames ||
-      (!ready_frames_.empty() && ready_frames_.back()->IsEndOfStream())) {
-    return;
-  }
+      (!ready_frames_.empty() && ready_frames_.back()->IsEndOfStream()) ||
+      state_ == kFlushingDecoder ||
+      state_ == kFlushing) {
+     return;
+   }
 
   pending_read_ = true;
   decoder_->Read(base::Bind(&VideoRendererBase::FrameReady, this));
 }
 
+void VideoRendererBase::OnDecoderFlushDone() {
+  base::AutoLock auto_lock(lock_);
+  DCHECK_EQ(kFlushingDecoder, state_);
+  DCHECK(!pending_read_);
+
+  state_ = kFlushing;
+  AttemptFlush_Locked();
+}
+
 void VideoRendererBase::AttemptFlush_Locked() {
   lock_.AssertAcquired();
   DCHECK_EQ(kFlushing, state_);
@@ -464,10 +485,7 @@ void VideoRendererBase::AttemptFlush_Locked() {
   if (!pending_paint_ && !pending_read_) {
     state_ = kFlushed;
     current_frame_ = NULL;
-
-    base::Closure flush_cb = flush_cb_;
-    flush_cb_.Reset();
-    decoder_->Flush(flush_cb);
+    base::ResetAndReturn(&flush_cb_).Run();
   }
 }