From f3dd296655d4be33a3b5ab8337e2fdd7b8e03f85 Mon Sep 17 00:00:00 2001 From: "jennyz@chromium.org" Date: Tue, 22 Apr 2014 04:48:38 +0000 Subject: Do not persist audio input device's mute state in prefs. BUG=365050 Review URL: https://codereview.chromium.org/242443010 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@265171 0039d316-1c4b-4281-b951-d872f2087c98 --- chromeos/audio/cras_audio_handler.cc | 44 +++++++++++++-------------- chromeos/audio/cras_audio_handler_unittest.cc | 24 +++++---------- 2 files changed, 28 insertions(+), 40 deletions(-) (limited to 'chromeos/audio') diff --git a/chromeos/audio/cras_audio_handler.cc b/chromeos/audio/cras_audio_handler.cc index f834b0c..aeda0eb 100644 --- a/chromeos/audio/cras_audio_handler.cc +++ b/chromeos/audio/cras_audio_handler.cc @@ -112,6 +112,7 @@ bool CrasAudioHandler::IsOutputMutedForDevice(uint64 device_id) { const AudioDevice* device = GetDeviceFromId(device_id); if (!device) return false; + DCHECK(!device->is_input); return audio_pref_handler_->GetMuteValue(*device); } @@ -127,7 +128,12 @@ bool CrasAudioHandler::IsInputMutedForDevice(uint64 device_id) { const AudioDevice* device = GetDeviceFromId(device_id); if (!device) return false; - return audio_pref_handler_->GetMuteValue(*device); + DCHECK(device->is_input); + // We don't record input mute state for each device in the prefs, + // for any non-active input device, we assume mute is off. + if (device->id == active_input_node_id_) + return input_mute_on_; + return false; } int CrasAudioHandler::GetOutputDefaultVolumeMuteThreshold() { @@ -225,8 +231,10 @@ void CrasAudioHandler::SetOutputMute(bool mute_on) { if (!SetOutputMuteInternal(mute_on)) return; - if (const AudioDevice* device = GetDeviceFromId(active_output_node_id_)) + if (const AudioDevice* device = GetDeviceFromId(active_output_node_id_)) { + DCHECK(!device->is_input); audio_pref_handler_->SetMuteValue(*device, output_mute_on_); + } FOR_EACH_OBSERVER(AudioObserver, observers_, OnOutputMuteChanged()); } @@ -243,12 +251,10 @@ void CrasAudioHandler::SetInputMute(bool mute_on) { if (!SetInputMuteInternal(mute_on)) return; + // Audio input mute state is not saved in prefs, see crbug.com/365050. LOG(WARNING) << "SetInputMute set active input id=" << "0x" << std::hex << active_input_node_id_ << " mute=" << mute_on; - AudioDevice device; - if (const AudioDevice* device = GetDeviceFromId(active_input_node_id_)) - audio_pref_handler_->SetMuteValue(*device, input_mute_on_); FOR_EACH_OBSERVER(AudioObserver, observers_, OnInputMuteChanged()); } @@ -296,14 +302,10 @@ void CrasAudioHandler::SetMuteForDevice(uint64 device_id, bool mute_on) { return; } - AudioDevice device; - if (const AudioDevice* device = GetDeviceFromId(device_id)) { - if (device->is_input) { - LOG(WARNING) << "SetMuteForDevice set mute pref for input device id=" - << "0x" << std::hex << device_id << " mute=" << mute_on; - } + const AudioDevice* device = GetDeviceFromId(device_id); + // Input device's mute state is not recorded in the pref. crbug.com/365050. + if (device && !device->is_input) audio_pref_handler_->SetMuteValue(*device, mute_on); - } } void CrasAudioHandler::LogErrors() { @@ -423,9 +425,8 @@ void CrasAudioHandler::SetupAudioInputState() { << "0x" << std::hex << active_input_node_id_; return; } - input_mute_on_ = audio_pref_handler_->GetMuteValue(*device); input_gain_ = audio_pref_handler_->GetInputGainValue(device); - LOG(WARNING) << "SetupAudioInputState from pref input device id=" + LOG(WARNING) << "SetupAudioInputState for active device id=" << "0x" << std::hex << device->id << " mute=" << input_mute_on_; SetInputMuteInternal(input_mute_on_); // TODO(rkc,jennyz): Set input gain once we decide on how to store @@ -440,6 +441,7 @@ void CrasAudioHandler::SetupAudioOutputState() { << "0x" << std::hex << active_output_node_id_; return; } + DCHECK(!device->is_input); output_mute_on_ = audio_pref_handler_->GetMuteValue(*device); output_volume_ = audio_pref_handler_->GetOutputVolumeValue(device); @@ -467,18 +469,14 @@ void CrasAudioHandler::ApplyAudioPolicy() { input_mute_locked_ = false; if (audio_pref_handler_->GetAudioCaptureAllowedValue()) { - // Set input mute if we have discovered active input device. - const AudioDevice* device = GetDeviceFromId(active_input_node_id_); - if (device) { - LOG(WARNING) << "Audio input allowed by policy, sets input id=" - << "0x" << std::hex << active_input_node_id_ - << " mute=false"; - SetInputMuteInternal(false); - } + LOG(WARNING) << "Audio input allowed by policy, sets input id=" + << "0x" << std::hex << active_input_node_id_ + << " mute=false"; + SetInputMuteInternal(false); } else { LOG(WARNING) << "Audio input NOT allowed by policy, sets input id=" << "0x" << std::hex << active_input_node_id_ << " mute=true"; - SetInputMute(true); + SetInputMuteInternal(true); input_mute_locked_ = true; } } diff --git a/chromeos/audio/cras_audio_handler_unittest.cc b/chromeos/audio/cras_audio_handler_unittest.cc index f4c3c99..ef706df 100644 --- a/chromeos/audio/cras_audio_handler_unittest.cc +++ b/chromeos/audio/cras_audio_handler_unittest.cc @@ -1499,21 +1499,16 @@ TEST_F(CrasAudioHandlerTest, SetInputMute) { // Mute the device. cras_audio_handler_->SetInputMute(true); - // Verify the input is muted, OnInputMuteChanged event is fired, - // and mute value is saved in the preferences. + // Verify the input is muted, OnInputMuteChanged event is fired. EXPECT_TRUE(cras_audio_handler_->IsInputMuted()); EXPECT_EQ(1, test_observer_->input_mute_changed_count()); - AudioDevice internal_mic(kInternalMic); - EXPECT_TRUE(audio_pref_handler_->GetMuteValue(internal_mic)); // Unmute the device. cras_audio_handler_->SetInputMute(false); - // Verify the input is unmuted, OnInputMuteChanged event is fired, - // and mute value is saved in the preferences. + // Verify the input is unmuted, OnInputMuteChanged event is fired. EXPECT_FALSE(cras_audio_handler_->IsInputMuted()); EXPECT_EQ(2, test_observer_->input_mute_changed_count()); - EXPECT_FALSE(audio_pref_handler_->GetMuteValue(internal_mic)); } TEST_F(CrasAudioHandlerTest, SetOutputVolumePercent) { @@ -1584,19 +1579,14 @@ TEST_F(CrasAudioHandlerTest, SetMuteForDevice) { EXPECT_EQ(kUSBMic.id, cras_audio_handler_->GetActiveInputNode()); cras_audio_handler_->SetMuteForDevice(kUSBMic.id, true); - // Verify the USB Mic is muted and mute state is saved in the preferences. - EXPECT_TRUE(cras_audio_handler_->IsOutputMutedForDevice(kUSBMic.id)); - AudioDevice usb_mic(kUSBMic); - EXPECT_TRUE(audio_pref_handler_->GetMuteValue(usb_mic)); + // Verify the USB Mic is muted. + EXPECT_TRUE(cras_audio_handler_->IsInputMutedForDevice(kUSBMic.id)); - // Mute the non-active input device. + // Mute the non-active input device should be a no-op, see crbug.com/365050. cras_audio_handler_->SetMuteForDevice(kInternalMic.id, true); - // Verify the internal mic is muted and mute value is saved in the - // preferences. - EXPECT_TRUE(cras_audio_handler_->IsOutputMutedForDevice(kInternalMic.id)); - AudioDevice internal_mic(kInternalMic); - EXPECT_TRUE(audio_pref_handler_->GetMuteValue(internal_mic)); + // Verify IsInputMutedForDevice returns false for non-active input device. + EXPECT_FALSE(cras_audio_handler_->IsInputMutedForDevice(kInternalMic.id)); } TEST_F(CrasAudioHandlerTest, SetVolumeGainPercentForDevice) { -- cgit v1.1