Associate audio streams with their source/destination RenderView.

content/renderer/media/audio_message_filter.cc

Index: content/renderer/media/audio_message_filter.cc
diff --git a/content/renderer/media/audio_message_filter.cc b/content/renderer/media/audio_message_filter.cc
index d8a6e70c77eac5fa03ad3251e617b6f7b670e02c..2a55d3a7076fcbd6079ff609e0eba26a2b0c195e 100644
--- a/content/renderer/media/audio_message_filter.cc
+++ b/content/renderer/media/audio_message_filter.cc
@@ -22,18 +22,23 @@ AudioMessageFilter* AudioMessageFilter::Get() {
 }
 
 AudioMessageFilter::AudioMessageFilter()
-    : channel_(NULL) {
+    : next_stream_id_(1), channel_(NULL) {

[DaleCurtis, 2012/11/28 19:29:50]

wrap to next line.

[miu, 2012/11/28 20:59:03]

Done.

   DVLOG(1) << "AudioMessageFilter::AudioMessageFilter()";
   DCHECK(!filter_);
   filter_ = this;
 }
 
 int AudioMessageFilter::AddDelegate(media::AudioOutputIPCDelegate* delegate) {
-  return delegates_.Add(delegate);
+  base::AutoLock guard(delegates_lock_);
+  const int id = next_stream_id_;
+  delegates_.insert(std::make_pair(id, delegate));
+  ++next_stream_id_;
+  return id;
 }
 
 void AudioMessageFilter::RemoveDelegate(int id) {
-  delegates_.Remove(id);
+  base::AutoLock guard(delegates_lock_);
+  delegates_.erase(id);
 }
 
 void AudioMessageFilter::CreateStream(int stream_id,
@@ -42,6 +47,11 @@ void AudioMessageFilter::CreateStream(int stream_id,
   Send(new AudioHostMsg_CreateStream(stream_id, params, input_channels));
 }
 
+void AudioMessageFilter::AssociateStreamWithProducer(int stream_id,
+                                                     int render_view_id) {
+  Send(new AudioHostMsg_AssociateStreamWithProducer(stream_id, render_view_id));
+}
+
 void AudioMessageFilter::PlayStream(int stream_id) {
   Send(new AudioHostMsg_PlayStream(stream_id));
 }
@@ -102,15 +112,17 @@ void AudioMessageFilter::OnFilterRemoved() {
 
 void AudioMessageFilter::OnChannelClosing() {
   channel_ = NULL;
-  LOG_IF(WARNING, !delegates_.IsEmpty())
+
+  base::AutoLock guard(delegates_lock_);
+
+  LOG_IF(WARNING, !delegates_.empty())
       << "Not all audio devices have been closed.";
 
-  IDMap<media::AudioOutputIPCDelegate>::iterator it(&delegates_);
-  while (!it.IsAtEnd()) {
-    it.GetCurrentValue()->OnIPCClosed();
-    delegates_.Remove(it.GetCurrentKey());
-    it.Advance();
+  for (DelegateMap::const_iterator it = delegates_.begin();
+       it != delegates_.end(); ++it) {
+    it->second->OnIPCClosed();

[DaleCurtis, 2012/11/28 19:29:50]

Is it possible there might be an outstanding Add/Remove Delegate() call at this
point? I.e. blocked waiting for the lock above.

[miu, 2012/11/28 20:59:03]

Within my understanding: I think it's possible, but highly implausible.  If the
IPC channel is closing, the render process is shutting down.  Therefore, I
wouldn't expect any entities being created/started that would be racing to call
AddDelegate().  It's possible there will be attempts to call RemoveDelegate(),
but that should be perfectly safe.

Another way to look at this: In the old code, the same situation can occur. 
Meaning, it's possible that the call to OnChannelClosing() would occur after an
AddDelegate() task is posted to the IO thread's message loop but before the task
is run.

Either way, safety is assured later on.  If an entity successfully calls
AddDelegate() after the IPC channel closes, it still cannot send messages.  The
Send() method includes a protection to simply no-op when the channel_ has gone
away.

   }
+  delegates_.clear();
 }
 
 AudioMessageFilter::~AudioMessageFilter() {
@@ -136,25 +148,31 @@ void AudioMessageFilter::OnStreamCreated(
 #if !defined(OS_WIN)
   base::SyncSocket::Handle socket_handle = socket_descriptor.fd;
 #endif
-  media::AudioOutputIPCDelegate* delegate = delegates_.Lookup(stream_id);
-  if (!delegate) {
-    DLOG(WARNING) << "Got audio stream event for a non-existent or removed"
-                    " audio renderer. (stream_id=" << stream_id << ").";
-    base::SharedMemory::CloseHandle(handle);
-    base::SyncSocket socket(socket_handle);
-    return;
+
+  {
+    base::AutoLock guard(delegates_lock_);
+    DelegateMap::const_iterator it = delegates_.find(stream_id);
+    if (it != delegates_.end()) {
+      it->second->OnStreamCreated(handle, socket_handle, length);
+      return;
+    }
   }
-  delegate->OnStreamCreated(handle, socket_handle, length);
+
+  DLOG(WARNING) << "Got audio stream event for a non-existent or removed"

[DaleCurtis, 2012/11/28 19:29:50]

align " or add << for second line.

[miu, 2012/11/28 20:59:03]

Done.

+      " audio renderer. (stream_id=" << stream_id << ").";
+  base::SharedMemory::CloseHandle(handle);
+  base::SyncSocket socket(socket_handle);
 }
 
 void AudioMessageFilter::OnStreamStateChanged(
     int stream_id, media::AudioOutputIPCDelegate::State state) {
-  media::AudioOutputIPCDelegate* delegate = delegates_.Lookup(stream_id);
-  if (!delegate) {
+  base::AutoLock guard(delegates_lock_);
+  DelegateMap::const_iterator it = delegates_.find(stream_id);
+  if (it == delegates_.end()) {
     DLOG(WARNING) << "No delegate found for state change. " << state;
     return;
   }
-  delegate->OnStateChanged(state);
+  it->second->OnStateChanged(state);
 }
 
 }  // namespace content