Use device_name + node_id to index devices in preferences.

Refactor the code to send the device object itself to the audio devices preferences handler. The preferences handler then pulls the device name and the node_id out of the object to generate it's device identification string. The 'id' field in the AudioDevice object is really
the device_id << 32 | node_id (it is sent as one value from cras), so to get the node_id, we need to & the value out. This changes how we store our audio device list internally, using a map instead of a list, making our active node lookups more efficient too.

Reviews requested,
hshi@ overall review
stevenjb@ chromeos/* and ash/system/chromeos/* OWNERs review
dgreid@ fyi

R=dgreid@chromium.org,hshi@chromium.org,stevenjb@chromium.org
BUG=241441
TEST=Tested with switching between various devices to ensure that preferences were remembered and switched per device correctly.

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

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

5 files changed, 86 insertions, 43 deletions

diff --git a/chromeos/audio/audio_device.cc b/chromeos/audio/audio_device.cc
index 5a70538..7ebdb78 100644
--- a/chromeos/audio/audio_device.cc
+++ b/chromeos/audio/audio_device.cc
@@ -92,9 +92,9 @@ AudioDevice::AudioDevice(const AudioNode& node) {
   id = node.id;
   type = GetAudioType(node.type);
   if (!node.name.empty() && node.name != "(default)")
-    display_name = UTF8ToUTF16(node.name);
+    display_name = node.name;
   else
-    display_name = UTF8ToUTF16(node.device_name);
+    display_name = node.device_name;
   priority = GetDevicePriority(type);
   active = node.active;
   plugged_time = node.plugged_time;
@@ -110,7 +110,7 @@ std::string AudioDevice::ToString() const {
                       base::Uint64ToString(id).c_str());
   base::StringAppendF(&result,
                       "display_name = %s ",
-                      UTF16ToUTF8(display_name).c_str());
+                      display_name.c_str());
   base::StringAppendF(&result,
                       "type = %s ",
                       GetTypeString(type).c_str());
diff --git a/chromeos/audio/audio_device.h b/chromeos/audio/audio_device.h
index 894c118..30712e2 100644
--- a/chromeos/audio/audio_device.h
+++ b/chromeos/audio/audio_device.h
@@ -5,11 +5,11 @@
 #ifndef CHROMEOS_AUDIO_AUDIO_DEVICE_H_
 #define CHROMEOS_AUDIO_AUDIO_DEVICE_H_
 
+#include <map>
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
-#include "base/strings/string16.h"
 #include "chromeos/chromeos_export.h"
 #include "chromeos/dbus/audio_node.h"
 
@@ -30,7 +30,7 @@ enum AudioDeviceType {
 struct CHROMEOS_EXPORT AudioDevice {
   bool is_input;
   uint64 id;
-  base::string16 display_name;
+  std::string display_name;
   AudioDeviceType type;
   uint8 priority;
   bool active;
@@ -42,6 +42,7 @@ struct CHROMEOS_EXPORT AudioDevice {
 };
 
 typedef std::vector<AudioDevice> AudioDeviceList;
+typedef std::map<uint64, AudioDevice> AudioDeviceMap;
 
 struct AudioDeviceCompare {
   // Rules used to discern which device is higher,
diff --git a/chromeos/audio/audio_devices_pref_handler.h b/chromeos/audio/audio_devices_pref_handler.h
index f528d25..08080b7 100644
--- a/chromeos/audio/audio_devices_pref_handler.h
+++ b/chromeos/audio/audio_devices_pref_handler.h
@@ -14,6 +14,8 @@ class PrefRegistrySimple;
 
 namespace chromeos {
 
+struct AudioDevice;
+
 // Interface that handles audio preference related work, reads and writes
 // audio preferences, and notifies AudioPrefObserver for audio preference
 // changes.
@@ -23,14 +25,14 @@ class CHROMEOS_EXPORT AudioDevicesPrefHandler
   // Gets the audio output volume value from prefs for a device. Since we can
   // only have either a gain or a volume for a device (depending on whether it
   // is input or output), we don't really care which value it is.
-  virtual double GetVolumeGainValue(uint64 device_id) = 0;
+  virtual double GetVolumeGainValue(const AudioDevice& device) = 0;
   // Sets the audio volume or gain value to prefs for a device.
-  virtual void SetVolumeGainValue(uint64 device_id, double value) = 0;
+  virtual void SetVolumeGainValue(const AudioDevice& device, double value) = 0;
 
   // Reads the audio mute value from prefs for a device.
-  virtual bool GetMuteValue(uint64 device_id) = 0;
+  virtual bool GetMuteValue(const AudioDevice& device) = 0;
   // Sets the audio mute value to prefs for a device.
-  virtual void SetMuteValue(uint64 device_id, bool mute_on) = 0;
+  virtual void SetMuteValue(const AudioDevice& device, bool mute_on) = 0;
 
   // Reads the audio capture allowed value from prefs.
   virtual bool GetAudioCaptureAllowedValue() = 0;
diff --git a/chromeos/audio/cras_audio_handler.cc b/chromeos/audio/cras_audio_handler.cc
index fdd7949..9e1a892 100644
--- a/chromeos/audio/cras_audio_handler.cc
+++ b/chromeos/audio/cras_audio_handler.cc
@@ -104,7 +104,10 @@ bool CrasAudioHandler::IsOutputMuted() {
 }
 
 bool CrasAudioHandler::IsOutputMutedForDevice(uint64 device_id) {
-  return audio_pref_handler_->GetMuteValue(device_id);
+  const AudioDevice* device = GetDeviceFromId(device_id);
+  if (!device)
+    return false;
+  return audio_pref_handler_->GetMuteValue(*device);
 }
 
 bool CrasAudioHandler::IsOutputVolumeBelowDefaultMuteLvel() {
@@ -116,7 +119,10 @@ bool CrasAudioHandler::IsInputMuted() {
 }
 
 bool CrasAudioHandler::IsInputMutedForDevice(uint64 device_id) {
-  return audio_pref_handler_->GetMuteValue(device_id);
+  const AudioDevice* device = GetDeviceFromId(device_id);
+  if (!device)
+    return false;
+  return audio_pref_handler_->GetMuteValue(*device);
 }
 
 int CrasAudioHandler::GetOutputVolumePercent() {
@@ -124,10 +130,14 @@ int CrasAudioHandler::GetOutputVolumePercent() {
 }
 
 int CrasAudioHandler::GetOutputVolumePercentForDevice(uint64 device_id) {
-  if (device_id == active_output_node_id_)
+  if (device_id == active_output_node_id_) {
     return output_volume_;
-  else
-    return (int) audio_pref_handler_->GetVolumeGainValue(device_id);
+  } else {
+    const AudioDevice* device = GetDeviceFromId(device_id);
+    if (!device)
+      return kDefaultVolumeGainPercent;
+    return static_cast<int>(audio_pref_handler_->GetVolumeGainValue(*device));
+  }
 }
 
 int CrasAudioHandler::GetInputGainPercent() {
@@ -135,10 +145,14 @@ int CrasAudioHandler::GetInputGainPercent() {
 }
 
 int CrasAudioHandler::GetInputGainPercentForDevice(uint64 device_id) {
-  if (device_id == active_input_node_id_)
+  if (device_id == active_input_node_id_) {
     return input_gain_;
-  else
-    return (int) audio_pref_handler_->GetVolumeGainValue(device_id);
+  } else {
+    const AudioDevice* device = GetDeviceFromId(device_id);
+    if (!device)
+      return kDefaultVolumeGainPercent;
+    return static_cast<int>(audio_pref_handler_->GetVolumeGainValue(*device));
+  }
 }
 
 uint64 CrasAudioHandler::GetActiveOutputNode() const {
@@ -150,18 +164,17 @@ uint64 CrasAudioHandler::GetActiveInputNode() const {
 }
 
 void CrasAudioHandler::GetAudioDevices(AudioDeviceList* device_list) const {
-  for (size_t i = 0; i < audio_devices_.size(); ++i)
-    device_list->push_back(audio_devices_[i]);
+  for (AudioDeviceMap::const_iterator it = audio_devices_.begin();
+       it != audio_devices_.end(); ++it)
+    device_list->push_back(it->second);
 }
 
 bool CrasAudioHandler::GetActiveOutputDevice(AudioDevice* device) const {
-  for (size_t i = 0; i < audio_devices_.size(); ++i) {
-    if (audio_devices_[i].id == active_output_node_id_) {
-      *device = audio_devices_[i];
-      return true;
-    }
-  }
-  return false;
+  const AudioDevice* active_device = GetDeviceFromId(active_output_node_id_);
+  if (!device)
+    return false;
+  *device = *active_device;
+  return true;
 }
 
 bool CrasAudioHandler::has_alternative_input() const {
@@ -177,8 +190,10 @@ void CrasAudioHandler::SetOutputVolumePercent(int volume_percent) {
   if (volume_percent <= kMuteThresholdPercent)
     volume_percent = 0;
   output_volume_ = volume_percent;
-  audio_pref_handler_->SetVolumeGainValue(active_output_node_id_,
-                                          output_volume_);
+
+  if (const AudioDevice* device = GetDeviceFromId(active_output_node_id_))
+    audio_pref_handler_->SetVolumeGainValue(*device, output_volume_);
+
   SetOutputVolumeInternal(output_volume_);
   FOR_EACH_OBSERVER(AudioObserver, observers_, OnOutputVolumeChanged());
 }
@@ -188,7 +203,10 @@ void CrasAudioHandler::SetInputGainPercent(int gain_percent) {
   if (gain_percent <= kMuteThresholdPercent)
     gain_percent = 0;
   input_gain_ = gain_percent;
-  audio_pref_handler_->SetVolumeGainValue(active_input_node_id_, input_gain_);
+
+  if (const AudioDevice* device = GetDeviceFromId(active_input_node_id_))
+    audio_pref_handler_->SetVolumeGainValue(*device, input_gain_);
+
   SetInputGainInternal(input_gain_);
   FOR_EACH_OBSERVER(AudioObserver, observers_, OnInputGainChanged());
 }
@@ -202,7 +220,10 @@ void CrasAudioHandler::SetOutputMute(bool mute_on) {
     return;
 
   output_mute_on_ = mute_on;
-  audio_pref_handler_->SetMuteValue(active_output_node_id_, output_mute_on_);
+
+  if (const AudioDevice* device = GetDeviceFromId(active_output_node_id_))
+    audio_pref_handler_->SetMuteValue(*device, output_mute_on_);
+
   FOR_EACH_OBSERVER(AudioObserver, observers_, OnOutputMuteChanged());
 }
 
@@ -219,7 +240,12 @@ void CrasAudioHandler::SetInputMute(bool mute_on) {
     return;
 
   input_mute_on_ = mute_on;
-  audio_pref_handler_->SetMuteValue(active_input_node_id_, input_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());
 }
 
@@ -247,7 +273,8 @@ void CrasAudioHandler::SetVolumeGainPercentForDevice(uint64 device_id,
   if (value <= kMuteThresholdPercent)
     value = 0;
 
-  audio_pref_handler_->SetVolumeGainValue(device_id, value);
+  if (const AudioDevice* device = GetDeviceFromId(device_id))
+    audio_pref_handler_->SetVolumeGainValue(*device, value);
 }
 
 void CrasAudioHandler::SetMuteForDevice(uint64 device_id, bool mute_on) {
@@ -258,7 +285,10 @@ void CrasAudioHandler::SetMuteForDevice(uint64 device_id, bool mute_on) {
     SetInputMute(mute_on);
     return;
   }
-  audio_pref_handler_->SetMuteValue(device_id, mute_on);
+
+  AudioDevice device;
+  if (const AudioDevice* device = GetDeviceFromId(device_id))
+    audio_pref_handler_->SetMuteValue(*device, mute_on);
 }
 
 CrasAudioHandler::CrasAudioHandler(
@@ -360,12 +390,20 @@ void CrasAudioHandler::OnAudioPolicyPrefChanged() {
   ApplyAudioPolicy();
 }
 
+const AudioDevice* CrasAudioHandler::GetDeviceFromId(uint64 device_id) const {
+  AudioDeviceMap::const_iterator it = audio_devices_.find(device_id);
+  if (it == audio_devices_.end())
+    return NULL;
+
+  return &(it->second);
+}
+
 void CrasAudioHandler::SetupAudioInputState() {
   // Set the initial audio state to the ones read from audio prefs.
-  if (active_input_node_id_) {
-    input_mute_on_ = audio_pref_handler_->GetMuteValue(active_input_node_id_);
-    input_gain_ = audio_pref_handler_->GetVolumeGainValue(
-        active_input_node_id_);
+  const AudioDevice* device = GetDeviceFromId(active_input_node_id_);
+  if (device) {
+    input_mute_on_ = audio_pref_handler_->GetMuteValue(*device);
+    input_gain_ = audio_pref_handler_->GetVolumeGainValue(*device);
   } else {
     input_mute_on_ = kPrefMuteOff;
     input_gain_ = kDefaultVolumeGainPercent;
@@ -376,10 +414,10 @@ void CrasAudioHandler::SetupAudioInputState() {
 }
 
 void CrasAudioHandler::SetupAudioOutputState() {
-  if (active_output_node_id_) {
-    output_mute_on_ = audio_pref_handler_->GetMuteValue(active_output_node_id_);
-    output_volume_ = audio_pref_handler_->GetVolumeGainValue(
-        active_output_node_id_);
+  const AudioDevice* device = GetDeviceFromId(active_output_node_id_);
+  if (device) {
+    output_mute_on_ = audio_pref_handler_->GetMuteValue(*device);
+    output_volume_ = audio_pref_handler_->GetVolumeGainValue(*device);
   } else {
     output_mute_on_ = kPrefMuteOff;
     output_volume_ = kDefaultVolumeGainPercent;
@@ -494,7 +532,7 @@ void CrasAudioHandler::UpdateDevicesAndSwitchActive(
     else if (!nodes[i].is_input && nodes[i].active)
       active_output_node_id_ = nodes[i].id;
     AudioDevice device(nodes[i]);
-    audio_devices_.push_back(device);
+    audio_devices_[device.id] = device;
 
     if (!has_alternative_input_ &&
         device.is_input &&
diff --git a/chromeos/audio/cras_audio_handler.h b/chromeos/audio/cras_audio_handler.h
index f9da570..cb26f23 100644
--- a/chromeos/audio/cras_audio_handler.h
+++ b/chromeos/audio/cras_audio_handler.h
@@ -193,6 +193,8 @@ class CHROMEOS_EXPORT CrasAudioHandler : public CrasAudioClient::Observer,
   void SetupAudioInputState();
   void SetupAudioOutputState();
 
+  const AudioDevice* GetDeviceFromId(uint64 device_id) const;
+
   // Initializes audio state, which should only be called when CrasAudioHandler
   // is created or cras audio client is restarted.
   void InitializeAudioState();
@@ -232,7 +234,7 @@ class CHROMEOS_EXPORT CrasAudioHandler : public CrasAudioClient::Observer,
   ObserverList<AudioObserver> observers_;
 
   // Audio data and state.
-  AudioDeviceList audio_devices_;
+  AudioDeviceMap audio_devices_;
 
   AudioDevicePriorityQueue input_devices_pq_;
   AudioDevicePriorityQueue output_devices_pq_;