Remove reference counting from media::VideoRenderer and friends.

BUG=173313

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

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

7 files changed, 44 insertions, 42 deletions

diff --git a/media/base/filter_collection.cc b/media/base/filter_collection.cc
index f82a61b..7824b1e 100644
--- a/media/base/filter_collection.cc
+++ b/media/base/filter_collection.cc
@@ -29,15 +29,20 @@ void FilterCollection::AddAudioRenderer(AudioRenderer* audio_renderer) {
   audio_renderers_.push_back(audio_renderer);
 }
 
-void FilterCollection::AddVideoRenderer(VideoRenderer* video_renderer) {
-  video_renderers_.push_back(video_renderer);
+void FilterCollection::SetVideoRenderer(
+    scoped_ptr<VideoRenderer> video_renderer) {
+  video_renderer_ = video_renderer.Pass();
+}
+
+scoped_ptr<VideoRenderer> FilterCollection::GetVideoRenderer() {
+  return video_renderer_.Pass();
 }
 
 void FilterCollection::Clear() {
   audio_decoders_.clear();
   video_decoders_.clear();
   audio_renderers_.clear();
-  video_renderers_.clear();
+  video_renderer_.reset();
 }
 
 void FilterCollection::SelectAudioRenderer(scoped_refptr<AudioRenderer>* out) {
@@ -49,15 +54,6 @@ void FilterCollection::SelectAudioRenderer(scoped_refptr<AudioRenderer>* out) {
   audio_renderers_.pop_front();
 }
 
-void FilterCollection::SelectVideoRenderer(scoped_refptr<VideoRenderer>* out) {
-  if (video_renderers_.empty()) {
-    *out = NULL;
-    return;
-  }
-  *out = video_renderers_.front();
-  video_renderers_.pop_front();
-}
-
 FilterCollection::AudioDecoderList* FilterCollection::GetAudioDecoders() {
   return &audio_decoders_;
 }
diff --git a/media/base/filter_collection.h b/media/base/filter_collection.h
index e9f2be5..100cd64 100644
--- a/media/base/filter_collection.h
+++ b/media/base/filter_collection.h
@@ -8,6 +8,7 @@
 #include <list>
 
 #include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
 #include "media/base/media_export.h"
 
 namespace media {
@@ -36,9 +37,10 @@ class MEDIA_EXPORT FilterCollection {
   const scoped_refptr<Demuxer>& GetDemuxer();
 
   // Adds a filter to the collection.
-  void AddAudioDecoder(AudioDecoder* audio_decoder);
   void AddAudioRenderer(AudioRenderer* audio_renderer);
-  void AddVideoRenderer(VideoRenderer* video_renderer);
+
+  void SetVideoRenderer(scoped_ptr<VideoRenderer> video_renderer);
+  scoped_ptr<VideoRenderer> GetVideoRenderer();
 
   // Remove remaining filters.
   void Clear();
@@ -48,7 +50,6 @@ class MEDIA_EXPORT FilterCollection {
   // If a filter is returned it is removed from the collection.
   // Filters are selected in FIFO order.
   void SelectAudioRenderer(scoped_refptr<AudioRenderer>* out);
-  void SelectVideoRenderer(scoped_refptr<VideoRenderer>* out);
 
   AudioDecoderList* GetAudioDecoders();
   VideoDecoderList* GetVideoDecoders();
@@ -58,7 +59,7 @@ class MEDIA_EXPORT FilterCollection {
   AudioDecoderList audio_decoders_;
   VideoDecoderList video_decoders_;
   std::list<scoped_refptr<AudioRenderer> > audio_renderers_;
-  std::list<scoped_refptr<VideoRenderer> > video_renderers_;
+  scoped_ptr<VideoRenderer> video_renderer_;
 
   DISALLOW_COPY_AND_ASSIGN(FilterCollection);
 };
diff --git a/media/base/mock_filters.h b/media/base/mock_filters.h
index 3050fb5..2050e87 100644
--- a/media/base/mock_filters.h
+++ b/media/base/mock_filters.h
@@ -134,6 +134,7 @@ class MockAudioDecoder : public AudioDecoder {
 class MockVideoRenderer : public VideoRenderer {
  public:
   MockVideoRenderer();
+  virtual ~MockVideoRenderer();
 
   // VideoRenderer implementation.
   MOCK_METHOD10(Initialize, void(const scoped_refptr<DemuxerStream>& stream,
@@ -153,9 +154,6 @@ class MockVideoRenderer : public VideoRenderer {
   MOCK_METHOD1(Stop, void(const base::Closure& callback));
   MOCK_METHOD1(SetPlaybackRate, void(float playback_rate));
 
- protected:
-  virtual ~MockVideoRenderer();
-
  private:
   DISALLOW_COPY_AND_ASSIGN(MockVideoRenderer);
 };
diff --git a/media/base/pipeline.cc b/media/base/pipeline.cc
index cc6419a..cdc0aa6 100644
--- a/media/base/pipeline.cc
+++ b/media/base/pipeline.cc
@@ -538,7 +538,8 @@ void Pipeline::DoInitialPreroll(const PipelineStatusCB& done_cb) {
 
   if (video_renderer_) {
     bound_fns.Push(base::Bind(
-        &VideoRenderer::Preroll, video_renderer_, seek_timestamp));
+        &VideoRenderer::Preroll, base::Unretained(video_renderer_.get()),
+        seek_timestamp));
   }
 
   pending_callbacks_ = SerialRunner::Run(bound_fns, done_cb);
@@ -554,14 +555,18 @@ void Pipeline::DoSeek(
   // Pause.
   if (audio_renderer_)
     bound_fns.Push(base::Bind(&AudioRenderer::Pause, audio_renderer_));
-  if (video_renderer_)
-    bound_fns.Push(base::Bind(&VideoRenderer::Pause, video_renderer_));
+  if (video_renderer_) {
+    bound_fns.Push(base::Bind(
+        &VideoRenderer::Pause, base::Unretained(video_renderer_.get())));
+  }
 
   // Flush.
   if (audio_renderer_)
     bound_fns.Push(base::Bind(&AudioRenderer::Flush, audio_renderer_));
-  if (video_renderer_)
-    bound_fns.Push(base::Bind(&VideoRenderer::Flush, video_renderer_));
+  if (video_renderer_) {
+    bound_fns.Push(base::Bind(
+        &VideoRenderer::Flush, base::Unretained(video_renderer_.get())));
+  }
 
   // Seek demuxer.
   bound_fns.Push(base::Bind(
@@ -575,7 +580,8 @@ void Pipeline::DoSeek(
 
   if (video_renderer_) {
     bound_fns.Push(base::Bind(
-        &VideoRenderer::Preroll, video_renderer_, seek_timestamp));
+        &VideoRenderer::Preroll, base::Unretained(video_renderer_.get()),
+        seek_timestamp));
   }
 
   pending_callbacks_ = SerialRunner::Run(bound_fns, done_cb);
@@ -592,8 +598,10 @@ void Pipeline::DoPlay(const PipelineStatusCB& done_cb) {
   if (audio_renderer_)
     bound_fns.Push(base::Bind(&AudioRenderer::Play, audio_renderer_));
 
-  if (video_renderer_)
-    bound_fns.Push(base::Bind(&VideoRenderer::Play, video_renderer_));
+  if (video_renderer_) {
+    bound_fns.Push(base::Bind(
+        &VideoRenderer::Play, base::Unretained(video_renderer_.get())));
+  }
 
   pending_callbacks_ = SerialRunner::Run(bound_fns, done_cb);
 }
@@ -609,8 +617,10 @@ void Pipeline::DoStop(const PipelineStatusCB& done_cb) {
   if (audio_renderer_)
     bound_fns.Push(base::Bind(&AudioRenderer::Stop, audio_renderer_));
 
-  if (video_renderer_)
-    bound_fns.Push(base::Bind(&VideoRenderer::Stop, video_renderer_));
+  if (video_renderer_) {
+    bound_fns.Push(base::Bind(
+        &VideoRenderer::Stop, base::Unretained(video_renderer_.get())));
+  }
 
   pending_callbacks_ = SerialRunner::Run(bound_fns, done_cb);
 }
@@ -627,7 +637,7 @@ void Pipeline::OnStopCompleted(PipelineStatus status) {
   pending_callbacks_.reset();
   filter_collection_.reset();
   audio_renderer_ = NULL;
-  video_renderer_ = NULL;
+  video_renderer_.reset();
   demuxer_ = NULL;
 
   // If we stop during initialization/seeking we want to run |seek_cb_|
@@ -912,7 +922,7 @@ void Pipeline::InitializeVideoRenderer(const PipelineStatusCB& done_cb) {
     natural_size_ = stream->video_decoder_config().natural_size();
   }
 
-  filter_collection_->SelectVideoRenderer(&video_renderer_);
+  video_renderer_ = filter_collection_->GetVideoRenderer();
   video_renderer_->Initialize(
       stream,
       *filter_collection_->GetVideoDecoders(),
diff --git a/media/base/pipeline.h b/media/base/pipeline.h
index 25e4dcb..54d7239 100644
--- a/media/base/pipeline.h
+++ b/media/base/pipeline.h
@@ -452,7 +452,7 @@ class MEDIA_EXPORT Pipeline
   // Renderer references used for setting the volume, playback rate, and
   // determining when playback has finished.
   scoped_refptr<AudioRenderer> audio_renderer_;
-  scoped_refptr<VideoRenderer> video_renderer_;
+  scoped_ptr<VideoRenderer> video_renderer_;
 
   // Demuxer reference used for setting the preload value.
   scoped_refptr<Demuxer> demuxer_;
diff --git a/media/base/pipeline_unittest.cc b/media/base/pipeline_unittest.cc
index 2d5b63b..cfe41b5 100644
--- a/media/base/pipeline_unittest.cc
+++ b/media/base/pipeline_unittest.cc
@@ -91,7 +91,8 @@ class PipelineTest : public ::testing::Test {
     filter_collection_->GetAudioDecoders()->push_back(audio_decoder_);
 
     video_renderer_ = new MockVideoRenderer();
-    filter_collection_->AddVideoRenderer(video_renderer_);
+    scoped_ptr<VideoRenderer> video_renderer(video_renderer_);
+    filter_collection_->SetVideoRenderer(video_renderer.Pass());
 
     audio_renderer_ = new MockAudioRenderer();
     filter_collection_->AddAudioRenderer(audio_renderer_);
@@ -299,7 +300,7 @@ class PipelineTest : public ::testing::Test {
   scoped_refptr<MockDemuxer> demuxer_;
   scoped_refptr<MockVideoDecoder> video_decoder_;
   scoped_refptr<MockAudioDecoder> audio_decoder_;
-  scoped_refptr<MockVideoRenderer> video_renderer_;
+  MockVideoRenderer* video_renderer_;
   scoped_refptr<MockAudioRenderer> audio_renderer_;
   scoped_refptr<StrictMock<MockDemuxerStream> > audio_stream_;
   scoped_refptr<StrictMock<MockDemuxerStream> > video_stream_;
diff --git a/media/base/video_renderer.h b/media/base/video_renderer.h
index c19c8f9..c609b68 100644
--- a/media/base/video_renderer.h
+++ b/media/base/video_renderer.h
@@ -22,8 +22,7 @@ namespace media {
 class DemuxerStream;
 class VideoDecoder;
 
-class MEDIA_EXPORT VideoRenderer
-    : public base::RefCountedThreadSafe<VideoRenderer> {
+class MEDIA_EXPORT VideoRenderer {
  public:
   typedef std::list<scoped_refptr<VideoDecoder> > VideoDecoderList;
 
@@ -37,6 +36,9 @@ class MEDIA_EXPORT VideoRenderer
   // Used to query the current time or duration of the media.
   typedef base::Callback<base::TimeDelta()> TimeDeltaCB;
 
+  VideoRenderer();
+  virtual ~VideoRenderer();
+
   // Initialize a VideoRenderer with the given DemuxerStream and
   // VideoDecoderList, executing |init_cb| callback upon completion.
   //
@@ -91,12 +93,6 @@ class MEDIA_EXPORT VideoRenderer
   // Updates the current playback rate.
   virtual void SetPlaybackRate(float playback_rate) = 0;
 
- protected:
-  friend class base::RefCountedThreadSafe<VideoRenderer>;
-
-  VideoRenderer();
-  virtual ~VideoRenderer();
-
  private:
   DISALLOW_COPY_AND_ASSIGN(VideoRenderer);
 };