Refactoring in media::PipelineImpl and media::MediaFilter.

A hack in media::PipelineImpl::BroadcastMessageTask() was previously required to remove the mime_type of the disabled renderer.

Since the only use of media::PipelineImpl::BroadcastMessage() is to disable the audio renderer, refactor BroadcastMessage() to DisableAudioRenderer() and refactor media::MediaFilter::OnReceivedMessage() to OnAudioRendererDisabled().

Patch by boliu@google.com:
http://codereview.chromium.org/2042014/show

BUG=19384
TEST=media_unittests

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

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

10 files changed, 48 insertions, 61 deletions

diff --git a/media/base/filter_host.h b/media/base/filter_host.h
index 0d9af0f..e90096f 100644
--- a/media/base/filter_host.h
+++ b/media/base/filter_host.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2008-2009 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -70,8 +70,9 @@ class FilterHost {
   // Sets the flag to indicate current network activity.
   virtual void SetNetworkActivity(bool network_activity) = 0;
 
-  // Broadcast a message of type |message| to all filters.
-  virtual void BroadcastMessage(FilterMessage message) = 0;
+  // Disable audio renderer by calling OnAudioRendererDisabled() on all
+  // filters.
+  virtual void DisableAudioRenderer() = 0;
 
  protected:
   virtual ~FilterHost() {}
diff --git a/media/base/filters.h b/media/base/filters.h
index e5eb4cc..980e700 100644
--- a/media/base/filters.h
+++ b/media/base/filters.h
@@ -54,12 +54,6 @@ enum FilterType {
   FILTER_VIDEO_RENDERER
 };
 
-// A filter can broadcast messages to all other filters.  This identifies
-// the type of message to be broadcasted.
-enum FilterMessage {
-  kMsgDisableAudio,
-};
-
 // Used for completing asynchronous methods.
 typedef Callback0::Type FilterCallback;
 
@@ -131,10 +125,10 @@ class MediaFilter : public base::RefCountedThreadSafe<MediaFilter> {
     }
   }
 
