Reland: Remove reference counting from media::Pipeline.

Now with a test that covers the refresh/teardown case.

BUG=173313

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

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

3 files changed, 50 insertions, 52 deletions

diff --git a/media/base/pipeline.cc b/media/base/pipeline.cc
index 84a1e18..292aef4 100644
--- a/media/base/pipeline.cc
+++ b/media/base/pipeline.cc
@@ -82,14 +82,14 @@ void Pipeline::Start(scoped_ptr<FilterCollection> collection,
 
   running_ = true;
   message_loop_->PostTask(FROM_HERE, base::Bind(
-      &Pipeline::StartTask, this, base::Passed(&collection),
+      &Pipeline::StartTask, base::Unretained(this), base::Passed(&collection),
       ended_cb, error_cb, seek_cb, buffering_state_cb, duration_change_cb));
 }
 
 void Pipeline::Stop(const base::Closure& stop_cb) {
   base::AutoLock auto_lock(lock_);
   message_loop_->PostTask(FROM_HERE, base::Bind(
-      &Pipeline::StopTask, this, stop_cb));
+      &Pipeline::StopTask, base::Unretained(this), stop_cb));
 }
 
 void Pipeline::Seek(TimeDelta time, const PipelineStatusCB& seek_cb) {
@@ -100,7 +100,7 @@ void Pipeline::Seek(TimeDelta time, const PipelineStatusCB& seek_cb) {
   }
 
   message_loop_->PostTask(FROM_HERE, base::Bind(
-      &Pipeline::SeekTask, this, time, seek_cb));
+      &Pipeline::SeekTask, base::Unretained(this), time, seek_cb));
 }
 
 bool Pipeline::IsRunning() const {
@@ -131,7 +131,8 @@ void Pipeline::SetPlaybackRate(float playback_rate) {
   playback_rate_ = playback_rate;
   if (running_) {
     message_loop_->PostTask(FROM_HERE, base::Bind(
-        &Pipeline::PlaybackRateChangedTask, this, playback_rate));
+        &Pipeline::PlaybackRateChangedTask, base::Unretained(this),
+        playback_rate));
   }
 }
 
@@ -148,7 +149,7 @@ void Pipeline::SetVolume(float volume) {
   volume_ = volume;
   if (running_) {
     message_loop_->PostTask(FROM_HERE, base::Bind(
-        &Pipeline::VolumeChangedTask, this, volume));
+        &Pipeline::VolumeChangedTask, base::Unretained(this), volume));
   }
 }
 
@@ -302,7 +303,7 @@ void Pipeline::SetError(PipelineStatus error) {
   VLOG(1) << "Media pipeline error: " << error;
 
   message_loop_->PostTask(FROM_HERE, base::Bind(
-      &Pipeline::ErrorChangedTask, this, error));
+      &Pipeline::ErrorChangedTask, base::Unretained(this), error));
 
   media_log_->AddEvent(media_log_->CreatePipelineErrorEvent(error));
 }
@@ -310,7 +311,7 @@ void Pipeline::SetError(PipelineStatus error) {
 void Pipeline::OnAudioDisabled() {
   DCHECK(IsRunning());
   message_loop_->PostTask(FROM_HERE, base::Bind(
-      &Pipeline::AudioDisabledTask, this));
+      &Pipeline::AudioDisabledTask, base::Unretained(this)));
   media_log_->AddEvent(
       media_log_->CreateEvent(MediaLogEvent::AUDIO_RENDERER_DISABLED));
 }
@@ -395,7 +396,7 @@ TimeDelta Pipeline::TimeForByteOffset_Locked(int64 byte_offset) const {
 void Pipeline::OnStateTransition(PipelineStatus status) {
   // Force post to process state transitions after current execution frame.
   message_loop_->PostTask(FROM_HERE, base::Bind(
-      &Pipeline::StateTransitionTask, this, status));
+      &Pipeline::StateTransitionTask, base::Unretained(this), status));
 }
 
 void Pipeline::StateTransitionTask(PipelineStatus status) {
@@ -423,7 +424,8 @@ void Pipeline::StateTransitionTask(PipelineStatus status) {
              state_ == kSeeking));
   pending_callbacks_.reset();
 
-  PipelineStatusCB done_cb = base::Bind(&Pipeline::OnStateTransition, this);
+  PipelineStatusCB done_cb = base::Bind(
+      &Pipeline::OnStateTransition, base::Unretained(this));
 
   // Switch states, performing any entrance actions for the new state as well.
   SetState(GetNextState());
