Use the browser UI thread for audio on OSX.

media/audio/audio_manager_base.cc

 // 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/audio_manager_base.h"
 
 #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 {
 
 static const int kStreamCloseDelaySeconds = 5;
 
 // Default maximum number of output streams that can be open simultaneously
 // for all platforms.
 static const int kDefaultMaxOutputStreams = 16;
 
 // Default maximum number of input streams that can be open simultaneously
@@ skipping 42 matching lines @@
  private:
   const DispatcherParams* dispatcher_;
 };
 
 AudioManagerBase::AudioManagerBase()
     : num_active_input_streams_(0),
       max_num_output_streams_(kDefaultMaxOutputStreams),
       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();
 }
 
 AudioManagerBase::~AudioManagerBase() {
   // The platform specific AudioManager implementation must have already
   // stopped the audio thread. Otherwise, we may destroy audio streams before
   // stopping the thread, resulting an unexpected behavior.
   // This way we make sure activities of the audio streams are all stopped
   // before we destroy them.
   CHECK(!audio_thread_.get());
   // All the output streams should have been deleted.
   DCHECK_EQ(0, num_output_streams_);
   // All the input streams should have been deleted.
   DCHECK_EQ(0, num_input_streams_);
 }
 
 string16 AudioManagerBase::GetAudioInputDeviceModel() {
   return string16();
 }
 
 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) {
   // TODO(miu): Fix ~50 call points across several unit test modules to call
   // this method on the audio thread, then uncomment the following:
   // DCHECK(message_loop_->BelongsToCurrentThread());
 
   if (!params.IsValid()) {
     DLOG(ERROR) << "Audio parameters are invalid";
     return NULL;
@@ skipping 183 matching lines @@
   // via a local variable while not holding the audio thread lock.
   scoped_ptr<base::Thread> audio_thread;
   {
     base::AutoLock lock(audio_thread_lock_);
     audio_thread_.swap(audio_thread);
   }
 
   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();
 }
 
 void AudioManagerBase::ShutdownOnAudioThread() {
 // IOS implements audio input only.
 #if defined(OS_IOS)
   return;
 #else
@@ skipping 42 matching lines @@
   return GetPreferredOutputStreamParameters(AudioParameters());
 }
 
 AudioParameters AudioManagerBase::GetInputStreamParameters(
     const std::string& device_id) {
   NOTREACHED();
   return AudioParameters();
 }
 
 }  // namespace media