-  // This method is called from the pipeline when a message of type |message|
-  // is broadcasted from |source|. Filters can ignore the message if they do
-  // not need to react to events raised from another filter.
-  virtual void OnReceivedMessage(FilterMessage message) {
+  // This method is called from the pipeline when the audio renderer
+  // is disabled. Filters can ignore the notification if they do not
+  // need to react to this event.
+  virtual void OnAudioRendererDisabled() {
   }
 
  protected:
diff --git a/media/base/mock_filter_host.h b/media/base/mock_filter_host.h
index a697226..a2fada9 100644
--- a/media/base/mock_filter_host.h
+++ b/media/base/mock_filter_host.h
@@ -1,6 +1,6 @@
-// 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.
+// 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.
 //
 // A FilterHost implementation based on gmock.  Combined with setting a message
 // loop on a filter, permits single-threaded testing of filters without
@@ -41,7 +41,7 @@ class MockFilterHost : public FilterHost {
   MOCK_METHOD1(SetLoaded, void(bool loaded));
   MOCK_METHOD1(SetNetworkActivity, void(bool network_activity));
   MOCK_METHOD0(NotifyEnded, void());
-  MOCK_METHOD1(BroadcastMessage, void(FilterMessage message));
+  MOCK_METHOD0(DisableAudioRenderer, void());
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockFilterHost);
diff --git a/media/base/mock_filters.h b/media/base/mock_filters.h
index 2886f3c..7f49fcf 100644
--- a/media/base/mock_filters.h
+++ b/media/base/mock_filters.h
@@ -97,7 +97,7 @@ class MockDataSource : public DataSource {
   MOCK_METHOD0(Stop, void());
   MOCK_METHOD1(SetPlaybackRate, void(float playback_rate));
   MOCK_METHOD2(Seek, void(base::TimeDelta time, FilterCallback* callback));
-  MOCK_METHOD1(OnReceivedMessage, void(FilterMessage message));
+  MOCK_METHOD0(OnAudioRendererDisabled, void());
 
   // DataSource implementation.
   MOCK_METHOD2(Initialize, void(const std::string& url,
@@ -123,7 +123,7 @@ class MockDemuxer : public Demuxer {
   MOCK_METHOD0(Stop, void());
   MOCK_METHOD1(SetPlaybackRate, void(float playback_rate));
   MOCK_METHOD2(Seek, void(base::TimeDelta time, FilterCallback* callback));
-  MOCK_METHOD1(OnReceivedMessage, void(FilterMessage message));
+  MOCK_METHOD0(OnAudioRendererDisabled, void());
 
   // Demuxer implementation.
   MOCK_METHOD2(Initialize, void(DataSource* data_source,
@@ -164,7 +164,7 @@ class MockVideoDecoder : public VideoDecoder {
   MOCK_METHOD0(Stop, void());
   MOCK_METHOD1(SetPlaybackRate, void(float playback_rate));
   MOCK_METHOD2(Seek, void(base::TimeDelta time, FilterCallback* callback));
-  MOCK_METHOD1(OnReceivedMessage, void(FilterMessage message));
+  MOCK_METHOD0(OnAudioRendererDisabled, void());
 
   // VideoDecoder implementation.
   MOCK_METHOD2(Initialize, void(DemuxerStream* stream,
@@ -187,7 +187,7 @@ class MockAudioDecoder : public AudioDecoder {
   MOCK_METHOD0(Stop, void());
   MOCK_METHOD1(SetPlaybackRate, void(float playback_rate));
   MOCK_METHOD2(Seek, void(base::TimeDelta time, FilterCallback* callback));
-  MOCK_METHOD1(OnReceivedMessage, void(FilterMessage message));
+  MOCK_METHOD0(OnAudioRendererDisabled, void());
 
   // AudioDecoder implementation.
   MOCK_METHOD2(Initialize, void(DemuxerStream* stream,
@@ -210,7 +210,7 @@ class MockVideoRenderer : public VideoRenderer {
   MOCK_METHOD0(Stop, void());
   MOCK_METHOD1(SetPlaybackRate, void(float playback_rate));
   MOCK_METHOD2(Seek, void(base::TimeDelta time, FilterCallback* callback));
-  MOCK_METHOD1(OnReceivedMessage, void(FilterMessage message));
+  MOCK_METHOD0(OnAudioRendererDisabled, void());
 
   // VideoRenderer implementation.
   MOCK_METHOD2(Initialize, void(VideoDecoder* decoder,
@@ -232,7 +232,7 @@ class MockAudioRenderer : public AudioRenderer {
   MOCK_METHOD0(Stop, void());
   MOCK_METHOD1(SetPlaybackRate, void(float playback_rate));
   MOCK_METHOD2(Seek, void(base::TimeDelta time, FilterCallback* callback));
-  MOCK_METHOD1(OnReceivedMessage, void(FilterMessage message));
+  MOCK_METHOD0(OnAudioRendererDisabled, void());
 
   // AudioRenderer implementation.
   MOCK_METHOD2(Initialize, void(AudioDecoder* decoder,
@@ -346,10 +346,10 @@ ACTION_P2(SetBufferedBytes, filter, bytes) {
   filter->host()->SetBufferedBytes(bytes);
 }
 
-// Helper gmock action that calls BroadcastMessage() on behalf of the provided
-// filter.
-ACTION_P2(BroadcastMessage, filter, message) {
-  filter->host()->BroadcastMessage(message);
+// Helper gmock action that calls DisableAudioRenderer() on behalf of the
+// provided filter.
+ACTION_P(DisableAudioRenderer, filter) {
+  filter->host()->DisableAudioRenderer();
 }
 
 }  // namespace media
diff --git a/media/base/pipeline_impl.cc b/media/base/pipeline_impl.cc
index cbc1061b..b5a193e 100644
--- a/media/base/pipeline_impl.cc
+++ b/media/base/pipeline_impl.cc
@@ -444,13 +444,12 @@ void PipelineImpl::SetNetworkActivity(bool network_activity) {
       NewRunnableMethod(this, &PipelineImpl::NotifyNetworkEventTask));
 }
 
-void PipelineImpl::BroadcastMessage(FilterMessage message) {
+void PipelineImpl::DisableAudioRenderer() {
   DCHECK(IsRunning());
 
-  // Broadcast the message on the message loop.
+  // Disable renderer on the message loop.
   message_loop_->PostTask(FROM_HERE,
-      NewRunnableMethod(this, &PipelineImpl::BroadcastMessageTask,
-                        message));
+      NewRunnableMethod(this, &PipelineImpl::DisableAudioRendererTask));
 }
 
 void PipelineImpl::InsertRenderedMimeType(const std::string& major_mime_type) {
@@ -752,23 +751,19 @@ void PipelineImpl::NotifyNetworkEventTask() {
   }
 }
 
-void PipelineImpl::BroadcastMessageTask(FilterMessage message) {
+void PipelineImpl::DisableAudioRendererTask() {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
 
-  // TODO(kylep): This is a horribly ugly hack, but we have no better way to
-  // log that audio is not and will not be working.
-  if (message == media::kMsgDisableAudio) {
-    // |rendered_mime_types_| is read through public methods so we need to lock
-    // this variable.
-    AutoLock auto_lock(lock_);
-    rendered_mime_types_.erase(mime_type::kMajorTypeAudio);
-  }
+  // |rendered_mime_types_| is read through public methods so we need to lock
+  // this variable.
+  AutoLock auto_lock(lock_);
+  rendered_mime_types_.erase(mime_type::kMajorTypeAudio);
 
-  // Broadcast the message to all filters.
+  // Notify all filters of disabled audio renderer.
   for (FilterVector::iterator iter = filters_.begin();
        iter != filters_.end();
        ++iter) {
-    (*iter)->OnReceivedMessage(message);
+    (*iter)->OnAudioRendererDisabled();
   }
 }
 
diff --git a/media/base/pipeline_impl.h b/media/base/pipeline_impl.h
index a1ca141..27ce43f 100644
--- a/media/base/pipeline_impl.h
+++ b/media/base/pipeline_impl.h
@@ -152,7 +152,7 @@ class PipelineImpl : public Pipeline, public FilterHost {
   virtual void SetLoaded(bool loaded);
   virtual void SetNetworkActivity(bool network_activity);
   virtual void NotifyEnded();
-  virtual void BroadcastMessage(FilterMessage message);
+  virtual void DisableAudioRenderer();
 
   // Method called during initialization to insert a mime type into the
   // |rendered_mime_types_| set.
@@ -203,8 +203,8 @@ class PipelineImpl : public Pipeline, public FilterHost {
   // Carries out handling a notification of network event.
   void NotifyNetworkEventTask();
 
-  // Carries out message broadcasting on the message loop.
-  void BroadcastMessageTask(FilterMessage message);
+  // Carries out disabling the audio renderer.
+  void DisableAudioRendererTask();
 
   // Carries out advancing to the next filter during Play()/Pause()/Seek().
   void FilterStateTransitionTask();
diff --git a/media/base/pipeline_impl_unittest.cc b/media/base/pipeline_impl_unittest.cc
index 9edae50..86b4e27 100644
--- a/media/base/pipeline_impl_unittest.cc
+++ b/media/base/pipeline_impl_unittest.cc
@@ -492,7 +492,7 @@ TEST_F(PipelineImplTest, Properties) {
             pipeline_->GetBufferedTime().ToInternalValue());
 }
 
-TEST_F(PipelineImplTest, BroadcastMessage) {
+TEST_F(PipelineImplTest, DisableAudioRenderer) {
   CreateAudioStream();
   CreateVideoStream();
   MockDemuxerStreamVector streams;
@@ -513,20 +513,19 @@ TEST_F(PipelineImplTest, BroadcastMessage) {
   EXPECT_TRUE(pipeline_->IsRendered(mime_type::kMajorTypeVideo));
 
   EXPECT_CALL(*mocks_->audio_renderer(), SetPlaybackRate(1.0f))
-      .WillOnce(BroadcastMessage(mocks_->audio_renderer(),
-                                 kMsgDisableAudio));
+      .WillOnce(DisableAudioRenderer(mocks_->audio_renderer()));
   EXPECT_CALL(*mocks_->data_source(),
-              OnReceivedMessage(kMsgDisableAudio));
+              OnAudioRendererDisabled());
   EXPECT_CALL(*mocks_->demuxer(),
-              OnReceivedMessage(kMsgDisableAudio));
+              OnAudioRendererDisabled());
   EXPECT_CALL(*mocks_->audio_decoder(),
-              OnReceivedMessage(kMsgDisableAudio));
+              OnAudioRendererDisabled());
   EXPECT_CALL(*mocks_->audio_renderer(),
-              OnReceivedMessage(kMsgDisableAudio));
+              OnAudioRendererDisabled());
   EXPECT_CALL(*mocks_->video_decoder(),
-              OnReceivedMessage(kMsgDisableAudio));
+              OnAudioRendererDisabled());
   EXPECT_CALL(*mocks_->video_renderer(),
-              OnReceivedMessage(kMsgDisableAudio));
+              OnAudioRendererDisabled());
 
   mocks_->audio_renderer()->SetPlaybackRate(1.0f);
 }
diff --git a/media/filters/ffmpeg_demuxer.cc b/media/filters/ffmpeg_demuxer.cc
index 5cc06d5..154e565 100644
--- a/media/filters/ffmpeg_demuxer.cc
+++ b/media/filters/ffmpeg_demuxer.cc
@@ -283,11 +283,9 @@ void FFmpegDemuxer::Seek(base::TimeDelta time, FilterCallback* callback) {
       NewRunnableMethod(this, &FFmpegDemuxer::SeekTask, time, callback));
 }
 
-void FFmpegDemuxer::OnReceivedMessage(FilterMessage message) {
-  if (message == kMsgDisableAudio) {
-    message_loop()->PostTask(FROM_HERE,
-        NewRunnableMethod(this, &FFmpegDemuxer::DisableAudioStreamTask));
-  }
+void FFmpegDemuxer::OnAudioRendererDisabled() {
+  message_loop()->PostTask(FROM_HERE,
+      NewRunnableMethod(this, &FFmpegDemuxer::DisableAudioStreamTask));
 }
 
 void FFmpegDemuxer::Initialize(DataSource* data_source,
diff --git a/media/filters/ffmpeg_demuxer.h b/media/filters/ffmpeg_demuxer.h
index bb55565..bc5cc62 100644
--- a/media/filters/ffmpeg_demuxer.h
+++ b/media/filters/ffmpeg_demuxer.h
@@ -133,7 +133,7 @@ class FFmpegDemuxer : public Demuxer,
   // MediaFilter implementation.
   virtual void Stop();
   virtual void Seek(base::TimeDelta time, FilterCallback* callback);
-  virtual void OnReceivedMessage(FilterMessage message);
+  virtual void OnAudioRendererDisabled();
 
   // Demuxer implementation.
   virtual void Initialize(DataSource* data_source, FilterCallback* callback);
diff --git a/media/filters/ffmpeg_demuxer_unittest.cc b/media/filters/ffmpeg_demuxer_unittest.cc
index 2e8eab9..8c3cb6a 100644
--- a/media/filters/ffmpeg_demuxer_unittest.cc
+++ b/media/filters/ffmpeg_demuxer_unittest.cc
@@ -647,7 +647,7 @@ TEST_F(FFmpegDemuxerTest, DisableAudioStream) {
   }
 
   // Submit a "disable audio stream" message to the demuxer.
-  demuxer_->OnReceivedMessage(kMsgDisableAudio);
+  demuxer_->OnAudioRendererDisabled();
   message_loop_.RunAllPending();
 
   // Ignore all AVFreePacket() calls.  We check this via valgrind.