@@ -672,14 +674,14 @@ void Pipeline::OnNaturalVideoSizeChanged(const gfx::Size& size) {
 void Pipeline::OnAudioRendererEnded() {
   // Force post to process ended messages after current execution frame.
   message_loop_->PostTask(FROM_HERE, base::Bind(
-      &Pipeline::DoAudioRendererEnded, this));
+      &Pipeline::DoAudioRendererEnded, base::Unretained(this)));
   media_log_->AddEvent(media_log_->CreateEvent(MediaLogEvent::AUDIO_ENDED));
 }
 
 void Pipeline::OnVideoRendererEnded() {
   // Force post to process ended messages after current execution frame.
   message_loop_->PostTask(FROM_HERE, base::Bind(
-      &Pipeline::DoVideoRendererEnded, this));
+      &Pipeline::DoVideoRendererEnded, base::Unretained(this)));
   media_log_->AddEvent(media_log_->CreateEvent(MediaLogEvent::VIDEO_ENDED));
 }
 
@@ -725,7 +727,7 @@ void Pipeline::StopTask(const base::Closure& stop_cb) {
   pending_callbacks_.reset();
   stop_cb_ = stop_cb;
 
-  DoStop(base::Bind(&Pipeline::OnStopCompleted, this));
+  DoStop(base::Bind(&Pipeline::OnStopCompleted, base::Unretained(this)));
 }
 
 void Pipeline::ErrorChangedTask(PipelineStatus error) {
@@ -739,7 +741,7 @@ void Pipeline::ErrorChangedTask(PipelineStatus error) {
   pending_callbacks_.reset();
   status_ = error;
 
-  DoStop(base::Bind(&Pipeline::OnStopCompleted, this));
+  DoStop(base::Bind(&Pipeline::OnStopCompleted, base::Unretained(this)));
 }
 
 void Pipeline::PlaybackRateChangedTask(float playback_rate) {
@@ -804,7 +806,8 @@ void Pipeline::SeekTask(TimeDelta time, const PipelineStatusCB& seek_cb) {
       clock_->Pause();
     clock_->SetTime(seek_timestamp, seek_timestamp);
   }
-  DoSeek(seek_timestamp, base::Bind(&Pipeline::OnStateTransition, this));
+  DoSeek(seek_timestamp, base::Bind(
+      &Pipeline::OnStateTransition, base::Unretained(this)));
 }
 
 void Pipeline::DoAudioRendererEnded() {
@@ -885,12 +888,12 @@ void Pipeline::InitializeAudioRenderer(const PipelineStatusCB& done_cb) {
   audio_renderer_->Initialize(
       demuxer_->GetStream(DemuxerStream::AUDIO),
       done_cb,
-      base::Bind(&Pipeline::OnUpdateStatistics, this),
-      base::Bind(&Pipeline::OnAudioUnderflow, this),
-      base::Bind(&Pipeline::OnAudioTimeUpdate, this),
-      base::Bind(&Pipeline::OnAudioRendererEnded, this),
-      base::Bind(&Pipeline::OnAudioDisabled, this),
-      base::Bind(&Pipeline::SetError, this));
+      base::Bind(&Pipeline::OnUpdateStatistics, base::Unretained(this)),
+      base::Bind(&Pipeline::OnAudioUnderflow, base::Unretained(this)),
+      base::Bind(&Pipeline::OnAudioTimeUpdate, base::Unretained(this)),
+      base::Bind(&Pipeline::OnAudioRendererEnded, base::Unretained(this)),
+      base::Bind(&Pipeline::OnAudioDisabled, base::Unretained(this)),
+      base::Bind(&Pipeline::SetError, base::Unretained(this)));
 }
 
 void Pipeline::InitializeVideoRenderer(const PipelineStatusCB& done_cb) {
@@ -909,19 +912,19 @@ void Pipeline::InitializeVideoRenderer(const PipelineStatusCB& done_cb) {
   video_renderer_->Initialize(
       stream,
       done_cb,
-      base::Bind(&Pipeline::OnUpdateStatistics, this),
-      base::Bind(&Pipeline::OnVideoTimeUpdate, this),
-      base::Bind(&Pipeline::OnNaturalVideoSizeChanged, this),
-      base::Bind(&Pipeline::OnVideoRendererEnded, this),
-      base::Bind(&Pipeline::SetError, this),
-      base::Bind(&Pipeline::GetMediaTime, this),
-      base::Bind(&Pipeline::GetMediaDuration, this));
+      base::Bind(&Pipeline::OnUpdateStatistics, base::Unretained(this)),
+      base::Bind(&Pipeline::OnVideoTimeUpdate, base::Unretained(this)),
+      base::Bind(&Pipeline::OnNaturalVideoSizeChanged, base::Unretained(this)),
+      base::Bind(&Pipeline::OnVideoRendererEnded, base::Unretained(this)),
+      base::Bind(&Pipeline::SetError, base::Unretained(this)),
+      base::Bind(&Pipeline::GetMediaTime, base::Unretained(this)),
+      base::Bind(&Pipeline::GetMediaDuration, base::Unretained(this)));
 }
 
 void Pipeline::OnAudioUnderflow() {
   if (!message_loop_->BelongsToCurrentThread()) {
     message_loop_->PostTask(FROM_HERE, base::Bind(
-        &Pipeline::OnAudioUnderflow, this));
+        &Pipeline::OnAudioUnderflow, base::Unretained(this)));
     return;
   }
 
