Fix the audio mute button broken issue.

chromeos/audio/cras_audio_handler.cc

Index: chromeos/audio/cras_audio_handler.cc
diff --git a/chromeos/audio/cras_audio_handler.cc b/chromeos/audio/cras_audio_handler.cc
index 10883334013a90f56adc24937b7e9ef6e6bfd9e5..19950229bdc4e099fe94132cbd24ad0593a275bf 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,15 +201,16 @@ 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);
   }
 }
 
@@ -212,21 +218,18 @@ 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 +314,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 +375,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;

[rkc, 2013/05/24 20:30:16]

This else block is redundant. The GetMuteValue and GetVolumeGainValue will
return the default values if we don't have a saved value for the device we're
checking.

We also should put a DCHECK here - there should never be a situation where we
don't actually have an active node id.

[jennyz, 2013/05/24 21:23:49]

This may not be always true. When we reboot the device, I remember I see in LOG
the case active_input_node_id_ and active_input_node_id_ being zero cases. It
looks like we called GetNodes when we initialized CrasAudioHandler, and we can
receive ActiveInputNodeChanged and ActiveOutputNodeChanged events after the
callback of our GetNodes call and cras seems don't send us active input/output
nodes until these events triggers.

[rkc, 2013/05/25 00:09:45]

Cool, the else block is then completely redundant. The Get calls will always
return at least the default values.

+    input_gain_ = kDefaultVolumeGainPercent;
   }
+
+  SetInputMuteInternal(input_mute_on_);
+  SetInputGainInternal(input_gain_);
 }
 
 void CrasAudioHandler::SetupAudioOutputState() {
@@ -434,6 +429,15 @@ void CrasAudioHandler::SetInputGainInternal(int gain) {
       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);

[rkc, 2013/05/24 20:30:16]

What happens if output mute is locked? We still want to be able to mute when
we're doing a switch.

[jennyz, 2013/05/24 21:23:49]

If output mute is locked, then the system is muted already by ApplyAudioPolicy.

     DBusThreadManager::Get()->GetCrasAudioClient()->SetActiveOutputNode(
         device.id);
   }