Fix special character handling in audio preferences.

This CL fixes the handling of the "." character in device names for audio
preferences. It also adds unit tests for the preference handler.

R=jennyz@chromium.org
BUG=294488,431133

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

Cr-Commit-Position: refs/heads/master@{#308460}

chrome/browser/chromeos/audio/audio_devices_pref_handler_impl.cc

@@ -33,9 +33,14 @@ const int kPrefMuteOn = 1;
 // parts of the string could be identical, only the last part will differentiate
 // them.
 std::string GetDeviceIdString(const chromeos::AudioDevice& device) {
-  return device.device_name + " : " +
-         base::Uint64ToString(device.id & static_cast<uint64>(0xffffffff)) +
-         " : " + (device.is_input ? "1" : "0");
+  std::string device_id_string =
+      device.device_name + " : " +
+      base::Uint64ToString(device.id & static_cast<uint64>(0xffffffff)) +
+      " : " + (device.is_input ? "1" : "0");
+  // Replace any periods from the device id string with a space, since setting
+  // names cannot contain periods.
+  std::replace(device_id_string.begin(), device_id_string.end(), '.', ' ');
+  return device_id_string;
 }
 
 }  // namespace
@@ -114,6 +119,8 @@ double AudioDevicesPrefHandlerImpl::GetVolumeGainPrefValue(
   // cras has added support for normalizing input gain range.
   double value = device.is_input ?
       0.0 : GetDeviceDefaultOutputVolume(device);
+  // TODO(rkc): The above code is completely ignored since we 'always' have a
+  // default pref value. Fix this. http://crbug.com/442489
   device_volume_settings_->GetDouble(device_id_str, &value);
 
   return value;
@@ -139,7 +146,7 @@ AudioDevicesPrefHandlerImpl::AudioDevicesPrefHandlerImpl(
 }
 
 AudioDevicesPrefHandlerImpl::~AudioDevicesPrefHandlerImpl() {
-};
+}
 
 void AudioDevicesPrefHandlerImpl::InitializePrefObservers() {
   pref_change_registrar_.Init(local_state_);

chrome/browser/chromeos/audio/audio_devices_pref_handler_impl_unittest.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/chromeos/audio/audio_devices_pref_handler_impl.h"
+
+#include "base/memory/ref_counted.h"
+#include "base/prefs/testing_pref_service.h"
+#include "chrome/browser/chromeos/audio/audio_devices_pref_handler_impl.h"
+#include "chromeos/audio/audio_device.h"
+#include "chromeos/audio/audio_devices_pref_handler.h"
+#include "chromeos/dbus/audio_node.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chromeos {
+
+const uint64 kInternalMicId = 10003;
+const uint64 kHeadphoneId = 10002;
+const uint64 kHDMIOutputId = 10006;
+const uint64 kOtherTypeOutputId = 90001;
+const uint64 kOtherTypeInputId = 90002;
+
+const AudioDevice kInternalMic(AudioNode(true,
+                                         kInternalMicId,
+                                         "Fake Mic",
+                                         "INTERNAL_MIC",
+                                         "Internal Mic",
+                                         false,
+                                         0));
+
+const AudioDevice kHeadphone(AudioNode(false,
+                                       kHeadphoneId,
+                                       "Fake Headphone",
+                                       "HEADPHONE",
+                                       "Headphone",
+                                       false,
+                                       0));
+
+const AudioDevice kHDMIOutput(AudioNode(false,
+                                        kHDMIOutputId,
+                                        "HDMI output",
+                                        "HDMI",
+                                        "HDMI output",
+                                        false,
+                                        0));
+
+const AudioDevice kInputDeviceWithSpecialCharacters(
+    AudioNode(true,
+              kOtherTypeInputId,
+              "Fake ~!@#$%^&*()_+`-=<>?,./{}|[]\\\\Mic",
+              "SOME_OTHER_TYPE",
+              "Other Type Input Device",
+              true,
+              0));
+
+const AudioDevice kOutputDeviceWithSpecialCharacters(
+    AudioNode(false,
+              kOtherTypeOutputId,
+              "Fake ~!@#$%^&*()_+`-=<>?,./{}|[]\\\\Headphone",
+              "SOME_OTHER_TYPE",
+              "Other Type Output Device",
+              false,
+              0));
+
+class AudioDevicesPrefHandlerTest : public testing::Test {
+ public:
+  AudioDevicesPrefHandlerTest() {}
+  ~AudioDevicesPrefHandlerTest() override {}
+
+  void SetUp() override {
+    pref_service_.reset(new TestingPrefServiceSimple());
+    AudioDevicesPrefHandlerImpl::RegisterPrefs(pref_service_->registry());
+    audio_pref_handler_ = new AudioDevicesPrefHandlerImpl(pref_service_.get());
+  }
+
+  void TearDown() override { audio_pref_handler_ = NULL; }
+
+ protected:
+  scoped_refptr<AudioDevicesPrefHandler> audio_pref_handler_;
+  scoped_ptr<TestingPrefServiceSimple> pref_service_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(AudioDevicesPrefHandlerTest);
+};
+
+TEST_F(AudioDevicesPrefHandlerTest, TestDefaultValues) {
+  // TODO(rkc): Once the bug with default preferences is fixed, fix this test
+  // also. http://crbug.com/442489
+  EXPECT_EQ(75.0, audio_pref_handler_->GetInputGainValue(&kInternalMic));
+  EXPECT_EQ(75.0, audio_pref_handler_->GetOutputVolumeValue(&kHeadphone));
+  EXPECT_EQ(75.0, audio_pref_handler_->GetOutputVolumeValue(&kHDMIOutput));
+}
+
+TEST_F(AudioDevicesPrefHandlerTest, TestBasicInputOutputDevices) {
+  audio_pref_handler_->SetVolumeGainValue(kInternalMic, 13.37);
+  EXPECT_EQ(13.37, audio_pref_handler_->GetInputGainValue(&kInternalMic));
+  audio_pref_handler_->SetVolumeGainValue(kHeadphone, 47.28);
+  EXPECT_EQ(47.28, audio_pref_handler_->GetOutputVolumeValue(&kHeadphone));
+}
+
+TEST_F(AudioDevicesPrefHandlerTest, TestSpecialCharactersInDeviceNames) {
+  audio_pref_handler_->SetVolumeGainValue(kInputDeviceWithSpecialCharacters,
+                                          73.31);
+  audio_pref_handler_->SetVolumeGainValue(kOutputDeviceWithSpecialCharacters,
+                                          85.92);
+
+  EXPECT_EQ(73.31, audio_pref_handler_->GetInputGainValue(
+      &kInputDeviceWithSpecialCharacters));
+  EXPECT_EQ(85.92, audio_pref_handler_->GetOutputVolumeValue(
+      &kOutputDeviceWithSpecialCharacters));
+}
+
+}  // namespace chromeos

chrome/chrome_tests_unit.gypi

@@ -78,6 +78,7 @@
       'browser/chromeos/attestation/fake_certificate.cc',
       'browser/chromeos/attestation/fake_certificate.h',
       'browser/chromeos/attestation/platform_verification_flow_unittest.cc',
+      'browser/chromeos/audio/audio_devices_pref_handler_impl_unittest.cc',
       'browser/chromeos/customization/customization_document_unittest.cc',
       'browser/chromeos/dbus/printer_service_provider_unittest.cc',
       'browser/chromeos/display/display_preferences_unittest.cc',