Simplify logic behind chrome.audio.setActiveDevices

extensions/browser/api/audio/audio_service_chromeos.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_service.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include "base/callback.h"
@@ skipping 30 matching lines @@
   AudioServiceImpl();
   ~AudioServiceImpl() override;
 
   // Called by listeners to this service to add/remove themselves as observers.
   void AddObserver(AudioService::Observer* observer) override;
   void RemoveObserver(AudioService::Observer* observer) override;
 
   // Start to query audio device information.
   bool GetInfo(OutputInfo* output_info_out, InputInfo* input_info_out) override;
   void SetActiveDevices(const DeviceIdList& device_list) override;
+  bool SetActiveDeviceLists(
+      const std::unique_ptr<DeviceIdList>& input_devices,
+      const std::unique_ptr<DeviceIdList>& output_devives) override;
   bool SetDeviceProperties(const std::string& device_id,
                            bool muted,
                            int volume,
                            int gain) override;
 
  protected:
   // chromeos::CrasAudioHandler::AudioObserver overrides.
   void OnOutputNodeVolumeChanged(uint64_t id, int volume) override;
   void OnInputNodeGainChanged(uint64_t id, int gain) override;
   void OnOutputMuteChanged(bool mute_on, bool system_adjust) override;
   void OnInputMuteChanged(bool mute_on) override;
   void OnAudioNodesChanged() override;
   void OnActiveOutputNodeChanged() override;
   void OnActiveInputNodeChanged() override;
 
  private:
   void NotifyDeviceChanged();
   void NotifyLevelChanged(uint64_t id, int level);
   void NotifyMuteChanged(bool is_input, bool is_muted);
   void NotifyDevicesChanged();
 
-  bool FindDevice(uint64_t id, chromeos::AudioDevice* device);
   uint64_t GetIdFromStr(const std::string& id_str);
+  bool GetAudioNodeIdList(const DeviceIdList& ids,
+                          bool is_input,
+                          chromeos::CrasAudioHandler::NodeIdList* node_ids);
 
   // List of observers.
   base::ObserverList<AudioService::Observer> observer_list_;
 
   chromeos::CrasAudioHandler* cras_audio_handler_;
 
   // Note: This should remain the last member so it'll be destroyed and
   // invalidate the weak pointers before any other members are destroyed.
   base::WeakPtrFactory<AudioServiceImpl> weak_ptr_factory_;
 
