Tell OSX to manage its own property listener callback thread.

Seems OSX might be expecting us to only make AudioObjectPropertyData
calls on the same thread as listener callbacks are fired on.  Setting the
kAudioHardwarePropertyRunLoop to NULL tells OSX to manage its own thread,
which will hopefully ensure it has to fire callbacks in a concurrency safe
manner.

Another option might be to set the run loop to the one used by the
audio thread.  However, currently we call AudioObjectPropertyData in
many places, not all of them on the audio thread.

In the WebRTC code base they have this code:

https://code.google.com/searchframe#OAMlx_jo-ck/src/third_party/webrtc/modules/audio_device/mac/audio_device_mac.cc&l=374

I also found:
http://stackoverflow.com/questions/9674666/default-audio-output-getting-device-changed-notification-coreaudio-mac-os-x
http://lists.apple.com/archives/coreaudio-api/2009/Oct/msg00212.html

BUG=158170
TEST=device changes still work.

Review URL: https://codereview.chromium.org/11413174

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

2 files changed, 23 insertions, 22 deletions

diff --git a/media/audio/mac/audio_manager_mac.cc b/media/audio/mac/audio_manager_mac.cc
index 9258929..d00a880e 100644
--- a/media/audio/mac/audio_manager_mac.cc
+++ b/media/audio/mac/audio_manager_mac.cc
@@ -11,6 +11,7 @@
 #include "base/command_line.h"
 #include "base/mac/mac_logging.h"
 #include "base/mac/scoped_cftyperef.h"
+#include "base/message_loop.h"
 #include "base/sys_string_conversions.h"
 #include "media/audio/audio_util.h"
 #include "media/audio/mac/audio_input_mac.h"
@@ -19,7 +20,6 @@
 #include "media/audio/mac/audio_output_mac.h"
 #include "media/audio/mac/audio_synchronized_mac.h"
 #include "media/audio/mac/audio_unified_mac.h"
-#include "media/base/bind_to_loop.h"
 #include "media/base/limits.h"
 #include "media/base/media_switches.h"
 
@@ -264,17 +264,29 @@ static OSStatus OnDefaultDeviceChangedCallback(
 }
 
 AudioManagerMac::AudioManagerMac()
-    : listener_registered_(false),
-      creating_message_loop_(base::MessageLoopProxy::current()) {
+    : listener_registered_(false) {
   SetMaxOutputStreamsAllowed(kMaxOutputStreams);
 
-  // AudioManagerMac is expected to be created by the root platform thread, this
-  // is generally BrowserMainLoop, it's MessageLoop will drive the NSApplication
-  // pump which in turn fires the property listener callbacks.
-  if (!creating_message_loop_)
+  // Setting RunLoop to NULL here instructs HAL to manage its own thread for
+  // notifications.  This was the default behaviour on OS X 10.5 and earlier,
+  // but now must be explicitly specified.  Without this, OS X will try to run
+  // device notification callbacks on BrowserMainLoop.
+  const AudioObjectPropertyAddress kRunLoopAddress = {
+    kAudioHardwarePropertyRunLoop,
+    kAudioObjectPropertyScopeGlobal,
+    kAudioObjectPropertyElementMaster
+  };
+
+  CFRunLoopRef run_loop = NULL;
+  UInt32 size = sizeof(CFRunLoopRef);
+  OSStatus result = AudioObjectSetPropertyData(
+      kAudioObjectSystemObject, &kRunLoopAddress, 0, 0, size, &run_loop);
+  if (result != noErr) {
+    OSSTATUS_DLOG(ERROR, result) << "Failed to set property listener thread.";
     return;
+  }
 
-  OSStatus result = AudioObjectAddPropertyListener(
+  result = AudioObjectAddPropertyListener(
       kAudioObjectSystemObject,
       &kDeviceChangePropertyAddress,
       &OnDefaultDeviceChangedCallback,
@@ -290,9 +302,6 @@ AudioManagerMac::AudioManagerMac()
 
 AudioManagerMac::~AudioManagerMac() {
   if (listener_registered_) {
-    // TODO(dalecurtis): CHECK destruction happens on |creating_message_loop_|,
-    // should be true, but currently several unit tests perform destruction in
-    // odd places so we can't CHECK here currently.
     OSStatus result = AudioObjectRemovePropertyListener(
         kAudioObjectSystemObject,
         &kDeviceChangePropertyAddress,
@@ -390,15 +399,9 @@ AudioParameters AudioManagerMac::GetPreferredLowLatencyOutputStreamParameters(
 }
 
 void AudioManagerMac::OnDeviceChange() {
-  CHECK(creating_message_loop_->BelongsToCurrentThread());
-
-  // Post the task to the |creating_message_loop_| to execute our listener
-  // callback.  The callback is created using BindToLoop() so will hop over
-  // to the audio thread upon execution.
-  creating_message_loop_->PostTask(FROM_HERE, BindToLoop(
-      GetMessageLoop(), base::Bind(
-          &AudioManagerMac::NotifyAllOutputDeviceChangeListeners,
-          base::Unretained(this))));
+  GetMessageLoop()->PostTask(FROM_HERE, base::Bind(
+      &AudioManagerMac::NotifyAllOutputDeviceChangeListeners,
+      base::Unretained(this)));
 }
 
 AudioManager* CreateAudioManager() {
diff --git a/media/audio/mac/audio_manager_mac.h b/media/audio/mac/audio_manager_mac.h
index d8b6b2d..0bb89e8 100644
--- a/media/audio/mac/audio_manager_mac.h
+++ b/media/audio/mac/audio_manager_mac.h
@@ -7,7 +7,6 @@
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
-#include "base/message_loop_proxy.h"
 #include "media/audio/audio_manager_base.h"
 
 namespace media {
@@ -46,7 +45,6 @@ class MEDIA_EXPORT AudioManagerMac : public AudioManagerBase {
 
  private:
   bool listener_registered_;
-  scoped_refptr<base::MessageLoopProxy> creating_message_loop_;
 
   DISALLOW_COPY_AND_ASSIGN(AudioManagerMac);
 };