Pipeline: Use WeakPtr for DemuxerHost Calls and Tasks.

media/base/pipeline.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/base/pipeline.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 54 matching lines @@
   DCHECK(thread_checker_.CalledOnValidThread())
       << "Pipeline must be destroyed on same thread that created it";
   DCHECK(!running_) << "Stop() must complete before destroying object";
   DCHECK(stop_cb_.is_null());
   DCHECK(seek_cb_.is_null());
 
   media_log_->AddEvent(
       media_log_->CreateEvent(MediaLogEvent::PIPELINE_DESTROYED));
 }
 
-// The base::Unretained(this) in these functions are safe because:
-// 1, No public methods (except for the dtor) should be called after Stop().
-// 2, |this| will not be destructed until the stop callback is fired.
-// 3, Stop() also posts StopTask(), hence all XxxTask() will be executed before
-//    StopTask(), and therefore before the dtor.
-
 void Pipeline::Start(scoped_ptr<FilterCollection> collection,
                      const base::Closure& ended_cb,
                      const PipelineStatusCB& error_cb,
                      const PipelineStatusCB& seek_cb,
                      const PipelineMetadataCB& metadata_cb,
                      const BufferingStateCB& buffering_state_cb,
                      const base::Closure& duration_change_cb) {
   DCHECK(!ended_cb.is_null());
   DCHECK(!error_cb.is_null());
   DCHECK(!seek_cb.is_null());
   DCHECK(!metadata_cb.is_null());
   DCHECK(!buffering_state_cb.is_null());
 
   base::AutoLock auto_lock(lock_);
   CHECK(!running_) << "Media pipeline is already running";
   running_ = true;
 
   filter_collection_ = collection.Pass();
   ended_cb_ = ended_cb;
   error_cb_ = error_cb;
   seek_cb_ = seek_cb;
   metadata_cb_ = metadata_cb;
   buffering_state_cb_ = buffering_state_cb;
   duration_change_cb_ = duration_change_cb;
 
   task_runner_->PostTask(
-      FROM_HERE, base::Bind(&Pipeline::StartTask, base::Unretained(this)));
+      FROM_HERE, base::Bind(&Pipeline::StartTask, weak_factory_.GetWeakPtr()));
 }
 
 void Pipeline::Stop(const base::Closure& stop_cb) {
   DVLOG(2) << __FUNCTION__;
   task_runner_->PostTask(
       FROM_HERE,
-      base::Bind(&Pipeline::StopTask, base::Unretained(this), stop_cb));
+      base::Bind(&Pipeline::StopTask, weak_factory_.GetWeakPtr(), stop_cb));
 }
 
 void Pipeline::Seek(TimeDelta time, const PipelineStatusCB& seek_cb) {
   base::AutoLock auto_lock(lock_);
   if (!running_) {
     DLOG(ERROR) << "Media pipeline isn't running. Ignoring Seek().";
     return;
   }
 
   task_runner_->PostTask(
       FROM_HERE,
-      base::Bind(&Pipeline::SeekTask, base::Unretained(this), time, seek_cb));
+      base::Bind(
+          &Pipeline::SeekTask, weak_factory_.GetWeakPtr(), time, seek_cb));
 }
 
 bool Pipeline::IsRunning() const {
   base::AutoLock auto_lock(lock_);
   return running_;
 }
 
 float Pipeline::GetPlaybackRate() const {
   base::AutoLock auto_lock(lock_);
   return playback_rate_;
 }
 
 void Pipeline::SetPlaybackRate(float playback_rate) {
   if (playback_rate < 0.0f)
     return;
 
   base::AutoLock auto_lock(lock_);
   playback_rate_ = playback_rate;
   if (running_) {
     task_runner_->PostTask(FROM_HERE,
                            base::Bind(&Pipeline::PlaybackRateChangedTask,
-                                      base::Unretained(this),
+                                      weak_factory_.GetWeakPtr(),
                                       playback_rate));
   }
 }
 
 float Pipeline::GetVolume() const {
   base::AutoLock auto_lock(lock_);
   return volume_;
 }
 
 void Pipeline::SetVolume(float volume) {
   if (volume < 0.0f || volume > 1.0f)
     return;
 
   base::AutoLock auto_lock(lock_);
   volume_ = volume;
   if (running_) {
     task_runner_->PostTask(
         FROM_HERE,
         base::Bind(
-            &Pipeline::VolumeChangedTask, base::Unretained(this), volume));
+            &Pipeline::VolumeChangedTask, weak_factory_.GetWeakPtr(), volume));
   }
 }
 
 TimeDelta Pipeline::GetMediaTime() const {
   base::AutoLock auto_lock(lock_);
   return std::min(interpolator_->GetInterpolatedTime(), duration_);
 }
 
 Ranges<TimeDelta> Pipeline::GetBufferedTimeRanges() const {
   base::AutoLock auto_lock(lock_);
@@ skipping 83 matching lines @@
 
     case kPlaying:
     case kStopping:
     case kStopped:
       break;
   }
   NOTREACHED() << "State has no transition: " << state_;
   return state_;
 }
 
