Final cleanup pass over audio device API

extensions/browser/api/audio/audio_api.cc

 // Copyright (c) 2013 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 "extensions/browser/api/audio/audio_api.h"
 
 #include <utility>
 
 #include "base/lazy_instance.h"
 #include "base/values.h"
@@ skipping 30 matching lines @@
 void AudioAPI::OnDeviceChanged() {
   if (EventRouter::Get(browser_context_)) {
     std::unique_ptr<Event> event(new Event(
         events::AUDIO_ON_DEVICE_CHANGED, audio::OnDeviceChanged::kEventName,
         std::unique_ptr<base::ListValue>(new base::ListValue())));
     EventRouter::Get(browser_context_)->BroadcastEvent(std::move(event));
   }
 }
 
 void AudioAPI::OnLevelChanged(const std::string& id, int level) {
-  if (EventRouter::Get(browser_context_)) {
-    std::unique_ptr<base::ListValue> args =
-        audio::OnLevelChanged::Create(id, level);
-    std::unique_ptr<Event> event(new Event(events::AUDIO_ON_LEVEL_CHANGED,
-                                           audio::OnLevelChanged::kEventName,
-                                           std::move(args)));
-    EventRouter::Get(browser_context_)->BroadcastEvent(std::move(event));
-  }
+  EventRouter* event_router = EventRouter::Get(browser_context_);
+  if (!event_router)
+    return;
+
+  audio::LevelChangedEvent raw_event;
+  raw_event.device_id = id;
+  raw_event.level = level;
+
+  std::unique_ptr<base::ListValue> event_args =
+      audio::OnLevelChanged::Create(raw_event);
+  std::unique_ptr<Event> event(new Event(events::AUDIO_ON_LEVEL_CHANGED,
+                                         audio::OnLevelChanged::kEventName,
+                                         std::move(event_args)));
+  event_router->BroadcastEvent(std::move(event));
 }
 
 void AudioAPI::OnMuteChanged(bool is_input, bool is_muted) {
   EventRouter* event_router = EventRouter::Get(browser_context_);
   if (!event_router)
     return;
 
   // Dispatch onMuteChanged event.
   audio::MuteChangedEvent raw_event;
   raw_event.stream_type =
       is_input ? audio::STREAM_TYPE_INPUT : audio::STREAM_TYPE_OUTPUT;
   raw_event.is_muted = is_muted;
   std::unique_ptr<base::ListValue> event_args =
       audio::OnMuteChanged::Create(raw_event);
   std::unique_ptr<Event> event(new Event(events::AUDIO_ON_MUTE_CHANGED,
                                          audio::OnMuteChanged::kEventName,
                                          std::move(event_args)));
   event_router->BroadcastEvent(std::move(event));
 }
 
 void AudioAPI::OnDevicesChanged(const DeviceInfoList& devices) {
-  if (EventRouter::Get(browser_context_)) {
-    std::unique_ptr<base::ListValue> args =
-        audio::OnDevicesChanged::Create(devices);
-    std::unique_ptr<Event> event(new Event(events::AUDIO_ON_DEVICES_CHANGED,
-                                           audio::OnDevicesChanged::kEventName,
-                                           std::move(args)));
-    EventRouter::Get(browser_context_)->BroadcastEvent(std::move(event));
-  }
+  EventRouter* event_router = EventRouter::Get(browser_context_);
+  if (!event_router)
+    return;
+
+  std::unique_ptr<base::ListValue> args =
+      audio::OnDeviceListChanged::Create(devices);
+  std::unique_ptr<Event> event(new Event(events::AUDIO_ON_DEVICES_CHANGED,
+                                         audio::OnDeviceListChanged::kEventName,
+                                         std::move(args)));
+  event_router->BroadcastEvent(std::move(event));
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
 ExtensionFunction::ResponseAction AudioGetInfoFunction::Run() {
   AudioService* service =
       AudioAPI::GetFactoryInstance()->Get(browser_context())->GetService();
   DCHECK(service);
   OutputInfo output_info;
   InputInfo input_info;
   if (!service->GetInfo(&output_info, &input_info)) {
     return RespondNow(
         Error("Error occurred when querying audio device information."));
   }
 
   return RespondNow(
       ArgumentList(audio::GetInfo::Results::Create(output_info, input_info)));
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
+ExtensionFunction::ResponseAction AudioGetDevicesFunction::Run() {
+  std::unique_ptr<audio::GetDevices::Params> params(
+      audio::GetDevices::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params.get());
+
+  AudioService* service =
+      AudioAPI::GetFactoryInstance()->Get(browser_context())->GetService();
+  DCHECK(service);
+
+  std::vector<api::audio::AudioDeviceInfo> devices;
+  if (!service->GetDevices(params->filter, &devices)) {
+    return RespondNow(
+        Error("Error occurred when querying audio device information."));
+  }
+
+  return RespondNow(ArgumentList(audio::GetDevices::Results::Create(devices)));
+}
+
+///////////////////////////////////////////////////////////////////////////////
+
 ExtensionFunction::ResponseAction AudioSetActiveDevicesFunction::Run() {
   std::unique_ptr<audio::SetActiveDevices::Params> params(
       audio::SetActiveDevices::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
 
   AudioService* service =
       AudioAPI::GetFactoryInstance()->Get(browser_context())->GetService();
   DCHECK(service);
 
   if (params->ids.as_device_id_lists) {
@@ skipping 15 matching lines @@
 
 ExtensionFunction::ResponseAction AudioSetPropertiesFunction::Run() {
   std::unique_ptr<audio::SetProperties::Params> params(
       audio::SetProperties::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
 
   AudioService* service =
       AudioAPI::GetFactoryInstance()->Get(browser_context())->GetService();
   DCHECK(service);
 
+  int level_value =
+      params->properties.level.get() ? *params->properties.level : -1;

[Devlin, 2017/02/10 20:24:29]

largely unrelated to this cl, but where do we validate that level, volume, and
gain are between 0 and 100?  I don't see a min/max set.

[tbarzic, 2017/02/10 21:05:18]

volume is normalized to 0-100 here:
https://cs.chromium.org/chromium/src/chromeos/audio/cras_audio_handler.cc?rcl...

gain is intentionally not normalized (due
https://bugs.chromium.org/p/chromium/issues/detail?id=273872)


(rewrote this to better handle the case level is set to a negative value.)

[Devlin, 2017/02/13 23:18:09]

Ideally, we'd have it set as a minimum/maximum in the bindings, so that the
developers are aware that calling it with anything but 0 <= x <= 100 is invalid.
 Any chance we can do that before moving the API to stable?  (Though doesn't
need to be in this cl)

[tbarzic, 2017/02/14 00:27:13]

I agree, but from what I can see [0, 100] is not the complete set of valid
values, at least not for input devices.

Let me see with jennyz if this could be enforced/changed.

+
   int volume_value = params->properties.volume.get() ?
       *params->properties.volume : -1;
 
   int gain_value = params->properties.gain.get() ?
       *params->properties.gain : -1;
 
-  if (!service->SetDeviceSoundLevel(params->id, volume_value, gain_value))
+  // |volume_value| and |gain_value| are deprecated in favor of |level_value|;
+  // they are kept around only to ensure backward-compatibility and should be
+  // ignored if |level_value| is set.
+  if (!service->SetDeviceSoundLevel(
+          params->id, level_value >= 0 ? level_value : volume_value,
+          level_value >= 0 ? level_value : gain_value))
     return RespondNow(Error("Could not set volume/gain properties"));
 
   if (params->properties.is_muted.get() &&
       !service->SetMuteForDevice(params->id, *params->properties.is_muted)) {
     return RespondNow(Error("Could not set mute property."));
   }
 
   return RespondNow(NoArguments());
 }
 
@@ skipping 28 matching lines @@
 
   bool value = false;
   if (!service->GetMute(params->stream_type == audio::STREAM_TYPE_INPUT,
                         &value)) {
     return RespondNow(Error("Could not get mute state."));
   }
   return RespondNow(ArgumentList(audio::GetMute::Results::Create(value)));
 }
 
 }  // namespace extensions