Merge AssociateStreamWithProducer message into CreateStream message for both audio output and input.

content/renderer/media/audio_input_message_filter.cc

Index: content/renderer/media/audio_input_message_filter.cc
diff --git a/content/renderer/media/audio_input_message_filter.cc b/content/renderer/media/audio_input_message_filter.cc
index 58dcc2ec7a06e789bec9d398a3f906e7f8fbad09..67c2752ef3e03fa0b7416d008bf70dfb4bb10c21 100644
--- a/content/renderer/media/audio_input_message_filter.cc
+++ b/content/renderer/media/audio_input_message_filter.cc
@@ -11,24 +11,53 @@
 
 namespace content {
 
-AudioInputMessageFilter* AudioInputMessageFilter::filter_ = NULL;
+namespace {

[no longer working on chromium, 2013/03/05 18:33:10]

nit, add an empty line.

[miu, 2013/03/06 19:25:41]

Done.

+const int kStreamIDNotSet = -1;
+
+// The singleton instance of AudioInputMessageFilter.  Set/unset by the
+// ctor/dtor.
+AudioInputMessageFilter* g_filter = NULL;

[DaleCurtis, 2013/03/05 23:29:54]

This should be a scoped_refptr() since IPC::ChannelProxy::MessageFilter is ref
counted.

[miu, 2013/03/06 22:36:52]

Done.  Removed the global pointer scheme in Audio*MessageFilter.  All accesses
are through RenderThreadImpl, and ref-counted pointers maintained where needed.

+}

[no longer working on chromium, 2013/03/05 18:33:10]

ditto

[miu, 2013/03/06 19:25:41]

Done.

+
+class AudioInputMessageFilter::AudioInputIPCImpl
+    : public NON_EXPORTED_BASE(media::AudioInputIPC) {
+ public:
+  explicit AudioInputIPCImpl(int render_view_id);
+  virtual ~AudioInputIPCImpl();
+
+  // media::AudioInputIPC implementation.
+  virtual void CreateStream(media::AudioInputIPCDelegate* delegate,
+                            const media::AudioParameters& params,
+                            const std::string& device_id,

[palmer, 2013/03/05 21:09:32]

It'd be better to constrain this type, rather than using string.

+                            bool automatic_gain_control) OVERRIDE;
+  virtual void StartDevice(media::AudioInputIPCDelegate* delegate,
+                           int session_id) OVERRIDE;
+  virtual void RecordStream() OVERRIDE;
+  virtual void SetVolume(double volume) OVERRIDE;
+  virtual void CloseStream() OVERRIDE;
+
+ private:
+  const int render_view_id_;
+  int stream_id_;
+};
 
 AudioInputMessageFilter::AudioInputMessageFilter(
     const scoped_refptr<base::MessageLoopProxy>& io_message_loop)
     : channel_(NULL),
       io_message_loop_(io_message_loop) {
-  DCHECK(!filter_);
-  filter_ = this;
+  DCHECK(!g_filter);
+  g_filter = this;
 }
 
 AudioInputMessageFilter::~AudioInputMessageFilter() {
-  DCHECK_EQ(filter_, this);
-  filter_ = NULL;
+  CHECK(delegates_.IsEmpty());

[DaleCurtis, 2013/03/05 23:29:54]

This can't be done here since the message filter is not destructed on the IO
thread.

[miu, 2013/03/06 22:36:52]

Added comment in code.  Explanation: When the destructor is called, there should
be no references outstanding (ref-count is zero).  That means there shouldn't be
code on any other thread attempting to reference delegates_.

BTW--This particular LOC was changed to make this match ~AudioMessageFilter().

+  DCHECK_EQ(g_filter, this);
+  g_filter = NULL;
 }
 
 // static.
 AudioInputMessageFilter* AudioInputMessageFilter::Get() {

[DaleCurtis, 2013/03/05 23:29:54]

Can we remove the singleton aspects in favor of scoped_refptr() and requesting
the MessageFilter from the RenderThread?

[miu, 2013/03/06 22:36:52]

Agreed and done.

-  return filter_;
+  return g_filter;
 }
 
 void AudioInputMessageFilter::Send(IPC::Message* message) {
@@ -146,45 +175,68 @@ void AudioInputMessageFilter::OnDeviceStarted(int stream_id,
   delegate->OnDeviceReady(device_id);
 }
 
-int AudioInputMessageFilter::AddDelegate(
-    media::AudioInputIPCDelegate* delegate) {
-  DCHECK(io_message_loop_->BelongsToCurrentThread());
-  return delegates_.Add(delegate);
-}
+AudioInputMessageFilter::AudioInputIPCImpl::AudioInputIPCImpl(
+    int render_view_id)
+    : render_view_id_(render_view_id), stream_id_(kStreamIDNotSet) {}

[no longer working on chromium, 2013/03/05 18:33:10]

nit, one line for each member initialization.

[miu, 2013/03/06 19:25:41]

Half the reviewers want fewer LOC, and half want more.  ;-)  Happy to do it
either way...

 
-void AudioInputMessageFilter::RemoveDelegate(int id) {
-  DCHECK(io_message_loop_->BelongsToCurrentThread());
-  delegates_.Remove(id);
-}
+AudioInputMessageFilter::AudioInputIPCImpl::~AudioInputIPCImpl() {}
 
-void AudioInputMessageFilter::CreateStream(int stream_id,
-                                           const media::AudioParameters& params,
-                                           const std::string& device_id,
-                                           bool automatic_gain_control) {
-  Send(new AudioInputHostMsg_CreateStream(
-      stream_id, params, device_id, automatic_gain_control));
+scoped_ptr<media::AudioInputIPC> AudioInputMessageFilter::CreateAudioInputIPC(
+    int render_view_id) {
+  DCHECK_LT(0, render_view_id);
+  return scoped_ptr<media::AudioInputIPC>(
+      new AudioInputIPCImpl(render_view_id));
 }
 
-void AudioInputMessageFilter::AssociateStreamWithConsumer(int stream_id,
-                                                          int render_view_id) {
-  Send(new AudioInputHostMsg_AssociateStreamWithConsumer(
-      stream_id, render_view_id));
+void AudioInputMessageFilter::AudioInputIPCImpl::CreateStream(
+    media::AudioInputIPCDelegate* delegate,
+    const media::AudioParameters& params,
+    const std::string& device_id,
+    bool automatic_gain_control) {
+  DCHECK(g_filter->io_message_loop_->BelongsToCurrentThread());
+  DCHECK(delegate);
+
+  // TODO(miu): This ugliness is due to the "start device" then "create stream"
+  // hacking that's been done for audio input.  Major IPC/impl clean-up is
+  // needed here.  http://crbug.com/179597.
+  if (stream_id_ == kStreamIDNotSet) {
+    stream_id_ = g_filter->delegates_.Add(delegate);
+  } else {
+    // Security CHECK, not a DCHECK, since using the StartDevice() path requires
+    // that permissions are obtained in the browser process.  This confirms the
+    // delegate that called StartDevice() is the same one doing the CreateStream
+    // operation here.
+    CHECK_EQ(delegate, g_filter->delegates_.Lookup(stream_id_));
+  }
+  g_filter->Send(new AudioInputHostMsg_CreateStream(
+      stream_id_, render_view_id_, params, device_id, automatic_gain_control));
 }
 
-void AudioInputMessageFilter::StartDevice(int stream_id, int session_id) {
-  Send(new AudioInputHostMsg_StartDevice(stream_id, session_id));
+void AudioInputMessageFilter::AudioInputIPCImpl::StartDevice(
+    media::AudioInputIPCDelegate* delegate,
+    int session_id) {
+  DCHECK(delegate);
+  DCHECK_EQ(kStreamIDNotSet, stream_id_);
+  stream_id_ = g_filter->delegates_.Add(delegate);
+  g_filter->Send(new AudioInputHostMsg_StartDevice(stream_id_, session_id));
 }
 
-void AudioInputMessageFilter::RecordStream(int stream_id) {
-  Send(new AudioInputHostMsg_RecordStream(stream_id));
+void AudioInputMessageFilter::AudioInputIPCImpl::RecordStream() {
+  DCHECK_NE(kStreamIDNotSet, stream_id_);
+  g_filter->Send(new AudioInputHostMsg_RecordStream(stream_id_));
 }
 
-void AudioInputMessageFilter::CloseStream(int stream_id) {
-  Send(new AudioInputHostMsg_CloseStream(stream_id));
+void AudioInputMessageFilter::AudioInputIPCImpl::SetVolume(double volume) {
+  DCHECK_NE(kStreamIDNotSet, stream_id_);
+  g_filter->Send(new AudioInputHostMsg_SetVolume(stream_id_, volume));
 }
 
-void AudioInputMessageFilter::SetVolume(int stream_id, double volume) {
-  Send(new AudioInputHostMsg_SetVolume(stream_id, volume));
+void AudioInputMessageFilter::AudioInputIPCImpl::CloseStream() {
+  DCHECK(g_filter->io_message_loop_->BelongsToCurrentThread());
+  DCHECK_NE(kStreamIDNotSet, stream_id_);
+  g_filter->Send(new AudioInputHostMsg_CloseStream(stream_id_));
+  g_filter->delegates_.Remove(stream_id_);
+  stream_id_ = kStreamIDNotSet;
 }
 
 }  // namespace content