Make calling FFmpegDemuxer::Stop() multiple times illegal.

First attempt r218110 was reverted in r218333 for two reasons:
1) There were still cases where Pipeline could call Stop() twice aside from test code, causing a null pointer dereference of data_source_
2) FFmpegDemuxer::OnFindStreamInfoDone() could run inbetween Stop() and OnDataSourceStopped(), causing a null pointer dereference of data_source_

The fix for (1) landed in r218625. The fix for (2) is included in this change.

BUG=235933

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

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

3 files changed, 11 insertions, 9 deletions

diff --git a/media/base/demuxer.h b/media/base/demuxer.h
index 6a91aab..81ac33b 100644
--- a/media/base/demuxer.h
+++ b/media/base/demuxer.h
@@ -47,8 +47,10 @@ class MEDIA_EXPORT Demuxer {
   // callback upon completion.
   virtual void Seek(base::TimeDelta time, const PipelineStatusCB& status_cb);
 
-  // The pipeline is being stopped either as a result of an error or because
-  // the client called Stop().
+  // Starts stopping this demuxer, executing the callback upon completion.
+  //
+  // After the callback completes the demuxer may be destroyed. It is illegal to
+  // call any method (including Stop()) after a demuxer has stopped.
   virtual void Stop(const base::Closure& callback);
 
   // This method is called from the pipeline when the audio renderer
diff --git a/media/filters/ffmpeg_demuxer.cc b/media/filters/ffmpeg_demuxer.cc
index 14b1fff..ddd3e80 100644
--- a/media/filters/ffmpeg_demuxer.cc
+++ b/media/filters/ffmpeg_demuxer.cc
@@ -316,12 +316,7 @@ void FFmpegDemuxer::Stop(const base::Closure& callback) {
   data_source_->Stop(BindToCurrentLoop(base::Bind(
       &FFmpegDemuxer::OnDataSourceStopped, weak_this_,
       BindToCurrentLoop(callback))));
-
-  // TODO(scherkus): Reenable after figuring why Stop() gets called multiple
-  // times, see http://crbug.com/235933
-#if 0
   data_source_ = NULL;
-#endif
 }
 
 void FFmpegDemuxer::Seek(base::TimeDelta time, const PipelineStatusCB& cb) {
@@ -476,7 +471,7 @@ void FFmpegDemuxer::OnOpenContextDone(const PipelineStatusCB& status_cb,
 void FFmpegDemuxer::OnFindStreamInfoDone(const PipelineStatusCB& status_cb,
                                          int result) {
   DCHECK(message_loop_->BelongsToCurrentThread());
-  if (!blocking_thread_.IsRunning()) {
+  if (!blocking_thread_.IsRunning() || !data_source_) {
     status_cb.Run(PIPELINE_ERROR_ABORT);
     return;
   }
diff --git a/media/filters/ffmpeg_demuxer_unittest.cc b/media/filters/ffmpeg_demuxer_unittest.cc
index c1da0cc..4fa2ab2 100644
--- a/media/filters/ffmpeg_demuxer_unittest.cc
+++ b/media/filters/ffmpeg_demuxer_unittest.cc
@@ -433,7 +433,9 @@ TEST_F(FFmpegDemuxerTest, Stop) {
   DemuxerStream* audio = demuxer_->GetStream(DemuxerStream::AUDIO);
   ASSERT_TRUE(audio);
 
-  demuxer_->Stop(NewExpectedClosure());
+  WaitableMessageLoopEvent event;
+  demuxer_->Stop(event.GetClosure());
+  event.RunAndWait();
 
   // Reads after being stopped are all EOS buffers.
   StrictMock<MockReadCB> callback;
@@ -442,6 +444,9 @@ TEST_F(FFmpegDemuxerTest, Stop) {
   // Attempt the read...
   audio->Read(base::Bind(&MockReadCB::Run, base::Unretained(&callback)));
   message_loop_.RunUntilIdle();
+
+  // Don't let the test call Stop() again.
+  demuxer_.reset();
 }
 
 TEST_F(FFmpegDemuxerTest, DisableAudioStream) {