Make DataSource::Stop() synchronous.

In a lot of filters in media code, Stop() has been folded into the dtor.
This model doesn't apply directly to DataSource because DataSource::Stop()
is called by the FFmpegDemuxer, but the DataSource is owned by
WebMediaPlayerImpl.

TBR=gbillock@chromium.org
BUG=349211
TEST=All existing tests pass.

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

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

9 files changed, 24 insertions, 42 deletions

diff --git a/media/base/data_source.h b/media/base/data_source.h
index dca1dd3..e0b7373 100644
--- a/media/base/data_source.h
+++ b/media/base/data_source.h
@@ -28,7 +28,7 @@ class MEDIA_EXPORT DataSource {
 
   // Stops the DataSource. Once this is called all future Read() calls will
   // return an error.
-  virtual void Stop(const base::Closure& callback) = 0;
+  virtual void Stop() = 0;
 
   // Returns true and the file size, false if the file size could not be
   // retrieved.
diff --git a/media/filters/blocking_url_protocol_unittest.cc b/media/filters/blocking_url_protocol_unittest.cc
index d8d1dfc3..ec55a00 100644
--- a/media/filters/blocking_url_protocol_unittest.cc
+++ b/media/filters/blocking_url_protocol_unittest.cc
@@ -24,10 +24,7 @@ class BlockingUrlProtocolTest : public testing::Test {
   }
 
   virtual ~BlockingUrlProtocolTest() {
-    base::WaitableEvent stop_event(false, false);
-    data_source_.Stop(base::Bind(
-        &base::WaitableEvent::Signal, base::Unretained(&stop_event)));
-    stop_event.Wait();
+    data_source_.Stop();
   }
 
   MOCK_METHOD0(OnDataSourceError, void());
diff --git a/media/filters/ffmpeg_demuxer.cc b/media/filters/ffmpeg_demuxer.cc
index a2264dc..c89c34d 100644
--- a/media/filters/ffmpeg_demuxer.cc
+++ b/media/filters/ffmpeg_demuxer.cc
@@ -530,11 +530,22 @@ FFmpegDemuxer::~FFmpegDemuxer() {}
 void FFmpegDemuxer::Stop(const base::Closure& callback) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   url_protocol_->Abort();
-  data_source_->Stop(
-      BindToCurrentLoop(base::Bind(&FFmpegDemuxer::OnDataSourceStopped,
-                                   weak_factory_.GetWeakPtr(),
-                                   BindToCurrentLoop(callback))));
+  data_source_->Stop();
+
+  // This will block until all tasks complete. Note that after this returns it's
+  // possible for reply tasks (e.g., OnReadFrameDone()) to be queued on this
+  // thread. Each of the reply task methods must check whether we've stopped the
+  // thread and drop their results on the floor.
+  blocking_thread_.Stop();
+
+  StreamVector::iterator iter;
+  for (iter = streams_.begin(); iter != streams_.end(); ++iter) {
+    if (*iter)
+      (*iter)->Stop();
+  }
+
   data_source_ = NULL;
+  task_runner_->PostTask(FROM_HERE, callback);
 }
 
 void FFmpegDemuxer::Seek(base::TimeDelta time, const PipelineStatusCB& cb) {
@@ -1120,23 +1131,6 @@ void FFmpegDemuxer::OnReadFrameDone(ScopedAVPacket packet, int result) {
   ReadFrameIfNeeded();
 }
 
-void FFmpegDemuxer::OnDataSourceStopped(const base::Closure& callback) {
-  // This will block until all tasks complete. Note that after this returns it's
-  // possible for reply tasks (e.g., OnReadFrameDone()) to be queued on this
-  // thread. Each of the reply task methods must check whether we've stopped the
-  // thread and drop their results on the floor.
-  DCHECK(task_runner_->BelongsToCurrentThread());
-  blocking_thread_.Stop();
-
-  StreamVector::iterator iter;
-  for (iter = streams_.begin(); iter != streams_.end(); ++iter) {
-    if (*iter)
-      (*iter)->Stop();
-  }
-
-  callback.Run();
-}
-
 bool FFmpegDemuxer::StreamsHaveAvailableCapacity() {
   DCHECK(task_runner_->BelongsToCurrentThread());
   StreamVector::iterator iter;
diff --git a/media/filters/ffmpeg_demuxer.h b/media/filters/ffmpeg_demuxer.h
index 3df575f..5d175a7 100644
--- a/media/filters/ffmpeg_demuxer.h
+++ b/media/filters/ffmpeg_demuxer.h
@@ -197,9 +197,6 @@ class MEDIA_EXPORT FFmpegDemuxer : public Demuxer {
   void ReadFrameIfNeeded();
   void OnReadFrameDone(ScopedAVPacket packet, int result);
 
-  // DataSource callbacks during stopping.
-  void OnDataSourceStopped(const base::Closure& callback);
-
   // Returns true iff any stream has additional capacity. Note that streams can
   // go over capacity depending on how the file is muxed.
   bool StreamsHaveAvailableCapacity();
diff --git a/media/filters/file_data_source.cc b/media/filters/file_data_source.cc
index e8b3292..5aad3f9 100644
--- a/media/filters/file_data_source.cc
+++ b/media/filters/file_data_source.cc
@@ -26,8 +26,7 @@ bool FileDataSource::Initialize(const base::FilePath& file_path) {
   return file_.Initialize(file_path);
 }
 
-void FileDataSource::Stop(const base::Closure& callback) {
-  callback.Run();
+void FileDataSource::Stop() {
 }
 
 void FileDataSource::Read(int64 position, int size, uint8* data,
diff --git a/media/filters/file_data_source.h b/media/filters/file_data_source.h
index 739bc2e..78884ea 100644
--- a/media/filters/file_data_source.h
+++ b/media/filters/file_data_source.h
@@ -25,7 +25,7 @@ class MEDIA_EXPORT FileDataSource : public DataSource {
   bool Initialize(const base::FilePath& file_path);
 
   // Implementation of DataSource.
-  virtual void Stop(const base::Closure& callback) OVERRIDE;
+  virtual void Stop() OVERRIDE;
   virtual void Read(int64 position, int size, uint8* data,
                     const DataSource::ReadCB& read_cb) OVERRIDE;
   virtual bool GetSize(int64* size_out) OVERRIDE;
diff --git a/media/filters/file_data_source_unittest.cc b/media/filters/file_data_source_unittest.cc
index 5eb94ca..0d57ae7 100644
--- a/media/filters/file_data_source_unittest.cc
+++ b/media/filters/file_data_source_unittest.cc
@@ -77,7 +77,7 @@ TEST(FileDataSourceTest, ReadData) {
       &ReadCBHandler::ReadCB, base::Unretained(&handler)));
   EXPECT_EQ('5', ten_bytes[0]);
 
-  data_source.Stop(NewExpectedClosure());
+  data_source.Stop();
 }
 
 }  // namespace media
diff --git a/media/tools/player_x11/data_source_logger.cc b/media/tools/player_x11/data_source_logger.cc
index 204d8b3..d09b6bf 100644
--- a/media/tools/player_x11/data_source_logger.cc
+++ b/media/tools/player_x11/data_source_logger.cc
@@ -6,11 +6,6 @@
 #include "base/logging.h"
 #include "media/tools/player_x11/data_source_logger.h"
 
-static void LogAndRunStopClosure(const base::Closure& closure) {
-  VLOG(1) << "Stop() finished";
-  closure.Run();
-}
-
 static void LogAndRunReadCB(
     int64 position, int size,
     const media::DataSource::ReadCB& read_cb, int result) {
@@ -25,9 +20,9 @@ DataSourceLogger::DataSourceLogger(
       streaming_(streaming) {
 }
 
-void DataSourceLogger::Stop(const base::Closure& closure) {
-  VLOG(1) << "Stop() started";
-  data_source_->Stop(base::Bind(&LogAndRunStopClosure, closure));
+void DataSourceLogger::Stop() {
+  VLOG(1) << "Stop()";
+  data_source_->Stop();
 }
 
 void DataSourceLogger::Read(
diff --git a/media/tools/player_x11/data_source_logger.h b/media/tools/player_x11/data_source_logger.h
index 5fdd9d4..1cb0a80 100644
--- a/media/tools/player_x11/data_source_logger.h
+++ b/media/tools/player_x11/data_source_logger.h
@@ -22,7 +22,7 @@ class DataSourceLogger : public media::DataSource {
   virtual ~DataSourceLogger();
 
   // media::DataSource implementation.
-  virtual void Stop(const base::Closure& closure) OVERRIDE;
+  virtual void Stop() OVERRIDE;
   virtual void Read(
       int64 position, int size, uint8* data,
       const media::DataSource::ReadCB& read_cb) OVERRIDE;