@@ skipping 59 matching lines @@
   }
   return true;
 }
 
 void AudioServiceImpl::SetActiveDevices(const DeviceIdList& device_list) {
   DCHECK(cras_audio_handler_);
   if (!cras_audio_handler_)
     return;
 
   chromeos::CrasAudioHandler::NodeIdList id_list;
-  for (size_t i = 0; i < device_list.size(); ++i) {
-    chromeos::AudioDevice device;
-    if (FindDevice(GetIdFromStr(device_list[i]), &device))
-      id_list.push_back(device.id);
+  for (const auto& id : device_list) {
+    const chromeos::AudioDevice* device =
+        cras_audio_handler_->GetDeviceFromId(GetIdFromStr(id));
+    if (device)
+      id_list.push_back(device->id);
   }
   cras_audio_handler_->ChangeActiveNodes(id_list);
 }
 
+bool AudioServiceImpl::SetActiveDeviceLists(
+    const std::unique_ptr<DeviceIdList>& input_ids,
+    const std::unique_ptr<DeviceIdList>& output_ids) {
+  DCHECK(cras_audio_handler_);
+  if (!cras_audio_handler_)
+    return false;
+
+  chromeos::CrasAudioHandler::NodeIdList input_nodes;
+  if (input_ids.get() && !GetAudioNodeIdList(*input_ids, true, &input_nodes))
+    return false;
+
+  chromeos::CrasAudioHandler::NodeIdList output_nodes;
+  if (output_ids.get() &&
+      !GetAudioNodeIdList(*output_ids, false, &output_nodes)) {
+    return false;
+  }
+
+  bool success = true;
+  if (input_ids.get()) {
+    success = success && cras_audio_handler_->SetActiveInputNodes(input_nodes);

[jennyz, 2017/01/07 00:33:00]

success is set initialized to true, what's the point && in the above line?

[tbarzic, 2017/01/07 05:16:31]

There is none - mostly was like this to keep it consistent with the line bellow
- fixed (and order of input and output handling, so both lines fit to 80 chars).

+    DCHECK(success);
+  }
+  if (output_ids.get()) {
+    success =
+        success && cras_audio_handler_->SetActiveOutputNodes(output_nodes);
+    DCHECK(success);
+  }
+  return success;
+}
+
 bool AudioServiceImpl::SetDeviceProperties(const std::string& device_id,
                                            bool muted,
                                            int volume,
                                            int gain) {
   DCHECK(cras_audio_handler_);
   if (!cras_audio_handler_)
     return false;
 
-  chromeos::AudioDevice device;
-  bool found = FindDevice(GetIdFromStr(device_id), &device);
-  if (!found)
+  const chromeos::AudioDevice* device =
+      cras_audio_handler_->GetDeviceFromId(GetIdFromStr(device_id));
+  if (!device)
     return false;
 
-  cras_audio_handler_->SetMuteForDevice(device.id, muted);
+  cras_audio_handler_->SetMuteForDevice(device->id, muted);
 
-  if (!device.is_input && volume != -1) {
-    cras_audio_handler_->SetVolumeGainPercentForDevice(device.id, volume);
+  if (!device->is_input && volume != -1) {
+    cras_audio_handler_->SetVolumeGainPercentForDevice(device->id, volume);
     return true;
-  } else if (device.is_input && gain != -1) {
-    cras_audio_handler_->SetVolumeGainPercentForDevice(device.id, gain);
+  } else if (device->is_input && gain != -1) {
+    cras_audio_handler_->SetVolumeGainPercentForDevice(device->id, gain);
     return true;
   }
 
   return false;
 }
 
-bool AudioServiceImpl::FindDevice(uint64_t id, chromeos::AudioDevice* device) {
-  chromeos::AudioDeviceList devices;
-  cras_audio_handler_->GetAudioDevices(&devices);
-
-  for (size_t i = 0; i < devices.size(); ++i) {
-    if (devices[i].id == id) {
-      *device = devices[i];
-      return true;
-    }
-  }
-  return false;
-}
-
 uint64_t AudioServiceImpl::GetIdFromStr(const std::string& id_str) {
   uint64_t device_id;
   if (!base::StringToUint64(id_str, &device_id))
     return 0;
   else
     return device_id;
 }
 
+bool AudioServiceImpl::GetAudioNodeIdList(
+    const DeviceIdList& ids,
+    bool is_input,
+    chromeos::CrasAudioHandler::NodeIdList* node_ids) {
+  for (const auto& device_id : ids) {
+    const chromeos::AudioDevice* device =
+        cras_audio_handler_->GetDeviceFromId(GetIdFromStr(device_id));
+    if (!device)
+      return false;
+    if (device->is_input != is_input)
+      return false;
+    node_ids->push_back(device->id);
+  }
+  return true;
+}
+
 void AudioServiceImpl::OnOutputNodeVolumeChanged(uint64_t id, int volume) {
   NotifyLevelChanged(id, volume);
 }
 
 void AudioServiceImpl::OnOutputMuteChanged(bool mute_on, bool system_adjust) {
   NotifyMuteChanged(false, mute_on);
 }
 
 void AudioServiceImpl::OnInputNodeGainChanged(uint64_t id, int gain) {
   NotifyLevelChanged(id, gain);
@@ skipping 74 matching lines @@
   // Notify DeviceChanged event for backward compatibility.
   // TODO(jennyz): remove this code when the old version of hotrod retires.
   NotifyDeviceChanged();
 }
 
 AudioService* AudioService::CreateInstance() {
   return new AudioServiceImpl;
 }
 
 }  // namespace extensions