Fix several audio issues:

1. Audio button on ash tray UI does not work when user toggles it.
2. On ash tray bubble, if audio is muted, when user moves slider, audio is not unmuted.
3. Change CrasAudioHandler to update audio state, preference and notify observers for audio state change when user call CrasAudioHandler APIs to change the audio state, not when the dbus signals are received.
4. Move the code for adjusting unmute output volume to minimum unmute level out of SetOutputMute, and it is up to user to decide if they can call AdjustOutputVolumeToAudibleLevel to adjust the volume.
5. This also make the volume bubble shows up each time user presses mute, volume up, volume down button even if there is no audio state changes.

BUG=242549,242964
TBR=jamescook@chromium.org

Review URL: https://chromiumcodereview.appspot.com/15927004

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

4 files changed, 148 insertions, 92 deletions

diff --git a/ash/system/chromeos/audio/tray_audio.cc b/ash/system/chromeos/audio/tray_audio.cc
index 7d85e08..22c15d7 100644
--- a/ash/system/chromeos/audio/tray_audio.cc
+++ b/ash/system/chromeos/audio/tray_audio.cc
@@ -37,6 +37,8 @@
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/view.h"
 
+using chromeos::CrasAudioHandler;
+
 namespace ash {
 namespace internal {
 
@@ -57,7 +59,7 @@ const int kVolumeLevels = 4;
 
 bool IsAudioMuted() {
   if(ash::switches::UseNewAudioHandler()) {
-    return chromeos::CrasAudioHandler::Get()->IsOutputMuted();
+    return CrasAudioHandler::Get()->IsOutputMuted();
   } else {
     return Shell::GetInstance()->system_tray_delegate()->
         GetVolumeControlDelegate()->IsAudioMuted();
@@ -66,7 +68,7 @@ bool IsAudioMuted() {
 
 float GetVolumeLevel() {
   if (ash::switches::UseNewAudioHandler()) {
-    return chromeos::CrasAudioHandler::Get()->GetOutputVolumePercent() / 100.0f;
+    return CrasAudioHandler::Get()->GetOutputVolumePercent() / 100.0f;
   } else {
     return Shell::GetInstance()->system_tray_delegate()->
         GetVolumeControlDelegate()->GetVolumeLevel();
@@ -220,7 +222,14 @@ class VolumeView : public ActionableView,
     Layout();
   }
 
+  // Sets volume level on slider_, |percent| is ranged from [0.00] to [1.00].
   void SetVolumeLevel(float percent) {
+    // Slider's value is in finer granularity than audio volume level(0.01),
+    // there will be a small discrepancy between slider's value and volume level
+    // on audio side. To avoid the jittering in slider UI, do not set change
+    // slider value if the change is less than 1%.
+    if (std::abs(percent-slider_->value()) < 0.01)
+      return;
     // The change in volume will be reflected via accessibility system events,
     // so we prevent the UI event from being sent here.
     slider_->set_enable_accessibility_events(false);
@@ -242,8 +251,7 @@ class VolumeView : public ActionableView,
       return;
     }
 
-    chromeos::CrasAudioHandler* audio_handler =
-        chromeos::CrasAudioHandler::Get();
+    CrasAudioHandler* audio_handler = CrasAudioHandler::Get();
     bool show_more = audio_handler->has_alternative_output() ||
                      audio_handler->has_alternative_input();
     more_->SetVisible(show_more);
@@ -264,6 +272,26 @@ class VolumeView : public ActionableView,
     }
   }
 
+  void HandleVolumeUp(int volume) {
+    CrasAudioHandler* audio_handler = CrasAudioHandler::Get();
+    audio_handler->SetOutputVolumePercent(volume);
+    if (audio_handler->IsOutputMuted() &&
+        !audio_handler->IsOutputVolumeBelowDefaultMuteLvel())
+      audio_handler->SetOutputMute(false);
+  }
+
+  void HandleVolumeDown(int volume) {
+    CrasAudioHandler* audio_handler =  CrasAudioHandler::Get();
+    audio_handler->SetOutputVolumePercent(volume);
+    if (audio_handler->IsOutputVolumeBelowDefaultMuteLvel() &&
+        !audio_handler->IsOutputMuted()) {
+      audio_handler->SetOutputMute(true);
+    } else if (!audio_handler->IsOutputVolumeBelowDefaultMuteLvel() &&
+               audio_handler->IsOutputMuted()) {
+      audio_handler->SetOutputMute(false);
+    }
+  }
+
   // Overridden from views::View.
   virtual void Layout() OVERRIDE {
     views::View::Layout();
@@ -309,7 +337,10 @@ class VolumeView : public ActionableView,
                              const ui::Event& event) OVERRIDE {
     CHECK(sender == icon_);
     if (ash::switches::UseNewAudioHandler()) {
-      chromeos::CrasAudioHandler::Get()->SetOutputMute(!IsAudioMuted());
+      bool mute_on = !IsAudioMuted();
+      CrasAudioHandler::Get()->SetOutputMute(mute_on);
+      if (!mute_on)
+        CrasAudioHandler::Get()->AdjustOutputVolumeToAudibleLevel();
     } else {
       ash::Shell::GetInstance()->system_tray_delegate()->
           GetVolumeControlDelegate()->SetAudioMuted(!IsAudioMuted());
@@ -323,10 +354,17 @@ class VolumeView : public ActionableView,
                                   views::SliderChangeReason reason) OVERRIDE {
     if (reason == views::VALUE_CHANGED_BY_USER) {
       if (ash::switches::UseNewAudioHandler()) {
-        chromeos::CrasAudioHandler::Get()->
-            SetOutputVolumePercent(value * 100.0f);
-      }
-      else {
+        int volume = value * 100.0f;
+        int old_volume = CrasAudioHandler::Get()->GetOutputVolumePercent();
+        // Do not call change audio volume if the difference is less than
+        // 1%, which is beyond cras audio api's granularity for output volume.
+        if (std::abs(volume - old_volume) < 1)
+          return;
+        if (volume > old_volume)
+          HandleVolumeUp(volume);
+        else
+          HandleVolumeDown(volume);
+      } else {
         ash::Shell::GetInstance()->system_tray_delegate()->
             GetVolumeControlDelegate()->SetVolumeLevel(value);
       }
@@ -385,7 +423,7 @@ class AudioDetailedView : public TrayDetailsView,
     output_devices_.clear();
     input_devices_.clear();
     chromeos::AudioDeviceList devices;
-    chromeos::CrasAudioHandler::Get()->GetAudioDevices(&devices);
+    CrasAudioHandler::Get()->GetAudioDevices(&devices);
     for (size_t i = 0; i < devices.size(); ++i) {
       if (devices[i].is_input)
         input_devices_.push_back(devices[i]);
@@ -450,9 +488,9 @@ class AudioDetailedView : public TrayDetailsView,
         return;
       chromeos::AudioDevice& device = iter->second;
       if (device.is_input)
-        chromeos::CrasAudioHandler::Get()->SetActiveInputNode(device.id);
+        CrasAudioHandler::Get()->SetActiveInputNode(device.id);
       else
-        chromeos::CrasAudioHandler::Get()->SetActiveOutputNode(device.id);
+        CrasAudioHandler::Get()->SetActiveOutputNode(device.id);
     }
   }
 
@@ -474,15 +512,15 @@ TrayAudio::TrayAudio(SystemTray* system_tray)
       audio_detail_(NULL),
       pop_up_volume_view_(false) {
   if (ash::switches::UseNewAudioHandler())
-    chromeos::CrasAudioHandler::Get()->AddAudioObserver(this);
+    CrasAudioHandler::Get()->AddAudioObserver(this);
   else
     Shell::GetInstance()->system_tray_notifier()->AddAudioObserver(this);
 }
 
 TrayAudio::~TrayAudio() {
   if (ash::switches::UseNewAudioHandler()) {
-    if (chromeos::CrasAudioHandler::IsInitialized())
-      chromeos::CrasAudioHandler::Get()->RemoveAudioObserver(this);
+    if (CrasAudioHandler::IsInitialized())
+      CrasAudioHandler::Get()->RemoveAudioObserver(this);
   } else {
     Shell::GetInstance()->system_tray_notifier()->RemoveAudioObserver(this);
   }
diff --git a/chrome/browser/ui/ash/volume_controller_chromeos.cc b/chrome/browser/ui/ash/volume_controller_chromeos.cc
index 2540a44..db93e82 100644
--- a/chrome/browser/ui/ash/volume_controller_chromeos.cc
+++ b/chrome/browser/ui/ash/volume_controller_chromeos.cc
@@ -10,6 +10,8 @@
 #include "chrome/browser/extensions/api/system_private/system_private_api.h"
 #include "content/public/browser/user_metrics.h"
 
+using chromeos::CrasAudioHandler;
+
 namespace {
 
 // Percent by which the volume should be changed when a volume key is pressed.
@@ -19,13 +21,12 @@ const double kStepPercentage = 4.0;
 
 VolumeController::VolumeController() {
   if (ash::switches::UseNewAudioHandler())
-    chromeos::CrasAudioHandler::Get()->AddAudioObserver(this);
+    CrasAudioHandler::Get()->AddAudioObserver(this);
 }
 
 VolumeController::~VolumeController() {
-  if (ash::switches::UseNewAudioHandler() &&
-      chromeos::CrasAudioHandler::IsInitialized())
-    chromeos::CrasAudioHandler::Get()->RemoveAudioObserver(this);
+  if (ash::switches::UseNewAudioHandler() && CrasAudioHandler::IsInitialized())
+    CrasAudioHandler::Get()->RemoveAudioObserver(this);
 }
 
 bool VolumeController::HandleVolumeMute(const ui::Accelerator& accelerator) {
@@ -33,7 +34,7 @@ bool VolumeController::HandleVolumeMute(const ui::Accelerator& accelerator) {
     content::RecordAction(content::UserMetricsAction("Accel_VolumeMute_F8"));
 
   if (ash::switches::UseNewAudioHandler()) {
-    chromeos::CrasAudioHandler::Get()->SetOutputMute(true);
+    CrasAudioHandler::Get()->SetOutputMute(true);
     return true;
   }
 
@@ -53,12 +54,15 @@ bool VolumeController::HandleVolumeDown(const ui::Accelerator& accelerator) {
     content::RecordAction(content::UserMetricsAction("Accel_VolumeDown_F9"));
 
   if (ash::switches::UseNewAudioHandler()) {
-    chromeos::CrasAudioHandler* audio_handler =
-        chromeos::CrasAudioHandler::Get();
-    if (audio_handler->IsOutputMuted())
+    CrasAudioHandler* audio_handler = CrasAudioHandler::Get();
+
+    if (audio_handler->IsOutputMuted()) {
       audio_handler->SetOutputVolumePercent(0);
-    else
+    } else {
       audio_handler->AdjustOutputVolumeByPercent(-kStepPercentage);
+      if (audio_handler->IsOutputVolumeBelowDefaultMuteLvel())
+        audio_handler->SetOutputMute(true);
+    }
     return true;
   }
 
@@ -78,12 +82,11 @@ bool VolumeController::HandleVolumeUp(const ui::Accelerator& accelerator) {
     content::RecordAction(content::UserMetricsAction("Accel_VolumeUp_F10"));
 
   if (ash::switches::UseNewAudioHandler()) {
-    chromeos::CrasAudioHandler* audio_handler =
-        chromeos::CrasAudioHandler::Get();
+    CrasAudioHandler* audio_handler = CrasAudioHandler::Get();
+
     if (audio_handler->IsOutputMuted())
       audio_handler->SetOutputMute(false);
-    else
-      audio_handler->AdjustOutputVolumeByPercent(kStepPercentage);
+    audio_handler->AdjustOutputVolumeByPercent(kStepPercentage);
     return true;
   }
 
@@ -130,7 +133,7 @@ void VolumeController::SetVolumePercent(double percent) {
 
 void VolumeController::OnOutputVolumeChanged() {
   DCHECK(ash::switches::UseNewAudioHandler());
-  chromeos::CrasAudioHandler* audio_handler = chromeos::CrasAudioHandler::Get();
+  CrasAudioHandler* audio_handler = CrasAudioHandler::Get();
   extensions::DispatchVolumeChangedEvent(
       audio_handler->GetOutputVolumePercent(),
       audio_handler->IsOutputMuted());
@@ -138,7 +141,7 @@ void VolumeController::OnOutputVolumeChanged() {
 
 void VolumeController::OnOutputMuteChanged() {
   DCHECK(ash::switches::UseNewAudioHandler());
-  chromeos::CrasAudioHandler* audio_handler = chromeos::CrasAudioHandler::Get();
+  CrasAudioHandler* audio_handler = CrasAudioHandler::Get();
   extensions::DispatchVolumeChangedEvent(
       audio_handler->GetOutputVolumePercent(),
       audio_handler->IsOutputMuted());
diff --git a/chromeos/audio/cras_audio_handler.cc b/chromeos/audio/cras_audio_handler.cc
index 1088333..7ad5c84 100644
--- a/chromeos/audio/cras_audio_handler.cc
+++ b/chromeos/audio/cras_audio_handler.cc
@@ -104,10 +104,11 @@ bool CrasAudioHandler::IsOutputMuted() {
 }
 
 bool CrasAudioHandler::IsOutputMutedForDevice(uint64 device_id) {
-  if (device_id == active_output_node_id_)
-    return output_mute_on_;
-  else
-    return audio_pref_handler_->GetMuteValue(device_id);
+  return audio_pref_handler_->GetMuteValue(device_id);
+}
+
+bool CrasAudioHandler::IsOutputVolumeBelowDefaultMuteLvel() {
+  return output_volume_ <= kMuteThresholdPercent;
 }
 
 bool CrasAudioHandler::IsInputMuted() {
@@ -115,10 +116,7 @@ bool CrasAudioHandler::IsInputMuted() {
 }
 
 bool CrasAudioHandler::IsInputMutedForDevice(uint64 device_id) {
-  if (device_id == active_input_node_id_)
-    return input_mute_on_;
-  else
-    return audio_pref_handler_->GetMuteValue(device_id);
+  return audio_pref_handler_->GetMuteValue(device_id);
 }
 
 int CrasAudioHandler::GetOutputVolumePercent() {
@@ -178,14 +176,21 @@ void CrasAudioHandler::SetOutputVolumePercent(int volume_percent) {
   volume_percent = min(max(volume_percent, 0), 100);
   if (volume_percent <= kMuteThresholdPercent)
     volume_percent = 0;
-  SetOutputVolumeInternal(volume_percent);
+  output_volume_ = volume_percent;
+  audio_pref_handler_->SetVolumeGainValue(active_output_node_id_,
+                                          output_volume_);
+  SetOutputVolumeInternal(output_volume_);
+  FOR_EACH_OBSERVER(AudioObserver, observers_, OnOutputVolumeChanged());
 }
 
 void CrasAudioHandler::SetInputGainPercent(int gain_percent) {
   gain_percent = min(max(gain_percent, 0), 100);
   if (gain_percent <= kMuteThresholdPercent)
     gain_percent = 0;
-  SetInputGainInternal(gain_percent);
+  input_gain_ = gain_percent;
+  audio_pref_handler_->SetVolumeGainValue(active_input_node_id_, input_gain_);
+  SetInputGainInternal(input_gain_);
+  FOR_EACH_OBSERVER(AudioObserver, observers_, OnInputGainChanged());
 }
 
 void CrasAudioHandler::AdjustOutputVolumeByPercent(int adjust_by_percent) {
@@ -196,37 +201,26 @@ void CrasAudioHandler::SetOutputMute(bool mute_on) {
   if (!SetOutputMuteInternal(mute_on))
     return;
 
-  if (mute_on)
-    return;
+  output_mute_on_ = mute_on;
+  audio_pref_handler_->SetMuteValue(active_output_node_id_, output_mute_on_);
+  FOR_EACH_OBSERVER(AudioObserver, observers_, OnOutputMuteChanged());
+}
 
-  // Adjust volume level if user unmute the device and makes sure the volume
-  // is set to a minimum audible level.
+void CrasAudioHandler::AdjustOutputVolumeToAudibleLevel() {
   if (output_volume_ <= kMuteThresholdPercent) {
     // Avoid the situation when sound has been unmuted, but the volume
     // is set to a very low value, so user still can't hear any sound.
-    SetOutputVolumeInternal(kDefaultUnmuteVolumePercent);
+    SetOutputVolumePercent(kDefaultUnmuteVolumePercent);
   }
 }
 
-bool  CrasAudioHandler::SetOutputMuteInternal(bool mute_on) {
-  if (output_mute_locked_)
-    return false;
-
-  output_mute_on_ = mute_on;
-  audio_pref_handler_->SetMuteValue(active_output_node_id_, mute_on);
-  chromeos::DBusThreadManager::Get()->GetCrasAudioClient()->
-      SetOutputMute(mute_on);
-  return true;
-}
-
 void CrasAudioHandler::SetInputMute(bool mute_on) {
-  if (input_mute_locked_)
+  if (!SetInputMuteInternal(mute_on))
     return;
 
   input_mute_on_ = mute_on;
-  audio_pref_handler_->SetMuteValue(active_input_node_id_, mute_on);
-  chromeos::DBusThreadManager::Get()->GetCrasAudioClient()->
-      SetInputMute(mute_on);
+  audio_pref_handler_->SetMuteValue(active_input_node_id_, input_mute_on_);
+  FOR_EACH_OBSERVER(AudioObserver, observers_, OnInputMuteChanged());
 }
 
 void CrasAudioHandler::SetActiveOutputNode(uint64 node_id) {
@@ -311,41 +305,32 @@ void CrasAudioHandler::AudioClientRestarted() {
 }
 
 void CrasAudioHandler::OutputVolumeChanged(int volume) {
-  if (output_volume_ == volume)
+  if (output_volume_ != volume) {
+    LOG(WARNING) << "Output volume state inconsistent, internal volume="
+        << output_volume_ << ", dbus signal volume=" << volume;
     return;
-
-  output_volume_ = volume;
-  audio_pref_handler_->SetVolumeGainValue(active_output_node_id_, volume);
-  FOR_EACH_OBSERVER(AudioObserver, observers_, OnOutputVolumeChanged());
+  }
 }
 
 void CrasAudioHandler::InputGainChanged(int gain) {
-  if (input_gain_ == gain)
-    return;
-
-  input_gain_ = gain;
-  audio_pref_handler_->SetVolumeGainValue(active_input_node_id_, gain);
-  FOR_EACH_OBSERVER(AudioObserver, observers_, OnInputGainChanged());
+  if (input_gain_ != gain) {
+    LOG(WARNING) << "input gain state inconsistent, internal gain="
+        << input_gain_  << ", dbus signal gain=" << gain;
+  }
 }
 
 void CrasAudioHandler::OutputMuteChanged(bool mute_on) {
-  if (output_mute_on_ == mute_on)
-    return;
-
-  output_mute_on_ = mute_on;
-  // TODO(rkc,jennyz): We need to save the mute preferences here. See
-  // crbug.com/239646.
-  FOR_EACH_OBSERVER(AudioObserver, observers_, OnOutputMuteChanged());
+  if (output_mute_on_ != mute_on) {
+    LOG(WARNING) << "output mute state inconsistent, internal mute="
+        << output_mute_on_  << ", dbus signal mute=" << mute_on;
+  }
 }
 
 void CrasAudioHandler::InputMuteChanged(bool mute_on) {
-  if (input_mute_on_ == mute_on)
-    return;
-
-  input_mute_on_ = mute_on;
-  // TODO(rkc,jennyz): Fix this also when fixing the output mute. See
-  // crbug.com/239646.
-  FOR_EACH_OBSERVER(AudioObserver, observers_, OnInputMuteChanged());
+  if (input_mute_on_ != mute_on) {
+    LOG(WARNING) << "input mute state inconsistent, internal mute="
+        << input_mute_on_  << ", dbus signal mute=" << mute_on;
+  }
 }
 
 void CrasAudioHandler::NodesChanged() {
@@ -381,12 +366,13 @@ void CrasAudioHandler::SetupAudioInputState() {
     input_mute_on_ = audio_pref_handler_->GetMuteValue(active_input_node_id_);
     input_gain_ = audio_pref_handler_->GetVolumeGainValue(
         active_input_node_id_);
-    SetInputMute(input_mute_on_);
-    SetInputGainInternal(input_gain_);
   } else {
-    SetInputMute(kPrefMuteOff);
-    SetInputGainInternal(kDefaultVolumeGainPercent);
+    input_mute_on_ = kPrefMuteOff;
+    input_gain_ = kDefaultVolumeGainPercent;
   }
+
+  SetInputMuteInternal(input_mute_on_);
+  SetInputGainInternal(input_gain_);
 }
 
 void CrasAudioHandler::SetupAudioOutputState() {
@@ -429,11 +415,29 @@ void CrasAudioHandler::SetOutputVolumeInternal(int volume) {
       SetOutputVolume(volume);
 }
 
+bool  CrasAudioHandler::SetOutputMuteInternal(bool mute_on) {
+  if (output_mute_locked_)
+    return false;
+
+  chromeos::DBusThreadManager::Get()->GetCrasAudioClient()->
+      SetOutputMute(mute_on);
+  return true;
+}
+
 void CrasAudioHandler::SetInputGainInternal(int gain) {
   chromeos::DBusThreadManager::Get()->GetCrasAudioClient()->
       SetInputGain(gain);
 }
 
+bool CrasAudioHandler::SetInputMuteInternal(bool mute_on) {
+  if (input_mute_locked_)
+    return false;
+
+  chromeos::DBusThreadManager::Get()->GetCrasAudioClient()->
+      SetInputMute(mute_on);
+  return true;
+}
+
 void CrasAudioHandler::GetNodes() {
   chromeos::DBusThreadManager::Get()->GetCrasAudioClient()->GetNodes(
       base::Bind(&CrasAudioHandler::HandleGetNodes,
@@ -455,11 +459,11 @@ void CrasAudioHandler::SwitchToDevice(const AudioDevice& device) {
   // to hear the wrong volume for a device.
   LOG(INFO) << "Switching active device to: " << device.ToString();
   if (device.is_input) {
-    DBusThreadManager::Get()->GetCrasAudioClient()->SetInputMute(true);
+    SetInputMuteInternal(true);
     DBusThreadManager::Get()->GetCrasAudioClient()->SetActiveInputNode(
         device.id);
   } else {
-    DBusThreadManager::Get()->GetCrasAudioClient()->SetOutputMute(true);
+    SetOutputMuteInternal(true);
     DBusThreadManager::Get()->GetCrasAudioClient()->SetActiveOutputNode(
         device.id);
   }
diff --git a/chromeos/audio/cras_audio_handler.h b/chromeos/audio/cras_audio_handler.h
index 54afeb8..f9da570 100644
--- a/chromeos/audio/cras_audio_handler.h
+++ b/chromeos/audio/cras_audio_handler.h
@@ -105,6 +105,9 @@ class CHROMEOS_EXPORT CrasAudioHandler : public CrasAudioClient::Observer,
   // Returns true if audio input is muted for a device.
   virtual bool IsInputMutedForDevice(uint64 device_id);
 
+  // Returns true if the output volume is below the default mute volume level.
+  virtual bool IsOutputVolumeBelowDefaultMuteLvel();
+
   // Gets volume level in 0-100% range (0 being pure silence) for the current
   // active node.
   virtual int GetOutputVolumePercent();
@@ -145,6 +148,9 @@ class CHROMEOS_EXPORT CrasAudioHandler : public CrasAudioClient::Observer,
   // Adjusts volume up (positive percentage) or down (negative percentage).
   virtual void AdjustOutputVolumeByPercent(int adjust_by_percent);
 
+  // Adjusts output volume to a minimum audible level if it is too low.
+  virtual void AdjustOutputVolumeToAudibleLevel();
+
   // Mutes or unmutes audio output device.
   virtual void SetOutputMute(bool mute_on);
 
@@ -198,12 +204,17 @@ class CHROMEOS_EXPORT CrasAudioHandler : public CrasAudioClient::Observer,
   // Sets output volume to specified value of |volume|.
   void SetOutputVolumeInternal(int volume);
 
-  // Sets output mute state to |mute_on|.
+  // Sets output mute state to |mute_on| internally, returns true if output mute
+  // is set.
   bool SetOutputMuteInternal(bool mute_on);
 
   // Sets output volume to specified value and notifies observers.
   void SetInputGainInternal(int gain);
 
+  // Sets input mute state to |mute_on| internally, returns true if input mute
+  // is set.
+  bool SetInputMuteInternal(bool mute_on);
+
   // Calling dbus to get nodes data.
   void GetNodes();