Asynchronous threaded volume get/set/init

Non-blocking version of init is now being called, and asynchronous set/get of mute and volume is implmenented at PulseAudioMixer level.

The logic has been updated in SystemKeyEventListener so that the audio key behavior follows the latest doc 'Chrome OS Audio Scenarios':
  Increase volume key: unmutes aduio if muted, otherwise increases volume.
  Decrease volume key: do nothing if muted, otherwise decrease volume.
  Mute volume key: always mutes volume.
  UI is displayed after any volume key press.

Currently only the init is being called asynchronously, but the next step will be to add a call allowing asynchronous operation of the volume adjust/unmute sequence from SystemKeyEventListener, and move the ShowVolumeBubble() calls into AudioHandler so that the volume bubble will update once the calls complete on their own thread.

Patch by davej@chromium.org:
http://codereview.chromium.org/2769008/show

BUG=2910
Test=Play video or audio in ChromeOS, and press the system volume up, down, and mute keys to see they follow the above behavior.  Alternating Mute and Increment Volume presses should always bring the volume back to the same level and not creep it upwards.

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

5 files changed, 248 insertions, 224 deletions

diff --git a/chrome/browser/chromeos/audio_handler.cc b/chrome/browser/chromeos/audio_handler.cc
index 05e19ec..f3157ee 100644
--- a/chrome/browser/chromeos/audio_handler.cc
+++ b/chrome/browser/chromeos/audio_handler.cc
@@ -15,10 +15,12 @@ namespace {
 
 const double kSilenceDb = -200.0;
 const double kMinVolumeDb = -90.0;
+// Choosing 6.0dB here instead of 0dB to give user chance to amplify audio some
+// in case sounds or their setup is too quiet for them.
 const double kMaxVolumeDb = 6.0;
 // A value of less than one adjusts quieter volumes in larger steps (giving
 // finer resolution in the higher volumes).
-const double kVolumeBias = 0.7;
+const double kVolumeBias = 0.5;
 
 }  // namespace
 
@@ -29,23 +31,6 @@ const double kVolumeBias = 0.7;
 // TODO(davej): Should we try to regain a connection if for some reason all was
 // initialized fine, but later IsValid() returned false?  Maybe N retries?
 
