Migrate Pipeline & PipelineImpl to PipelineStatusCB.

BUG=90214
TEST=PipelineImplTest.*

Review URL: http://codereview.chromium.org/7484054

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

7 files changed, 86 insertions, 97 deletions

diff --git a/media/base/mock_filters.cc b/media/base/mock_filters.cc
index 6fe84c3..07466a1 100644
--- a/media/base/mock_filters.cc
+++ b/media/base/mock_filters.cc
@@ -173,10 +173,8 @@ void RunFilterStatusCB(::testing::Unused, const FilterStatusCB& cb) {
   cb.Run(PIPELINE_OK);
 }
 
-void RunPipelineStatusCallback(
-    PipelineStatus status, PipelineStatusCallback* callback) {
-  callback->Run(status);
-  delete callback;
+void RunPipelineStatusCB(PipelineStatus status, const PipelineStatusCB& cb) {
+  cb.Run(status);
 }
 
 void RunFilterCallback3(::testing::Unused, FilterCallback* callback,
diff --git a/media/base/mock_filters.h b/media/base/mock_filters.h
index 635659e..e34fb8f 100644
--- a/media/base/mock_filters.h
+++ b/media/base/mock_filters.h
@@ -312,8 +312,7 @@ class MockFilterCollection {
 // the Initialize() and Seek() methods.
 void RunFilterCallback(::testing::Unused, FilterCallback* callback);
 void RunFilterStatusCB(::testing::Unused, const FilterStatusCB& cb);
-void RunPipelineStatusCallback(PipelineStatus status,
-                               PipelineStatusCallback* callback);
+void RunPipelineStatusCB(PipelineStatus status, const PipelineStatusCB& cb);
 void RunFilterCallback3(::testing::Unused, FilterCallback* callback,
                         ::testing::Unused);
 
diff --git a/media/base/pipeline.h b/media/base/pipeline.h
index 9686bb6..cbfc8b0 100644
--- a/media/base/pipeline.h
+++ b/media/base/pipeline.h
@@ -11,7 +11,6 @@
 
 #include <string>
 
-#include "base/callback.h"
 #include "media/base/filters.h"
 #include "media/base/pipeline_status.h"
 
@@ -46,9 +45,9 @@ class Pipeline : public base::RefCountedThreadSafe<Pipeline> {
   // |ended_callback| will be executed when the media reaches the end.
   // |error_callback_| will be executed upon an error in the pipeline.
   // |network_callback_| will be executed when there's a network event.
-  virtual void Init(PipelineStatusCallback* ended_callback,
-                    PipelineStatusCallback* error_callback,
-                    PipelineStatusCallback* network_callback) = 0;
+  virtual void Init(const PipelineStatusCB& ended_callback,
+                    const PipelineStatusCB& error_callback,
+                    const PipelineStatusCB& network_callback) = 0;
 
   // Build a pipeline to render the given URL using the given filter collection
   // to construct a filter chain.  Returns true if successful, false otherwise
@@ -63,7 +62,7 @@ class Pipeline : public base::RefCountedThreadSafe<Pipeline> {
   // pipeline initialization completes.
   virtual bool Start(FilterCollection* filter_collection,
                      const std::string& url,
-                     PipelineStatusCallback* start_callback) = 0;
+                     const PipelineStatusCB& start_callback) = 0;
 
   // Asynchronously stops the pipeline and resets it to an uninitialized state.
   // If provided, |stop_callback| will be executed when the pipeline has been
@@ -75,7 +74,7 @@ class Pipeline : public base::RefCountedThreadSafe<Pipeline> {
   //
   // TODO(scherkus): ideally clients would destroy the pipeline after calling
   // Stop() and create a new pipeline as needed.
-  virtual void Stop(PipelineStatusCallback* stop_callback) = 0;
+  virtual void Stop(const PipelineStatusCB& stop_callback) = 0;
 
   // Attempt to seek to the position specified by time.  |seek_callback| will be
   // executed when the all filters in the pipeline have processed the seek.
@@ -83,7 +82,7 @@ class Pipeline : public base::RefCountedThreadSafe<Pipeline> {
   // Clients are expected to call GetCurrentTime() to check whether the seek
   // succeeded.
   virtual void Seek(base::TimeDelta time,
-                    PipelineStatusCallback* seek_callback) = 0;
+                    const PipelineStatusCB& seek_callback) = 0;
 
   // Returns true if the pipeline has been started via Start().  If IsRunning()
   // returns true, it is expected that Stop() will be called before destroying
diff --git a/media/base/pipeline_impl.cc b/media/base/pipeline_impl.cc
index 0a88b2d..cdb3311 100644
--- a/media/base/pipeline_impl.cc
+++ b/media/base/pipeline_impl.cc
@@ -25,15 +25,15 @@ const char kRawMediaScheme[] = "x-raw-media";
 
 PipelineStatusNotification::PipelineStatusNotification()
     : cv_(&lock_), status_(PIPELINE_OK), notified_(false) {
-  callback_.reset(NewCallback(this, &PipelineStatusNotification::Notify));
 }
 
 PipelineStatusNotification::~PipelineStatusNotification() {
   DCHECK(notified_);
 }
 
-media::PipelineStatusCallback* PipelineStatusNotification::Callback() {
-  return callback_.release();
+PipelineStatusCB PipelineStatusNotification::Callback() {
+  return base::Bind(&PipelineStatusNotification::Notify,
+                    base::Unretained(this));
 }
 
 void PipelineStatusNotification::Notify(media::PipelineStatus status) {
@@ -86,22 +86,21 @@ PipelineImpl::~PipelineImpl() {
       media_log_->CreateEvent(MediaLogEvent::PIPELINE_DESTROYED));
 }
 
-void PipelineImpl::Init(PipelineStatusCallback* ended_callback,
-                        PipelineStatusCallback* error_callback,
-                        PipelineStatusCallback* network_callback) {
+void PipelineImpl::Init(const PipelineStatusCB& ended_callback,
+                        const PipelineStatusCB& error_callback,
+                        const PipelineStatusCB& network_callback) {
   DCHECK(!IsRunning())
       << "Init() should be called before the pipeline has started";
-  ended_callback_.reset(ended_callback);
-  error_callback_.reset(error_callback);
-  network_callback_.reset(network_callback);
+  ended_callback_ = ended_callback;
+  error_callback_ = error_callback;
+  network_callback_ = network_callback;
 }
 
 // Creates the PipelineInternal and calls it's start method.
 bool PipelineImpl::Start(FilterCollection* collection,
                          const std::string& url,
-                         PipelineStatusCallback* start_callback) {
+                         const PipelineStatusCB& start_callback) {
   base::AutoLock auto_lock(lock_);
-  scoped_ptr<PipelineStatusCallback> callback(start_callback);
   scoped_ptr<FilterCollection> filter_collection(collection);
 
   if (running_) {
@@ -121,13 +120,12 @@ bool PipelineImpl::Start(FilterCollection* collection,
                         &PipelineImpl::StartTask,
                         filter_collection.release(),
                         url,
-                        callback.release()));
+                        start_callback));
   return true;
 }
 
-void PipelineImpl::Stop(PipelineStatusCallback* stop_callback) {
+void PipelineImpl::Stop(const PipelineStatusCB& stop_callback) {
   base::AutoLock auto_lock(lock_);
-  scoped_ptr<PipelineStatusCallback> callback(stop_callback);
   if (!running_) {
     VLOG(1) << "Media pipeline has already stopped";
     return;
@@ -135,21 +133,19 @@ void PipelineImpl::Stop(PipelineStatusCallback* stop_callback) {
 
   // Stop the pipeline, which will set |running_| to false on behalf.
   message_loop_->PostTask(FROM_HERE,
-      NewRunnableMethod(this, &PipelineImpl::StopTask, callback.release()));
+      NewRunnableMethod(this, &PipelineImpl::StopTask, stop_callback));
 }
 
 void PipelineImpl::Seek(base::TimeDelta time,
-                        PipelineStatusCallback* seek_callback) {
+                        const PipelineStatusCB& seek_callback) {
   base::AutoLock auto_lock(lock_);
-  scoped_ptr<PipelineStatusCallback> callback(seek_callback);
   if (!running_) {
     VLOG(1) << "Media pipeline must be running";
     return;
   }
 
   message_loop_->PostTask(FROM_HERE,
-      NewRunnableMethod(this, &PipelineImpl::SeekTask, time,
-                        callback.release()));
+      NewRunnableMethod(this, &PipelineImpl::SeekTask, time, seek_callback));
 }
 
 bool PipelineImpl::IsRunning() const {
@@ -428,9 +424,9 @@ void PipelineImpl::FinishInitialization() {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
   // Execute the seek callback, if present.  Note that this might be the
   // initial callback passed into Start().
-  if (seek_callback_.get()) {
-    seek_callback_->Run(status_);
-    seek_callback_.reset();
+  if (!seek_callback_.is_null()) {
+    seek_callback_.Run(status_);
+    seek_callback_.Reset();
   }
 }
 
@@ -615,12 +611,12 @@ void PipelineImpl::OnUpdateStatistics(const PipelineStatistics& stats) {
 
 void PipelineImpl::StartTask(FilterCollection* filter_collection,
                              const std::string& url,
-                             PipelineStatusCallback* start_callback) {
+                             const PipelineStatusCB& start_callback) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
   DCHECK_EQ(kCreated, state_);
   filter_collection_.reset(filter_collection);
   url_ = url;
-  seek_callback_.reset(start_callback);
+  seek_callback_ = start_callback;
 
   // Kick off initialization.
   pipeline_init_state_.reset(new PipelineInitState());
@@ -754,15 +750,14 @@ void PipelineImpl::InitializeTask() {
 // TODO(scherkus): beware!  this can get posted multiple times since we post
 // Stop() tasks even if we've already stopped.  Perhaps this should no-op for
 // additional calls, however most of this logic will be changing.
-void PipelineImpl::StopTask(PipelineStatusCallback* stop_callback) {
+void PipelineImpl::StopTask(const PipelineStatusCB& stop_callback) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
   DCHECK(!IsPipelineStopPending());
   DCHECK_NE(state_, kStopped);
 
   if (state_ == kStopped) {
     // Already stopped so just run callback.
-    stop_callback->Run(status_);
-    delete stop_callback;
+    stop_callback.Run(status_);
     return;
   }
 
@@ -776,7 +771,7 @@ void PipelineImpl::StopTask(PipelineStatusCallback* stop_callback) {
     error_caused_teardown_ = false;
   }
 
-  stop_callback_.reset(stop_callback);
+  stop_callback_ = stop_callback;
 
   stop_pending_ = true;
   if (!IsPipelineSeeking() && !IsPipelineTearingDown()) {
@@ -850,7 +845,7 @@ void PipelineImpl::PreloadChangedTask(Preload preload) {
 }
 
 void PipelineImpl::SeekTask(base::TimeDelta time,
-                            PipelineStatusCallback* seek_callback) {
+                            const PipelineStatusCB& seek_callback) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
   DCHECK(!IsPipelineStopPending());
 
@@ -860,7 +855,6 @@ void PipelineImpl::SeekTask(base::TimeDelta time,
     // will only execute the first Seek() request.
     VLOG(1) << "Media pipeline has not started, ignoring seek to "
               << time.InMicroseconds();
-    delete seek_callback;
     return;
   }
 
@@ -876,7 +870,7 @@ void PipelineImpl::SeekTask(base::TimeDelta time,
   //   kStarted
   SetState(kPausing);
   seek_timestamp_ = time;
-  seek_callback_.reset(seek_callback);
+  seek_callback_ = seek_callback;
 
   // Kick off seeking!
   {
@@ -919,15 +913,15 @@ void PipelineImpl::NotifyEndedTask() {
 
   // Transition to ended, executing the callback if present.
   SetState(kEnded);
-  if (ended_callback_.get()) {
-    ended_callback_->Run(status_);
+  if (!ended_callback_.is_null()) {
+    ended_callback_.Run(status_);
   }
 }
 
 void PipelineImpl::NotifyNetworkEventTask() {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
-  if (network_callback_.get()) {
-    network_callback_->Run(status_);
+  if (!network_callback_.is_null()) {
+    network_callback_.Run(status_);
   }
 }
 
@@ -1072,16 +1066,17 @@ void PipelineImpl::FinishDestroyingFiltersTask() {
 
   pipeline_filter_ = NULL;
 
-  if (error_caused_teardown_ && !IsPipelineOk() && error_callback_.get())
-    error_callback_->Run(status_);
+  if (error_caused_teardown_ && !IsPipelineOk() && !error_callback_.is_null())
+    error_callback_.Run(status_);
 
   if (stop_pending_) {
     stop_pending_ = false;
     ResetState();
-    scoped_ptr<PipelineStatusCallback> stop_callback(stop_callback_.release());
+    PipelineStatusCB stop_cb;
+    std::swap(stop_cb, stop_callback_);
     // Notify the client that stopping has finished.
-    if (stop_callback.get()) {
-      stop_callback->Run(status_);
+    if (!stop_cb.is_null()) {
+      stop_cb.Run(status_);
     }
   }
 
diff --git a/media/base/pipeline_impl.h b/media/base/pipeline_impl.h
index 6fce5dc..5176d58 100644
--- a/media/base/pipeline_impl.h
+++ b/media/base/pipeline_impl.h
@@ -41,17 +41,17 @@ class PipelineStatusNotification {
   ~PipelineStatusNotification();
 
   // See class-level comment for usage.
-  media::PipelineStatusCallback* Callback();
-  void Notify(media::PipelineStatus status);
+  PipelineStatusCB Callback();
   void Wait();
-  media::PipelineStatus status();
+  PipelineStatus status();
 
  private:
+  void Notify(media::PipelineStatus status);
+
   base::Lock lock_;
   base::ConditionVariable cv_;
   media::PipelineStatus status_;
   bool notified_;
-  scoped_ptr<media::PipelineStatusCallback> callback_;
 
   DISALLOW_COPY_AND_ASSIGN(PipelineStatusNotification);
 };
@@ -98,15 +98,15 @@ class PipelineImpl : public Pipeline, public FilterHost {
   explicit PipelineImpl(MessageLoop* message_loop, MediaLog* media_log);
 
   // Pipeline implementation.
-  virtual void Init(PipelineStatusCallback* ended_callback,
-                    PipelineStatusCallback* error_callback,
-                    PipelineStatusCallback* network_callback);
+  virtual void Init(const PipelineStatusCB& ended_callback,
+                    const PipelineStatusCB& error_callback,
+                    const PipelineStatusCB& network_callback);
   virtual bool Start(FilterCollection* filter_collection,
                      const std::string& uri,
-                     PipelineStatusCallback* start_callback);
-  virtual void Stop(PipelineStatusCallback* stop_callback);
+                     const PipelineStatusCB& start_callback);
+  virtual void Stop(const PipelineStatusCB& stop_callback);
   virtual void Seek(base::TimeDelta time,
-                    PipelineStatusCallback* seek_callback);
+                    const PipelineStatusCB& seek_callback);
   virtual bool IsRunning() const;
   virtual bool IsInitialized() const;
   virtual bool IsNetworkActive() const;
@@ -226,7 +226,7 @@ class PipelineImpl : public Pipeline, public FilterHost {
   // message loop.
   void StartTask(FilterCollection* filter_collection,
                  const std::string& url,
-                 PipelineStatusCallback* start_callback);
+                 const PipelineStatusCB& start_callback);
 
   // InitializeTask() performs initialization in multiple passes. It is executed
   // as a result of calling Start() or InitializationComplete() that advances
@@ -235,7 +235,7 @@ class PipelineImpl : public Pipeline, public FilterHost {
   void InitializeTask();
 
   // Stops and destroys all filters, placing the pipeline in the kStopped state.
-  void StopTask(PipelineStatusCallback* stop_callback);
+  void StopTask(const PipelineStatusCB& stop_callback);
 
   // Carries out stopping and destroying all filters, placing the pipeline in
   // the kError state.
@@ -254,7 +254,7 @@ class PipelineImpl : public Pipeline, public FilterHost {
   void PreloadChangedTask(Preload preload);
 
   // Carries out notifying filters that we are seeking to a new timestamp.
-  void SeekTask(base::TimeDelta time, PipelineStatusCallback* seek_callback);
+  void SeekTask(base::TimeDelta time, const PipelineStatusCB& seek_callback);
 
   // Carries out handling a notification from a filter that it has ended.
   void NotifyEndedTask();
@@ -436,11 +436,11 @@ class PipelineImpl : public Pipeline, public FilterHost {
   std::string url_;
 
   // Callbacks for various pipeline operations.
-  scoped_ptr<PipelineStatusCallback> seek_callback_;
-  scoped_ptr<PipelineStatusCallback> stop_callback_;
-  scoped_ptr<PipelineStatusCallback> ended_callback_;
-  scoped_ptr<PipelineStatusCallback> error_callback_;
-  scoped_ptr<PipelineStatusCallback> network_callback_;
+  PipelineStatusCB seek_callback_;
+  PipelineStatusCB stop_callback_;
+  PipelineStatusCB ended_callback_;
+  PipelineStatusCB error_callback_;
+  PipelineStatusCB network_callback_;
 
   // Reference to the filter(s) that constitute the pipeline.
   scoped_refptr<Filter> pipeline_filter_;
diff --git a/media/base/pipeline_impl_unittest.cc b/media/base/pipeline_impl_unittest.cc
index 18b9576..4dd4da2 100644
--- a/media/base/pipeline_impl_unittest.cc
+++ b/media/base/pipeline_impl_unittest.cc
@@ -4,6 +4,7 @@
 
 #include <string>
 
+#include "base/bind.h"
 #include "base/callback.h"
 #include "base/stl_util.h"
 #include "base/threading/simple_thread.h"
@@ -64,11 +65,9 @@ class PipelineImplTest : public ::testing::Test {
   PipelineImplTest()
       : pipeline_(new PipelineImpl(&message_loop_, new MediaLog())) {
     pipeline_->Init(
-        NewCallback(reinterpret_cast<CallbackHelper*>(&callbacks_),
-                    &CallbackHelper::OnEnded),
-        NewCallback(reinterpret_cast<CallbackHelper*>(&callbacks_),
-                    &CallbackHelper::OnError),
-        static_cast<PipelineStatusCallback*>(NULL));
+        base::Bind(&CallbackHelper::OnEnded, base::Unretained(&callbacks_)),
+        base::Bind(&CallbackHelper::OnError, base::Unretained(&callbacks_)),
+        PipelineStatusCB());
     mocks_.reset(new MockFilterCollection());
 
     // InitializeDemuxer adds overriding expectations for expected non-NULL
@@ -88,8 +87,8 @@ class PipelineImplTest : public ::testing::Test {
 
     // Expect a stop callback if we were started.
     EXPECT_CALL(callbacks_, OnStop(PIPELINE_OK));
-    pipeline_->Stop(NewCallback(reinterpret_cast<CallbackHelper*>(&callbacks_),
-                                &CallbackHelper::OnStop));
+    pipeline_->Stop(base::Bind(&CallbackHelper::OnStop,
+                               base::Unretained(&callbacks_)));
     message_loop_.RunAllPending();
 
     mocks_.reset();
@@ -214,8 +213,8 @@ class PipelineImplTest : public ::testing::Test {
                                                run_build,
                                                build_status),
                      url,
-                     NewCallback(reinterpret_cast<CallbackHelper*>(&callbacks_),
-                                 &CallbackHelper::OnStart));
+                     base::Bind(&CallbackHelper::OnStart,
+                                base::Unretained(&callbacks_)));
 
     message_loop_.RunAllPending();
   }
@@ -261,8 +260,8 @@ class PipelineImplTest : public ::testing::Test {
 
   void DoSeek(const base::TimeDelta& seek_time) {
     pipeline_->Seek(seek_time,
-                    NewCallback(reinterpret_cast<CallbackHelper*>(&callbacks_),
-                                &CallbackHelper::OnSeek));
+                    base::Bind(&CallbackHelper::OnSeek,
+                               base::Unretained(&callbacks_)));
 
     // We expect the time to be updated only after the seek has completed.
     EXPECT_NE(seek_time, pipeline_->GetCurrentTime());
@@ -289,11 +288,11 @@ TEST_F(PipelineImplTest, NotStarted) {
 
   // StrictMock<> will ensure these never get called, and valgrind/purify will
   // make sure the callbacks are instantly deleted.
-  pipeline_->Stop(NewCallback(reinterpret_cast<CallbackHelper*>(&callbacks_),
-                              &CallbackHelper::OnStop));
+  pipeline_->Stop(base::Bind(&CallbackHelper::OnStop,
+                             base::Unretained(&callbacks_)));
   pipeline_->Seek(kZero,
-                  NewCallback(reinterpret_cast<CallbackHelper*>(&callbacks_),
-                              &CallbackHelper::OnSeek));
+                  base::Bind(&CallbackHelper::OnSeek,
+                             base::Unretained(&callbacks_)));
 
   EXPECT_FALSE(pipeline_->IsRunning());
   EXPECT_FALSE(pipeline_->IsInitialized());
@@ -338,8 +337,8 @@ TEST_F(PipelineImplTest, NeverInitializes) {
                                              true,
                                              PIPELINE_OK),
                    "",
-                   NewCallback(reinterpret_cast<CallbackHelper*>(&callbacks_),
-                               &CallbackHelper::OnStart));
+                   base::Bind(&CallbackHelper::OnStart,
+                              base::Unretained(&callbacks_)));
   message_loop_.RunAllPending();
 
   EXPECT_FALSE(pipeline_->IsInitialized());
@@ -366,8 +365,8 @@ TEST_F(PipelineImplTest, RequiredFilterMissing) {
                                 true,
                                 PIPELINE_ERROR_REQUIRED_FILTER_MISSING);
   pipeline_->Start(collection, "",
-                   NewCallback(reinterpret_cast<CallbackHelper*>(&callbacks_),
-                               &CallbackHelper::OnStart));
+                   base::Bind(&CallbackHelper::OnStart,
+                              base::Unretained(&callbacks_)));
   message_loop_.RunAllPending();
 
   EXPECT_FALSE(pipeline_->IsInitialized());
@@ -800,8 +799,8 @@ TEST_F(PipelineImplTest, ErrorDuringSeek) {
   EXPECT_CALL(*mocks_->demuxer(), Seek(seek_time, _))
       .WillOnce(Invoke(&SendReadErrorToCB));
 
-  pipeline_->Seek(seek_time, NewCallback(
-      reinterpret_cast<CallbackHelper*>(&callbacks_), &CallbackHelper::OnSeek));
+  pipeline_->Seek(seek_time,base::Bind(&CallbackHelper::OnSeek,
+                                       base::Unretained(&callbacks_)));
   EXPECT_CALL(callbacks_, OnSeek(PIPELINE_ERROR_READ));
   EXPECT_CALL(callbacks_, OnError(PIPELINE_ERROR_READ));
   message_loop_.RunAllPending();
@@ -851,23 +850,23 @@ TEST_F(PipelineImplTest, StartTimeIsNonZero) {
 class FlexibleCallbackRunner : public base::DelegateSimpleThread::Delegate {
  public:
   FlexibleCallbackRunner(int delayInMs, PipelineStatus status,
-                         PipelineStatusCallback* callback)
+                         const PipelineStatusCB& callback)
       : delayInMs_(delayInMs), status_(status), callback_(callback) {
     if (delayInMs_ < 0) {
-      callback_->Run(status_);
+      callback_.Run(status_);
       return;
     }
   }
   virtual void Run() {
     if (delayInMs_ < 0) return;
     base::PlatformThread::Sleep(delayInMs_);
-    callback_->Run(status_);
+    callback_.Run(status_);
   }
 
  private:
   int delayInMs_;
   PipelineStatus status_;
-  scoped_ptr<PipelineStatusCallback> callback_;
+  PipelineStatusCB callback_;
 };
 
 void TestPipelineStatusNotification(int delayInMs) {
diff --git a/media/base/pipeline_status.h b/media/base/pipeline_status.h
index ba6b931..dcbda09 100644
--- a/media/base/pipeline_status.h
+++ b/media/base/pipeline_status.h
@@ -34,7 +34,6 @@ enum PipelineStatus {
   DATASOURCE_ERROR_URL_NOT_SUPPORTED,
 };
 
-typedef Callback1<media::PipelineStatus>::Type PipelineStatusCallback;
 typedef base::Callback<void(PipelineStatus)> PipelineStatusCB;
 
 }  // namespace media