Handle audio device changes on Mac (Take 2).

media/audio/mac/audio_device_listener_mac.cc

Index: media/audio/mac/audio_device_listener_mac.cc
diff --git a/media/audio/mac/audio_device_listener_mac.cc b/media/audio/mac/audio_device_listener_mac.cc
new file mode 100644
index 0000000000000000000000000000000000000000..7f0e8f15decc51c9f3eed31ea0a53086f7fd5247
--- /dev/null
+++ b/media/audio/mac/audio_device_listener_mac.cc
@@ -0,0 +1,67 @@
+// Copyright (c) 2012 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.
+
+#include "media/audio/mac/audio_device_listener_mac.h"
+
+#include <CoreAudio/CoreAudio.h>
+
+#include "base/logging.h"
+#include "base/mac/mac_logging.h"
+
+namespace media {
+
+static const AudioObjectPropertyAddress kPropertyAddress = {
+  kAudioHardwarePropertyDefaultOutputDevice,
+  kAudioObjectPropertyScopeGlobal,
+  kAudioObjectPropertyElementMaster
+};
+
+// Callback from the system when the default device changes.  This can be called
+// either on the main thread or on an audio thread managed by the system
+// depending on what kAudioHardwarePropertyRunLoop is set to.
+static OSStatus OnDefaultDeviceChangedCallback(
+    AudioObjectID object,
+    UInt32 size,
+    const AudioObjectPropertyAddress addresses[],
+    void* context) {
+  static_cast<base::Closure*>(context)->Run();

[Scott Hess - ex-Googler, 2012/10/31 23:39:11]

If you've arranged for the callback to live forever, then you don't need
AudioDeviceListenerMac class and manual scoped_ptr junk at all, it can just be
static methods to register/unregister.

If you _haven't_ arranged for the callback to live forever, then I don't see how
you're providing thread-safety.

AFAICT, you've only arranged for the context-callback to outlive
AudioDeviceListenerMac, and for AudioDeviceListenerMac to be created and deleted
on the "audio" thread (which AFAICT has no relationship to the thread that this
callback function is called on).  So this callback function could be executing
when ~AudioDeviceListenerMac() has completed.

[scherkus (not reviewing), 2012/11/01 00:15:40]

I do like having register/unregister auto-scoped, but I will concede we're not
taking advantage of that today :)
While it's certainly possible to have the callback execute _during_ ~ADLM(), I
haven't seen any documentation/evidence that states whether it's possible for
the callback to execute after removing the listener.

I certainly hope AudioObjectRemovePropertyListener() guarantees that any
existing callback would complete and that no further callbacks are executed --
otherwise it forces everyone to handle a spurious callback firing w/ potentially
invalid userdata pointer!

[DaleCurtis, 2012/11/01 00:16:33]

Indeed it could, which is exactly what the method documentation says.

The callback now lives nearly "forever" since AudioManager won't be destroyed
until BrowserMainLoop dies which is near enough to process shutdown that I don't
think we need to worry about it any further.  If we see crashes in the field we
can consider a singleton type pattern.

static register/unregister methods aren't a bad idea and less crazy than keeping
a local pointer copy. I'll switch to that in the next patch set.

+  return noErr;
+}
+
+AudioDeviceListenerMac::AudioDeviceListenerMac(base::Closure* listener_cb)
+    : listener_cb_(listener_cb) {
+  CHECK(listener_cb_);
+  DCHECK(!listener_cb_->is_null());
+
+  OSStatus result = AudioObjectAddPropertyListener(
+      kAudioObjectSystemObject,
+      &kPropertyAddress,
+      &OnDefaultDeviceChangedCallback,
+      listener_cb_);
+
+  // Clear |listener_cb_| so we don't try to remove the listener later.
+  if (result != noErr) {
+    OSSTATUS_DLOG(ERROR, result) << "AudioObjectAddPropertyListener() failed!";
+    listener_cb_ = NULL;
+  }
+}
+
+AudioDeviceListenerMac::~AudioDeviceListenerMac() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  // If adding the listener failed, don't try and remove it.
+  if (!listener_cb_)

[scherkus (not reviewing), 2012/10/31 23:17:22]

you can use is_null() instead since you DCHECK it can't be null when created

[DaleCurtis, 2012/11/01 00:16:33]

No, it's set to NULL if AddPropertyListener fails.

+    return;
+
+  OSStatus result = AudioObjectRemovePropertyListener(
+      kAudioObjectSystemObject,
+      &kPropertyAddress,
+      &OnDefaultDeviceChangedCallback,
+      listener_cb_);
+
+  OSSTATUS_DLOG_IF(ERROR, result != noErr, result)
+      << "AudioObjectRemovePropertyListener() failed!";
+}
+
+}  // namespace media