-// The use of base::Unretained(this) in the following 3 functions is safe
-// because these functions are called by the Demuxer directly, before the stop
-// callback is posted by the Demuxer. So the posted tasks will always be
-// executed before the stop callback is executed, and hence before the Pipeline
-// is destructed.
-
 void Pipeline::OnDemuxerError(PipelineStatus error) {
   task_runner_->PostTask(FROM_HERE,
                          base::Bind(&Pipeline::ErrorChangedTask,
-                                    base::Unretained(this),
+                                    weak_factory_.GetWeakPtr(),
                                     error));
 }
 
 void Pipeline::AddTextStream(DemuxerStream* text_stream,
                              const TextTrackConfig& config) {
   task_runner_->PostTask(FROM_HERE,
                          base::Bind(&Pipeline::AddTextStreamTask,
-                                    base::Unretained(this),
+                                    weak_factory_.GetWeakPtr(),
                                     text_stream,
                                     config));
 }
 
 void Pipeline::RemoveTextStream(DemuxerStream* text_stream) {
   task_runner_->PostTask(FROM_HERE,
                          base::Bind(&Pipeline::RemoveTextStreamTask,
-                                    base::Unretained(this),
+                                    weak_factory_.GetWeakPtr(),
                                     text_stream));
 }
 
 void Pipeline::OnError(PipelineStatus error) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK(IsRunning());
   DCHECK_NE(PIPELINE_OK, error);
   VLOG(1) << "Media pipeline error: " << error;
 
   task_runner_->PostTask(FROM_HERE, base::Bind(
@@ skipping 243 matching lines @@
   demuxer_ = NULL;
 
   // If we stop during initialization/seeking we want to run |seek_cb_|
   // followed by |stop_cb_| so we don't leave outstanding callbacks around.
   if (!seek_cb_.is_null()) {
     base::ResetAndReturn(&seek_cb_).Run(status_);
     error_cb_.Reset();
   }
   if (!stop_cb_.is_null()) {
     error_cb_.Reset();
+
+    // Invalid all weak pointers so it's safe to destroy |this| on the render
+    // main thread.
+    weak_factory_.InvalidateWeakPtrs();

[xhwang, 2014/08/05 20:44:47]

Move this down here because OnStopComplete() can also be triggered by error.

+
     base::ResetAndReturn(&stop_cb_).Run();
 
     // NOTE: pipeline may be deleted at this point in time as a result of
     // executing |stop_cb_|.
     return;
   }
   if (!error_cb_.is_null()) {
     DCHECK_NE(status_, PIPELINE_OK);
     base::ResetAndReturn(&error_cb_).Run(status_);
   }
@@ skipping 346 matching lines @@
 void Pipeline::StartClockIfWaitingForTimeUpdate_Locked() {
   lock_.AssertAcquired();
   if (interpolation_state_ != INTERPOLATION_WAITING_FOR_AUDIO_TIME_UPDATE)
     return;
 
   interpolation_state_ = INTERPOLATION_STARTED;
   interpolator_->StartInterpolating();
 }
 
 }  // namespace media