Use the browser UI thread for audio on OSX.

media/audio/audio_manager_base.cc

Index: media/audio/audio_manager_base.cc
diff --git a/media/audio/audio_manager_base.cc b/media/audio/audio_manager_base.cc
index 43dc31e20e742cbe56554c307ad1cf3efdd5b4c0..f95faef36f0fd09a068cb87cec1f5ffca191fb8e 100644
--- a/media/audio/audio_manager_base.cc
+++ b/media/audio/audio_manager_base.cc
@@ -6,14 +6,17 @@
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
+#include "base/command_line.h"
 #include "base/message_loop_proxy.h"
 #include "base/threading/thread.h"
+#include "build/build_config.h"
 #include "media/audio/audio_output_dispatcher_impl.h"
 #include "media/audio/audio_output_proxy.h"
 #include "media/audio/audio_output_resampler.h"
 #include "media/audio/audio_util.h"
 #include "media/audio/fake_audio_input_stream.h"
 #include "media/audio/fake_audio_output_stream.h"
+#include "media/base/media_switches.h"
 
 namespace media {
 
@@ -76,20 +79,28 @@ AudioManagerBase::AudioManagerBase()
       max_num_input_streams_(kDefaultMaxInputStreams),
       num_output_streams_(0),
       num_input_streams_(0),
+      // TODO(dalecurtis): Switch this to an ObserverListThreadSafe, so we don't
+      // block the UI thread when swapping devices.
       output_listeners_(
           ObserverList<AudioDeviceListener>::NOTIFY_EXISTING_ONLY),
       audio_thread_(new base::Thread("AudioThread")) {
 #if defined(OS_WIN)
   audio_thread_->init_com_with_mta(true);
+#elif defined(OS_MACOSX)

[Chris Rogers, 2013/06/04 19:16:52]

It'd be nice to have the ability to run on the main thread be a run-time feature
instead of compile-time.  We could have a method UseMainThread() (or something
like that, which would compile-time conditionally return |true| for OS_MACOSX. 
Then down below of line 339 we could avoid another #if defined(OS_MACOSX) and
instead call UseMainThread()

That way, in the future, if there are additional places in the code that need to
be treated specially, we can avoid a proliferation of #if defined(OS_MACOSX)
each time it's necessary to check

[DaleCurtis, 2013/06/04 20:03:14]

I've changed the code so the only OS_MACOSX necessary is here.  Long term the
flag will go away if this sticks since it doesn't make sense to offer a runtime
selection for bad behavior :)

+  // CoreAudio calls must occur on the main thread of the process, which in our
+  // case is sadly the browser UI thread.  Failure to execute calls on the right
+  // thread leads to crashes and odd behavior.  See http://crbug.com/158170.
+  // TODO(dalecurtis): We should require the message loop to be passed in.
+  const CommandLine* cmd_line = CommandLine::ForCurrentProcess();
+  if (!cmd_line->HasSwitch(switches::kDisableMainThreadAudio) &&
+      base::MessageLoopProxy::current() &&
+      base::MessageLoop::current()->IsType(base::MessageLoop::TYPE_UI)) {
+    message_loop_ = base::MessageLoopProxy::current();
+    return;
+  }
 #endif
-#if defined(OS_MACOSX)
-  // On Mac, use a UI loop to get native message pump so that CoreAudio property
-  // listener callbacks fire.
-  CHECK(audio_thread_->StartWithOptions(
-      base::Thread::Options(base::MessageLoop::TYPE_UI, 0)));
-#else
+
   CHECK(audio_thread_->Start());
-#endif
   message_loop_ = audio_thread_->message_loop_proxy();
 }
 
@@ -114,6 +125,14 @@ scoped_refptr<base::MessageLoopProxy> AudioManagerBase::GetMessageLoop() {
   return message_loop_;
 }
 
+scoped_refptr<base::MessageLoopProxy> AudioManagerBase::GetWorkerLoop() {
+  // Lazily start the worker thread.
+  if (!audio_thread_->IsRunning())
+    CHECK(audio_thread_->Start());
+
+  return audio_thread_->message_loop_proxy();
+}
+
 AudioOutputStream* AudioManagerBase::MakeAudioOutputStream(
     const AudioParameters& params,
     const std::string& input_device_id) {
@@ -317,13 +336,20 @@ void AudioManagerBase::Shutdown() {
   if (!audio_thread)
     return;
 
-  CHECK_NE(base::MessageLoop::current(), audio_thread->message_loop());
+#if defined(OS_MACOSX)
+  // Only true when we're sharing the UI message loop with the browser.  The UI
+  // loop is no longer running at this time and browser destruction is imminent.
+  if (message_loop_->BelongsToCurrentThread()) {
+    // After ShutdownOnAudioThread() there should be no tasks running on the
+    // |worker_loop_|.

[Chris Rogers, 2013/06/04 19:16:52]

I see no member variable called |worker_loop_| - can you please update the
comment

[DaleCurtis, 2013/06/04 20:03:14]

Comment removed. It's not really helpful given the comment above it.

+    ShutdownOnAudioThread();
+    audio_thread->Stop();
+    return;
+  }
+#endif
 
-  // We must use base::Unretained since Shutdown might have been called from
-  // the destructor and we can't alter the refcount of the object at that point.
-  audio_thread->message_loop()->PostTask(FROM_HERE, base::Bind(
-      &AudioManagerBase::ShutdownOnAudioThread,
-      base::Unretained(this)));
+  message_loop_->PostTask(FROM_HERE, base::Bind(
+      &AudioManagerBase::ShutdownOnAudioThread, base::Unretained(this)));
 
   // Stop() will wait for any posted messages to be processed first.
   audio_thread->Stop();