diff --git a/media/base/pipeline.h b/media/base/pipeline.h
index aa92269..a91176e 100644
--- a/media/base/pipeline.h
+++ b/media/base/pipeline.h
@@ -63,9 +63,7 @@ class VideoRenderer;
 // If any error ever happens, this object will transition to the "Error" state
 // from any state. If Stop() is ever called, this object will transition to
 // "Stopped" state.
-class MEDIA_EXPORT Pipeline
-    : public base::RefCountedThreadSafe<Pipeline>,
-      public DemuxerHost {
+class MEDIA_EXPORT Pipeline : public DemuxerHost {
  public:
   // Buffering states the pipeline transitions between during playback.
   // kHaveMetadata:
@@ -85,6 +83,7 @@ class MEDIA_EXPORT Pipeline
   // Constructs a media pipeline that will execute on |message_loop|.
   Pipeline(const scoped_refptr<base::MessageLoopProxy>& message_loop,
            MediaLog* media_log);
+  virtual ~Pipeline();
 
   // Build a pipeline to using the given filter collection to construct a filter
   // chain, executing |seek_cb| when the initial seek/preroll has completed.
@@ -198,10 +197,6 @@ class MEDIA_EXPORT Pipeline
   FRIEND_TEST_ALL_PREFIXES(PipelineTest, AudioStreamShorterThanVideo);
   friend class MediaLog;
 
-  // Only allow ourselves to be deleted by reference counting.
-  friend class base::RefCountedThreadSafe<Pipeline>;
-  virtual ~Pipeline();
-
   // Pipeline states, as described above.
   enum State {
     kCreated,
diff --git a/media/base/pipeline_unittest.cc b/media/base/pipeline_unittest.cc
index ca0cc98..725695f 100644
--- a/media/base/pipeline_unittest.cc
+++ b/media/base/pipeline_unittest.cc
@@ -125,8 +125,8 @@ class PipelineTest : public ::testing::Test {
                                base::Unretained(&callbacks_)));
     message_loop_.RunUntilIdle();
 
-    pipeline_ = NULL;
     filter_collection_.reset();
+    pipeline_.reset();
   }
 
  protected:
@@ -289,7 +289,7 @@ class PipelineTest : public ::testing::Test {
   StrictMock<CallbackHelper> callbacks_;
   base::SimpleTestClock test_clock_;
   base::MessageLoop message_loop_;
-  scoped_refptr<Pipeline> pipeline_;
+  scoped_ptr<Pipeline> pipeline_;
 
   scoped_ptr<FilterCollection> filter_collection_;
   scoped_ptr<MockDemuxer> demuxer_;
@@ -711,7 +711,7 @@ TEST_F(PipelineTest, NoMessageDuringTearDownFromError) {
 
   // Trigger additional requests on the pipeline during tear down from error.
   base::Callback<void(PipelineStatus)> cb = base::Bind(
-      &TestNoCallsAfterError, pipeline_, &message_loop_);
+      &TestNoCallsAfterError, pipeline_.get(), &message_loop_);
   ON_CALL(callbacks_, OnError(_))
       .WillByDefault(Invoke(&cb, &base::Callback<void(PipelineStatus)>::Run));
 
@@ -912,7 +912,7 @@ class PipelineTeardownTest : public PipelineTest {
     if (state == kInitDemuxer) {
       if (stop_or_error == kStop) {
         EXPECT_CALL(*demuxer_, Initialize(_, _))
-            .WillOnce(DoAll(Stop(pipeline_, stop_cb),
+            .WillOnce(DoAll(Stop(pipeline_.get(), stop_cb),
                             RunCallback<1>(PIPELINE_OK)));
         EXPECT_CALL(callbacks_, OnStop());
       } else {
@@ -935,7 +935,7 @@ class PipelineTeardownTest : public PipelineTest {
     if (state == kInitAudioRenderer) {
       if (stop_or_error == kStop) {
         EXPECT_CALL(*audio_renderer_, Initialize(_, _, _, _, _, _, _, _))
-            .WillOnce(DoAll(Stop(pipeline_, stop_cb),
+            .WillOnce(DoAll(Stop(pipeline_.get(), stop_cb),
                             RunCallback<1>(PIPELINE_OK)));
         EXPECT_CALL(callbacks_, OnStop());
       } else {
@@ -955,7 +955,7 @@ class PipelineTeardownTest : public PipelineTest {
     if (state == kInitVideoRenderer) {
       if (stop_or_error == kStop) {
         EXPECT_CALL(*video_renderer_, Initialize(_, _, _, _, _, _, _, _, _))
-            .WillOnce(DoAll(Stop(pipeline_, stop_cb),
+            .WillOnce(DoAll(Stop(pipeline_.get(), stop_cb),
                             RunCallback<1>(PIPELINE_OK)));
         EXPECT_CALL(callbacks_, OnStop());
       } else {
@@ -1024,11 +1024,11 @@ class PipelineTeardownTest : public PipelineTest {
     if (state == kPausing) {
       if (stop_or_error == kStop) {
         EXPECT_CALL(*audio_renderer_, Pause(_))
-            .WillOnce(DoAll(Stop(pipeline_, stop_cb), RunClosure<0>()));
+            .WillOnce(DoAll(Stop(pipeline_.get(), stop_cb), RunClosure<0>()));
       } else {
         status = PIPELINE_ERROR_READ;
-        EXPECT_CALL(*audio_renderer_, Pause(_))
-            .WillOnce(DoAll(SetError(pipeline_, status), RunClosure<0>()));
+        EXPECT_CALL(*audio_renderer_, Pause(_)).WillOnce(
+            DoAll(SetError(pipeline_.get(), status), RunClosure<0>()));
       }
 
       return status;
@@ -1040,11 +1040,11 @@ class PipelineTeardownTest : public PipelineTest {
     if (state == kFlushing) {
       if (stop_or_error == kStop) {
         EXPECT_CALL(*audio_renderer_, Flush(_))
-            .WillOnce(DoAll(Stop(pipeline_, stop_cb), RunClosure<0>()));
+            .WillOnce(DoAll(Stop(pipeline_.get(), stop_cb), RunClosure<0>()));
       } else {
         status = PIPELINE_ERROR_READ;
-        EXPECT_CALL(*audio_renderer_, Flush(_))
-            .WillOnce(DoAll(SetError(pipeline_, status), RunClosure<0>()));
+        EXPECT_CALL(*audio_renderer_, Flush(_)).WillOnce(
+            DoAll(SetError(pipeline_.get(), status), RunClosure<0>()));
       }
 
       return status;
@@ -1056,7 +1056,7 @@ class PipelineTeardownTest : public PipelineTest {
     if (state == kSeeking) {
       if (stop_or_error == kStop) {
         EXPECT_CALL(*demuxer_, Seek(_, _))
-            .WillOnce(DoAll(Stop(pipeline_, stop_cb),
+            .WillOnce(DoAll(Stop(pipeline_.get(), stop_cb),
                             RunCallback<1>(PIPELINE_OK)));
       } else {
         status = PIPELINE_ERROR_READ;
@@ -1073,7 +1073,7 @@ class PipelineTeardownTest : public PipelineTest {
     if (state == kPrerolling) {
       if (stop_or_error == kStop) {
         EXPECT_CALL(*audio_renderer_, Preroll(_, _))
-            .WillOnce(DoAll(Stop(pipeline_, stop_cb),
+            .WillOnce(DoAll(Stop(pipeline_.get(), stop_cb),
                             RunCallback<1>(PIPELINE_OK)));
       } else {
         status = PIPELINE_ERROR_READ;
@@ -1098,11 +1098,11 @@ class PipelineTeardownTest : public PipelineTest {
     if (state == kStarting) {
       if (stop_or_error == kStop) {
         EXPECT_CALL(*audio_renderer_, Play(_))
-            .WillOnce(DoAll(Stop(pipeline_, stop_cb), RunClosure<0>()));
+            .WillOnce(DoAll(Stop(pipeline_.get(), stop_cb), RunClosure<0>()));
       } else {
         status = PIPELINE_ERROR_READ;
-        EXPECT_CALL(*audio_renderer_, Play(_))
-            .WillOnce(DoAll(SetError(pipeline_, status), RunClosure<0>()));
+        EXPECT_CALL(*audio_renderer_, Play(_)).WillOnce(
+            DoAll(SetError(pipeline_.get(), status), RunClosure<0>()));
       }
       return status;
     }