Fix the bluetooth audio device id changing on the fly issue and the active device inconsistent issue caused by multiple NodesChanged signal received for the same node change.

BUG=290485

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

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

3 files changed, 77 insertions, 1 deletions

diff --git a/chromeos/audio/cras_audio_handler.cc b/chromeos/audio/cras_audio_handler.cc
index 5460466..60663eb 100644
--- a/chromeos/audio/cras_audio_handler.cc
+++ b/chromeos/audio/cras_audio_handler.cc
@@ -30,6 +30,11 @@ const int kMuteThresholdPercent = 1;
 
 static CrasAudioHandler* g_cras_audio_handler = NULL;
 
+bool IsSameAudioDevice(const AudioDevice& a, const AudioDevice& b) {
+  return a.id == b.id && a.is_input == b.is_input && a.type == b.type
+      && a.device_name == b.device_name;
+}
+
 }  // namespace
 
 CrasAudioHandler::AudioObserver::AudioObserver() {
@@ -562,12 +567,31 @@ bool CrasAudioHandler::HasDeviceChange(const AudioNodeList& new_nodes,
 
   for (AudioNodeList::const_iterator it = new_nodes.begin();
        it != new_nodes.end(); ++it) {
-    if (is_input == it->is_input)
+    if (is_input == it->is_input) {
       ++num_new_devices;
+      // Look to see if the new device not in the old device list.
+      AudioDevice device(*it);
+      if (FoundNewDevice(device))
+        return true;
+    }
   }
   return num_old_devices != num_new_devices;
 }
 
+bool CrasAudioHandler::FoundNewDevice(const AudioDevice& device) {
+  const AudioDevice* device_found = GetDeviceFromId(device.id);
+  if (!device_found)
+    return true;
+
+  if (!IsSameAudioDevice(device, *device_found)) {
+    LOG(WARNING) << "Different Audio devices with same id:"
+        << " new device: " << device.ToString()
+        << " old device: " << device_found->ToString();
+    return true;
+  }
+  return false;
+}
+
 void CrasAudioHandler::UpdateDevicesAndSwitchActive(
     const AudioNodeList& nodes) {
   size_t old_audio_devices_size = audio_devices_.size();
diff --git a/chromeos/audio/cras_audio_handler.h b/chromeos/audio/cras_audio_handler.h
index a3e706a..6777ced 100644
--- a/chromeos/audio/cras_audio_handler.h
+++ b/chromeos/audio/cras_audio_handler.h
@@ -238,6 +238,9 @@ class CHROMEOS_EXPORT CrasAudioHandler : public CrasAudioClient::Observer,
   void HandleGetNodesError(const std::string& error_name,
                            const std::string& error_msg);
 
+  // Returns true if |device| is not found in audio_devices_.
+  bool FoundNewDevice(const AudioDevice& device);
+
   scoped_refptr<AudioDevicesPrefHandler> audio_pref_handler_;
   base::WeakPtrFactory<CrasAudioHandler> weak_ptr_factory_;
   ObserverList<AudioObserver> observers_;
diff --git a/chromeos/audio/cras_audio_handler_unittest.cc b/chromeos/audio/cras_audio_handler_unittest.cc
index 03dd4aa..f584f21 100644
--- a/chromeos/audio/cras_audio_handler_unittest.cc
+++ b/chromeos/audio/cras_audio_handler_unittest.cc
@@ -935,6 +935,55 @@ TEST_F(CrasAudioHandlerTest, OneActiveAudioOutputAfterLoginNewUserSession) {
   }
 }
 
+TEST_F(CrasAudioHandlerTest, BluetoothSpeakerIdChangedOnFly) {
+  // Initialize with internal speaker and bluetooth headset.
+  AudioNodeList audio_nodes;
+  audio_nodes.push_back(kInternalSpeaker);
+  audio_nodes.push_back(kBluetoothHeadset);
+  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 bluetooth headset 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(kBluetoothHeadset.id, active_output.id);
+  EXPECT_EQ(kBluetoothHeadset.id, cras_audio_handler_->GetActiveOutputNode());
+  EXPECT_TRUE(cras_audio_handler_->has_alternative_output());
+
+  // Cras changes the bluetooth headset's id on the fly.
+  audio_nodes.clear();
+  AudioNode internal_speaker(kInternalSpeaker);
+  internal_speaker.active = false;
+  audio_nodes.push_back(internal_speaker);
+  AudioNode bluetooth_headphone(kBluetoothHeadset);
+  // Change bluetooth headphone id.
+  bluetooth_headphone.id = kBluetoothHeadsetId + 20000;
+  bluetooth_headphone.active = false;
+  audio_nodes.push_back(bluetooth_headphone);
+  ChangeAudioNodes(audio_nodes);
+
+  // Verify NodesChanged event is fired, and the audio devices size is not
+  // changed.
+  audio_devices.clear();
+  cras_audio_handler_->GetAudioDevices(&audio_devices);
+  EXPECT_EQ(init_nodes_size, audio_devices.size());
+  EXPECT_EQ(1, test_observer_->audio_nodes_changed_count());
+
+  // Verify ActiveOutputNodeChanged event is fired, and active device should be
+  // bluetooth headphone.
+  EXPECT_EQ(1, test_observer_->active_output_node_changed_count());
+  EXPECT_TRUE(cras_audio_handler_->GetActiveOutputDevice(&active_output));
+  EXPECT_EQ(bluetooth_headphone.id, active_output.id);
+}
+
 TEST_F(CrasAudioHandlerTest, PlugUSBMic) {
   // Set up initial audio devices, only with internal mic.
   AudioNodeList audio_nodes;