From 9e166974e33f359467458af13987c3d87663c163 Mon Sep 17 00:00:00 2001
From: "scherkus@chromium.org"
 <scherkus@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Wed, 3 Nov 2010 05:40:13 +0000
Subject: [PATCH] Move MediaFilterCollection code into a class.

Refactored FilterType usage a bit to remove the need to manually associate
FilterType values to filter base classes.

Patch by acolwell@chromium.org:
http://codereview.chromium.org/4176006/show

BUG=60778
TEST=media_unittests

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@64885 0039d316-1c4b-4281-b951-d872f2087c98
---
 chrome/renderer/render_view.cc                |  17 +-
 media/base/filters.cc                         |  12 +-
 media/base/filters.h                          |  12 +-
 media/base/media_filter_collection.cc         |  41 ++++
 media/base/media_filter_collection.h          |  54 ++++++
 .../base/media_filter_collection_unittest.cc  |  99 ++++++++++
 media/base/mock_filters.h                     |  24 ++-
 media/base/pipeline.h                         |   3 +-
 media/base/pipeline_impl.cc                   | 178 ++++++++----------
 media/base/pipeline_impl.h                    |  18 +-
 media/base/pipeline_impl_unittest.cc          |   3 +-
 media/media.gyp                               |   3 +
 media/tools/player_wtl/movie.cc               |  18 +-
 media/tools/player_x11/player_x11.cc          |  21 ++-
 webkit/glue/webmediaplayer_impl.cc            |  24 +--
 webkit/glue/webmediaplayer_impl.h             |   4 +-
 webkit/support/webkit_support.cc              |   9 +-
 .../tools/test_shell/test_webview_delegate.cc |   9 +-
 18 files changed, 359 insertions(+), 190 deletions(-)
 create mode 100644 media/base/media_filter_collection.cc
 create mode 100644 media/base/media_filter_collection.h
 create mode 100644 media/base/media_filter_collection_unittest.cc

diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index f8eba644d4cc6..f918b5d96c5cc 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -2601,14 +2601,14 @@ WebSharedWorker* RenderView::createSharedWorker(
 
 WebMediaPlayer* RenderView::createMediaPlayer(
     WebFrame* frame, WebMediaPlayerClient* client) {
-  media::MediaFilterCollection collection;
+  scoped_ptr<media::MediaFilterCollection> collection(
+      new media::MediaFilterCollection());
 
   // Add in any custom filter factories first.
   const CommandLine* cmd_line = CommandLine::ForCurrentProcess();
   if (!cmd_line->HasSwitch(switches::kDisableAudio)) {
     // Add the chrome specific audio renderer.
-    collection.push_back(make_scoped_refptr(
-        new AudioRendererImpl(audio_message_filter())));
+    collection->AddFilter(new AudioRendererImpl(audio_message_filter()));
   }
 
   if (cmd_line->HasSwitch(switches::kEnableAcceleratedDecoding) &&
@@ -2619,8 +2619,8 @@ WebMediaPlayer* RenderView::createMediaPlayer(
     bool ret = frame->view()->graphicsContext3D()->makeContextCurrent();
     CHECK(ret) << "Failed to switch context";
 
-    collection.push_back(make_scoped_refptr(new IpcVideoDecoder(
-        MessageLoop::current(), ggl::GetCurrentContext())));
+    collection->AddFilter(new IpcVideoDecoder(
+        MessageLoop::current(), ggl::GetCurrentContext()));
   }
 
   WebApplicationCacheHostImpl* appcache_host =
@@ -2650,18 +2650,19 @@ WebMediaPlayer* RenderView::createMediaPlayer(
   if (cmd_line->HasSwitch(switches::kEnableVideoLayering)) {
     scoped_refptr<IPCVideoRenderer> renderer(
         new IPCVideoRenderer(routing_id_));
-    collection.push_back(renderer);
+    collection->AddFilter(renderer);
     video_renderer = renderer;
   } else {
     bool pts_logging = cmd_line->HasSwitch(switches::kEnableVideoLogging);
     scoped_refptr<webkit_glue::VideoRendererImpl> renderer(
         new webkit_glue::VideoRendererImpl(pts_logging));
-    collection.push_back(renderer);
+    collection->AddFilter(renderer);
     video_renderer = renderer;
   }
 
   return new webkit_glue::WebMediaPlayerImpl(
-      client, collection, bridge_factory_simple, bridge_factory_buffered,
+      client, collection.release(), bridge_factory_simple,
+      bridge_factory_buffered,
       cmd_line->HasSwitch(switches::kSimpleDataSource),video_renderer);
 }
 
diff --git a/media/base/filters.cc b/media/base/filters.cc
index 4c9ab4ff9aa32..fb8f66a7c141c 100644
--- a/media/base/filters.cc
+++ b/media/base/filters.cc
@@ -86,15 +86,15 @@ bool DataSource::IsUrlSupported(const std::string& url) {
 }
 
 FilterType DataSource::filter_type() const {
-  return FILTER_DATA_SOURCE;
+  return static_filter_type();
 }
 
 FilterType Demuxer::filter_type() const {
-  return FILTER_DEMUXER;
+  return static_filter_type();
 }
 
 FilterType AudioDecoder::filter_type() const {
-  return FILTER_AUDIO_DECODER;
+  return static_filter_type();
 }
 
 const char* AudioDecoder::major_mime_type() const {
@@ -102,7 +102,7 @@ const char* AudioDecoder::major_mime_type() const {
 }
 
 FilterType AudioRenderer::filter_type() const {
-  return FILTER_AUDIO_RENDERER;
+  return static_filter_type();
 }
 
 const char* AudioRenderer::major_mime_type() const {
@@ -110,7 +110,7 @@ const char* AudioRenderer::major_mime_type() const {
 }
 
 FilterType VideoDecoder::filter_type() const {
-  return FILTER_VIDEO_DECODER;
+  return static_filter_type();
 }
 
 const char* VideoDecoder::major_mime_type() const {
@@ -118,7 +118,7 @@ const char* VideoDecoder::major_mime_type() const {
 }
 
 FilterType VideoRenderer::filter_type() const {
-  return FILTER_VIDEO_RENDERER;
+  return static_filter_type();
 }
 
 const char* VideoRenderer::major_mime_type() const {
diff --git a/media/base/filters.h b/media/base/filters.h
index 02bc59602807f..f386111a165a6 100644
--- a/media/base/filters.h
+++ b/media/base/filters.h
@@ -24,7 +24,6 @@
 #define MEDIA_BASE_FILTERS_H_
 
 #include <limits>
-#include <list>
 #include <string>
 
 #include "base/callback.h"
@@ -60,11 +59,6 @@ enum FilterType {
 // Used for completing asynchronous methods.
 typedef Callback0::Type FilterCallback;
 
-// This is a list of MediaFilter objects. Objects in this list is used to
-// form a media playback pipeline. See src/media/base/pipeline.h for more
-// information.
-typedef std::list<scoped_refptr<MediaFilter> > MediaFilterCollection;
-
 class MediaFilter : public base::RefCountedThreadSafe<MediaFilter> {
  public:
   MediaFilter();
@@ -147,6 +141,7 @@ class DataSource : public MediaFilter {
 
   virtual bool IsUrlSupported(const std::string& url);
 
+  static FilterType static_filter_type() { return FILTER_DATA_SOURCE; }
   virtual FilterType filter_type() const;
 
   // Initialize a DataSource for the given URL, executing the callback upon
@@ -173,6 +168,7 @@ class DataSource : public MediaFilter {
 
 class Demuxer : public MediaFilter {
  public:
+  static FilterType static_filter_type() { return FILTER_DEMUXER; }
   virtual FilterType filter_type() const;
 
   // Initialize a Demuxer with the given DataSource, executing the callback upon
@@ -228,6 +224,7 @@ class DemuxerStream : public base::RefCountedThreadSafe<DemuxerStream> {
 
 class VideoDecoder : public MediaFilter {
  public:
+  static FilterType static_filter_type() { return FILTER_VIDEO_DECODER; }
   virtual FilterType filter_type() const;
 
   virtual const char* major_mime_type() const;
@@ -272,6 +269,7 @@ class VideoDecoder : public MediaFilter {
 
 class AudioDecoder : public MediaFilter {
  public:
+  static FilterType static_filter_type() { return FILTER_AUDIO_DECODER; }
   virtual FilterType filter_type() const;
 
   virtual const char* major_mime_type() const;
@@ -311,6 +309,7 @@ class AudioDecoder : public MediaFilter {
 
 class VideoRenderer : public MediaFilter {
  public:
+  static FilterType static_filter_type() { return FILTER_VIDEO_RENDERER; }
   virtual FilterType filter_type() const;
 
   virtual const char* major_mime_type() const;
@@ -327,6 +326,7 @@ class VideoRenderer : public MediaFilter {
 
 class AudioRenderer : public MediaFilter {
  public:
+  static FilterType static_filter_type() { return FILTER_AUDIO_RENDERER; }
   virtual FilterType filter_type() const;
 
   virtual const char* major_mime_type() const;
diff --git a/media/base/media_filter_collection.cc b/media/base/media_filter_collection.cc
new file mode 100644
index 0000000000000..2c90412761516
--- /dev/null
+++ b/media/base/media_filter_collection.cc
@@ -0,0 +1,41 @@
+// Copyright (c) 2010 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/media_filter_collection.h"
+
+namespace media {
+
+MediaFilterCollection::MediaFilterCollection() {
+}
+
+void MediaFilterCollection::AddFilter(MediaFilter* filter) {
+  filters_.push_back(filter);
+}
+
+bool MediaFilterCollection::IsEmpty() const {
+  return filters_.empty();
+}
+
+void MediaFilterCollection::Clear() {
+  filters_.clear();
+}
+
+void MediaFilterCollection::SelectFilter(
+    FilterType filter_type,
+    scoped_refptr<MediaFilter>* filter_out)  {
+
+  std::list<scoped_refptr<MediaFilter> >::iterator it = filters_.begin();
+  while (it != filters_.end()) {
+    if ((*it)->filter_type() == filter_type)
+      break;
+    ++it;
+  }
+
+  if (it != filters_.end()) {
+    *filter_out = it->get();
+    filters_.erase(it);
+  }
+}
+
+}  // namespace media
diff --git a/media/base/media_filter_collection.h b/media/base/media_filter_collection.h
new file mode 100644
index 0000000000000..31ea69c315dfc
--- /dev/null
+++ b/media/base/media_filter_collection.h
@@ -0,0 +1,54 @@
+// Copyright (c) 2010 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.
+
+#ifndef MEDIA_BASE_MEDIA_FILTER_COLLECTION_H_
+#define MEDIA_BASE_MEDIA_FILTER_COLLECTION_H_
+
+#include <list>
+
+#include "base/ref_counted.h"
+#include "media/base/filters.h"
+
+namespace media {
+
+// This is a collection of MediaFilter objects used to form a media playback
+// pipeline. See src/media/base/pipeline.h for more information.
+class MediaFilterCollection {
+ public:
+  MediaFilterCollection();
+
+  // Adds a filter to the collection.
+  void AddFilter(MediaFilter* filter);
+
+  // Is the collection empty?
+  bool IsEmpty() const;
+
+  // Remove remaining filters.
+  void Clear();
+
+  // Selects a filter of the specified type from the collection.
+  // If the required filter cannot be found, NULL is returned.
+  // If a filter is returned it is removed from the collection.
+  template <class Filter>
+  void SelectFilter(scoped_refptr<Filter>* filter_out) {
+    scoped_refptr<MediaFilter> filter;
+    SelectFilter(Filter::static_filter_type(), &filter);
+    *filter_out = reinterpret_cast<Filter*>(filter.get());
+  }
+
+ private:
+  // List of filters managed by this collection.
+  std::list<scoped_refptr<MediaFilter> > filters_;
+
+  // Helper function that searches the filters list for a specific
+  // filter type.
+  void SelectFilter(FilterType filter_type,
+                    scoped_refptr<MediaFilter>* filter_out);
+
+  DISALLOW_COPY_AND_ASSIGN(MediaFilterCollection);
+};
+
+}  // namespace media
+
+#endif  // MEDIA_BASE_MEDIA_FILTER_COLLECTION_H_
diff --git a/media/base/media_filter_collection_unittest.cc b/media/base/media_filter_collection_unittest.cc
new file mode 100644
index 0000000000000..d974b4fda17bf
--- /dev/null
+++ b/media/base/media_filter_collection_unittest.cc
@@ -0,0 +1,99 @@
+// Copyright (c) 2010 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/media_filter_collection.h"
+#include "media/base/mock_filters.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace media {
+
+class MediaFilterCollectionTest : public ::testing::Test {
+ public:
+  MediaFilterCollectionTest() {}
+  virtual ~MediaFilterCollectionTest() {}
+
+ protected:
+  MediaFilterCollection collection_;
+  MockFilterCollection mock_filters_;
+
+  DISALLOW_COPY_AND_ASSIGN(MediaFilterCollectionTest);
+};
+
+TEST_F(MediaFilterCollectionTest, TestIsEmptyAndClear) {
+  EXPECT_TRUE(collection_.IsEmpty());
+
+  collection_.AddFilter(mock_filters_.data_source());
+
+  EXPECT_FALSE(collection_.IsEmpty());
+
+  collection_.Clear();
+
+  EXPECT_TRUE(collection_.IsEmpty());
+}
+
+TEST_F(MediaFilterCollectionTest, SelectFilter) {
+  scoped_refptr<AudioDecoder> audio_decoder;
+  scoped_refptr<DataSource> data_source;
+
+  collection_.AddFilter(mock_filters_.data_source());
+  EXPECT_FALSE(collection_.IsEmpty());
+
+  // Verify that the data source will not be returned if we
+  // ask for a different type.
+  collection_.SelectFilter(&audio_decoder);
+  EXPECT_FALSE(audio_decoder);
+  EXPECT_FALSE(collection_.IsEmpty());
+
+  // Verify that we can actually retrieve the data source
+  // and that it is removed from the collection.
+  collection_.SelectFilter(&data_source);
+  EXPECT_TRUE(data_source);
+  EXPECT_TRUE(collection_.IsEmpty());
+
+  // Add a data source and audio decoder.
+  collection_.AddFilter(mock_filters_.data_source());
+  collection_.AddFilter(mock_filters_.audio_decoder());
+
+  // Verify that we can select the audio decoder.
+  collection_.SelectFilter(&audio_decoder);
+  EXPECT_TRUE(audio_decoder);
+  EXPECT_FALSE(collection_.IsEmpty());
+
+  // Verify that we can't select it again since only one has been added.
+  collection_.SelectFilter(&audio_decoder);
+  EXPECT_FALSE(audio_decoder);
+
+  // Verify that we can select the data source and that doing so will
+  // empty the collection again.
+  collection_.SelectFilter(&data_source);
+  EXPECT_TRUE(collection_.IsEmpty());
+}
+
+TEST_F(MediaFilterCollectionTest, MultipleFiltersOfSameType) {
+  scoped_refptr<DataSource> data_source_a(new MockDataSource());
+  scoped_refptr<DataSource> data_source_b(new MockDataSource());
+
+  scoped_refptr<DataSource> data_source;
+
+  collection_.AddFilter(data_source_a.get());
+  collection_.AddFilter(data_source_b.get());
+
+  // Verify that first SelectFilter() returns data_source_a.
+  collection_.SelectFilter(&data_source);
+  EXPECT_TRUE(data_source);
+  EXPECT_EQ(data_source, data_source_a);
+  EXPECT_FALSE(collection_.IsEmpty());
+
+  // Verify that second SelectFilter() returns data_source_b.
+  collection_.SelectFilter(&data_source);
+  EXPECT_TRUE(data_source);
+  EXPECT_EQ(data_source, data_source_b);
+  EXPECT_TRUE(collection_.IsEmpty());
+
+  // Verify that third SelectFilter returns nothing.
+  collection_.SelectFilter(&data_source);
+  EXPECT_FALSE(data_source);
+}
+
+}  // namespace media
diff --git a/media/base/mock_filters.h b/media/base/mock_filters.h
index 97710feee4697..ba15293480cab 100644
--- a/media/base/mock_filters.h
+++ b/media/base/mock_filters.h
@@ -17,6 +17,7 @@
 
 #include "base/callback.h"
 #include "media/base/filters.h"
+#include "media/base/media_filter_collection.h"
 #include "media/base/video_frame.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
@@ -284,14 +285,21 @@ class MockFilterCollection {
   MockVideoRenderer* video_renderer() const { return video_renderer_; }
   MockAudioRenderer* audio_renderer() const { return audio_renderer_; }
 
-  MediaFilterCollection filter_collection() const {
-    MediaFilterCollection collection;
-    collection.push_back(data_source_);
-    collection.push_back(demuxer_);
-    collection.push_back(video_decoder_);
-    collection.push_back(audio_decoder_);
-    collection.push_back(video_renderer_);
-    collection.push_back(audio_renderer_);
+  MediaFilterCollection* filter_collection() const {
+    return filter_collection(true);
+  }
+
+  MediaFilterCollection* filter_collection(bool include_data_source) const {
+    MediaFilterCollection* collection = new MediaFilterCollection();
+
+    if (include_data_source) {
+      collection->AddFilter(data_source_);
+    }
+    collection->AddFilter(demuxer_);
+    collection->AddFilter(video_decoder_);
+    collection->AddFilter(audio_decoder_);
+    collection->AddFilter(video_renderer_);
+    collection->AddFilter(audio_renderer_);
     return collection;
   }
 
diff --git a/media/base/pipeline.h b/media/base/pipeline.h
index 0519b3505f3ca..ab08cc47f346b 100644
--- a/media/base/pipeline.h
+++ b/media/base/pipeline.h
@@ -13,6 +13,7 @@
 
 #include "base/callback.h"
 #include "media/base/filters.h"
+#include "media/base/media_filter_collection.h"
 
 namespace base {
 class TimeDelta;
@@ -59,7 +60,7 @@ class Pipeline : public base::RefCountedThreadSafe<Pipeline> {
   // If the caller provides a |start_callback|, it will be called when the
   // pipeline initialization completes.  Clients are expected to call GetError()
   // to check whether initialization succeeded.
-  virtual bool Start(const MediaFilterCollection& filter_collection,
+  virtual bool Start(MediaFilterCollection* filter_collection,
                      const std::string& url,
                      PipelineCallback* start_callback) = 0;
 
diff --git a/media/base/pipeline_impl.cc b/media/base/pipeline_impl.cc
index 214be7d07c034..2493f3b7dba32 100644
--- a/media/base/pipeline_impl.cc
+++ b/media/base/pipeline_impl.cc
@@ -75,18 +75,21 @@ PipelineImpl::~PipelineImpl() {
 }
 
 // Creates the PipelineInternal and calls it's start method.
-bool PipelineImpl::Start(const MediaFilterCollection& collection,
+bool PipelineImpl::Start(MediaFilterCollection* collection,
                          const std::string& url,
                          PipelineCallback* start_callback) {
   AutoLock auto_lock(lock_);
   scoped_ptr<PipelineCallback> callback(start_callback);
+  scoped_ptr<MediaFilterCollection> filter_collection(collection);
+
   if (running_) {
     VLOG(1) << "Media pipeline is already running";
     return false;
   }
 
-  if (collection.empty())
+  if (collection->IsEmpty()) {
     return false;
+  }
 
   // Kick off initialization!
   running_ = true;
@@ -94,7 +97,7 @@ bool PipelineImpl::Start(const MediaFilterCollection& collection,
       FROM_HERE,
       NewRunnableMethod(this,
                         &PipelineImpl::StartTask,
-                        collection,
+                        filter_collection.release(),
                         url,
                         callback.release()));
   return true;
@@ -566,12 +569,12 @@ void PipelineImpl::OnFilterStateTransition() {
       NewRunnableMethod(this, &PipelineImpl::FilterStateTransitionTask));
 }
 
-void PipelineImpl::StartTask(const MediaFilterCollection& filter_collection,
+void PipelineImpl::StartTask(MediaFilterCollection* filter_collection,
                              const std::string& url,
                              PipelineCallback* start_callback) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
   DCHECK_EQ(kCreated, state_);
-  filter_collection_ = filter_collection;
+  filter_collection_.reset(filter_collection);
   url_ = url;
   seek_callback_.reset(start_callback);
 
@@ -664,7 +667,7 @@ void PipelineImpl::InitializeTask() {
     }
 
     // Clear the collection of filters.
-    filter_collection_.clear();
+    filter_collection_->Clear();
 
     // Initialization was successful, we are now considered paused, so it's safe
     // to set the initial playback rate and volume.
@@ -750,7 +753,7 @@ void PipelineImpl::VolumeChangedTask(float volume) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
 
   scoped_refptr<AudioRenderer> audio_renderer;
-  GetInitializedFilter(FILTER_AUDIO_RENDERER, &audio_renderer);
+  GetInitializedFilter(&audio_renderer);
   if (audio_renderer) {
     audio_renderer->SetVolume(volume);
   }
@@ -808,8 +811,8 @@ void PipelineImpl::NotifyEndedTask() {
   // Grab the renderers, if they exist.
   scoped_refptr<AudioRenderer> audio_renderer;
   scoped_refptr<VideoRenderer> video_renderer;
-  GetInitializedFilter(FILTER_AUDIO_RENDERER, &audio_renderer);
-  GetInitializedFilter(FILTER_VIDEO_RENDERER, &video_renderer);
+  GetInitializedFilter(&audio_renderer);
+  GetInitializedFilter(&video_renderer);
   DCHECK(audio_renderer || video_renderer);
 
   // Make sure every extant renderer has ended.
@@ -972,38 +975,6 @@ void PipelineImpl::FinishDestroyingFiltersTask() {
   }
 }
 
-template <class Filter>
-void PipelineImpl::SelectFilter(
-    FilterType filter_type, scoped_refptr<Filter>* filter_out) const {
-  DCHECK_EQ(MessageLoop::current(), message_loop_);
-
-  MediaFilterCollection::const_iterator it = filter_collection_.begin();
-  while (it != filter_collection_.end()) {
-    if ((*it)->filter_type() == filter_type)
-      break;
-    ++it;
-  }
-
-  if (it == filter_collection_.end())
-    *filter_out = NULL;
-  else
-    *filter_out = reinterpret_cast<Filter*>(it->get());
-}
-
-void PipelineImpl::RemoveFilter(scoped_refptr<MediaFilter> filter) {
-  MediaFilterCollection::iterator it = filter_collection_.begin();
-  while (it != filter_collection_.end()) {
-    if (*it == filter)
-      break;
-    ++it;
-  }
-  if (it == filter_collection_.end()) {
-    NOTREACHED() << "Removing a filter that doesn't exist";
-    return;
-  }
-  filter_collection_.erase(it);
-}
-
 void PipelineImpl::PrepareFilter(scoped_refptr<MediaFilter> filter) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
   DCHECK(IsPipelineOk());
@@ -1037,10 +1008,9 @@ void PipelineImpl::InitializeDataSource() {
 
   scoped_refptr<DataSource> data_source;
   while (true) {
-    SelectFilter(FILTER_DATA_SOURCE, &data_source);
+    filter_collection_->SelectFilter(&data_source);
     if (!data_source || data_source->IsUrlSupported(url_))
       break;
-    RemoveFilter(data_source);
   }
 
   if (!data_source) {
@@ -1048,7 +1018,6 @@ void PipelineImpl::InitializeDataSource() {
     return;
   }
 
-  RemoveFilter(data_source);
   PrepareFilter(data_source);
   data_source->Initialize(
       url_, NewCallback(this, &PipelineImpl::OnFilterInitialize));
@@ -1061,16 +1030,15 @@ void PipelineImpl::InitializeDemuxer() {
   scoped_refptr<DataSource> data_source;
   scoped_refptr<Demuxer> demuxer;
 
-  GetInitializedFilter(FILTER_DATA_SOURCE, &data_source);
+  GetInitializedFilter(&data_source);
   CHECK(data_source);
 
-  SelectFilter(FILTER_DEMUXER, &demuxer);
+  filter_collection_->SelectFilter(&demuxer);
   if (!demuxer) {
     SetError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING);
     return;
   }
 
-  RemoveFilter(demuxer);
   PrepareFilter(demuxer);
   demuxer->Initialize(data_source,
                       NewCallback(this, &PipelineImpl::OnFilterInitialize));
@@ -1082,21 +1050,23 @@ bool PipelineImpl::InitializeAudioDecoder() {
 
   scoped_refptr<DemuxerStream> stream =
       FindDemuxerStream(mime_type::kMajorTypeAudio);
-  scoped_refptr<AudioDecoder> audio_decoder;
-  SelectFilter(FILTER_AUDIO_DECODER, &audio_decoder);
 
-  // If there is such stream but audio decoder is not found.
-  if (stream && !audio_decoder)
-    SetError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING);
-  if (!stream || !audio_decoder)
-    return false;
+  if (stream) {
+    scoped_refptr<AudioDecoder> audio_decoder;
+    filter_collection_->SelectFilter(&audio_decoder);
 
-  RemoveFilter(audio_decoder);
-  PrepareFilter(audio_decoder);
-  audio_decoder->Initialize(
-      stream,
-      NewCallback(this, &PipelineImpl::OnFilterInitialize));
-  return true;
+    if (audio_decoder) {
+      PrepareFilter(audio_decoder);
+      audio_decoder->Initialize(
+          stream,
+          NewCallback(this, &PipelineImpl::OnFilterInitialize));
+      return true;
+    } else {
+      SetError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING);
+    }
+  }
+
+  return false;
 }
 
 bool PipelineImpl::InitializeVideoDecoder() {
@@ -1105,21 +1075,22 @@ bool PipelineImpl::InitializeVideoDecoder() {
 
   scoped_refptr<DemuxerStream> stream =
       FindDemuxerStream(mime_type::kMajorTypeVideo);
-  scoped_refptr<VideoDecoder> video_decoder;
-  SelectFilter(FILTER_VIDEO_DECODER, &video_decoder);
 
-  // If there is such stream but video decoder is not found.
-  if (stream && !video_decoder)
-    SetError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING);
-  if (!stream || !video_decoder)
-    return false;
+  if (stream) {
+    scoped_refptr<VideoDecoder> video_decoder;
+    filter_collection_->SelectFilter(&video_decoder);
 
-  RemoveFilter(video_decoder);
-  PrepareFilter(video_decoder);
-  video_decoder->Initialize(
-      stream,
-      NewCallback(this, &PipelineImpl::OnFilterInitialize));
-  return true;
+    if (video_decoder) {
+      PrepareFilter(video_decoder);
+      video_decoder->Initialize(
+          stream,
+          NewCallback(this, &PipelineImpl::OnFilterInitialize));
+      return true;
+    } else {
+      SetError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING);
+    }
+  }
+  return false;
 }
 
 bool PipelineImpl::InitializeAudioRenderer() {
@@ -1127,21 +1098,22 @@ bool PipelineImpl::InitializeAudioRenderer() {
   DCHECK(IsPipelineOk());
 
   scoped_refptr<AudioDecoder> decoder;
-  GetInitializedFilter(FILTER_AUDIO_DECODER, &decoder);
-  scoped_refptr<AudioRenderer> audio_renderer;
-  SelectFilter(FILTER_AUDIO_RENDERER, &audio_renderer);
+  GetInitializedFilter(&decoder);
 
-  // If there is such decoder but renderer is not found.
-  if (decoder && !audio_renderer)
-    SetError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING);
-  if (!decoder || !audio_renderer)
-    return false;
+  if (decoder) {
+    scoped_refptr<AudioRenderer> audio_renderer;
+    filter_collection_->SelectFilter(&audio_renderer);
 
-  RemoveFilter(audio_renderer);
-  PrepareFilter(audio_renderer);
-  audio_renderer->Initialize(
-      decoder, NewCallback(this, &PipelineImpl::OnFilterInitialize));
-  return true;
+    if (audio_renderer) {
+      PrepareFilter(audio_renderer);
+      audio_renderer->Initialize(
+          decoder, NewCallback(this, &PipelineImpl::OnFilterInitialize));
+      return true;
+    } else {
+      SetError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING);
+    }
+  }
+  return false;
 }
 
 bool PipelineImpl::InitializeVideoRenderer() {
@@ -1149,27 +1121,28 @@ bool PipelineImpl::InitializeVideoRenderer() {
   DCHECK(IsPipelineOk());
 
   scoped_refptr<VideoDecoder> decoder;
-  GetInitializedFilter(FILTER_VIDEO_DECODER, &decoder);
-  scoped_refptr<VideoRenderer> video_renderer;
-  SelectFilter(FILTER_VIDEO_RENDERER, &video_renderer);
+  GetInitializedFilter(&decoder);
 
-  // If there is such decoder but renderer is not found.
-  if (decoder && !video_renderer)
-    SetError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING);
-  if (!decoder || !video_renderer)
-    return false;
+  if (decoder) {
+    scoped_refptr<VideoRenderer> video_renderer;
+    filter_collection_->SelectFilter(&video_renderer);
 
-  RemoveFilter(video_renderer);
-  PrepareFilter(video_renderer);
-  video_renderer->Initialize(
-      decoder, NewCallback(this, &PipelineImpl::OnFilterInitialize));
-  return true;
+    if (video_renderer) {
+      PrepareFilter(video_renderer);
+      video_renderer->Initialize(
+          decoder, NewCallback(this, &PipelineImpl::OnFilterInitialize));
+      return true;
+    } else {
+      SetError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING);
+    }
+  }
+  return false;
 }
 
 scoped_refptr<DemuxerStream> PipelineImpl::FindDemuxerStream(
     std::string major_mime_type) {
   scoped_refptr<Demuxer> demuxer;
-  GetInitializedFilter(FILTER_DEMUXER, &demuxer);
+  GetInitializedFilter(&demuxer);
   DCHECK(demuxer);
 
   const int num_outputs = demuxer->GetNumberOfStreams();
@@ -1186,10 +1159,11 @@ scoped_refptr<DemuxerStream> PipelineImpl::FindDemuxerStream(
 
 template <class Filter>
 void PipelineImpl::GetInitializedFilter(
-    FilterType filter_type, scoped_refptr<Filter>* filter_out) const {
+    scoped_refptr<Filter>* filter_out) const {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
 
-  FilterTypeMap::const_iterator ft = filter_types_.find(filter_type);
+  FilterTypeMap::const_iterator ft =
+      filter_types_.find(Filter::static_filter_type());
   if (ft == filter_types_.end()) {
     *filter_out = NULL;
   } else {
diff --git a/media/base/pipeline_impl.h b/media/base/pipeline_impl.h
index 0370fd55b9a42..c03f8c5e81fa3 100644
--- a/media/base/pipeline_impl.h
+++ b/media/base/pipeline_impl.h
@@ -66,7 +66,7 @@ class PipelineImpl : public Pipeline, public FilterHost {
   explicit PipelineImpl(MessageLoop* message_loop);
 
   // Pipeline implementation.
-  virtual bool Start(const MediaFilterCollection& filter_collection,
+  virtual bool Start(MediaFilterCollection* filter_collection,
                      const std::string& uri,
                      PipelineCallback* start_callback);
   virtual void Stop(PipelineCallback* stop_callback);
@@ -194,7 +194,7 @@ class PipelineImpl : public Pipeline, public FilterHost {
   // The following "task" methods correspond to the public methods, but these
   // methods are run as the result of posting a task to the PipelineInternal's
   // message loop.
-  void StartTask(const MediaFilterCollection& filter_collection,
+  void StartTask(MediaFilterCollection* filter_collection,
                  const std::string& url,
                  PipelineCallback* start_callback);
 
@@ -241,15 +241,6 @@ class PipelineImpl : public Pipeline, public FilterHost {
   // Internal methods used in the implementation of the pipeline thread.  All
   // of these methods are only called on the pipeline thread.
 
-  // Uses the MediaFilterCollection to return a filter of |filter_type|.
-  // If the required filter cannot be found, NULL is returned.
-  template <class Filter>
-  void SelectFilter(FilterType filter_type,
-                    scoped_refptr<Filter>* filter_out) const;
-
-  // Remove a filer from MediaFilterCollection.
-  void RemoveFilter(scoped_refptr<MediaFilter> filter);
-
   // PrepareFilter() creates the filter's thread and injects a FilterHost and
   // MessageLoop.
   void PrepareFilter(scoped_refptr<MediaFilter> filter);
@@ -280,8 +271,7 @@ class PipelineImpl : public Pipeline, public FilterHost {
   // specified filter type. If one exists, the |filter_out| will contain
   // the filter, |*filter_out| will be NULL.
   template <class Filter>
-  void GetInitializedFilter(FilterType filter_type,
-                            scoped_refptr<Filter>* filter_out) const;
+  void GetInitializedFilter(scoped_refptr<Filter>* filter_out) const;
 
   // Kicks off destroying filters. Called by StopTask() and ErrorChangedTask().
   // When we start to tear down the pipeline, we will consider two cases:
@@ -400,7 +390,7 @@ class PipelineImpl : public Pipeline, public FilterHost {
   base::TimeDelta max_buffered_time_;
 
   // Filter collection as passed in by Start().
-  MediaFilterCollection filter_collection_;
+  scoped_ptr<MediaFilterCollection> filter_collection_;
 
   // URL for the data source as passed in by Start().
   std::string url_;
diff --git a/media/base/pipeline_impl_unittest.cc b/media/base/pipeline_impl_unittest.cc
index eef635666e991..02d1165026392 100644
--- a/media/base/pipeline_impl_unittest.cc
+++ b/media/base/pipeline_impl_unittest.cc
@@ -351,8 +351,7 @@ TEST_F(PipelineImplTest, RequiredFilterMissing) {
   EXPECT_CALL(callbacks_, OnStart());
 
   // Create a filter collection with missing filter.
-  MediaFilterCollection collection = mocks_->filter_collection();
-  collection.pop_front();
+  MediaFilterCollection* collection = mocks_->filter_collection(false);
   pipeline_->Start(collection, "",
                    NewCallback(reinterpret_cast<CallbackHelper*>(&callbacks_),
                                &CallbackHelper::OnStart));
diff --git a/media/media.gyp b/media/media.gyp
index f844f12596cf4..53c59f43908b7 100644
--- a/media/media.gyp
+++ b/media/media.gyp
@@ -80,6 +80,8 @@
         'base/filters.cc',
         'base/filters.h',
         'base/media.h',
+        'base/media_filter_collection.cc',
+        'base/media_filter_collection.h',
         'base/media_format.cc',
         'base/media_format.h',
         'base/media_posix.cc',
@@ -262,6 +264,7 @@
         'base/clock_impl_unittest.cc',
         'base/data_buffer_unittest.cc',
         'base/djb2_unittest.cc',
+        'base/media_filter_collection_unittest.cc',
         'base/mock_ffmpeg.cc',
         'base/mock_ffmpeg.h',
         'base/mock_filter_host.h',
diff --git a/media/tools/player_wtl/movie.cc b/media/tools/player_wtl/movie.cc
index 90dae4fa51b99..c12d724ce3dd2 100644
--- a/media/tools/player_wtl/movie.cc
+++ b/media/tools/player_wtl/movie.cc
@@ -54,25 +54,25 @@ bool Movie::Open(const wchar_t* url, WtlVideoRenderer* video_renderer) {
   }
 
   // Create filter collection.
-  MediaFilterCollection collection;
-  collection.push_back(new FileDataSource());
-  collection.push_back(new FFmpegAudioDecoder());
-  collection.push_back(new FFmpegDemuxer());
-  collection.push_back(new FFmpegVideoDecoder(NULL));
+  scoped_ptr<MediaFilterCollection> collection(new MediaFilterCollection());
+  collection->AddFilter(new FileDataSource());
+  collection->AddFilter(new FFmpegAudioDecoder());
+  collection->AddFilter(new FFmpegDemuxer());
+  collection->AddFilter(new FFmpegVideoDecoder(NULL));
 
   if (enable_audio_) {
-    collection.push_back(new AudioRendererImpl());
+    collection->AddFilter(new AudioRendererImpl());
   } else {
-    collection.push_back(new media::NullAudioRenderer());
+    collection->AddFilter(new media::NullAudioRenderer());
   }
-  collection.push_back(video_renderer);
+  collection->AddFilter(video_renderer);
 
   thread_.reset(new base::Thread("PipelineThread"));
   thread_->Start();
   pipeline_ = new PipelineImpl(thread_->message_loop());
 
   // Create and start our pipeline.
-  pipeline_->Start(collection, WideToUTF8(std::wstring(url)), NULL);
+  pipeline_->Start(collection.release(), WideToUTF8(std::wstring(url)), NULL);
   while (true) {
     PlatformThread::Sleep(100);
     if (pipeline_->IsInitialized())
diff --git a/media/tools/player_x11/player_x11.cc b/media/tools/player_x11/player_x11.cc
index d1a7ffeaa4d0a..ad307c4891ef0 100644
--- a/media/tools/player_x11/player_x11.cc
+++ b/media/tools/player_x11/player_x11.cc
@@ -97,26 +97,27 @@ bool InitPipeline(MessageLoop* message_loop,
   }
 
   // Create our filter factories.
-  media::MediaFilterCollection collection;
-  collection.push_back(new media::FileDataSource());
-  collection.push_back(new media::FFmpegDemuxer());
-  collection.push_back(new media::FFmpegAudioDecoder());
+  scoped_ptr<media::MediaFilterCollection> collection(
+      new media::MediaFilterCollection());
+  collection->AddFilter(new media::FileDataSource());
+  collection->AddFilter(new media::FFmpegDemuxer());
+  collection->AddFilter(new media::FFmpegAudioDecoder());
   if (CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableOpenMax)) {
-    collection.push_back(new media::OmxVideoDecoder(NULL));
+    collection->AddFilter(new media::OmxVideoDecoder(NULL));
   } else {
-    collection.push_back(new media::FFmpegVideoDecoder(NULL));
+    collection->AddFilter(new media::FFmpegVideoDecoder(NULL));
   }
-  collection.push_back(new Renderer(g_display, g_window, paint_message_loop));
+  collection->AddFilter(new Renderer(g_display, g_window, paint_message_loop));
 
   if (enable_audio)
-    collection.push_back(new media::AudioRendererImpl());
+    collection->AddFilter(new media::AudioRendererImpl());
   else
-    collection.push_back(new media::NullAudioRenderer());
+    collection->AddFilter(new media::NullAudioRenderer());
 
   // Creates the pipeline and start it.
   *pipeline = new media::PipelineImpl(message_loop);
-  (*pipeline)->Start(collection, filename, NULL);
+  (*pipeline)->Start(collection.release(), filename, NULL);
 
   // Wait until the pipeline is fully initialized.
   while (true) {
diff --git a/webkit/glue/webmediaplayer_impl.cc b/webkit/glue/webmediaplayer_impl.cc
index 21a083d7a0d99..919cf2e350f19 100644
--- a/webkit/glue/webmediaplayer_impl.cc
+++ b/webkit/glue/webmediaplayer_impl.cc
@@ -224,7 +224,7 @@ void WebMediaPlayerImpl::Proxy::PutCurrentFrame(
 
 WebMediaPlayerImpl::WebMediaPlayerImpl(
     WebKit::WebMediaPlayerClient* client,
-    const media::MediaFilterCollection& collection,
+    media::MediaFilterCollection* collection,
     MediaResourceLoaderBridgeFactory* bridge_factory_simple,
     MediaResourceLoaderBridgeFactory* bridge_factory_buffered,
     bool use_simple_data_source,
@@ -276,22 +276,18 @@ WebMediaPlayerImpl::WebMediaPlayerImpl(
   proxy_->SetDataSource(buffered_data_source);
 
   if (use_simple_data_source) {
-    filter_collection_.push_back(simple_data_source);
-    filter_collection_.push_back(buffered_data_source);
+    filter_collection_->AddFilter(simple_data_source);
+    filter_collection_->AddFilter(buffered_data_source);
   } else {
-    filter_collection_.push_back(buffered_data_source);
-    filter_collection_.push_back(simple_data_source);
+    filter_collection_->AddFilter(buffered_data_source);
+    filter_collection_->AddFilter(simple_data_source);
   }
 
   // Add in the default filter factories.
-  filter_collection_.push_back(make_scoped_refptr(
-      new media::FFmpegDemuxer()));
-  filter_collection_.push_back(make_scoped_refptr(
-      new media::FFmpegAudioDecoder()));
-  filter_collection_.push_back(make_scoped_refptr(
-      new media::FFmpegVideoDecoder(NULL)));
-  filter_collection_.push_back(make_scoped_refptr(
-      new media::NullAudioRenderer()));
+  filter_collection_->AddFilter(new media::FFmpegDemuxer());
+  filter_collection_->AddFilter(new media::FFmpegAudioDecoder());
+  filter_collection_->AddFilter(new media::FFmpegVideoDecoder(NULL));
+  filter_collection_->AddFilter(new media::NullAudioRenderer());
 }
 
 WebMediaPlayerImpl::~WebMediaPlayerImpl() {
@@ -315,7 +311,7 @@ void WebMediaPlayerImpl::load(const WebKit::WebURL& url) {
   SetNetworkState(WebKit::WebMediaPlayer::Loading);
   SetReadyState(WebKit::WebMediaPlayer::HaveNothing);
   pipeline_->Start(
-      filter_collection_,
+      filter_collection_.release(),
       url.spec(),
       NewCallback(proxy_.get(),
                   &WebMediaPlayerImpl::Proxy::PipelineInitializationCallback));
diff --git a/webkit/glue/webmediaplayer_impl.h b/webkit/glue/webmediaplayer_impl.h
index f1860f4265508..23f414805fe4a 100644
--- a/webkit/glue/webmediaplayer_impl.h
+++ b/webkit/glue/webmediaplayer_impl.h
@@ -169,7 +169,7 @@ class WebMediaPlayerImpl : public WebKit::WebMediaPlayer,
   // |collection| before calling this method.
   //
   WebMediaPlayerImpl(WebKit::WebMediaPlayerClient* client,
-                     const media::MediaFilterCollection& collection,
+                     media::MediaFilterCollection* collection,
                      MediaResourceLoaderBridgeFactory* bridge_factory_simple,
                      MediaResourceLoaderBridgeFactory* bridge_factory_buffered,
                      bool use_simple_data_source,
@@ -280,7 +280,7 @@ class WebMediaPlayerImpl : public WebKit::WebMediaPlayer,
   MessageLoop* main_loop_;
 
   // A collection of filters.
-  media::MediaFilterCollection filter_collection_;
+  scoped_ptr<media::MediaFilterCollection> filter_collection_;
 
   // The actual pipeline and the thread it runs on.
   scoped_refptr<media::PipelineImpl> pipeline_;
diff --git a/webkit/support/webkit_support.cc b/webkit/support/webkit_support.cc
index e2dbf4cff09cf..b80c0af09083d 100644
--- a/webkit/support/webkit_support.cc
+++ b/webkit/support/webkit_support.cc
@@ -265,7 +265,8 @@ WebPlugin* CreateWebPlugin(WebFrame* frame,
 
 WebKit::WebMediaPlayer* CreateMediaPlayer(WebFrame* frame,
                                           WebMediaPlayerClient* client) {
-  media::MediaFilterCollection collection;
+  scoped_ptr<media::MediaFilterCollection> collection(
+      new media::MediaFilterCollection());
 
   appcache::WebApplicationCacheHostImpl* appcache_host =
       appcache::WebApplicationCacheHostImpl::FromFrame(frame);
@@ -291,11 +292,11 @@ WebKit::WebMediaPlayer* CreateMediaPlayer(WebFrame* frame,
 
   scoped_refptr<webkit_glue::VideoRendererImpl> video_renderer(
       new webkit_glue::VideoRendererImpl(false));
-  collection.push_back(video_renderer);
+  collection->AddFilter(video_renderer);
 
   return new webkit_glue::WebMediaPlayerImpl(
-      client, collection, bridge_factory_simple, bridge_factory_buffered,
-      false, video_renderer);
+      client, collection.release(), bridge_factory_simple,
+      bridge_factory_buffered, false, video_renderer);
 }
 
 WebKit::WebApplicationCacheHost* CreateApplicationCacheHost(
diff --git a/webkit/tools/test_shell/test_webview_delegate.cc b/webkit/tools/test_shell/test_webview_delegate.cc
index 9bbf1a617cd79..3a423826f264a 100644
--- a/webkit/tools/test_shell/test_webview_delegate.cc
+++ b/webkit/tools/test_shell/test_webview_delegate.cc
@@ -716,7 +716,8 @@ WebWorker* TestWebViewDelegate::createWorker(
 
 WebMediaPlayer* TestWebViewDelegate::createMediaPlayer(
     WebFrame* frame, WebMediaPlayerClient* client) {
-  media::MediaFilterCollection collection;
+  scoped_ptr<media::MediaFilterCollection> collection(
+      new media::MediaFilterCollection());
 
   appcache::WebApplicationCacheHostImpl* appcache_host =
       appcache::WebApplicationCacheHostImpl::FromFrame(frame);
@@ -742,11 +743,11 @@ WebMediaPlayer* TestWebViewDelegate::createMediaPlayer(
 
   scoped_refptr<webkit_glue::VideoRendererImpl> video_renderer(
       new webkit_glue::VideoRendererImpl(false));
-  collection.push_back(video_renderer);
+  collection->AddFilter(video_renderer);
 
   return new webkit_glue::WebMediaPlayerImpl(
-      client, collection, bridge_factory_simple, bridge_factory_buffered,
-      false, video_renderer);
+      client, collection.release(), bridge_factory_simple,
+      bridge_factory_buffered, false, video_renderer);
 }
 
 WebApplicationCacheHost* TestWebViewDelegate::createApplicationCacheHost(
-- 
GitLab