-double AudioHandler::GetVolumeDb() const {
-  if (!SanityCheck())
-    return kSilenceDb;
-
-  double volume_db = mixer_->GetVolumeDb();
-  if (volume_db <= kSilenceDb)
-    return kSilenceDb;
-  return volume_db;
-}
-
-void AudioHandler::SetVolumeDb(double volume_db) {
-  if (!SanityCheck())
-    return;
-
-  mixer_->SetVolumeDb(volume_db);
-}
-
 double AudioHandler::GetVolumePercent() const {
   if (!SanityCheck())
     return 0;
@@ -69,16 +54,15 @@ void AudioHandler::SetVolumePercent(double volume_percent) {
   mixer_->SetVolumeDb(vol_db);
 }
 
-// Volume range is from kMinVolumeDb at just above 0% to kMaxVolumeDb at 100%
-// with a special case at 0% which maps to kSilenceDb.
 void AudioHandler::AdjustVolumeByPercent(double adjust_by_percent) {
   if (!SanityCheck())
     return;
 
   DLOG(INFO) << "Adjusting Volume by " << adjust_by_percent << " percent";
 
-  double vol = mixer_->GetVolumeDb();
-  double pct = VolumeDbToPercent(vol);
+  double volume = mixer_->GetVolumeDb();
+  double pct = VolumeDbToPercent(volume);
+
   if (pct < 0)
     pct = 0;
   pct = pct + adjust_by_percent;
@@ -86,13 +70,12 @@ void AudioHandler::AdjustVolumeByPercent(double adjust_by_percent) {
     pct = 100.0;
 
   double new_volume;
-
   if (pct <= 0.1)
     new_volume = kSilenceDb;
   else
     new_volume = PercentToVolumeDb(pct);
 
-  if (new_volume != vol)
+  if (new_volume != volume)
     mixer_->SetVolumeDb(new_volume);
 }
 
@@ -112,19 +95,15 @@ void AudioHandler::SetMute(bool do_mute) {
   mixer_->SetMute(do_mute);
 }
 
-void AudioHandler::ToggleMute() {
-  if (!SanityCheck())
-    return;
-
-  mixer_->ToggleMute();
+void AudioHandler::OnMixerInitialized(bool success) {
+  connected_ = success;
+  DLOG(INFO) << "OnMixerInitialized, success = " << success;
 }
 
 AudioHandler::AudioHandler()
     : connected_(false) {
   mixer_.reset(new PulseAudioMixer());
-  if (mixer_->Init()) {
-    connected_ = true;
-  } else {
+  if (!mixer_->Init(NewCallback(this, &AudioHandler::OnMixerInitialized))) {
     LOG(ERROR) << "Unable to connect to PulseAudio";
   }
 }
@@ -135,7 +114,13 @@ AudioHandler::~AudioHandler() {
 inline bool AudioHandler::SanityCheck() const {
   if (!mixer_->IsValid()) {
     if (connected_) {
+      // Something happened and the mixer is no longer valid after having been
+      // initialized earlier.
+      // TODO(davej): Try to reconnect now?
+      connected_ = false;
       LOG(ERROR) << "Lost connection to PulseAudio";
+    } else {
+      LOG(ERROR) << "Mixer not valid";
     }
     return false;
   }
@@ -144,6 +129,8 @@ inline bool AudioHandler::SanityCheck() const {
 
 // VolumeDbToPercent() and PercentToVolumeDb() conversion functions allow us
 // complete control over how the 0 to 100% range is mapped to actual loudness.
+// Volume range is from kMinVolumeDb at just above 0% to kMaxVolumeDb at 100%
+// with a special case at 0% which maps to kSilenceDb.
 //
 // The mapping is confined to these two functions to make it easy to adjust and
 // have everything else just work.  The range is biased to give finer resolution
diff --git a/chrome/browser/chromeos/audio_handler.h b/chrome/browser/chromeos/audio_handler.h
index 54b134d4..7e443a6 100644
--- a/chrome/browser/chromeos/audio_handler.h
+++ b/chrome/browser/chromeos/audio_handler.h
@@ -18,16 +18,6 @@ class AudioHandler {
     return Singleton<AudioHandler>::get();
   }
 
-  // Get current volume level in terms of decibels (dB), silence will return
-  // -200 dB.  Returns default of -200.0 on error.  This function is designed
-  // to block until the volume is retrieved or fails.  Blocking call.
-  double GetVolumeDb() const;
-
-  // Blocking call.
-  // TODO(davej): Verify this becomes non-blocking after underlying calls are
-  // made non-blocking.
-  void SetVolumeDb(double volume_db);
-
   // Get volume level in our internal 0-100% range, 0 being pure silence.
   // Volume may go above 100% if another process changes PulseAudio's volume.
   // Returns default of 0 on error.  This function will block until the volume
@@ -37,38 +27,27 @@ class AudioHandler {
   // Set volume level from 0-100%.  Volumes above 100% are OK and boost volume,
   // although clipping will occur more at higher volumes.  Volume gets quieter
   // as the percentage gets lower, and then switches to silence at 0%.
-  // Blocking call.
-  // TODO(davej): Verify this becomes non-blocking after underlying calls are
-  // made non-blocking.
   void SetVolumePercent(double volume_percent);
 
   // Adust volume up (positive percentage) or down (negative percentage),
-  // capping at 100%.  Call GetVolumePercent() afterwards to get the new level.
-  // Blocking call.
-  // TODO(davej): Verify this becomes non-blocking after underlying calls are
-  // made non-blocking.
+  // capping at 100%.  GetVolumePercent() will be accurate after this
+  // blocking call.
   void AdjustVolumeByPercent(double adjust_by_percent);
 
-  // Just returns true if mute, false if not or an error occurred.  This call
-  // will block until the mute state is retrieved or fails.  Blocking call.
+  // Just returns true if mute, false if not or an error occurred.
+  // Blocking call.
   bool IsMute() const;
 
   // Mutes all audio.  Non-blocking call.
-  // TODO(davej): Verify this becomes non-blocking after underlying calls are
-  // made non-blocking.
   void SetMute(bool do_mute);
 
-  // Toggle mute.  Use this if you do not need to know the mute state, so it is
-  // possible to operate asynchronously.  Blocking call.
-  // TODO(davej): Verify this becomes non-blocking after underlying calls are
-  // made non-blocking.
-  void ToggleMute();
-
  private:
   // Defines the delete on exit Singleton traits we like.  Best to have this
   // and constructor/destructor private as recommended for Singletons.
   friend struct DefaultSingletonTraits<AudioHandler>;
 
+  void OnMixerInitialized(bool success);
+
   AudioHandler();
   virtual ~AudioHandler();
   inline bool SanityCheck() const;
@@ -78,7 +57,7 @@ class AudioHandler {
   static double PercentToVolumeDb(double volume_percent);
 
   scoped_ptr<PulseAudioMixer> mixer_;
-  bool connected_;
+  mutable bool connected_;  // Mutable for sanity checking only
 
   DISALLOW_COPY_AND_ASSIGN(AudioHandler);
 };
diff --git a/chrome/browser/chromeos/pulse_audio_mixer.cc b/chrome/browser/chromeos/pulse_audio_mixer.cc
index b183bf4..53c23d0 100644
--- a/chrome/browser/chromeos/pulse_audio_mixer.cc
+++ b/chrome/browser/chromeos/pulse_audio_mixer.cc
@@ -7,20 +7,19 @@
 #include <pulse/pulseaudio.h>
 
 #include "base/logging.h"
+#include "base/task.h"
 
 namespace chromeos {
 
-// TODO(davej): Do asynchronous versions by using the threaded PulseAudio API
-// so gets, sets, and init sequence do not block calling thread.  GetVolume()
-// and IsMute() may still need to be synchronous since caller is asking for the
-// value specifically, which we will not know until the operation completes.
-// So new asynchronous versions of the Get routines could be added with a
-// callback parameter.  Currently, all Get, Set and Init calls are blocking on
-// PulseAudio.
+// Using asynchronous versions of the threaded PulseAudio API, as well
+// as a worker thread so gets, sets, and the init sequence do not block the
+// calling thread.  GetVolume() and IsMute() can still be called synchronously
+// if needed, but take a bit longer (~2ms vs ~0.3ms).
+//
+// Set calls just return without waiting.  If you must guarantee the value has
+// been set before continuing, immediately call the blocking Get version to
+// synchronously get the value back.
 //
-// Set calls can just return without waiting.  When a set operation completes,
-// we could proxy to the UI thread to notify there was a volume change update
-// so the new volume can be displayed to the user.
 // TODO(davej): Serialize volume/mute to preserve settings when restarting?
 // TODO(davej): Check if we need some thread safety mechanism (will someone be
 // calling GetVolume while another process is calling SetVolume?)
@@ -29,6 +28,12 @@ namespace {
 
 const int kInvalidDeviceId = -1;
 
+// Used for passing custom data to the PulseAudio callbacks.
+struct CallbackWrapper {
+  pa_threaded_mainloop* mainloop;
+  void* data;
+};
+
 }  // namespace
 
 // AudioInfo contains all the values we care about when getting info for a
@@ -38,87 +43,102 @@ struct PulseAudioMixer::AudioInfo {
   bool muted;
 };
 
-// This class will set volume using PulseAudio to adjust volume and mute.
-
 PulseAudioMixer::PulseAudioMixer()
     : device_id_(kInvalidDeviceId),
-      pa_mainloop_(NULL),
+      last_channels_(0),
+      mixer_state_(UNINITIALIZED),
       pa_context_(NULL),
-      connect_started_(false),
-      last_channels_(0) {
+      pa_mainloop_(NULL),
+      thread_(NULL) {
 }
 
 PulseAudioMixer::~PulseAudioMixer() {
   PulseAudioFree();
+  thread_->Stop();
 }
 
-bool PulseAudioMixer::Init() {
-  // Find main device for changing 'master' default volume.
-  if (!PulseAudioInit())
+bool PulseAudioMixer::Init(InitDoneCallback* callback) {
+  // Just start up worker thread, then post the task of starting up, which can
+  // block for 200-500ms, so best not to do it on this thread.
+  if (mixer_state_ != UNINITIALIZED)
     return false;
-  device_id_ = GetDefaultPlaybackDevice();
-  last_channels_ = 0;
-  LOG(INFO) << "PulseAudioMixer initialized OK";
+
+  mixer_state_ = INITIALIZING;
+  if (thread_ == NULL) {
+    thread_.reset(new base::Thread("PulseAudioMixer"));
+    if (!thread_->Start()) {
+      thread_.reset();
+      return false;
+    }
+  }
+  thread_->message_loop()->PostTask(FROM_HERE,
+      NewRunnableMethod(this, &PulseAudioMixer::DoInit,
+                        callback));
   return true;
 }
 
 double PulseAudioMixer::GetVolumeDb() const {
-  AudioInfo data;
-  if (!GetAudioInfo(&data))
+  if (!PulseAudioValid())
     return pa_sw_volume_to_dB(0);  // this returns -inf
+  AudioInfo data;
+  GetAudioInfo(&data);
   return pa_sw_volume_to_dB(data.cvolume.values[0]);
 }
 
+void PulseAudioMixer::GetVolumeDbAsync(GetVolumeCallback* callback,
+                                       void* user) {
+  if (!PulseAudioValid())
+    return;
+  thread_->message_loop()->PostTask(FROM_HERE,
+      NewRunnableMethod(this,
+                        &PulseAudioMixer::DoGetVolume,
+                        callback, user));
+}
+
 void PulseAudioMixer::SetVolumeDb(double vol_db) {
-  if (!PulseAudioValid()) {
-    DLOG(ERROR) << "Called SetVolume when mixer not valid";
+  if (!PulseAudioValid())
     return;
-  }
 
   // last_channels_ determines the number of channels on the main output device,
   // and is used later to set the volume on all channels at once.
   if (!last_channels_) {
     AudioInfo data;
-    if (!GetAudioInfo(&data))
-      return;
+    GetAudioInfo(&data);
     last_channels_ = data.cvolume.channels;
   }
 
   pa_operation* pa_op;
   pa_cvolume cvolume;
   pa_cvolume_set(&cvolume, last_channels_, pa_sw_volume_from_dB(vol_db));
+  pa_threaded_mainloop_lock(pa_mainloop_);
   pa_op = pa_context_set_sink_volume_by_index(pa_context_, device_id_,
                                               &cvolume, NULL, NULL);
-  CompleteOperationHelper(pa_op);
+  pa_operation_unref(pa_op);
+  pa_threaded_mainloop_unlock(pa_mainloop_);
 }
 
 bool PulseAudioMixer::IsMute() const {
-  AudioInfo data;
-  if (!GetAudioInfo(&data))
+  if (!PulseAudioValid())
     return false;
-
+  AudioInfo data;
+  GetAudioInfo(&data);
   return data.muted;
 }
 
 void PulseAudioMixer::SetMute(bool mute) {
-  if (!PulseAudioValid()) {
-    DLOG(ERROR) << "Called SetMute when mixer not valid";
+  if (!PulseAudioValid())
     return;
-  }
-
   pa_operation* pa_op;
+  pa_threaded_mainloop_lock(pa_mainloop_);
   pa_op = pa_context_set_sink_mute_by_index(pa_context_, device_id_,
                                             mute ? 1 : 0, NULL, NULL);
-  CompleteOperationHelper(pa_op);
-}
-
-void PulseAudioMixer::ToggleMute() {
-  SetMute(!IsMute());
+  pa_operation_unref(pa_op);
+  pa_threaded_mainloop_unlock(pa_mainloop_);
 }
 
 bool PulseAudioMixer::IsValid() const {
-  if (device_id_ == kInvalidDeviceId)
-    return false;
+  if (mixer_state_ == READY)
+    return true;
   if (!pa_context_)
     return false;
   if (pa_context_get_state(pa_context_) != PA_CONTEXT_READY)
@@ -127,49 +147,60 @@ bool PulseAudioMixer::IsValid() const {
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-// Private functions that deal with PulseAudio directly
+// Private functions follow
+
+void PulseAudioMixer::DoInit(InitDoneCallback* callback) {
+  bool success = PulseAudioInit();
+  callback->Run(success);
+  delete callback;
+}
+
+void PulseAudioMixer::DoGetVolume(GetVolumeCallback* callback,
+                                            void* user) {
+  callback->Run(GetVolumeDb(), user);
+  delete callback;
+}
 
 bool PulseAudioMixer::PulseAudioInit() {
-  pa_mainloop_api* pa_mlapi;
   pa_context_state_t state;
 
-  // It's OK to call Init again to re-initialize.  This could be useful if
-  // !IsValid() and we want to try reconnecting.
-  if (pa_mainloop_)
-    PulseAudioFree();
-
   while (true) {
     // Create connection to default server.
-    pa_mainloop_ = pa_mainloop_new();
+    pa_mainloop_ = pa_threaded_mainloop_new();
     if (!pa_mainloop_) {
       LOG(ERROR) << "Can't create PulseAudio mainloop";
       break;
     }
-    pa_mlapi = pa_mainloop_get_api(pa_mainloop_);
+    pa_mainloop_api* pa_mlapi = pa_threaded_mainloop_get_api(pa_mainloop_);
     if (!pa_mlapi) {
       LOG(ERROR) << "Can't get PulseAudio mainloop api";
       break;
     }
+    // This one takes the most time if run at app startup.
     pa_context_ = pa_context_new(pa_mlapi, "ChromeAudio");
     if (!pa_context_) {
       LOG(ERROR) << "Can't create new PulseAudio context";
       break;
     }
+    pa_context_set_state_callback(pa_context_, ContextStateCallback,
+                                  pa_mainloop_);
 
-    // Using simpler method of just checking state after each iterate.
-    // Connect to PulseAudio sound server.
     if (pa_context_connect(pa_context_, NULL,
                            PA_CONTEXT_NOAUTOSPAWN, NULL) != 0) {
       LOG(ERROR) << "Can't start connection to PulseAudio sound server";
       break;
     }
 
-    connect_started_ = true;
+    pa_threaded_mainloop_lock(pa_mainloop_);
+
+    if (pa_threaded_mainloop_start(pa_mainloop_) < 0) {
+      pa_threaded_mainloop_unlock(pa_mainloop_);
+      break;
+    }
 
     // Wait until we have a completed connection or fail.
-    // TODO(davej): Remove blocking waits during init as well.
     do {
-      pa_mainloop_iterate(pa_mainloop_, 1, NULL);  // blocking wait
+      pa_threaded_mainloop_wait(pa_mainloop_);
       state = pa_context_get_state(pa_context_);
       if (state == PA_CONTEXT_FAILED) {
         LOG(ERROR) << "PulseAudio context connection failed";
@@ -181,9 +212,14 @@ bool PulseAudioMixer::PulseAudioInit() {
       }
     } while (state != PA_CONTEXT_READY);
 
+    pa_threaded_mainloop_unlock(pa_mainloop_);
     if (state != PA_CONTEXT_READY)
       break;
 
+    last_channels_ = 0;
+    GetDefaultPlaybackDevice();
+    mixer_state_ = READY;
+
     return true;
   }
 
@@ -194,115 +230,100 @@ bool PulseAudioMixer::PulseAudioInit() {
 }
 
 void PulseAudioMixer::PulseAudioFree() {
+  if (pa_mainloop_) {
+    DCHECK_NE(mixer_state_, UNINITIALIZED);
+    pa_threaded_mainloop_lock(pa_mainloop_);
+    mixer_state_ = SHUTTING_DOWN;
+    pa_threaded_mainloop_unlock(pa_mainloop_);
+    pa_threaded_mainloop_stop(pa_mainloop_);
+  }
   if (pa_context_) {
-    if (connect_started_)
-      pa_context_disconnect(pa_context_);
+    pa_context_set_state_callback(pa_context_, NULL, NULL);
+    pa_context_disconnect(pa_context_);
     pa_context_unref(pa_context_);
     pa_context_ = NULL;
   }
-  connect_started_ = false;
   if (pa_mainloop_) {
-    pa_mainloop_free(pa_mainloop_);
+    pa_threaded_mainloop_free(pa_mainloop_);
     pa_mainloop_ = NULL;
   }
+  mixer_state_ = UNINITIALIZED;
 }
 
 bool PulseAudioMixer::PulseAudioValid() const {
+  if (mixer_state_ != READY)
+    return false;
   if (!pa_context_) {
     DLOG(ERROR) << "Trying to use PulseAudio when no context";
     return false;
   }
-
   if (pa_context_get_state(pa_context_) != PA_CONTEXT_READY) {
-    LOG(ERROR) << "PulseAudio context not ready";
+    LOG(ERROR) << "PulseAudio context not ready ("
+               << pa_context_get_state(pa_context_) << ")";
     return false;
   }
-
-  if (device_id_ == kInvalidDeviceId) {
-    DLOG(ERROR) << "Trying to use PulseAudio when no device id";
+  if (device_id_ == kInvalidDeviceId)
     return false;
-  }
+
   return true;
 }
 
-bool PulseAudioMixer::CompleteOperationHelper(pa_operation* pa_op) {
+void PulseAudioMixer::CompleteOperationAndUnlock(pa_operation* pa_op) const {
   // After starting any operation, this helper checks if it started OK, then
   // waits for it to complete by iterating through the mainloop until the
   // operation is not running anymore.
-  if (!pa_op) {
-    LOG(ERROR) << "Failed to start operation";
-    return false;
-  }
+  CHECK(pa_op);
+
   while (pa_operation_get_state(pa_op) == PA_OPERATION_RUNNING) {
-    pa_mainloop_iterate(pa_mainloop_, 1, NULL);
+    pa_threaded_mainloop_wait(pa_mainloop_);
   }
   pa_operation_unref(pa_op);
-  return true;
+  pa_threaded_mainloop_unlock(pa_mainloop_);
 }
 
-int PulseAudioMixer::GetDefaultPlaybackDevice() {
-  int device = kInvalidDeviceId;
-  pa_operation* pa_op;
-
-  if (!pa_context_) {
-    DLOG(ERROR) << "Trying to use PulseAudio when no context";
-    return kInvalidDeviceId;
-  }
-
-  if (pa_context_get_state(pa_context_) != PA_CONTEXT_READY) {
-    LOG(ERROR) << "PulseAudio context not ready";
-    return kInvalidDeviceId;
-  }
+void PulseAudioMixer::GetDefaultPlaybackDevice() {
+  DCHECK(pa_context_);
+  DCHECK(pa_context_get_state(pa_context_) == PA_CONTEXT_READY);
 
-  pa_op = pa_context_get_sink_info_list(pa_context_,
+  pa_threaded_mainloop_lock(pa_mainloop_);
+  pa_operation* pa_op = pa_context_get_sink_info_list(pa_context_,
                                         EnumerateDevicesCallback,
-                                        &device);
-  CompleteOperationHelper(pa_op);
-  return device;
+                                        this);
+  CompleteOperationAndUnlock(pa_op);
+  return;
 }
 
-// static
-void PulseAudioMixer::EnumerateDevicesCallback(pa_context* unused,
-                                               const pa_sink_info* sink_info,
-                                               int eol,
-                                               void* userdata) {
-  int* pa_device = static_cast<int*>(userdata);
-
+void PulseAudioMixer::OnEnumerateDevices(const pa_sink_info* sink_info,
+                                         int eol) {
   // If eol is set to a positive number, you're at the end of the list.
   if (eol > 0)
     return;
 
   // TODO(davej): Should we handle cases of more than one output sink device?
-  if (*pa_device == kInvalidDeviceId)
-    *pa_device = sink_info->index;
+  if (device_id_ == kInvalidDeviceId)
+    device_id_ = sink_info->index;
+
+  pa_threaded_mainloop_signal(pa_mainloop_, 0);
 }
 
-bool PulseAudioMixer::GetAudioInfo(AudioInfo* info) const {
-  DCHECK(info);
-  if (!PulseAudioValid()) {
-    DLOG(ERROR) << "Called GetAudioInfo when mixer not valid";
-    return false;
-  }
-  if (!info)
-    return false;
+// static
+void PulseAudioMixer::EnumerateDevicesCallback(pa_context* unused,
+                                               const pa_sink_info* sink_info,
+                                               int eol,
+                                               void* userdata) {
+  PulseAudioMixer* inst = static_cast<PulseAudioMixer*>(userdata);
+  inst->OnEnumerateDevices(sink_info, eol);
+}
 
+void PulseAudioMixer::GetAudioInfo(AudioInfo* info) const {
+  CallbackWrapper cb_data = {pa_mainloop_, info};
+  pa_threaded_mainloop_lock(pa_mainloop_);
   pa_operation* pa_op;
   pa_op = pa_context_get_sink_info_by_index(pa_context_,
                                             device_id_,
                                             GetAudioInfoCallback,
-                                            info);
-
-  // Duplicating some code in CompleteOperationHelper because this function
-  // needs to stay 'const', and this isn't allowed if that is called.
-  if (!pa_op) {
-    LOG(ERROR) << "Failed to start operation";
-    return false;
-  }
-  while (pa_operation_get_state(pa_op) == PA_OPERATION_RUNNING) {
-    pa_mainloop_iterate(pa_mainloop_, 1, NULL);
-  }
-  pa_operation_unref(pa_op);
-  return true;
+                                            &cb_data);
+  CompleteOperationAndUnlock(pa_op);
 }
 
 // static
@@ -310,17 +331,29 @@ void PulseAudioMixer::GetAudioInfoCallback(pa_context* unused,
                                            const pa_sink_info* sink_info,
                                            int eol,
                                            void* userdata) {
-  if (!userdata) {
-    DLOG(ERROR) << "userdata NULL";
-    return;
-  }
-  AudioInfo* data = static_cast<AudioInfo*>(userdata);
+  CallbackWrapper* cb_data = static_cast<CallbackWrapper*>(userdata);
+  AudioInfo* data = static_cast<AudioInfo*>(cb_data->data);
 
   // Copy just the information we care about.
   if (eol == 0) {
     data->cvolume = sink_info->volume;
     data->muted = sink_info->mute ? true : false;
   }
+  pa_threaded_mainloop_signal(cb_data->mainloop, 0);
+}
+
+// static
+void PulseAudioMixer::ContextStateCallback(pa_context* c, void* userdata) {
+  pa_threaded_mainloop* mainloop = static_cast<pa_threaded_mainloop*>(userdata);
+  switch (pa_context_get_state(c)) {
+    case PA_CONTEXT_READY:
+    case PA_CONTEXT_TERMINATED:
+    case PA_CONTEXT_FAILED:
+      pa_threaded_mainloop_signal(mainloop, 0);
+      break;
+    default:
+      break;
+  }
 }
 
 }  // namespace chromeos
diff --git a/chrome/browser/chromeos/pulse_audio_mixer.h b/chrome/browser/chromeos/pulse_audio_mixer.h
index 7bc213f..c74563a 100644
--- a/chrome/browser/chromeos/pulse_audio_mixer.h
+++ b/chrome/browser/chromeos/pulse_audio_mixer.h
@@ -6,10 +6,13 @@
 #define CHROME_BROWSER_CHROMEOS_PULSE_AUDIO_MIXER_H_
 
 #include "base/basictypes.h"
+#include "base/callback.h"
+#include "base/scoped_ptr.h"
+#include "base/thread.h"
 
 struct pa_context;
 struct pa_cvolume;
-struct pa_mainloop;
+struct pa_threaded_mainloop;
 struct pa_operation;
 struct pa_sink_info;
 
@@ -20,32 +23,28 @@ class PulseAudioMixer {
   PulseAudioMixer();
   ~PulseAudioMixer();
 
-  // These public functions all return true if the operation completed
-  // successfully, or false if not set up to talk with the default audio device.
-
-  // Connect to PulseAudio and find a default device.  Blocking call.
-  // TODO(davej): Remove blocking waits in PA.
-  bool Init();
+  // Non-blocking, connect to PulseAudio and find a default device, and call
+  // callback when complete with success code.
+  typedef Callback1<bool>::Type InitDoneCallback;
+  bool Init(InitDoneCallback* callback);
 
   // Blocking call. Returns a default of -inf on error.
   double GetVolumeDb() const;
 
-  // Blocking call.
-  // TODO(davej): Remove blocking waits in PA.
+  // Non-blocking, volume sent in as first param to callback
+  typedef Callback2<double, void*>::Type GetVolumeCallback;
+  void GetVolumeDbAsync(GetVolumeCallback* callback, void* user);
+
+  // Non-blocking call.
   void SetVolumeDb(double vol_db);
 
   // Gets the mute state of the default device (true == mute). Blocking call.
   // Returns a default of false on error.
   bool IsMute() const;
 
-  // Blocking call.
-  // TODO(davej): Remove blocking waits in PA.
+  // Non-Blocking call.
   void SetMute(bool mute);
 
-  // Blocking call.
-  // TODO(davej): Remove blocking waits in PA.
-  void ToggleMute();
-
   // Call any time to see if we have a valid working connection to PulseAudio.
   // Non-blocking call.
   bool IsValid() const;
@@ -53,11 +52,20 @@ class PulseAudioMixer {
  private:
   struct AudioInfo;
 
+  enum State {
+    UNINITIALIZED = 0,
+    INITIALIZING,
+    READY,
+    SHUTTING_DOWN
+  };
+
+  // These are the tasks to be run in the background on the worker thread.
+  void DoInit(InitDoneCallback* callback);
+  void DoGetVolume(GetVolumeCallback* callback, void* user);
+
   // This goes through sequence of connecting to the default PulseAudio server.
   // We will block until we either have a valid connection or something failed.
-  // It's safe to call again, in case a connection is lost for some reason
-  // (e.g. if other functions fail, or IsValid() is false).
-  // TODO(davej): Remove blocking waits in PA.
+  // If a connection is lost for some reason, delete and recreate the object.
   bool PulseAudioInit();
 
   // PulseAudioFree.  Disconnect from server.
@@ -69,46 +77,48 @@ class PulseAudioMixer {
   bool PulseAudioValid() const;
 
   // Iterates the PA mainloop and only returns once an operation has completed
-  // (successfully or unsuccessfully).  This blocking call is needed for
-  // synchronous getting of data.
-  // TODO(davej): This function will go away after doing asynchronous versions
-  // so we don't have to block when not necessary.
-  bool CompleteOperationHelper(pa_operation* pa_op);
+  // (successfully or unsuccessfully).  This call only blocks the worker thread.
+  void CompleteOperationAndUnlock(pa_operation* pa_op) const;
 
   // For now, this just gets the first device returned from the enumeration
   // request.  This will be the 'default' or 'master' device that all further
   // actions are taken on.  Blocking call.
-  int GetDefaultPlaybackDevice();
+  void GetDefaultPlaybackDevice();
   static void EnumerateDevicesCallback(pa_context* unused,
                                        const pa_sink_info* sink_info,
                                        int eol,
                                        void* userdata);
+  void OnEnumerateDevices(const pa_sink_info* sink_info, int eol);
 
   // Get the info we're interested in from the default device. Currently this
   // is an array of volumes, and the mute state.  Blocking call.
-  bool GetAudioInfo(AudioInfo* info) const;
+  void GetAudioInfo(AudioInfo* info) const;
   static void GetAudioInfoCallback(pa_context* unused,
                                    const pa_sink_info* sink_info,
                                    int eol,
                                    void* userdata);
 
+  static void ContextStateCallback(pa_context* c, void* userdata);
+
   // The PulseAudio index of the main device being used.
-  int device_id_;
+  mutable int device_id_;
+
+  // Set to the number of channels on the main device.
+  int last_channels_;
+  State mixer_state_;
 
   // Cached contexts for use in PulseAudio calls.
-  pa_mainloop* pa_mainloop_;
   pa_context* pa_context_;
+  pa_threaded_mainloop* pa_mainloop_;
 
-  // Set if context connect succeeded so we can safely disconnect later.
-  bool connect_started_;
-
-  // Set to the number of channels on the main device.
-  int last_channels_;
+  scoped_ptr<base::Thread> thread_;
 
   DISALLOW_COPY_AND_ASSIGN(PulseAudioMixer);
 };
 
 }  // namespace chromeos
 
+DISABLE_RUNNABLE_METHOD_REFCOUNT(chromeos::PulseAudioMixer);
+
 #endif  // CHROME_BROWSER_CHROMEOS_PULSE_AUDIO_MIXER_H_
 
diff --git a/chrome/browser/chromeos/system_key_event_listener.cc b/chrome/browser/chromeos/system_key_event_listener.cc
index e0853c2..24bfa3e 100644
--- a/chrome/browser/chromeos/system_key_event_listener.cc
+++ b/chrome/browser/chromeos/system_key_event_listener.cc
@@ -30,6 +30,14 @@ SystemKeyEventListener::~SystemKeyEventListener() {
   WmMessageListener::instance()->RemoveObserver(this);
 }
 
+// TODO(davej): Move the ShowVolumeBubble() calls in to AudioHandler so that
+// this function returns faster without blocking on GetVolumePercent(), and
+// still guarantees that the volume displayed will be that after the adjustment.
+
+// TODO(davej): The IsMute() check can also be made non-blocking by changing
+// to an AdjustVolumeByPercentOrUnmute() function which can do the steps off
+// of this thread when ShowVolumeBubble() is moved in to AudioHandler.
+
 void SystemKeyEventListener::ProcessWmMessage(const WmIpc::Message& message,
                                               GdkWindow* window) {
   if (message.type() != WM_IPC_MESSAGE_CHROME_NOTIFY_SYSKEY_PRESSED)
@@ -37,18 +45,25 @@ void SystemKeyEventListener::ProcessWmMessage(const WmIpc::Message& message,
 
   switch (message.param(0)) {
     case WM_IPC_SYSTEM_KEY_VOLUME_MUTE:
+      // Always muting (and not toggling) as per final decision on
+      // http://crosbug.com/3751
       audio_handler_->SetMute(true);
       VolumeBubble::instance()->ShowVolumeBubble(0);
       break;
     case WM_IPC_SYSTEM_KEY_VOLUME_DOWN:
-      audio_handler_->AdjustVolumeByPercent(-kStepPercentage);
-      audio_handler_->SetMute(false);
-      VolumeBubble::instance()->ShowVolumeBubble(
-          audio_handler_->GetVolumePercent());
+      if (audio_handler_->IsMute()) {
+        VolumeBubble::instance()->ShowVolumeBubble(0);
+      } else {
+        audio_handler_->AdjustVolumeByPercent(-kStepPercentage);
+        VolumeBubble::instance()->ShowVolumeBubble(
+            audio_handler_->GetVolumePercent());
+      }
       break;
     case WM_IPC_SYSTEM_KEY_VOLUME_UP:
-      audio_handler_->AdjustVolumeByPercent(kStepPercentage);
-      audio_handler_->SetMute(false);
+      if (audio_handler_->IsMute())
+        audio_handler_->SetMute(false);
+      else
+        audio_handler_->AdjustVolumeByPercent(kStepPercentage);
       VolumeBubble::instance()->ShowVolumeBubble(
           audio_handler_->GetVolumePercent());
       break;