Remove MessageLoop methods from Filter interface to

separate Filter management from MessageLoop management.
This sets the stage for filters to share threads in the
future which will reduce resource consumption when
multiple <video> tags are on the same page.

BUG=69451
TEST=None

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

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

11 files changed, 153 insertions, 216 deletions

diff --git a/media/base/composite_filter.cc b/media/base/composite_filter.cc
index 94665fd..41eb31c 100644
--- a/media/base/composite_filter.cc
+++ b/media/base/composite_filter.cc
@@ -4,6 +4,7 @@
 
 #include "media/base/composite_filter.h"
 
+#include "base/message_loop.h"
 #include "base/stl_util-inl.h"
 #include "media/base/callback.h"
 
@@ -40,46 +41,21 @@ class CompositeFilter::FilterHostImpl : public FilterHost {
   DISALLOW_COPY_AND_ASSIGN(FilterHostImpl);
 };
 
-CompositeFilter::CompositeFilter(MessageLoop* message_loop) {
-  Init(message_loop, NULL);
-}
-
-CompositeFilter::CompositeFilter(MessageLoop* message_loop,
-                                 ThreadFactoryFunction thread_factory) {
-  DCHECK(thread_factory);
-  Init(message_loop, thread_factory);
-}
-
-void CompositeFilter::Init(MessageLoop* message_loop,
-                           ThreadFactoryFunction thread_factory) {
+CompositeFilter::CompositeFilter(MessageLoop* message_loop)
+    : state_(kCreated),
+      sequence_index_(0),
+      message_loop_(message_loop),
+      error_(PIPELINE_OK) {
   DCHECK(message_loop);
-  message_loop_ = message_loop;
-  thread_factory_ = thread_factory;
   runnable_factory_.reset(
       new ScopedRunnableMethodFactory<CompositeFilter>(this));
-
-  if (!thread_factory_) {
-    thread_factory_ = &CompositeFilter::DefaultThreadFactory;
-  }
-
-  state_ = kCreated;
-  sequence_index_ = 0;
-  error_ = PIPELINE_OK;
 }
 
 CompositeFilter::~CompositeFilter() {
   DCHECK_EQ(message_loop_, MessageLoop::current());
   DCHECK(state_ == kCreated || state_ == kStopped);
 
-  // Stop every running filter thread.
-  for (FilterThreadVector::iterator iter = filter_threads_.begin();
-       iter != filter_threads_.end();
-       ++iter) {
-    (*iter)->Stop();
-  }
-
   filters_.clear();
-  STLDeleteElements(&filter_threads_);
 }
 
 bool CompositeFilter::AddFilter(scoped_refptr<Filter> filter) {
@@ -87,19 +63,6 @@ bool CompositeFilter::AddFilter(scoped_refptr<Filter> filter) {
   if (!filter.get() || state_ != kCreated || !host())
     return false;
 
-  // Create a dedicated thread for this filter if applicable.
-  if (filter->requires_message_loop()) {
-    scoped_ptr<base::Thread> thread(
-        thread_factory_(filter->message_loop_name()));
-
-    if (!thread.get() || !thread->Start()) {
-      return false;
-    }
-
-    filter->set_message_loop(thread->message_loop());
-    filter_threads_.push_back(thread.release());
-  }
-
   // Register ourselves as the filter's host.
   filter->set_host(host_impl_.get());
   filters_.push_back(make_scoped_refptr(filter.get()));
@@ -121,22 +84,6 @@ FilterHost* CompositeFilter::host() {
   return host_impl_.get() ? host_impl_->host() : NULL;
 }
 
-bool CompositeFilter::requires_message_loop() const {
-  return false;
-}
-
-const char* CompositeFilter::message_loop_name() const {
-  return "CompositeFilter";
-}
-
-void CompositeFilter::set_message_loop(MessageLoop* message_loop) {
-  NOTREACHED() << "Message loop should not be set.";
-}
-
-MessageLoop* CompositeFilter::message_loop() {
-  return NULL;
-}
-
 void CompositeFilter::Play(FilterCallback* play_callback) {
   DCHECK_EQ(message_loop_, MessageLoop::current());
   scoped_ptr<FilterCallback> callback(play_callback);
@@ -278,11 +225,6 @@ void CompositeFilter::OnAudioRendererDisabled() {
   }
 }
 
-base::Thread* CompositeFilter::DefaultThreadFactory(
-    const char* thread_name) {
-  return new base::Thread(thread_name);
-}
-
 void CompositeFilter::ChangeState(State new_state) {
   DCHECK_EQ(message_loop_, MessageLoop::current());
   state_ = new_state;
@@ -529,9 +471,9 @@ void CompositeFilter::SetError(PipelineError error) {
 }
 
 CompositeFilter::FilterHostImpl::FilterHostImpl(CompositeFilter* parent,
-                                                FilterHost* host) :
-    parent_(parent),
-    host_(host) {
+                                                FilterHost* host)
+    : parent_(parent),
+      host_(host) {
 }
 
 FilterHost* CompositeFilter::FilterHostImpl::host() {
diff --git a/media/base/composite_filter.h b/media/base/composite_filter.h
index c1bd8c0..daac421 100644
--- a/media/base/composite_filter.h
+++ b/media/base/composite_filter.h
@@ -5,39 +5,29 @@
 #ifndef MEDIA_BASE_COMPOSITE_FILTER_H_
 #define MEDIA_BASE_COMPOSITE_FILTER_H_
 
-#include "base/threading/thread.h"
+#include "base/task.h"
 #include "media/base/filter_host.h"
 #include "media/base/filters.h"
 
+class MessageLoop;
+
 namespace media {
 
 class CompositeFilter : public Filter {
  public:
-  typedef base::Thread* (*ThreadFactoryFunction)(const char* thread_name);
-
-  CompositeFilter(MessageLoop* message_loop);
-
-  // Constructor that allows the default thread creation strategy to be
-  // overridden.
-  CompositeFilter(MessageLoop* message_loop,
-                  ThreadFactoryFunction thread_factory);
+  explicit CompositeFilter(MessageLoop* message_loop);
 
   // Adds a filter to the composite. This is only allowed after set_host()
   // is called and before the first state changing operation such as Play(),
   // Flush(), Stop(), or Seek(). True is returned if the filter was successfully
   // added to the composite. False is returned if the filter couldn't be added
-  // because the composite is in the wrong state or the filter needed a thread
-  // and the composite was unable to create one.
+  // because the composite is in the wrong state.
   bool AddFilter(scoped_refptr<Filter> filter);
 
   // media::Filter methods.
   virtual const char* major_mime_type() const;
   virtual void set_host(FilterHost* host);
   virtual FilterHost* host();
-  virtual bool requires_message_loop() const;
-  virtual const char* message_loop_name() const;
-  virtual void set_message_loop(MessageLoop* message_loop);
-  virtual MessageLoop* message_loop();
   virtual void Play(FilterCallback* play_callback);
   virtual void Pause(FilterCallback* pause_callback);
   virtual void Flush(FilterCallback* flush_callback);
@@ -49,9 +39,6 @@ class CompositeFilter : public Filter {
  protected:
   virtual ~CompositeFilter();
 
-  /// Default thread factory strategy.
-  static base::Thread* DefaultThreadFactory(const char* thread_name);
-
   void SetError(PipelineError error);
 
  private:
@@ -75,9 +62,6 @@ class CompositeFilter : public Filter {
     kError
   };
 
-  // Initialization method called by constructors.
-  void Init(MessageLoop* message_loop, ThreadFactoryFunction thread_factory);
-
   // Transition to a new state.
   void ChangeState(State new_state);
 
@@ -124,17 +108,10 @@ class CompositeFilter : public Filter {
   // to the host of this filter.
   bool CanForwardError();
 
-  // Vector of threads owned by the composite and used by filters in |filters_|.
-  typedef std::vector<base::Thread*> FilterThreadVector;
-  FilterThreadVector filter_threads_;
-
   // Vector of the filters added to the composite.
   typedef std::vector<scoped_refptr<Filter> > FilterVector;
   FilterVector filters_;
 
-  // Factory function used to create filter threads.
-  ThreadFactoryFunction thread_factory_;
-
   // Callback for the pending request.
   scoped_ptr<FilterCallback> callback_;
 
diff --git a/media/base/composite_filter_unittest.cc b/media/base/composite_filter_unittest.cc
index 4ce96c1..0be2185 100644
--- a/media/base/composite_filter_unittest.cc
+++ b/media/base/composite_filter_unittest.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/message_loop.h"
 #include "media/base/composite_filter.h"
 #include "media/base/mock_callback.h"
 #include "media/base/mock_filter_host.h"
@@ -297,56 +298,29 @@ void CompositeFilterTest::RunFilter2Callback() {
   delete callback;
 }
 
-static base::Thread* NullThreadFactory(const char* thread_name) {
-  return NULL;
-}
-
 // Test AddFilter() failure cases.
 TEST_F(CompositeFilterTest, TestAddFilterFailCases) {
   // Test adding a null pointer.
   EXPECT_FALSE(composite_->AddFilter(NULL));
 
-  scoped_refptr<StrictMock<MockFilter> > filter =
-      new StrictMock<MockFilter>(true);
+  scoped_refptr<StrictMock<MockFilter> > filter = new StrictMock<MockFilter>();
   EXPECT_EQ(NULL, filter->host());
-  EXPECT_EQ(NULL, filter->message_loop());
 
   // Test failing because set_host() hasn't been called yet.
   EXPECT_FALSE(composite_->AddFilter(filter));
-
-  // Test thread creation failure.
-  composite_ = new CompositeFilter(&message_loop_, &NullThreadFactory);
-  composite_->set_host(mock_filter_host_.get());
-  EXPECT_FALSE(composite_->AddFilter(filter));
-  EXPECT_EQ(NULL, filter->host());
-  EXPECT_EQ(NULL, filter->message_loop());
 }
 
 // Test successful AddFilter() cases.
 TEST_F(CompositeFilterTest, TestAddFilter) {
   composite_->set_host(mock_filter_host_.get());
 
-  // Add a filter that doesn't require a message loop.
+  // Add a filter.
   scoped_refptr<StrictMock<MockFilter> > filter = new StrictMock<MockFilter>();
   EXPECT_EQ(NULL, filter->host());
-  EXPECT_EQ(NULL, filter->message_loop());
 
   EXPECT_TRUE(composite_->AddFilter(filter));
 
   EXPECT_TRUE(filter->host() != NULL);
-  EXPECT_EQ(NULL, filter->message_loop());
-
-  // Add a filter that requires a message loop.
-  scoped_refptr<StrictMock<MockFilter> > filter_2 =
-      new StrictMock<MockFilter>(true);
-
-  EXPECT_EQ(NULL, filter_2->host());
-  EXPECT_EQ(NULL, filter_2->message_loop());
-
-  EXPECT_TRUE(composite_->AddFilter(filter_2));
-
-  EXPECT_TRUE(filter_2->host() != NULL);
-  EXPECT_TRUE(filter_2->message_loop() != NULL);
 }
 
 TEST_F(CompositeFilterTest, TestPlay) {
diff --git a/media/base/filters.cc b/media/base/filters.cc
index d3d3bcb..eff50b4 100644
--- a/media/base/filters.cc
+++ b/media/base/filters.cc
@@ -5,11 +5,10 @@
 #include "media/base/filters.h"
 
 #include "base/logging.h"
-#include "base/message_loop.h"
 
 namespace media {
 
-Filter::Filter() : host_(NULL), message_loop_(NULL) {}
+Filter::Filter() : host_(NULL) {}
 
 Filter::~Filter() {}
 
@@ -27,24 +26,6 @@ FilterHost* Filter::host() {
   return host_;
 }
 
-bool Filter::requires_message_loop() const {
-  return false;
-}
-
-const char* Filter::message_loop_name() const {
-  return "FilterThread";
-}
-
-void Filter::set_message_loop(MessageLoop* message_loop) {
-  DCHECK(message_loop);
-  DCHECK(!message_loop_);
-  message_loop_ = message_loop;
-}
-
-MessageLoop* Filter::message_loop() {
-  return message_loop_;
-}
-
 void Filter::Play(FilterCallback* callback) {
   DCHECK(callback);
   if (callback) {
@@ -93,26 +74,10 @@ bool DataSource::IsUrlSupported(const std::string& url) {
   return true;
 }
 
-bool Demuxer::requires_message_loop() const {
-  return true;
-}
-
-const char* Demuxer::message_loop_name() const {
-  return "DemuxerThread";
-}
-
 const char* AudioDecoder::major_mime_type() const {
   return mime_type::kMajorTypeAudio;
 }
 
-bool AudioDecoder::requires_message_loop() const {
-  return true;
-}
-
-const char* AudioDecoder::message_loop_name() const {
-  return "AudioDecoderThread";
-}
-
 const char* AudioRenderer::major_mime_type() const {
   return mime_type::kMajorTypeAudio;
 }
@@ -121,14 +86,6 @@ const char* VideoDecoder::major_mime_type() const {
   return mime_type::kMajorTypeVideo;
 }
 
-bool VideoDecoder::requires_message_loop() const {
-  return true;
-}
-
-const char* VideoDecoder::message_loop_name() const {
-  return "VideoDecoderThread";
-}
-
 const char* VideoRenderer::major_mime_type() const {
   return mime_type::kMajorTypeVideo;
 }
diff --git a/media/base/filters.h b/media/base/filters.h
index 49fa347..84c466d 100644
--- a/media/base/filters.h
+++ b/media/base/filters.h
@@ -33,8 +33,6 @@
 #include "media/base/media_format.h"
 #include "media/base/video_frame.h"
 
-class MessageLoop;
-
 namespace media {
 
 class Buffer;
@@ -61,20 +59,6 @@ class Filter : public base::RefCountedThreadSafe<Filter> {
 
   virtual FilterHost* host();
 
-  // Indicates whether this filter requires a message loop to operate.
-  virtual bool requires_message_loop() const;
-
-  // The name to associate with this filter's message loop.
-  virtual const char* message_loop_name() const;
-
-  // Sets the private member |message_loop_|, which is used by filters for
-  // processing asynchronous tasks and maintaining synchronized access to
-  // internal data members.  The message loop should be running and exceed the
-  // lifetime of the filter.
-  virtual void set_message_loop(MessageLoop* message_loop);
-
-  virtual MessageLoop* message_loop();
-
   // The pipeline has resumed playback.  Filters can continue requesting reads.
   // Filters may implement this method if they need to respond to this call.
   // TODO(boliu): Check that callback is not NULL in subclasses.
@@ -114,11 +98,9 @@ class Filter : public base::RefCountedThreadSafe<Filter> {
   virtual ~Filter();
 
   FilterHost* host() const { return host_; }
-  MessageLoop* message_loop() const { return message_loop_; }
 
  private:
   FilterHost* host_;
-  MessageLoop* message_loop_;
 
   DISALLOW_COPY_AND_ASSIGN(Filter);
 };
@@ -154,9 +136,6 @@ class DataSource : public Filter {
 
 class Demuxer : public Filter {
  public:
-  virtual bool requires_message_loop() const;
-  virtual const char* message_loop_name() const;
-
   // Initialize a Demuxer with the given DataSource, executing the callback upon
   // completion.
   virtual void Initialize(DataSource* data_source,
@@ -211,9 +190,6 @@ class DemuxerStream : public base::RefCountedThreadSafe<DemuxerStream> {
 class VideoDecoder : public Filter {
  public:
   virtual const char* major_mime_type() const;
-  virtual bool requires_message_loop() const;
-  virtual const char* message_loop_name() const;
-
 
   // Initialize a VideoDecoder with the given DemuxerStream, executing the
   // callback upon completion.
@@ -256,8 +232,6 @@ class VideoDecoder : public Filter {
 class AudioDecoder : public Filter {
  public:
   virtual const char* major_mime_type() const;
-  virtual bool requires_message_loop() const;
-  virtual const char* message_loop_name() const;
 
   // Initialize a AudioDecoder with the given DemuxerStream, executing the
   // callback upon completion.
diff --git a/media/base/message_loop_factory.cc b/media/base/message_loop_factory.cc
new file mode 100644
index 0000000..e5b1d33
--- /dev/null
+++ b/media/base/message_loop_factory.cc
@@ -0,0 +1,11 @@
+// Copyright (c) 2011 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/message_loop_factory.h"
+
+namespace media {
+
+MessageLoopFactory::~MessageLoopFactory() {}
+
+}  // namespace media
diff --git a/media/base/message_loop_factory.h b/media/base/message_loop_factory.h
new file mode 100644
index 0000000..dacfab2
--- /dev/null
+++ b/media/base/message_loop_factory.h
@@ -0,0 +1,34 @@
+// Copyright (c) 2011 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_MESSAGE_LOOP_FACTORY_H_
+#define MEDIA_BASE_MESSAGE_LOOP_FACTORY_H_
+
+#include <string>
+
+#include "base/scoped_ptr.h"
+
+class MessageLoop;
+
+namespace media {
+
+// Factory object that manages named MessageLoops.
+class MessageLoopFactory {
+ public:
+  // Get the message loop associated with |name|. A new MessageLoop
+  // is created if the factory doesn't have one associated with |name|.
+  // NULL is returned if |name| is an empty string, or a new
+  // MessageLoop needs to be created and a failure occurs during the
+  // creation process.
+  virtual MessageLoop* GetMessageLoop(const std::string& name) = 0;
+
+ protected:
+  // Only allow scoped_ptr<> to delete factory.
+  friend class scoped_ptr<MessageLoopFactory>;
+  virtual ~MessageLoopFactory();
+};
+
+}  // namespace media
+
+#endif  // MEDIA_BASE_MESSAGE_LOOP_FACTORY_H_
diff --git a/media/base/message_loop_factory_impl.cc b/media/base/message_loop_factory_impl.cc
new file mode 100644
index 0000000..bcf3bf8
--- /dev/null
+++ b/media/base/message_loop_factory_impl.cc
@@ -0,0 +1,51 @@
+// Copyright (c) 2011 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/message_loop_factory_impl.h"
+
+namespace media {
+
+MessageLoopFactoryImpl::MessageLoopFactoryImpl() {}
+
+MessageLoopFactoryImpl::~MessageLoopFactoryImpl() {
+  AutoLock auto_lock(lock_);
+
+  for (ThreadMap::iterator iter = thread_map_.begin();
+       iter != thread_map_.end();
+       ++iter) {
+    base::Thread* thread = (*iter).second;
+
+    if (thread) {
+      thread->Stop();
+      delete thread;
+    }
+  }
+  thread_map_.clear();
+}
+
+// MessageLoopFactory methods.
+MessageLoop* MessageLoopFactoryImpl::GetMessageLoop(const std::string& name) {
+  if (name.empty()) {
+    return NULL;
+  }
+
+  AutoLock auto_lock(lock_);
+
+  ThreadMap::iterator it = thread_map_.find(name);
+  if (it != thread_map_.end())
+    return (*it).second->message_loop();
+
+  scoped_ptr<base::Thread> thread(new base::Thread(name.c_str()));
+
+  if (thread->Start()) {
+    MessageLoop* message_loop = thread->message_loop();
+    thread_map_[name] = thread.release();
+    return message_loop;
+  }
+
+  LOG(ERROR) << "Failed to start '" << name << "' thread!";
+  return NULL;
+}
+
+}  // namespace media
diff --git a/media/base/message_loop_factory_impl.h b/media/base/message_loop_factory_impl.h
new file mode 100644
index 0000000..1c8a61a
--- /dev/null
+++ b/media/base/message_loop_factory_impl.h
@@ -0,0 +1,38 @@
+// Copyright (c) 2011 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_MESSAGE_LOOP_FACTORY_IMPL_H_
+#define MEDIA_BASE_MESSAGE_LOOP_FACTORY_IMPL_H_
+
+#include <map>
+#include <string>
+
+#include "base/threading/thread.h"
+#include "media/base/message_loop_factory.h"
+
+namespace media {
+
+class MessageLoopFactoryImpl : public MessageLoopFactory {
+ public:
+  MessageLoopFactoryImpl();
+
+  // MessageLoopFactory methods.
+  virtual MessageLoop* GetMessageLoop(const std::string& name);
+
+ protected:
+  virtual ~MessageLoopFactoryImpl();
+
+ private:
+  // Lock used to serialize access for the following data members.
+  Lock lock_;
+
+  typedef std::map<std::string, base::Thread*> ThreadMap;
+  ThreadMap thread_map_;
+
+  DISALLOW_COPY_AND_ASSIGN(MessageLoopFactoryImpl);
+};
+
+}  // namespace media
+
+#endif  // MEDIA_BASE_MESSAGE_LOOP_FACTORY_IMPL_H_
diff --git a/media/base/mock_filters.cc b/media/base/mock_filters.cc
index 90df69b..8aa5458 100644
--- a/media/base/mock_filters.cc
+++ b/media/base/mock_filters.cc
@@ -74,23 +74,9 @@ void RunStopFilterCallback(FilterCallback* callback) {
   delete callback;
 }
 
-MockFilter::MockFilter() :
-    requires_message_loop_(false) {
-}
-
-MockFilter::MockFilter(bool requires_message_loop) :
-    requires_message_loop_(requires_message_loop) {
+MockFilter::MockFilter() {
 }
 
 MockFilter::~MockFilter() {}
 
-bool MockFilter::requires_message_loop() const {
-  return requires_message_loop_;
-}
-
-const char* MockFilter::message_loop_name() const {
-  return "MockFilter";
-}
-
-
 }  // namespace media
diff --git a/media/base/mock_filters.h b/media/base/mock_filters.h
index 45c1e29..3439597 100644
--- a/media/base/mock_filters.h
+++ b/media/base/mock_filters.h
@@ -46,12 +46,8 @@ class Destroyable : public MockClass {
 class MockFilter : public Filter {
  public:
   MockFilter();
-  MockFilter(bool requires_message_loop);
 
   // Filter implementation.
-  virtual bool requires_message_loop() const;
-  virtual const char* message_loop_name() const;
-
   MOCK_METHOD1(Play, void(FilterCallback* callback));
   MOCK_METHOD1(Pause, void(FilterCallback* callback));
   MOCK_METHOD1(Flush, void(FilterCallback* callback));
@@ -64,9 +60,6 @@ class MockFilter : public Filter {
   virtual ~MockFilter();
 
  private:
-
-  bool requires_message_loop_;
-
   DISALLOW_COPY_AND_ASSIGN(MockFilter);
 };