Implemented Pipeline seek callbacks and a big refactor of PipelineImplTest.

Now that I'm wiser writing tests, I'm trying to get rid of InitializationHelper and rely on simplier test fixtures classes.  I also introduced MockFilterFactory which is capable of creating all the mock filters and is able to return instantiated instances of filters to permit testing for expectations (refer to PipelineImplTest.Seek for an example).

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

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

8 files changed, 399 insertions, 240 deletions

diff --git a/media/base/mock_media_filters.h b/media/base/mock_media_filters.h
index d043999..f2f57e1 100644
--- a/media/base/mock_media_filters.h
+++ b/media/base/mock_media_filters.h
@@ -37,7 +37,8 @@ enum MockDataSourceBehavior {
 // is typically allocated on the stack.
 struct MockFilterConfig {
   MockFilterConfig()
-      : data_source_behavior(MOCK_DATA_SOURCE_NORMAL_INIT),
+      : create_filter(true),
+        data_source_behavior(MOCK_DATA_SOURCE_NORMAL_INIT),
         data_source_value('!'),
         has_video(true),
         video_width(1280u),
@@ -53,6 +54,7 @@ struct MockFilterConfig {
         media_total_bytes(media_duration.InMilliseconds() * 250) {
   }
 
+  bool create_filter;
   MockDataSourceBehavior data_source_behavior;
   char data_source_value;
   bool has_video;
@@ -72,11 +74,6 @@ struct MockFilterConfig {
 
 class MockDataSource : public DataSource {
  public:
-  static FilterFactory* CreateFactory(const MockFilterConfig* config) {
-     return new FilterFactoryImpl1<MockDataSource,
-                                   const MockFilterConfig*>(config);
-  }
-
   explicit MockDataSource(const MockFilterConfig* config)
       : config_(config),
         position_(0),
@@ -94,6 +91,10 @@ class MockDataSource : public DataSource {
   // Implementation of MediaFilter.
   virtual void Stop() {}
 
+  virtual void Seek(base::TimeDelta time) {
+    seek_time_ = time;
+  }
+
   // Implementation of DataSource.
   virtual bool Initialize(const std::string& url) {
     media_format_.SetAsString(MediaFormat::kMimeType,
@@ -162,8 +163,9 @@ class MockDataSource : public DataSource {
     return true;
   }
 
-  // Simple position getter for unit testing.
+  // Mock accessors.
   int64 position() const { return position_; }
+  const base::TimeDelta& seek_time() const { return seek_time_; }
 
  private:
   virtual ~MockDataSource() {
@@ -189,6 +191,7 @@ class MockDataSource : public DataSource {
   const MockFilterConfig* config_;
   int64 position_;
   MediaFormat media_format_;
+  base::TimeDelta seek_time_;
 
   // Set to true inside the destructor.  Used in FFmpegGlue unit tests for
   // testing proper reference counting.
@@ -197,7 +200,6 @@ class MockDataSource : public DataSource {
   DISALLOW_COPY_AND_ASSIGN(MockDataSource);
 };
 
-//------------------------------------------------------------------------------
 
 class MockDemuxerStream : public DemuxerStream {
  public:
@@ -230,15 +232,9 @@ class MockDemuxerStream : public DemuxerStream {
   DISALLOW_COPY_AND_ASSIGN(MockDemuxerStream);
 };
 
-//------------------------------------------------------------------------------
 
 class MockDemuxer : public Demuxer {
  public:
-  static FilterFactory* CreateFactory(const MockFilterConfig* config) {
-     return new FilterFactoryImpl1<MockDemuxer,
-                                   const MockFilterConfig*>(config);
-  }
-
   explicit MockDemuxer(const MockFilterConfig* config)
       : config_(config),
         mock_audio_stream_(new MockDemuxerStream(config, true)),
@@ -248,6 +244,10 @@ class MockDemuxer : public Demuxer {
   // Implementation of MediaFilter.
   virtual void Stop() {}
 
+  virtual void Seek(base::TimeDelta time) {
+    seek_time_ = time;
+  }
+
   // Implementation of Demuxer.
   virtual bool Initialize(DataSource* data_source) {
     host_->InitializationComplete();
@@ -284,29 +284,23 @@ class MockDemuxer : public Demuxer {
     return NULL;
   }
 
+  // Mock accessors.
+  const base::TimeDelta& seek_time() const { return seek_time_; }
+
  private:
   virtual ~MockDemuxer() {}
 
   const MockFilterConfig* config_;
   scoped_refptr<DemuxerStream> mock_audio_stream_;
   scoped_refptr<DemuxerStream> mock_video_stream_;
+  base::TimeDelta seek_time_;
 
   DISALLOW_COPY_AND_ASSIGN(MockDemuxer);
 };
 
-//------------------------------------------------------------------------------
 
 class MockAudioDecoder : public AudioDecoder {
  public:
-  static FilterFactory* CreateFactory(const MockFilterConfig* config) {
-    return new FilterFactoryImpl1<MockAudioDecoder,
-                                  const MockFilterConfig*>(config);
-  }
-
-  static bool IsMediaFormatSupported(const MediaFormat& media_format) {
-    return true;  // TODO(ralphl): check for a supported format.
-  }
-
   explicit MockAudioDecoder(const MockFilterConfig* config) {
     media_format_.SetAsString(MediaFormat::kMimeType,
                               config->uncompressed_audio_mime_type);
@@ -315,6 +309,10 @@ class MockAudioDecoder : public AudioDecoder {
   // Implementation of MediaFilter.
   virtual void Stop() {}
 
+  virtual void Seek(base::TimeDelta time) {
+    seek_time_ = time;
+  }
+
   // Implementation of AudioDecoder.
   virtual bool Initialize(DemuxerStream* stream) {
     host_->InitializationComplete();
@@ -330,32 +328,30 @@ class MockAudioDecoder : public AudioDecoder {
     NOTREACHED();
   }
 
+  // Mock accessors.
+  const base::TimeDelta& seek_time() const { return seek_time_; }
+
  private:
   virtual ~MockAudioDecoder() {}
 
   MediaFormat media_format_;
+  base::TimeDelta seek_time_;
 
   DISALLOW_COPY_AND_ASSIGN(MockAudioDecoder);
 };
 
-//------------------------------------------------------------------------------
 
 class MockAudioRenderer : public AudioRenderer {
  public:
-  static FilterFactory* CreateFactory(const MockFilterConfig* config) {
-    return new FilterFactoryImpl1<MockAudioRenderer,
-                                  const MockFilterConfig*>(config);
-  }
-
-  static bool IsMediaFormatSupported(const MediaFormat& media_format) {
-    return true;  // TODO(ralphl): check for a supported format
-  }
-
   explicit MockAudioRenderer(const MockFilterConfig* config) {}
 
   // Implementation of MediaFilter.
   virtual void Stop() {}
 
+  virtual void Seek(base::TimeDelta time) {
+    seek_time_ = time;
+  }
+
   // Implementation of AudioRenderer.
   virtual bool Initialize(AudioDecoder* decoder) {
     host_->InitializationComplete();
@@ -364,25 +360,20 @@ class MockAudioRenderer : public AudioRenderer {
 
   virtual void SetVolume(float volume) {}
 
+  // Mock accessors.
+  const base::TimeDelta& seek_time() const { return seek_time_; }
+
  private:
   virtual ~MockAudioRenderer() {}
 
+  base::TimeDelta seek_time_;
+
   DISALLOW_COPY_AND_ASSIGN(MockAudioRenderer);
 };
 
-//------------------------------------------------------------------------------
 
 class MockVideoDecoder : public VideoDecoder {
  public:
-  static FilterFactory* CreateFactory(const MockFilterConfig* config) {
-    return new FilterFactoryImpl1<MockVideoDecoder,
-                                  const MockFilterConfig*>(config);
-  }
-
-  static bool IsMediaFormatSupported(const MediaFormat& media_format) {
-    return true;  // TODO(ralphl): check for a supported format.
-  }
-
   // Helper function that initializes a YV12 frame with white and black scan
   // lines based on the |white_to_black| parameter.  If 0, then the entire
   // frame will be black, if 1 then the entire frame will be white.
@@ -423,6 +414,10 @@ class MockVideoDecoder : public VideoDecoder {
   // Implementation of MediaFilter.
   virtual void Stop() {}
 
+  virtual void Seek(base::TimeDelta time) {
+    seek_time_ = time;
+  }
+
   // Implementation of VideoDecoder.
   virtual bool Initialize(DemuxerStream* stream) {
     host_->InitializationComplete();
@@ -438,6 +433,9 @@ class MockVideoDecoder : public VideoDecoder {
         this, &MockVideoDecoder::DoRead, callback));
   }
 
+  // Mock accessors.
+  const base::TimeDelta& seek_time() const { return seek_time_; }
+
  private:
   virtual ~MockVideoDecoder() {}
 
@@ -471,24 +469,15 @@ class MockVideoDecoder : public VideoDecoder {
 
   MediaFormat media_format_;
   base::TimeDelta mock_frame_time_;
+  base::TimeDelta seek_time_;
   const MockFilterConfig* config_;
 
   DISALLOW_COPY_AND_ASSIGN(MockVideoDecoder);
 };
 
-//------------------------------------------------------------------------------
 
 class MockVideoRenderer : public VideoRenderer {
  public:
-  static FilterFactory* CreateFactory(const MockFilterConfig* config) {
-    return new FilterFactoryImpl1<MockVideoRenderer,
-                                  const MockFilterConfig*>(config);
-  }
-
-  static bool IsMediaFormatSupported(const MediaFormat& media_format) {
-    return true;  // TODO(ralphl): check for a supported format
-  }
-
   explicit MockVideoRenderer(const MockFilterConfig* config)
       : config_(config) {
   }
@@ -496,6 +485,10 @@ class MockVideoRenderer : public VideoRenderer {
   // Implementation of MediaFilter.
   virtual void Stop() {}
 
+  virtual void Seek(base::TimeDelta time) {
+    seek_time_ = time;
+  }
+
   // Implementation of VideoRenderer.
   virtual bool Initialize(VideoDecoder* decoder) {
     host_->SetVideoSize(config_->video_width, config_->video_height);
@@ -503,19 +496,94 @@ class MockVideoRenderer : public VideoRenderer {
     return true;
   }
 
+  // Mock accessors.
+  const base::TimeDelta& seek_time() const { return seek_time_; }
+
  private:
   virtual ~MockVideoRenderer() {}
 
+  base::TimeDelta seek_time_;
   const MockFilterConfig* config_;
 
   DISALLOW_COPY_AND_ASSIGN(MockVideoRenderer);
 };
 
 
-//------------------------------------------------------------------------------
+// FilterFactory capable of creating each mock filter type.  Only one instance
+// of each filter type can exist at any time.  Filters can be inspected for
+// expectations using the accessors, which may return NULL if the filter was
+// never created (i.e., streams containing no video).
+class MockFilterFactory : public FilterFactory {
+ public:
+  explicit MockFilterFactory(const MockFilterConfig* config)
+      : config_(config) {
+  }
+
+  // Mock accessors.
+  MockDataSource* data_source() const { return data_source_; }
+  MockDemuxer* demuxer() const { return demuxer_; }
+  MockAudioDecoder* audio_decoder() const { return audio_decoder_; }
+  MockVideoDecoder* video_decoder() const { return video_decoder_; }
+  MockAudioRenderer* audio_renderer() const { return audio_renderer_; }
+  MockVideoRenderer* video_renderer() const { return video_renderer_; }
+
+ protected:
+  MediaFilter* Create(FilterType filter_type, const MediaFormat& media_format) {
+    if (!config_->create_filter)
+      return NULL;
+
+    switch (filter_type) {
+      case FILTER_DATA_SOURCE:
+        DCHECK(!data_source_);
+        data_source_ = new MockDataSource(config_);
+        return data_source_;
+
+      case FILTER_DEMUXER:
+        DCHECK(!demuxer_);
+        demuxer_ = new MockDemuxer(config_);
+        return demuxer_;
+
+      case FILTER_AUDIO_DECODER:
+        DCHECK(!audio_decoder_);
+        audio_decoder_ = new MockAudioDecoder(config_);
+        return audio_decoder_;
+
+      case FILTER_VIDEO_DECODER:
+        DCHECK(!video_decoder_);
+        video_decoder_ = new MockVideoDecoder(config_);
+        return video_decoder_;
+
+      case FILTER_AUDIO_RENDERER:
+        DCHECK(!audio_renderer_);
+        audio_renderer_ = new MockAudioRenderer(config_);
+        return audio_renderer_;
+
+      case FILTER_VIDEO_RENDERER:
+        DCHECK(!video_renderer_);
+        video_renderer_ = new MockVideoRenderer(config_);
+        return video_renderer_;
+
+      default:
+        NOTREACHED();
+    }
+    return NULL;
+  }
+
+ private:
+  const MockFilterConfig* config_;
+  scoped_refptr<MockDataSource> data_source_;
+  scoped_refptr<MockDemuxer> demuxer_;
+  scoped_refptr<MockAudioDecoder> audio_decoder_;
+  scoped_refptr<MockVideoDecoder> video_decoder_;
+  scoped_refptr<MockAudioRenderer> audio_renderer_;
+  scoped_refptr<MockVideoRenderer> video_renderer_;
+
+  DISALLOW_COPY_AND_ASSIGN(MockFilterFactory);
+};
+
 // A simple class that waits for a pipeline to be started and checks some
 // basic initialization values.  The Start() method will not return until
-// either a pre-dermined amount of time has passed or the pipeline calls the
+// either a pre-determined amount of time has passed or the pipeline calls the
 // InitCallback() callback.  A typical use would be:
 //   Pipeline p;
 //   FilterFactoryCollection f;
@@ -531,6 +599,8 @@ class MockVideoRenderer : public VideoRenderer {
 // If the test expects the pipeline to hang during initialization (a filter
 // never calls FilterHost::InitializationComplete()) then the use would be:
 //   h.Start(&p, f, uri, PIPELINE_OK, true);
+//
+// TODO(scherkus): Keep refactoring tests until we can remove this entirely.
 class InitializationHelper {
  public:
   InitializationHelper()
@@ -543,7 +613,7 @@ class InitializationHelper {
   bool callback_success_status() { return callback_success_status_; }
 
   // Returns true if Start has been called, but the pipeline has not yet
-  // called the intialization complete callback.
+  // called the initialization complete callback.
   bool waiting_for_callback() { return waiting_for_callback_; }
 
   // Starts the pipeline, providing an initialization callback that points
diff --git a/media/base/mock_pipeline.h b/media/base/mock_pipeline.h
index e317a5d..dd31ca6 100644
--- a/media/base/mock_pipeline.h
+++ b/media/base/mock_pipeline.h
@@ -82,7 +82,7 @@ class MockPipeline : public media::Pipeline {
   // Implementation of Pipeline interface.
   virtual bool Start(FilterFactory* filter_factory,
                      const std::string& url,
-                     Callback1<bool>::Type* init_complete_callback) {
+                     PipelineCallback* init_complete_callback) {
     EXPECT_FALSE(initialized_);
     initialized_ = true;
     if (init_complete_callback) {
@@ -101,7 +101,8 @@ class MockPipeline : public media::Pipeline {
     playback_rate_ = playback_rate;
   }
 
-  virtual void Seek(base::TimeDelta time) {
+  virtual void Seek(base::TimeDelta time,
+                    PipelineCallback* seek_complete_callback) {
     time_ = time;
   }
 
diff --git a/media/base/pipeline.h b/media/base/pipeline.h
index 3b18b81..0120863 100644
--- a/media/base/pipeline.h
+++ b/media/base/pipeline.h
@@ -116,6 +116,11 @@ class PipelineStatus {
   virtual ~PipelineStatus() {}
 };
 
+// Client-provided callbacks for various pipeline operations.
+//
+// TODO(scherkus): consider returning a PipelineError instead of a bool, or
+// perhaps a client callback interface.
+typedef Callback1<bool>::Type PipelineCallback;
 
 class Pipeline : public PipelineStatus {
  public:
@@ -123,28 +128,28 @@ class Pipeline : public PipelineStatus {
   // construct a filter chain.  Returns true if successful, false otherwise
   // (i.e., pipeline already started).  Note that a return value of true
   // only indicates that the initialization process has started successfully.
-  // Pipeline initializaion is an inherently asynchronous process.  Clients
-  // should not call SetPlaybackRate, Seek, or SetVolume until initialization
-  // is complete.  Clients can either poll the IsInitialized() method (which is
-  // discouraged) or use the init_complete_callback as described below.
+  // Pipeline initialization is an inherently asynchronous process.  Clients
+  // should not call SetPlaybackRate(), Seek(), or SetVolume() until
+  // initialization is complete.  Clients can either poll the IsInitialized()
+  // method (which is discouraged) or use the |start_callback| as described
+  // below.
   //
   // This method is asynchronous and can execute a callback when completed.
-  // If the caller provides an init_complete_callback, it will be
-  // called when the pipeline initiailization completes.  If successful, the
-  // callback's bool parameter will be true.  If the callback is called with
-  // false, then the client can use the GetError method to obtain more
-  // information about the reason initialization failed.  The prototype for
-  // the client callback is:
+  // If the caller provides a |start_callback|, it will be called when the
+  // pipeline initialization completes.  If successful, the callback's bool
+  // parameter will be true.  If the callback is called with false, then the
+  // client can use the GetError() method to obtain more information about the
+  // reason initialization failed.  The prototype for the client callback is:
   //    void Client::PipelineInitComplete(bool init_was_successful);
   //
   // Note that clients must not call the Stop method from within the
-  // init_complete_callback.  Other methods, including SetPlaybackRate,
-  // Seek, and SetVolume may be called.  The client will be called on a
-  // thread owned by the pipeline class, not on the thread that originally
-  // called the Start method.
+  // |start_callback|.  Other methods, including SetPlaybackRate(), Seek(), and
+  // SetVolume() may be called.  The client will be called on a thread owned by
+  // the pipeline class, not on the thread that originally called the Start()
+  // method.
   virtual bool Start(FilterFactory* filter_factory,
                      const std::string& uri,
-                     Callback1<bool>::Type* init_complete_callback) = 0;
+                     PipelineCallback* start_callback) = 0;
 
   // Stops the pipeline and resets to an uninitialized state.  This method
   // will block the calling thread until the pipeline has been completely
@@ -158,15 +163,19 @@ class Pipeline : public PipelineStatus {
   // Attempt to adjust the playback rate. Setting a playback rate of 0.0f pauses
   // all rendering of the media.  A rate of 1.0f indicates a normal playback
   // rate.  Values for the playback rate must be greater than or equal to 0.0f.
-  // TODO(ralphl) What about maximum rate?  Does HTML5 specify a max?
+  // TODO(ralphl): What about maximum rate?  Does HTML5 specify a max?
   //
   // This method must be called only after initialization has completed.
   virtual void SetPlaybackRate(float playback_rate) = 0;
 
-  // Attempt to seek to the position in microseconds.
+  // 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.
+  // The callback will return true if the seek was carried out, false otherwise
+  // (i.e., streaming media).
   //
   // This method must be called only after initialization has completed.
-  virtual void Seek(base::TimeDelta time) = 0;
+  virtual void Seek(base::TimeDelta time,
+                    PipelineCallback* seek_callback) = 0;
 
   // Attempt to set the volume of the audio renderer.  Valid values for volume
   // range from 0.0f (muted) to 1.0f (full volume).  This value affects all
diff --git a/media/base/pipeline_impl.cc b/media/base/pipeline_impl.cc
index a4c85d5..88c3537 100644
--- a/media/base/pipeline_impl.cc
+++ b/media/base/pipeline_impl.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/compiler_specific.h"
+#include "base/stl_util-inl.h"
 #include "media/base/filter_host_impl.h"
 #include "media/base/media_format.h"
 #include "media/base/pipeline_impl.h"
@@ -124,10 +125,11 @@ bool PipelineImpl::InternalSetError(PipelineError error) {
 // Creates the PipelineThread and calls it's start method.
 bool PipelineImpl::Start(FilterFactory* factory,
                          const std::string& url,
-                         Callback1<bool>::Type* init_complete_callback) {
+                         PipelineCallback* init_complete_callback) {
   DCHECK(!pipeline_thread_);
   DCHECK(factory);
   DCHECK(!initialized_);
+  DCHECK(!IsPipelineThread());
   if (!pipeline_thread_ && factory) {
     pipeline_thread_ = new PipelineThread(this);
     if (pipeline_thread_) {
@@ -146,16 +148,18 @@ bool PipelineImpl::Start(FilterFactory* factory,
 // Stop the PipelineThread and return to a state identical to that of a newly
 // created PipelineImpl object.
 void PipelineImpl::Stop() {
+  DCHECK(!IsPipelineThread());
+
   if (pipeline_thread_) {
     pipeline_thread_->Stop();
   }
   ResetState();
 }
 
-
-
 void PipelineImpl::SetPlaybackRate(float rate) {
-  if (OkToCallThread() && rate >= 0.0f) {
+  DCHECK(!IsPipelineThread());
+
+  if (IsPipelineOk() && rate >= 0.0f) {
     pipeline_thread_->SetPlaybackRate(rate);
   } else {
     // It's OK for a client to call SetPlaybackRate(0.0f) if we're stopped.
@@ -163,16 +167,21 @@ void PipelineImpl::SetPlaybackRate(float rate) {
   }
 }
 
-void PipelineImpl::Seek(base::TimeDelta time) {
-  if (OkToCallThread()) {
-    pipeline_thread_->Seek(time);
+void PipelineImpl::Seek(base::TimeDelta time,
+                        PipelineCallback* seek_callback) {
+  DCHECK(!IsPipelineThread());
+
+  if (IsPipelineOk()) {
+    pipeline_thread_->Seek(time, seek_callback);
   } else {
     NOTREACHED();
   }
 }
 
 void PipelineImpl::SetVolume(float volume) {
-  if (OkToCallThread() && volume >= 0.0f && volume <= 1.0f) {
+  DCHECK(!IsPipelineThread());
+
+  if (IsPipelineOk() && volume >= 0.0f && volume <= 1.0f) {
     pipeline_thread_->SetVolume(volume);
   } else {
     NOTREACHED();
@@ -197,6 +206,15 @@ void PipelineImpl::ResetState() {
   rendered_mime_types_.clear();
 }
 
+bool PipelineImpl::IsPipelineOk() const {
+  return pipeline_thread_ && initialized_ && PIPELINE_OK == error_;
+}
+
+bool PipelineImpl::IsPipelineThread() const {
+  return pipeline_thread_ &&
+      PlatformThread::CurrentId() == pipeline_thread_->thread_id();
+}
+
 void PipelineImpl::SetDuration(base::TimeDelta duration) {
   AutoLock auto_lock(lock_);
   duration_ = duration;
@@ -247,7 +265,7 @@ PipelineThread::~PipelineThread() {
 // thread.
 bool PipelineThread::Start(FilterFactory* filter_factory,
                            const std::string& url,
-                           Callback1<bool>::Type* init_complete_callback) {
+                           PipelineCallback* init_complete_callback) {
   if (thread_.Start()) {
     filter_factory->AddRef();
     PostTask(NewRunnableMethod(this,
@@ -284,8 +302,10 @@ void PipelineThread::SetPlaybackRate(float rate) {
 }
 
 // Called on client's thread.
-void PipelineThread::Seek(base::TimeDelta time) {
-  PostTask(NewRunnableMethod(this, &PipelineThread::SeekTask, time));
+void PipelineThread::Seek(base::TimeDelta time,
+                          PipelineCallback* seek_callback) {
+  PostTask(NewRunnableMethod(this, &PipelineThread::SeekTask, time,
+                             seek_callback));
 }
 
 // Called on client's thread.
@@ -355,7 +375,7 @@ void PipelineThread::PostTask(Task* task) {
 // handled by the CreateFilter method.
 void PipelineThread::StartTask(FilterFactory* filter_factory,
                                const std::string& url,
-                               Callback1<bool>::Type* init_complete_callback) {
+                               PipelineCallback* init_complete_callback) {
   // During the entire StartTask we hold the initialization_lock_ so that
   // if the client calls the Pipeline::Stop method while we are running a
   // nested message loop, we can correctly unwind out of it before calling
@@ -402,10 +422,11 @@ void PipelineThread::StopTask() {
   if (PipelineOk()) {
     pipeline_->error_ = PIPELINE_STOPPING;
   }
-  FilterHostVector::iterator iter = filter_hosts_.begin();
-  while (iter != filter_hosts_.end()) {
+
+  for (FilterHostVector::iterator iter = filter_hosts_.begin();
+       iter != filter_hosts_.end();
+       ++iter) {
     (*iter)->Stop();
-    ++iter;
   }
   if (host_initializing_) {
     host_initializing_ = NULL;
@@ -453,18 +474,23 @@ void PipelineThread::InitializationCompleteTask(FilterHostImpl* host) {
 
 void PipelineThread::SetPlaybackRateTask(float rate) {
   pipeline_->InternalSetPlaybackRate(rate);
-  FilterHostVector::iterator iter = filter_hosts_.begin();
-  while (iter != filter_hosts_.end()) {
+  for (FilterHostVector::iterator iter = filter_hosts_.begin();
+       iter != filter_hosts_.end();
+       ++iter) {
     (*iter)->media_filter()->SetPlaybackRate(rate);
-    ++iter;
   }
 }
 
-void PipelineThread::SeekTask(base::TimeDelta time) {
-  FilterHostVector::iterator iter = filter_hosts_.begin();
-  while (iter != filter_hosts_.end()) {
+void PipelineThread::SeekTask(base::TimeDelta time,
+                              PipelineCallback* seek_callback) {
+  for (FilterHostVector::iterator iter = filter_hosts_.begin();
+       iter != filter_hosts_.end();
+       ++iter) {
     (*iter)->media_filter()->Seek(time);
-    ++iter;
+  }
+  if (seek_callback) {
+    seek_callback->Run(true);
+    delete seek_callback;
   }
 }
 
@@ -479,10 +505,10 @@ void PipelineThread::SetVolumeTask(float volume) {
 
 void PipelineThread::SetTimeTask() {
   time_update_callback_scheduled_ = false;
-  FilterHostVector::iterator iter = filter_hosts_.begin();
-  while (iter != filter_hosts_.end()) {
+  for (FilterHostVector::iterator iter = filter_hosts_.begin();
+       iter != filter_hosts_.end();
+       ++iter) {
     (*iter)->RunTimeUpdateCallback(pipeline_->time_);
-    ++iter;
   }
 }
 
@@ -554,10 +580,7 @@ scoped_refptr<DataSource> PipelineThread::CreateDataSource(
 
 // Called as a result of destruction of the thread.
 void PipelineThread::WillDestroyCurrentMessageLoop() {
-  while (!filter_hosts_.empty()) {
-    delete filter_hosts_.back();
-    filter_hosts_.pop_back();
-  }
+  STLDeleteElements(&filter_hosts_);
 }
 
 }  // namespace media
diff --git a/media/base/pipeline_impl.h b/media/base/pipeline_impl.h
index c4c4393..ad57444 100644
--- a/media/base/pipeline_impl.h
+++ b/media/base/pipeline_impl.h
@@ -46,29 +46,32 @@ class PipelineImpl : public Pipeline {
   virtual PipelineError GetError() const;
   virtual bool IsRendered(const std::string& major_mime_type) const;
 
-  // Impementation of Pipeline methods.
+  // Implementation of Pipeline methods.
   virtual bool Start(FilterFactory* filter_factory,
                      const std::string& url,
-                     Callback1<bool>::Type* init_complete_callback);
+                     PipelineCallback* start_callback);
   virtual void Stop();
   virtual void SetPlaybackRate(float rate);
-  virtual void Seek(base::TimeDelta time);
+  virtual void Seek(base::TimeDelta time,
+                    PipelineCallback* seek_callback);
   virtual void SetVolume(float volume);
 
  private:
   friend class FilterHostImpl;
   friend class PipelineThread;
 
-  // Reset the state of the pipline object to the initial state.  This method
+  // Reset the state of the pipeline object to the initial state.  This method
   // is used by the constructor, and the Stop method.
   void ResetState();
 
   // Used internally to make sure that the thread is in a state that is
   // acceptable to post a task to.  It must exist, be initialized, and there
   // must not be an error.
-  bool OkToCallThread() const {
-    return (pipeline_thread_ && initialized_ && PIPELINE_OK == error_);
-  }
+  bool IsPipelineOk() const;
+
+  // Returns true if we're currently executing on the pipeline thread.  Mostly
+  // used in DCHECKs.
+  bool IsPipelineThread() const;
 
   // Methods called by FilterHostImpl to update pipeline state.
   void SetDuration(base::TimeDelta duration);
@@ -170,10 +173,10 @@ class PipelineThread : public base::RefCountedThreadSafe<PipelineThread>,
 
   bool Start(FilterFactory* filter_factory,
              const std::string& url_media_source,
-             Callback1<bool>::Type* init_complete_callback);
+             PipelineCallback* init_complete_callback);
   void Stop();
   void SetPlaybackRate(float rate);
-  void Seek(base::TimeDelta time);
+  void Seek(base::TimeDelta time, PipelineCallback* seek_callback);
   void SetVolume(float volume);
 
   // Methods called by a FilterHostImpl object.  These methods may be called
@@ -203,6 +206,10 @@ class PipelineThread : public base::RefCountedThreadSafe<PipelineThread>,
   // Accessor used to post messages to thread's message loop.
   MessageLoop* message_loop() const { return thread_.message_loop(); }
 
+  // Accessor used by PipelineImpl to check if we're executing on the pipeline
+  // thread.
+  PlatformThreadId thread_id() const { return thread_.thread_id(); }
+
  private:
   // Implementation of MessageLoop::DestructionObserver.  StartTask registers
   // this class as a destruction observer on the thread's message loop.
@@ -223,10 +230,10 @@ class PipelineThread : public base::RefCountedThreadSafe<PipelineThread>,
   // StartTask message on the pipeline thread.
   void StartTask(FilterFactory* filter_factory,
                  const std::string& url,
-                 Callback1<bool>::Type* init_complete_callback);
+                 PipelineCallback* init_complete_callback);
   void StopTask();
   void SetPlaybackRateTask(float rate);
-  void SeekTask(base::TimeDelta time);
+  void SeekTask(base::TimeDelta time, PipelineCallback* seek_callback);
   void SetVolumeTask(float volume);
   void SetTimeTask();
   void InitializationCompleteTask(FilterHostImpl* FilterHost);
diff --git a/media/base/pipeline_impl_unittest.cc b/media/base/pipeline_impl_unittest.cc
index 8e300a1..adfc3a6 100644
--- a/media/base/pipeline_impl_unittest.cc
+++ b/media/base/pipeline_impl_unittest.cc
@@ -13,140 +13,194 @@
 #include "media/base/mock_media_filters.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-using media::FilterFactory;
-using media::FilterFactoryCollection;
-using media::FilterHost;
-using media::InitializationHelper;
-using media::MediaFormat;
-using media::MockAudioDecoder;
-using media::MockAudioRenderer;
-using media::MockDataSource;
-using media::MockDemuxer;
-using media::MockFilterConfig;
-using media::MockVideoDecoder;
-using media::MockVideoRenderer;
-using media::PipelineImpl;
-
 namespace {
 
-void AddAllMockFilters(FilterFactoryCollection* factories,
-                       const MockFilterConfig* config) {
-  factories->AddFactory(MockDataSource::CreateFactory(config));
-  factories->AddFactory(MockDemuxer::CreateFactory(config));
-  factories->AddFactory(MockAudioDecoder::CreateFactory(config));
-  factories->AddFactory(MockAudioRenderer::CreateFactory(config));
-  factories->AddFactory(MockVideoDecoder::CreateFactory(config));
-  factories->AddFactory(MockVideoRenderer::CreateFactory(config));
+class PipelineImplTest : public testing::Test {
+ protected:
+  PipelineImplTest()
+      : initialize_result_(false),
+        seek_result_(false),
+        initialize_event_(false, false),
+        seek_event_(false, false) {
+  }
+
+  virtual ~PipelineImplTest() {}
+
+  virtual void TearDown() {
+    // Force the pipeline to shut down its thread.
+    pipeline_.Stop();
+  }
+
+  // Called by tests after they have finished setting up MockFilterConfig.
+  // Initializes the pipeline and returns true if the initialization callback
+  // was executed, false otherwise.
+  bool InitializeAndWait() {
+    DCHECK(!filters_);
+    filters_ = new media::MockFilterFactory(&config_);
+    pipeline_.Start(filters_, "",
+                    NewCallback(this, &PipelineImplTest::OnInitialize));
+    return initialize_event_.TimedWait(base::TimeDelta::FromMilliseconds(500));
+  }
+
+  // Issues a seek on the pipeline and returns true if the seek callback was
+  // executed, false otherwise.
+  bool SeekAndWait(const base::TimeDelta& time) {
+    pipeline_.Seek(time, NewCallback(this, &PipelineImplTest::OnSeek));
+    return seek_event_.TimedWait(base::TimeDelta::FromMilliseconds(500));
+  }
+
+  // Fixture members.
+  media::PipelineImpl pipeline_;
+  scoped_refptr<media::MockFilterFactory> filters_;
+  media::MockFilterConfig config_;
+  bool initialize_result_;
+  bool seek_result_;
+
+ private:
+  void OnInitialize(bool result) {
+    initialize_result_ = result;
+    initialize_event_.Signal();
+  }
+
+  void OnSeek(bool result) {
+    seek_result_ = result;
+    seek_event_.Signal();
+  }
+
+  // Used to wait for callbacks.
+  base::WaitableEvent initialize_event_;
+  base::WaitableEvent seek_event_;
+
+  DISALLOW_COPY_AND_ASSIGN(PipelineImplTest);
+};
+
+TEST_F(PipelineImplTest, NeverInitializes) {
+  config_.data_source_behavior = media::MOCK_DATA_SOURCE_NEVER_INIT;
+
+  // This test hangs during initialization by never calling
+  // InitializationComplete().  Make sure we tear down the pipeline properly.
+  ASSERT_FALSE(InitializeAndWait());
+  EXPECT_FALSE(initialize_result_);
+  EXPECT_FALSE(pipeline_.IsInitialized());
+  EXPECT_EQ(media::PIPELINE_OK, pipeline_.GetError());
 }
 
-}  // namespace
+TEST_F(PipelineImplTest, RequiredFilterMissing) {
+  config_.create_filter = false;
+
+  ASSERT_TRUE(InitializeAndWait());
+  EXPECT_FALSE(initialize_result_);
+  EXPECT_FALSE(pipeline_.IsInitialized());
+  EXPECT_EQ(media::PIPELINE_ERROR_REQUIRED_FILTER_MISSING,
+            pipeline_.GetError());
+}
+
+TEST_F(PipelineImplTest, URLNotFound) {
+  config_.data_source_behavior = media::MOCK_DATA_SOURCE_URL_ERROR_IN_INIT;
 
-// TODO(ralphl): Get rid of single character variable names in these tests.
-TEST(PipelineImplTest, Initialization) {
-  std::string u("");
-
-  // This test hangs during initialization of the data source (it never
-  // calls InitializationComplete).  Make sure we tear down the pipeline
-  // propertly.
-  PipelineImpl p;
-  InitializationHelper h;
-  MockFilterConfig config;
-  config.data_source_behavior = media::MOCK_DATA_SOURCE_NEVER_INIT;
-  h.Start(&p, MockDataSource::CreateFactory(&config), u,
-          media::PIPELINE_OK, true);
-  p.Stop();
-  EXPECT_FALSE(h.waiting_for_callback());
-  EXPECT_FALSE(h.callback_success_status());
-  EXPECT_TRUE(media::PIPELINE_OK == p.GetError());
-
-  // This test should not hang.  Should return an error indicating that we are
-  // missing a requried filter.
-  config.data_source_behavior = media::MOCK_DATA_SOURCE_TASK_INIT;
-  h.Start(&p, MockDataSource::CreateFactory(&config), u,
-          media::PIPELINE_ERROR_REQUIRED_FILTER_MISSING);
-  p.Stop();
-
-  // This test should return a specific error from the mock data source.
-  config.data_source_behavior = media::MOCK_DATA_SOURCE_URL_ERROR_IN_INIT;
-  h.Start(&p, MockDataSource::CreateFactory(&config), u,
-          media::PIPELINE_ERROR_URL_NOT_FOUND);
-  p.Stop();
+  ASSERT_TRUE(InitializeAndWait());
+  EXPECT_FALSE(initialize_result_);
+  EXPECT_FALSE(pipeline_.IsInitialized());
+  EXPECT_EQ(media::PIPELINE_ERROR_URL_NOT_FOUND, pipeline_.GetError());
 }
 
-TEST(PipelineImplTest, MockAudioPipeline) {
-  std::string url("");
-  PipelineImpl p;
-  MockFilterConfig config;
-  config.has_video = false;
-  scoped_refptr<FilterFactoryCollection> c = new FilterFactoryCollection();
-  AddAllMockFilters(c, &config);
-  InitializationHelper h;
-  h.Start(&p, c, url);
+TEST_F(PipelineImplTest, NoStreams) {
+  config_.has_audio = false;
+  config_.has_video = false;
+
+  ASSERT_TRUE(InitializeAndWait());
+  EXPECT_FALSE(initialize_result_);
+  EXPECT_FALSE(pipeline_.IsInitialized());
+  EXPECT_EQ(media::PIPELINE_ERROR_COULD_NOT_RENDER, pipeline_.GetError());
+
+  EXPECT_FALSE(filters_->audio_decoder());
+  EXPECT_FALSE(filters_->audio_renderer());
+  EXPECT_FALSE(filters_->video_decoder());
+  EXPECT_FALSE(filters_->video_renderer());
+}
+
+TEST_F(PipelineImplTest, AudioStream) {
+  config_.has_video = false;
+
+  ASSERT_TRUE(InitializeAndWait());
+  EXPECT_TRUE(initialize_result_);
+  EXPECT_TRUE(pipeline_.IsInitialized());
+  EXPECT_EQ(media::PIPELINE_OK, pipeline_.GetError());
+
   size_t width, height;
-  p.GetVideoSize(&width, &height);
-  EXPECT_TRUE(p.IsRendered(media::mime_type::kMajorTypeAudio));
-  EXPECT_FALSE(p.IsRendered(media::mime_type::kMajorTypeVideo));
+  pipeline_.GetVideoSize(&width, &height);
   EXPECT_EQ(0u, width);
   EXPECT_EQ(0u, height);
-  p.SetPlaybackRate(1.0f);
-  p.SetVolume(0.5f);
-  p.Stop();
-  EXPECT_FALSE(p.IsInitialized());
+  EXPECT_TRUE(pipeline_.IsRendered(media::mime_type::kMajorTypeAudio));
+  EXPECT_FALSE(pipeline_.IsRendered(media::mime_type::kMajorTypeVideo));
+
+  EXPECT_TRUE(filters_->audio_decoder());
+  EXPECT_TRUE(filters_->audio_renderer());
+  EXPECT_FALSE(filters_->video_decoder());
+  EXPECT_FALSE(filters_->video_renderer());
 }
 
-TEST(PipelineImplTest, MockVideoPipeline) {
-  std::string url("");
-  PipelineImpl p;
-  scoped_refptr<FilterFactoryCollection> c = new FilterFactoryCollection();
-  MockFilterConfig config;
-  AddAllMockFilters(c, &config);
-  InitializationHelper h;
-  h.Start(&p, c, url);
+TEST_F(PipelineImplTest, VideoStream) {
+  config_.has_audio = false;
+
+  ASSERT_TRUE(InitializeAndWait());
+  EXPECT_TRUE(initialize_result_);
+  EXPECT_TRUE(pipeline_.IsInitialized());
+  EXPECT_EQ(media::PIPELINE_OK, pipeline_.GetError());
+
   size_t width, height;
-  p.GetVideoSize(&width, &height);
-  EXPECT_EQ(config.video_width, width);
-  EXPECT_EQ(config.video_height, height);
-  EXPECT_TRUE(p.IsRendered(media::mime_type::kMajorTypeAudio));
-  EXPECT_TRUE(p.IsRendered(media::mime_type::kMajorTypeVideo));
-  p.SetPlaybackRate(1.0f);
-  p.SetVolume(0.5f);
-  p.Stop();
-  EXPECT_FALSE(p.IsInitialized());
+  pipeline_.GetVideoSize(&width, &height);
+  EXPECT_EQ(config_.video_width, width);
+  EXPECT_EQ(config_.video_height, height);
+  EXPECT_FALSE(pipeline_.IsRendered(media::mime_type::kMajorTypeAudio));
+  EXPECT_TRUE(pipeline_.IsRendered(media::mime_type::kMajorTypeVideo));
+
+  EXPECT_FALSE(filters_->audio_decoder());
+  EXPECT_FALSE(filters_->audio_renderer());
+  EXPECT_TRUE(filters_->video_decoder());
+  EXPECT_TRUE(filters_->video_renderer());
 }
 
-TEST(PipelineImplTest, MockVideoOnlyPipeline) {
-  std::string url("");
-  PipelineImpl p;
-  scoped_refptr<FilterFactoryCollection> c = new FilterFactoryCollection();
-  MockFilterConfig config;
-  config.has_audio = false;
-  AddAllMockFilters(c, &config);
-  InitializationHelper h;
-  h.Start(&p, c, url);
+TEST_F(PipelineImplTest, AudioVideoStream) {
+  ASSERT_TRUE(InitializeAndWait());
+  EXPECT_TRUE(initialize_result_);
+  EXPECT_TRUE(pipeline_.IsInitialized());
+  EXPECT_EQ(media::PIPELINE_OK, pipeline_.GetError());
+
   size_t width, height;
-  p.GetVideoSize(&width, &height);
-  EXPECT_EQ(config.video_width, width);
-  EXPECT_EQ(config.video_height, height);
-  EXPECT_FALSE(p.IsRendered(media::mime_type::kMajorTypeAudio));
-  EXPECT_TRUE(p.IsRendered(media::mime_type::kMajorTypeVideo));
-  p.SetPlaybackRate(1.0f);
-  p.Stop();
-  EXPECT_FALSE(p.IsInitialized());
+  pipeline_.GetVideoSize(&width, &height);
+  EXPECT_EQ(config_.video_width, width);
+  EXPECT_EQ(config_.video_height, height);
+  EXPECT_TRUE(pipeline_.IsRendered(media::mime_type::kMajorTypeAudio));
+  EXPECT_TRUE(pipeline_.IsRendered(media::mime_type::kMajorTypeVideo));
+
+  EXPECT_TRUE(filters_->audio_decoder());
+  EXPECT_TRUE(filters_->audio_renderer());
+  EXPECT_TRUE(filters_->video_decoder());
+  EXPECT_TRUE(filters_->video_renderer());
 }
 
-TEST(PipelineImplTest, MockNothingToRenderPipeline) {
-  std::string url("");
-  PipelineImpl p;
-  scoped_refptr<FilterFactoryCollection> c = new FilterFactoryCollection();
-  MockFilterConfig config;
-  config.has_audio = false;
-  config.has_video = false;
-  AddAllMockFilters(c, &config);
-  InitializationHelper h;
-  h.Start(&p, c, url, media::PIPELINE_ERROR_COULD_NOT_RENDER);
-  p.Stop();
+TEST_F(PipelineImplTest, Seek) {
+  ASSERT_TRUE(InitializeAndWait());
+
+  // Seek and verify callback returned true.
+  base::TimeDelta expected = base::TimeDelta::FromSeconds(2000);
+  EXPECT_TRUE(SeekAndWait(expected));
+  EXPECT_TRUE(seek_result_);
+
+  // Verify every filter received the seek.
+  // TODO(scherkus): implement whatever it takes so I can use EXPECT_EQ with
+  // base::TimeDelta.
+  EXPECT_TRUE(expected == filters_->data_source()->seek_time());
+  EXPECT_TRUE(expected == filters_->demuxer()->seek_time());
+  EXPECT_TRUE(expected == filters_->audio_decoder()->seek_time());
+  EXPECT_TRUE(expected == filters_->audio_renderer()->seek_time());
+  EXPECT_TRUE(expected == filters_->video_decoder()->seek_time());
+  EXPECT_TRUE(expected == filters_->video_renderer()->seek_time());
 }
 
 // TODO(ralphl): Add a unit test that makes sure that the mock audio filter
 // is actually called on a SetVolume() call to the pipeline.  I almost checked
 // in code that broke this, but all unit tests were passing.
+
+}  // namespace
diff --git a/media/filters/file_data_source_unittest.cc b/media/filters/file_data_source_unittest.cc
index 8512300..1d2f054 100644
--- a/media/filters/file_data_source_unittest.cc
+++ b/media/filters/file_data_source_unittest.cc
@@ -31,6 +31,7 @@ using media::MockDemuxer;
 using media::MockAudioDecoder;
 using media::MockAudioRenderer;
 using media::MockFilterConfig;
+using media::MockFilterFactory;
 using media::MockFilterHost;
 using media::MockPipeline;
 using media::PipelineImpl;
@@ -66,9 +67,7 @@ TEST(FileDataSourceTest, OpenFile) {
   config.has_video = false;
   scoped_refptr<FilterFactoryCollection> c = new FilterFactoryCollection();
   c->AddFactory(FileDataSource::CreateFactory());
-  c->AddFactory(MockDemuxer::CreateFactory(&config));
-  c->AddFactory(MockAudioDecoder::CreateFactory(&config));
-  c->AddFactory(MockAudioRenderer::CreateFactory(&config));
+  c->AddFactory(new MockFilterFactory(&config));
   InitializationHelper h;
   h.Start(&pipeline, c, TestFileURL());
   EXPECT_EQ(pipeline.GetTotalBytes(), 10);
diff --git a/media/filters/video_decoder_unittest.cc b/media/filters/video_decoder_unittest.cc
index d6bb9dc..d400d2f 100644
--- a/media/filters/video_decoder_unittest.cc
+++ b/media/filters/video_decoder_unittest.cc
@@ -20,6 +20,7 @@ using media::MockAudioRenderer;
 using media::MockDataSource;
 using media::MockDemuxer;
 using media::MockFilterConfig;
+using media::MockFilterFactory;
 using media::MockVideoRenderer;
 using media::PipelineImpl;
 using media::TestVideoDecoder;
@@ -31,15 +32,10 @@ TEST(VideoDecoder, CreateTestDecoder) {
   scoped_refptr<TestVideoDecoder> test_decoder = new TestVideoDecoder();
   MockFilterConfig config;
   scoped_refptr<FilterFactoryCollection> c = new FilterFactoryCollection();
-  c->AddFactory(MockDataSource::CreateFactory(&config));
-  c->AddFactory(MockDemuxer::CreateFactory(&config));
   c->AddFactory(new InstanceFilterFactory<TestVideoDecoder>(test_decoder));
-  c->AddFactory(MockAudioDecoder::CreateFactory(&config));
-  c->AddFactory(MockAudioRenderer::CreateFactory(&config));
-  c->AddFactory(MockVideoRenderer::CreateFactory(&config));
+  c->AddFactory(new MockFilterFactory(&config));
   media::InitializationHelper h;
   h.Start(&p, c, url);
   p.SetPlaybackRate(1.0f);
   p.Stop();
 }
-