Make Demuxer::Stop() synchronous.

BUG=349211
TEST=All existing tests pass.

Review URL: https://codereview.chromium.org/416393003

Cr-Commit-Position: refs/heads/master@{#292958}

content/renderer/media/android/media_source_delegate.cc

@@ -139,10 +139,14 @@ void MediaSourceDelegate::StopDemuxer() {
   media_weak_factory_.InvalidateWeakPtrs();
   DCHECK(!media_weak_factory_.HasWeakPtrs());
 
-  // The callback OnDemuxerStopDone() owns |this| and will delete it when
-  // called. Hence using base::Unretained(this) is safe here.
-  chunk_demuxer_->Stop(base::Bind(&MediaSourceDelegate::OnDemuxerStopDone,
-                                  base::Unretained(this)));
+  chunk_demuxer_->Stop();
+  chunk_demuxer_.reset();
+
+  // The callback DeleteSelf() owns |this| and will delete it when called.
+  // Hence using base::Unretained(this) is safe here.
+  media_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&MediaSourceDelegate::DeleteSelf, base::Unretained(this)));
 }
 
 void MediaSourceDelegate::InitializeMediaSource(
@@ -626,18 +630,9 @@ void MediaSourceDelegate::FinishResettingDecryptingDemuxerStreams() {
   demuxer_client_->DemuxerSeekDone(demuxer_client_id_, browser_seek_time_);
 }
 
-void MediaSourceDelegate::OnDemuxerStopDone() {
-  DCHECK(media_task_runner_->BelongsToCurrentThread());
-  DVLOG(1) << __FUNCTION__ << " : " << demuxer_client_id_;
-  main_task_runner_->PostTask(
-      FROM_HERE,
-      base::Bind(&MediaSourceDelegate::DeleteSelf, base::Unretained(this)));
-}
-
 void MediaSourceDelegate::DeleteSelf() {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
   DVLOG(1) << __FUNCTION__ << " : " << demuxer_client_id_;
-  chunk_demuxer_.reset();
   delete this;
 }
 

content/renderer/media/android/media_source_delegate.h

@@ -146,9 +146,7 @@ class MediaSourceDelegate : public media::DemuxerHost {
   void ResetVideoDecryptingDemuxerStream();
   void FinishResettingDecryptingDemuxerStreams();
 
-  // Callback for ChunkDemuxer::Stop() and helper for deleting |this| on the
-  // main thread.
-  void OnDemuxerStopDone();
+  // Helper for deleting |this| on the main thread.
   void DeleteSelf();
 
   void OnDemuxerOpened();

media/base/demuxer.h

@@ -72,11 +72,11 @@ class MEDIA_EXPORT Demuxer {
   virtual void Seek(base::TimeDelta time,
                     const PipelineStatusCB& status_cb) = 0;
 
-  // Starts stopping this demuxer, executing the callback upon completion.
+  // Stops this demuxer.
   //
-  // 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) = 0;
+  // After this call the demuxer may be destroyed. It is illegal to call any
+  // method (including Stop()) after a demuxer has stopped.
+  virtual void Stop() = 0;
 
   // Returns the first stream of the given stream type (which is not allowed
   // to be DemuxerStream::TEXT), or NULL if that type of stream is not present.

media/base/demuxer_perftest.cc

@@ -38,7 +38,7 @@ class DemuxerHostImpl : public media::DemuxerHost {
 };
 
 static void QuitLoopWithStatus(base::MessageLoop* message_loop,
-                        media::PipelineStatus status) {
+                               media::PipelineStatus status) {
   CHECK_EQ(status, media::PIPELINE_OK);
   message_loop->PostTask(FROM_HERE, base::MessageLoop::QuitWhenIdleClosure());
 }
@@ -194,8 +194,8 @@ static void RunDemuxerBenchmark(const std::string& filename) {
     }
     base::TimeTicks end = base::TimeTicks::HighResNow();
     total_time += (end - start).InSecondsF();
-    demuxer.Stop(base::Bind(
-        &QuitLoopWithStatus, &message_loop, PIPELINE_OK));
+    demuxer.Stop();
+    QuitLoopWithStatus(&message_loop, PIPELINE_OK);
     message_loop.Run();
   }
 

media/base/mock_filters.h

@@ -36,7 +36,7 @@ class MockDemuxer : public Demuxer {
                void(DemuxerHost* host, const PipelineStatusCB& cb, bool));
   MOCK_METHOD1(SetPlaybackRate, void(float playback_rate));
   MOCK_METHOD2(Seek, void(base::TimeDelta time, const PipelineStatusCB& cb));
-  MOCK_METHOD1(Stop, void(const base::Closure& callback));
+  MOCK_METHOD0(Stop, void());
   MOCK_METHOD0(OnAudioRendererDisabled, void());
   MOCK_METHOD1(GetStream, DemuxerStream*(DemuxerStream::Type));
   MOCK_CONST_METHOD0(GetTimelineOffset, base::Time());

media/base/pipeline.cc

@@ -410,8 +410,8 @@ void Pipeline::DoStop(const PipelineStatusCB& done_cb) {
   text_renderer_.reset();
 
   if (demuxer_) {
-    demuxer_->Stop(base::Bind(done_cb, PIPELINE_OK));
-    return;
+    demuxer_->Stop();
+    demuxer_ = NULL;
   }
 
   task_runner_->PostTask(FROM_HERE, base::Bind(done_cb, PIPELINE_OK));

media/base/pipeline_unittest.cc

@@ -276,7 +276,7 @@ class PipelineTest : public ::testing::Test {
 
   void ExpectDemuxerStop() {
     if (demuxer_)
-      EXPECT_CALL(*demuxer_, Stop(_)).WillOnce(RunClosure<0>());
+      EXPECT_CALL(*demuxer_, Stop());
   }
 
   void ExpectPipelineStopAndDestroyPipeline() {
@@ -371,8 +371,7 @@ TEST_F(PipelineTest, StopWithoutStart) {
 TEST_F(PipelineTest, StartThenStopImmediately) {
   EXPECT_CALL(*demuxer_, Initialize(_, _, _))
       .WillOnce(RunCallback<1>(PIPELINE_OK));
-  EXPECT_CALL(*demuxer_, Stop(_))
-      .WillOnce(RunClosure<0>());
+  EXPECT_CALL(*demuxer_, Stop());
 
   EXPECT_CALL(callbacks_, OnStart(_));
   StartPipeline();
@@ -394,9 +393,8 @@ TEST_F(PipelineTest, DemuxerErrorDuringStop) {
 
   StartPipelineAndExpect(PIPELINE_OK);
 
-  EXPECT_CALL(*demuxer_, Stop(_))
-      .WillOnce(DoAll(InvokeWithoutArgs(this, &PipelineTest::OnDemuxerError),
-                      RunClosure<0>()));
+  EXPECT_CALL(*demuxer_, Stop())
+      .WillOnce(InvokeWithoutArgs(this, &PipelineTest::OnDemuxerError));
   ExpectPipelineStopAndDestroyPipeline();
 
   pipeline_->Stop(
@@ -407,8 +405,7 @@ TEST_F(PipelineTest, DemuxerErrorDuringStop) {
 TEST_F(PipelineTest, URLNotFound) {
   EXPECT_CALL(*demuxer_, Initialize(_, _, _))
       .WillOnce(RunCallback<1>(PIPELINE_ERROR_URL_NOT_FOUND));
-  EXPECT_CALL(*demuxer_, Stop(_))
-      .WillOnce(RunClosure<0>());
+  EXPECT_CALL(*demuxer_, Stop());
 
   StartPipelineAndExpect(PIPELINE_ERROR_URL_NOT_FOUND);
 }
@@ -416,8 +413,7 @@ TEST_F(PipelineTest, URLNotFound) {
 TEST_F(PipelineTest, NoStreams) {
   EXPECT_CALL(*demuxer_, Initialize(_, _, _))
       .WillOnce(RunCallback<1>(PIPELINE_OK));
-  EXPECT_CALL(*demuxer_, Stop(_))
-      .WillOnce(RunClosure<0>());
+  EXPECT_CALL(*demuxer_, Stop());
 
   StartPipelineAndExpect(PIPELINE_ERROR_COULD_NOT_RENDER);
 }
@@ -528,8 +524,7 @@ TEST_F(PipelineTest, SeekAfterError) {
   // Initialize then seek!
   StartPipelineAndExpect(PIPELINE_OK);
 
-  EXPECT_CALL(*demuxer_, Stop(_))
-      .WillOnce(RunClosure<0>());
+  EXPECT_CALL(*demuxer_, Stop());
   EXPECT_CALL(callbacks_, OnError(_));
 
   static_cast<DemuxerHost*>(pipeline_.get())
@@ -649,8 +644,7 @@ TEST_F(PipelineTest, ErrorDuringSeek) {
 
   EXPECT_CALL(*demuxer_, Seek(seek_time, _))
       .WillOnce(RunCallback<1>(PIPELINE_ERROR_READ));
-  EXPECT_CALL(*demuxer_, Stop(_))
-      .WillOnce(RunClosure<0>());
+  EXPECT_CALL(*demuxer_, Stop());
 
   pipeline_->Seek(seek_time, base::Bind(&CallbackHelper::OnSeek,
                                         base::Unretained(&callbacks_)));
@@ -703,8 +697,7 @@ TEST_F(PipelineTest, NoMessageDuringTearDownFromError) {
 
   EXPECT_CALL(*demuxer_, Seek(seek_time, _))
       .WillOnce(RunCallback<1>(PIPELINE_ERROR_READ));
-  EXPECT_CALL(*demuxer_, Stop(_))
-      .WillOnce(RunClosure<0>());
+  EXPECT_CALL(*demuxer_, Stop());
 
   pipeline_->Seek(seek_time, base::Bind(&CallbackHelper::OnSeek,
                                         base::Unretained(&callbacks_)));
@@ -819,7 +812,7 @@ class PipelineTeardownTest : public PipelineTest {
             .WillOnce(RunCallback<1>(status));
       }
 
-      EXPECT_CALL(*demuxer_, Stop(_)).WillOnce(RunClosure<0>());
+      EXPECT_CALL(*demuxer_, Stop());
       return status;
     }
 
@@ -845,7 +838,7 @@ class PipelineTeardownTest : public PipelineTest {
             .WillOnce(RunCallback<0>(status));
       }
 
-      EXPECT_CALL(*demuxer_, Stop(_)).WillOnce(RunClosure<0>());
+      EXPECT_CALL(*demuxer_, Stop());
       return status;
     }
 
@@ -872,7 +865,7 @@ class PipelineTeardownTest : public PipelineTest {
     InSequence s;
     PipelineStatus status = SetSeekExpectations(state, stop_or_error);
 
-    EXPECT_CALL(*demuxer_, Stop(_)).WillOnce(RunClosure<0>());
+    EXPECT_CALL(*demuxer_, Stop());
     EXPECT_CALL(callbacks_, OnSeek(status));
 
     if (status == PIPELINE_OK) {
@@ -938,7 +931,7 @@ class PipelineTeardownTest : public PipelineTest {
   void DoStopOrError(StopOrError stop_or_error) {
     InSequence s;
 
-    EXPECT_CALL(*demuxer_, Stop(_)).WillOnce(RunClosure<0>());
+    EXPECT_CALL(*demuxer_, Stop());
 
     switch (stop_or_error) {
       case kStop:
@@ -956,6 +949,7 @@ class PipelineTeardownTest : public PipelineTest {
         EXPECT_CALL(callbacks_, OnError(PIPELINE_ERROR_READ));
         ExpectPipelineStopAndDestroyPipeline();
         pipeline_->SetErrorForTesting(PIPELINE_ERROR_READ);
+        message_loop_.RunUntilIdle();
         pipeline_->Stop(base::Bind(
             &CallbackHelper::OnStop, base::Unretained(&callbacks_)));
         break;

media/filters/chunk_demuxer.cc

@@ -1057,10 +1057,9 @@ void ChunkDemuxer::Initialize(
   base::ResetAndReturn(&open_cb_).Run();
 }
 
-void ChunkDemuxer::Stop(const base::Closure& callback) {
+void ChunkDemuxer::Stop() {
   DVLOG(1) << "Stop()";
   Shutdown();
-  callback.Run();
 }
 
 void ChunkDemuxer::Seek(TimeDelta time, const PipelineStatusCB& cb) {

media/filters/chunk_demuxer.h

@@ -159,7 +159,7 @@ class MEDIA_EXPORT ChunkDemuxer : public Demuxer {
   virtual void Initialize(DemuxerHost* host,
                           const PipelineStatusCB& cb,
                           bool enable_text_tracks) OVERRIDE;
-  virtual void Stop(const base::Closure& callback) OVERRIDE;
+  virtual void Stop() OVERRIDE;
   virtual void Seek(base::TimeDelta time, const PipelineStatusCB&  cb) OVERRIDE;
   virtual DemuxerStream* GetStream(DemuxerStream::Type type) OVERRIDE;
   virtual base::Time GetTimelineOffset() const OVERRIDE;

media/filters/ffmpeg_demuxer.cc

@@ -555,7 +555,7 @@ FFmpegDemuxer::FFmpegDemuxer(
 
 FFmpegDemuxer::~FFmpegDemuxer() {}
 
-void FFmpegDemuxer::Stop(const base::Closure& callback) {
+void FFmpegDemuxer::Stop() {
   DCHECK(task_runner_->BelongsToCurrentThread());
   url_protocol_->Abort();
   data_source_->Stop();
@@ -573,7 +573,6 @@ void FFmpegDemuxer::Stop(const base::Closure& callback) {
   }
 
   data_source_ = NULL;
-  task_runner_->PostTask(FROM_HERE, callback);
 }
 
 void FFmpegDemuxer::Seek(base::TimeDelta time, const PipelineStatusCB& cb) {

media/filters/ffmpeg_demuxer.h

@@ -164,7 +164,7 @@ class MEDIA_EXPORT FFmpegDemuxer : public Demuxer {
   virtual void Initialize(DemuxerHost* host,
                           const PipelineStatusCB& status_cb,
                           bool enable_text_tracks) OVERRIDE;
-  virtual void Stop(const base::Closure& callback) OVERRIDE;
+  virtual void Stop() OVERRIDE;
   virtual void Seek(base::TimeDelta time, const PipelineStatusCB& cb) OVERRIDE;
   virtual DemuxerStream* GetStream(DemuxerStream::Type type) OVERRIDE;
   virtual base::Time GetTimelineOffset() const OVERRIDE;

media/filters/ffmpeg_demuxer_unittest.cc

@@ -67,11 +67,8 @@ class FFmpegDemuxerTest : public testing::Test {
   FFmpegDemuxerTest() {}
 
   virtual ~FFmpegDemuxerTest() {
-    if (demuxer_) {
-      WaitableMessageLoopEvent event;
-      demuxer_->Stop(event.GetClosure());
-      event.RunAndWait();
-    }
+    if (demuxer_)
+      demuxer_->Stop();
   }
 
   void CreateDemuxer(const std::string& name) {
@@ -753,9 +750,7 @@ TEST_F(FFmpegDemuxerTest, Stop) {
   DemuxerStream* audio = demuxer_->GetStream(DemuxerStream::AUDIO);
   ASSERT_TRUE(audio);
 
-  WaitableMessageLoopEvent event;
-  demuxer_->Stop(event.GetClosure());
-  event.RunAndWait();
+  demuxer_->Stop();
 
   // Reads after being stopped are all EOS buffers.
   StrictMock<MockReadCB> callback;
@@ -914,9 +909,7 @@ TEST_F(FFmpegDemuxerTest, IsValidAnnexB) {
     stream->Read(base::Bind(&ValidateAnnexB, stream));
     message_loop_.Run();
 
-    WaitableMessageLoopEvent event;
-    demuxer_->Stop(event.GetClosure());
-    event.RunAndWait();
+    demuxer_->Stop();
     demuxer_.reset();
     data_source_.reset();
   }