Clear all other input/output audio devices' active status before switching to a new active device.

Fix a racing issue caused by 2 NodesChanged signals triggered by cras for unplugging headhpne, which caused audio_devices_ got override by stale audio nodes data from 2nd NodesChanged signal.

BUG=273271, 274641
R=rkc@chromium.org

Review URL: https://codereview.chromium.org/23190009

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

2 files changed, 64 insertions, 7 deletions

diff --git a/chromeos/audio/cras_audio_handler.cc b/chromeos/audio/cras_audio_handler.cc
index 736af14..15911ed 100644
--- a/chromeos/audio/cras_audio_handler.cc
+++ b/chromeos/audio/cras_audio_handler.cc
@@ -466,8 +466,18 @@ bool CrasAudioHandler::ChangeActiveDevice(const AudioDevice& new_active_device,
       new_active_device.id == *current_active_node_id) {
     return false;
   }
-  if (GetDeviceFromId(*current_active_node_id))
-    audio_devices_[*current_active_node_id].active = false;
+
+  // Reset all other input or output devices' active status. The active audio
+  // device from the previous user session can be remembered by cras, but not
+  // in chrome. see crbug.com/273271.
+  for (AudioDeviceMap::iterator it = audio_devices_.begin();
+       it != audio_devices_.end(); ++it) {
+    if (it->second.is_input == new_active_device.is_input &&
+        it->second.id != new_active_device.id)
+      it->second.active = false;
+  }
+
+  // Set the current active input/output device to the new_active_device.
   *current_active_node_id = new_active_device.id;
   audio_devices_[*current_active_node_id].active = true;
   return true;
@@ -477,8 +487,16 @@ bool CrasAudioHandler::NonActiveDeviceUnplugged(
     size_t old_devices_size,
     size_t new_devices_size,
     uint64 current_active_node) {
+  // There could be cases that more than one NodesChanged signals are
+  // triggered by cras for unplugging or plugging one audio devices, both coming
+  // with the same node data. After handling the first NodesChanged signal, the
+  // audio_devices_ can be overwritten by staled node data from handling 2nd
+  // NodesChanged signal. Therefore, we need to check if the device with
+  // current_active_node is consistently active or not.
+  // crbug.com/274641.
   return (new_devices_size <= old_devices_size &&
-          GetDeviceFromId(current_active_node));
+          GetDeviceFromId(current_active_node) &&
+          audio_devices_[current_active_node].active);
 }
 
 void CrasAudioHandler::SwitchToDevice(const AudioDevice& device) {
@@ -531,13 +549,13 @@ void CrasAudioHandler::UpdateDevicesAndSwitchActive(
   // devices, the previously set active audio device will stay active.
   // Otherwise, switch to a new active audio device according to their priority.
   if (!NonActiveDeviceUnplugged(old_audio_devices_size,
-                                     audio_devices_.size(),
-                                     active_input_node_id_) &&
+                                audio_devices_.size(),
+                                active_input_node_id_) &&
       !input_devices_pq_.empty())
     SwitchToDevice(input_devices_pq_.top());
   if (!NonActiveDeviceUnplugged(old_audio_devices_size,
-                                     audio_devices_.size(),
-                                     active_output_node_id_) &&
+                                audio_devices_.size(),
+                                active_output_node_id_) &&
       !output_devices_pq_.empty()) {
     SwitchToDevice(output_devices_pq_.top());
   }
diff --git a/chromeos/audio/cras_audio_handler_unittest.cc b/chromeos/audio/cras_audio_handler_unittest.cc
index d18488a..48a1200 100644
--- a/chromeos/audio/cras_audio_handler_unittest.cc
+++ b/chromeos/audio/cras_audio_handler_unittest.cc
@@ -896,6 +896,45 @@ TEST_F(CrasAudioHandlerTest, UnplugUSBHeadphonesWithActiveSpeaker) {
   EXPECT_TRUE(cras_audio_handler_->has_alternative_output());
 }
 
+TEST_F(CrasAudioHandlerTest, OneActiveAudioOutputAfterLoginNewUserSession) {
+  // This tests the case found with crbug.com/273271.
+  // Initialize with internal speaker, bluetooth headphone and headphone jack
+  // for a new chrome session after user signs out from the previous session.
+  // Headphone jack is plugged in later than bluetooth headphone, but bluetooth
+  // headphone is selected as the active output by user from previous user
+  // session.
+  AudioNodeList audio_nodes;
+  audio_nodes.push_back(kInternalSpeaker);
+  AudioNode bluetooth_headphone(kBluetoothHeadset);
+  bluetooth_headphone.active = true;
+  bluetooth_headphone.plugged_time = 70000000;
+  audio_nodes.push_back(bluetooth_headphone);
+  AudioNode headphone_jack(kHeadphone);
+  headphone_jack.plugged_time = 80000000;
+  audio_nodes.push_back(headphone_jack);
+  SetUpCrasAudioHandler(audio_nodes);
+  const size_t init_nodes_size = audio_nodes.size();
+
+  // Verify the audio devices size.
+  AudioDeviceList audio_devices;
+  cras_audio_handler_->GetAudioDevices(&audio_devices);
+  EXPECT_EQ(init_nodes_size, audio_devices.size());
+  EXPECT_EQ(0, test_observer_->audio_nodes_changed_count());
+
+  // Verify the headphone jack is selected as the active output and all other
+  // audio devices are not active.
+  EXPECT_EQ(0, test_observer_->active_output_node_changed_count());
+  AudioDevice active_output;
+  EXPECT_TRUE(cras_audio_handler_->GetActiveOutputDevice(&active_output));
+  EXPECT_EQ(kHeadphone.id, active_output.id);
+  EXPECT_EQ(kHeadphone.id, cras_audio_handler_->GetActiveOutputNode());
+  EXPECT_TRUE(cras_audio_handler_->has_alternative_output());
+  for (size_t i = 0; i < audio_devices.size(); ++i) {
+    if (audio_devices[i].id != kHeadphone.id)
+      EXPECT_FALSE(audio_devices[i].active);
+  }
+}
+
 TEST_F(CrasAudioHandlerTest, PlugUSBMic) {
   // Set up initial audio devices, only with internal mic.
   AudioNodeList audio_nodes;