Construct AudioRendererHost in BrowserRendererProcessHost, also construct

ResourceMessageFilter with pointer to it so we can delegate requests to it from IPC later.

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

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

8 files changed, 114 insertions, 18 deletions

diff --git a/chrome/browser/browser.scons b/chrome/browser/browser.scons
index d1dcd6a..de44b18 100644
--- a/chrome/browser/browser.scons
+++ b/chrome/browser/browser.scons
@@ -15,6 +15,7 @@ env.SConscript([
     '$ICU38_DIR/using_icu38.scons',
     '$LIBPNG_DIR/using_libpng.scons',
     '$LIBXML_DIR/using_libxml.scons',
+    '$MEDIA_DIR/using_media.scons',
     '$NPAPI_DIR/using_npapi.scons',
     '$SKIA_DIR/using_skia.scons',
     '$ZLIB_DIR/using_zlib.scons',
@@ -508,6 +509,8 @@ input_files = ChromeFileList([
   MSVSFilter('Renderer Host', [
     'renderer_host/async_resource_handler.cc',
     'renderer_host/async_resource_handler.h',
+    'renderer_host/audio_renderer_host.cc',
+    'renderer_host/audio_renderer_host.h',
     'renderer_host/backing_store.h',
     'renderer_host/backing_store.cc',
     'renderer_host/backing_store_win.cc',
diff --git a/chrome/browser/renderer_host/audio_renderer_host.cc b/chrome/browser/renderer_host/audio_renderer_host.cc
index 66ee9ff..fd798ac 100644
--- a/chrome/browser/renderer_host/audio_renderer_host.cc
+++ b/chrome/browser/renderer_host/audio_renderer_host.cc
@@ -68,18 +68,21 @@ void AudioRendererHost::IPCAudioSource::NotifyPacketReady() {
 
 AudioRendererHost::AudioRendererHost(MessageLoop* message_loop)
     : next_id_(INVALID_ID+1),
-      message_loop_(message_loop) {
+      io_loop_(message_loop) {
+  // Make sure we perform actual initialization operations in the thread where
+  // this object should live.
+  io_loop_->PostTask(FROM_HERE,
+      NewRunnableMethod(this, &AudioRendererHost::OnInitialized));
 }
 
 AudioRendererHost::~AudioRendererHost() {
-  DestroyAllStreams();
 }
 
 int AudioRendererHost::CreateStream(
     IPC::Message::Sender* sender, base::ProcessHandle handle,
     AudioManager::Format format, int channels, int sample_rate,
     int bits_per_sample, size_t packet_size) {
-  DCHECK(MessageLoop::current() == message_loop_);
+  DCHECK(MessageLoop::current() == io_loop_);
 
   // Create the stream in the first place.
   AudioOutputStream* stream = AudioManager::GetAudioManager()->MakeAudioStream(
@@ -100,7 +103,7 @@ int AudioRendererHost::CreateStream(
 }
 
 bool AudioRendererHost::Start(int stream_id) {
-  DCHECK(MessageLoop::current() == message_loop_);
+  DCHECK(MessageLoop::current() == io_loop_);
   IPCAudioSource* source = sources_.Lookup(stream_id);
   if (source) {
     source->stream()->Start(source);
@@ -110,7 +113,7 @@ bool AudioRendererHost::Start(int stream_id) {
 }
 
 bool AudioRendererHost::Stop(int stream_id) {
-  DCHECK(MessageLoop::current() == message_loop_);
+  DCHECK(MessageLoop::current() == io_loop_);
   IPCAudioSource* source = sources_.Lookup(stream_id);
   if (source) {
     source->stream()->Stop();
@@ -120,7 +123,7 @@ bool AudioRendererHost::Stop(int stream_id) {
 }
 
 bool AudioRendererHost::Close(int stream_id) {
-  DCHECK(MessageLoop::current() == message_loop_);
+  DCHECK(MessageLoop::current() == io_loop_);
   IPCAudioSource* source = sources_.Lookup(stream_id);
   if (source) {
     source->stream()->Close();
@@ -131,7 +134,7 @@ bool AudioRendererHost::Close(int stream_id) {
 
 bool AudioRendererHost::SetVolume(
     int stream_id, double left_channel, double right_channel) {
-  DCHECK(MessageLoop::current() == message_loop_);
+  DCHECK(MessageLoop::current() == io_loop_);
   IPCAudioSource* source = sources_.Lookup(stream_id);
   if (source) {
     source->stream()->SetVolume(left_channel, right_channel);
@@ -141,7 +144,7 @@ bool AudioRendererHost::SetVolume(
 
 bool AudioRendererHost::GetVolume(
     int stream_id, double* left_channel, double* right_channel) {
-  DCHECK(MessageLoop::current() == message_loop_);
+  DCHECK(MessageLoop::current() == io_loop_);
   IPCAudioSource* source = sources_.Lookup(stream_id);
   if (source) {
     source->stream()->GetVolume(left_channel, right_channel);
@@ -151,7 +154,7 @@ bool AudioRendererHost::GetVolume(
 }
 
 void AudioRendererHost::NotifyPacketReady(int stream_id) {
-  DCHECK(MessageLoop::current() == message_loop_);
+  DCHECK(MessageLoop::current() == io_loop_);
   IPCAudioSource* source = sources_.Lookup(stream_id);
   if (source) {
     source->NotifyPacketReady();
@@ -159,16 +162,48 @@ void AudioRendererHost::NotifyPacketReady(int stream_id) {
 }
 
 void AudioRendererHost::DestroyAllStreams() {
-  DCHECK(MessageLoop::current() == message_loop_);
+  DCHECK(MessageLoop::current() == io_loop_);
   // TODO(hclam): iterate on the map, close and delete every stream, and clear
   // the map.
 }
 
 void AudioRendererHost::DestroySource(int stream_id) {
-  DCHECK(MessageLoop::current() == message_loop_);
+  DCHECK(MessageLoop::current() == io_loop_);
   IPCAudioSource* source = sources_.Lookup(stream_id);
   if (source) {
     sources_.Remove(stream_id);
     delete source;
   }
 }
+
+void AudioRendererHost::Destroy() {
+  // Post a message to the thread where this object should live and do the
+  // actual operations there.
+  io_loop_->PostTask(
+      FROM_HERE, NewRunnableMethod(this, &AudioRendererHost::OnDestroyed));
+}
+
+void AudioRendererHost::OnInitialized() {
+  DCHECK(MessageLoop::current() == io_loop_);
+
+  // Increase the ref count of this object so it is active until we do
+  // Release().
+  AddRef();
+
+  // Also create the AudioManager singleton in this thread.
+  // TODO(hclam): figure out a better location to initialize the AudioManager
+  // singleton.
+  AudioManager::GetAudioManager();
+}
+
+void AudioRendererHost::OnDestroyed() {
+  DCHECK(MessageLoop::current() == io_loop_);
+
+  // Destroy audio streams only in the thread it should happen.
+  // TODO(hclam): make sure we don't call IPC::Message::Sender inside
+  // IPCAudioSource because it is most likely be destroyed.
+  DestroyAllStreams();
+
+  // Decrease the reference to this object, which may lead to self-destruction.
+  Release();
+}
diff --git a/chrome/browser/renderer_host/audio_renderer_host.h b/chrome/browser/renderer_host/audio_renderer_host.h
index 4df24ba..8128d61 100644
--- a/chrome/browser/renderer_host/audio_renderer_host.h
+++ b/chrome/browser/renderer_host/audio_renderer_host.h
@@ -17,11 +17,31 @@
 // passing a SharedMemoryHandle for filling the buffer.
 // NotifyPacketReady(|stream_id|) would be called when the buffer is filled
 // and ready to be consumed.
+//
+// This class is owned by BrowserRenderProcessHost, and instantiated on UI
+// thread, but all other operations and method calls (except Destroy()) happens
+// in IO thread, so we need to be extra careful about the lifetime of this
+// object. AudioManager is a singleton and created in IO thread, audio output
+// streams are also created in the IO thread, so we need to destroy them also
+// in IO thread. After this class is created, a task of OnInitialized() is
+// posted on IO thread in which singleton of AudioManager is created and
+// AddRef() is called to increase one ref count of this object. Owner of this
+// class should call Destroy() before decrementing the ref count to this object,
+// which essentially post a task of OnDestroyed() on IO thread. Inside
+// OnDestroyed(), audio output streams are destroyed and Release() is called
+// which may result in self-destruction.
+//
+// TODO(hclam): Have these things done before having real implementations:
+//   1. Make sure this class has greater or equal lifetime to
+//      IPC:Message::Sender, essentially ResourceMessageFilter.
+//   2. Listen to destruction event of the browser and do cleanup in case this
+//      class is not destructed nicely during browser close.
 
 #ifndef CHROME_BROWSER_RENDERER_HOST_AUDIO_RENDERER_HOST_H_
 #define CHROME_BROWSER_RENDERER_HOST_AUDIO_RENDERER_HOST_H_
 
 #include "base/id_map.h"
+#include "base/ref_counted.h"
 #include "base/shared_memory.h"
 #include "chrome/common/ipc_message.h"
 #include "media/audio/audio_output.h"
@@ -29,7 +49,7 @@
 class AudioManager;
 class MessageLoop;
 
-class AudioRendererHost {
+class AudioRendererHost : public base::RefCountedThreadSafe<AudioRendererHost> {
  public:
   static const int32 INVALID_ID = 0;
 
@@ -74,14 +94,24 @@ class AudioRendererHost {
   // consumed.
   void NotifyPacketReady(int32 stream_id);
 
-  // Destroy all audio output streams.
-  void DestroyAllStreams();
+  // Called from UI thread from the owner of this object.
+  void Destroy();
 
   // Destroy the stream specified by |stream_id| and remove it from map.
   // *DO NOT* call this method other than from IPCAudioSource.
   void DestroySource(int32 stream_id);
 
  private:
+  // Methods called on IO thread.
+  // Called on IO thread when this object is created and initialized.
+  void OnInitialized();
+  // Called on IO thread when this object needs to be destroyed and after
+  // Destroy() is called from owner of this class in UI thread.
+  void OnDestroyed();
+
+  // Destroy all audio output streams.
+  void DestroyAllStreams();
+
   // The container for AudioOutputStream and serves audio packet for it by IPC.
   class IPCAudioSource : public AudioOutputStream::AudioSourceCallback {
    public:
@@ -124,7 +154,7 @@ class AudioRendererHost {
 
   // Only used for DCHECKs to make sure all methods calls are from the same
   // thread as this object is created.
-  MessageLoop* message_loop_;
+  MessageLoop* io_loop_;
 
   DISALLOW_COPY_AND_ASSIGN(AudioRendererHost);
 };
diff --git a/chrome/browser/renderer_host/audio_renderer_host_unittest.cc b/chrome/browser/renderer_host/audio_renderer_host_unittest.cc
index 443966b..d7ab831 100644
--- a/chrome/browser/renderer_host/audio_renderer_host_unittest.cc
+++ b/chrome/browser/renderer_host/audio_renderer_host_unittest.cc
@@ -11,10 +11,17 @@
 class AudioRendererHostTest : public testing::Test {
  protected:
   virtual void SetUp() {
-    host_.reset(new AudioRendererHost(MessageLoop::current()));
+    // Create a message loop so AudioRendererHost can use it.
+    message_loop_.reset(new MessageLoop(MessageLoop::TYPE_IO));
+    host_ = new AudioRendererHost(MessageLoop::current());
   }
 
-  scoped_ptr<AudioRendererHost> host_;
+  virtual void TearDown() {
+    host_->Destroy();
+  }
+
+  scoped_refptr<AudioRendererHost> host_;
+  scoped_ptr<MessageLoop> message_loop_;
 };
 
 TEST_F(AudioRendererHostTest, NoTest) {
diff --git a/chrome/browser/renderer_host/browser_render_process_host.cc b/chrome/browser/renderer_host/browser_render_process_host.cc
index b020feb..24d5f4d 100644
--- a/chrome/browser/renderer_host/browser_render_process_host.cc
+++ b/chrome/browser/renderer_host/browser_render_process_host.cc
@@ -155,6 +155,10 @@ BrowserRenderProcessHost::~BrowserRenderProcessHost() {
   // We may have some unsent messages at this point, but that's OK.
   channel_.reset();
 
+  // Destroy the AudioRendererHost properly.
+  if (audio_renderer_host_.get())
+    audio_renderer_host_->Destroy();
+
   if (process_.handle() && !run_renderer_in_process()) {
     ProcessWatcher::EnsureProcessTerminated(process_.handle());
   }
@@ -195,8 +199,13 @@ bool BrowserRenderProcessHost::Init() {
   // run the IPC channel on the shared IO thread.
   base::Thread* io_thread = g_browser_process->io_thread();
 
+  // Construct the AudioRendererHost with the IO thread.
+  audio_renderer_host_ =
+      new AudioRendererHost(io_thread->message_loop());
+
   scoped_refptr<ResourceMessageFilter> resource_message_filter =
       new ResourceMessageFilter(g_browser_process->resource_dispatcher_host(),
+                                audio_renderer_host_.get(),
                                 PluginService::GetInstance(),
                                 g_browser_process->print_job_manager(),
                                 host_id(),
diff --git a/chrome/browser/renderer_host/browser_render_process_host.h b/chrome/browser/renderer_host/browser_render_process_host.h
index 3e40d50..d089516 100644
--- a/chrome/browser/renderer_host/browser_render_process_host.h
+++ b/chrome/browser/renderer_host/browser_render_process_host.h
@@ -13,6 +13,7 @@
 #include "base/ref_counted.h"
 #include "base/scoped_ptr.h"
 #include "base/shared_memory.h"
+#include "chrome/browser/renderer_host/audio_renderer_host.h"
 #include "chrome/browser/renderer_host/render_process_host.h"
 #include "chrome/common/notification_observer.h"
 #include "webkit/glue/cache_manager.h"
@@ -136,6 +137,9 @@ class BrowserRenderProcessHost : public RenderProcessHost,
   // IO thread.
   scoped_refptr<RenderWidgetHelper> widget_helper_;
 
+  // The host of audio renderers in the renderer process.
+  scoped_refptr<AudioRendererHost> audio_renderer_host_;
+
   DISALLOW_COPY_AND_ASSIGN(BrowserRenderProcessHost);
 };
 
diff --git a/chrome/browser/renderer_host/resource_message_filter.cc b/chrome/browser/renderer_host/resource_message_filter.cc
index 2db7124..19f0ad9 100644
--- a/chrome/browser/renderer_host/resource_message_filter.cc
+++ b/chrome/browser/renderer_host/resource_message_filter.cc
@@ -13,6 +13,7 @@
 #include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/net/dns_global.h"
 #include "chrome/browser/profile.h"
+#include "chrome/browser/renderer_host/audio_renderer_host.h"
 #include "chrome/browser/renderer_host/browser_render_process_host.h"
 #include "chrome/browser/renderer_host/render_widget_helper.h"
 #include "chrome/common/chrome_plugin_lib.h"
@@ -92,6 +93,7 @@ class WriteClipboardTask : public Task {
 
 ResourceMessageFilter::ResourceMessageFilter(
     ResourceDispatcherHost* resource_dispatcher_host,
+    AudioRendererHost* audio_renderer_host,
     PluginService* plugin_service,
     printing::PrintJobManager* print_job_manager,
     int render_process_host_id,
@@ -108,7 +110,8 @@ ResourceMessageFilter::ResourceMessageFilter(
       render_handle_(NULL),
       request_context_(profile->GetRequestContext()),
       profile_(profile),
-      render_widget_helper_(render_widget_helper) {
+      render_widget_helper_(render_widget_helper),
+      audio_renderer_host_(audio_renderer_host) {
 
   DCHECK(request_context_.get());
   DCHECK(request_context_->cookie_store());
diff --git a/chrome/browser/renderer_host/resource_message_filter.h b/chrome/browser/renderer_host/resource_message_filter.h
index c4d75e3..34d3c3d 100644
--- a/chrome/browser/renderer_host/resource_message_filter.h
+++ b/chrome/browser/renderer_host/resource_message_filter.h
@@ -25,6 +25,7 @@
 #include "chrome/common/temp_scaffolding_stubs.h"
 #endif
 
+class AudioRendererHost;
 class ClipboardService;
 class Profile;
 class RenderWidgetHelper;
@@ -57,6 +58,7 @@ class ResourceMessageFilter : public IPC::ChannelProxy::MessageFilter,
   //        ResourceMessageFilter is 'given' ownership of the spellchecker
   //        object and must clean it up on exit.
   ResourceMessageFilter(ResourceDispatcherHost* resource_dispatcher_host,
+                        AudioRendererHost* audio_renderer_host,
                         PluginService* plugin_service,
                         printing::PrintJobManager* print_job_manager,
                         int render_process_host_id,
@@ -239,6 +241,9 @@ class ResourceMessageFilter : public IPC::ChannelProxy::MessageFilter,
 
   scoped_refptr<RenderWidgetHelper> render_widget_helper_;
 
+  // Object that should take care of audio related resource requests.
+  scoped_refptr<AudioRendererHost> audio_renderer_host_;
+
   DISALLOW_COPY_AND_ASSIGN(ResourceMessageFilter);
 };