Switching WebRtc private API from AudioManager to AudioSystem

chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc

 // Copyright 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 "chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.h"
 
 #include <utility>
 #include <vector>
 
 #include "base/lazy_instance.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/task_runner_util.h"
 #include "chrome/browser/extensions/api/tabs/tabs_constants.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "content/public/browser/media_device_id.h"
 #include "content/public/browser/web_contents.h"
 #include "extensions/browser/event_router.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/common/error_utils.h"
 #include "extensions/common/permissions/permissions_data.h"
-#include "media/audio/audio_device_description.h"
+#include "media/audio/audio_system.h"
 #include "url/gurl.h"
 #include "url/origin.h"
 
 namespace extensions {
 
 using content::BrowserThread;
-using content::RenderProcessHost;
-using media::AudioDeviceDescriptions;
-using media::AudioManager;
-
 namespace wap = api::webrtc_audio_private;
 
 using api::webrtc_audio_private::RequestInfo;
 
 static base::LazyInstance<BrowserContextKeyedAPIFactory<
     WebrtcAudioPrivateEventService>>::DestructorAtExit g_factory =
     LAZY_INSTANCE_INITIALIZER;
 
 WebrtcAudioPrivateEventService::WebrtcAudioPrivateEventService(
     content::BrowserContext* context)
@@ skipping 25 matching lines @@
   return "WebrtcAudioPrivateEventService";
 }
 
 void WebrtcAudioPrivateEventService::OnDevicesChanged(
     base::SystemMonitor::DeviceType device_type) {
   switch (device_type) {
     case base::SystemMonitor::DEVTYPE_AUDIO:
     case base::SystemMonitor::DEVTYPE_VIDEO_CAPTURE:
       SignalEvent();
       break;
-
     default:
       // No action needed.
       break;
   }
 }
 
 void WebrtcAudioPrivateEventService::SignalEvent() {
   using api::webrtc_audio_private::OnSinksChanged::kEventName;
 
   EventRouter* router = EventRouter::Get(browser_context_);
   if (!router || !router->HasEventListener(kEventName))
     return;
 
   for (const scoped_refptr<const extensions::Extension>& extension :
        ExtensionRegistry::Get(browser_context_)->enabled_extensions()) {
     const std::string& extension_id = extension->id();
     if (router->ExtensionHasEventListener(extension_id, kEventName) &&
         extension->permissions_data()->HasAPIPermission("webrtcAudioPrivate")) {
       std::unique_ptr<Event> event = base::MakeUnique<Event>(
           events::WEBRTC_AUDIO_PRIVATE_ON_SINKS_CHANGED, kEventName,
           base::MakeUnique<base::ListValue>());
       router->DispatchEventToExtension(extension_id, std::move(event));
     }
   }
 }
 
 WebrtcAudioPrivateFunction::WebrtcAudioPrivateFunction() {}
 
-WebrtcAudioPrivateFunction::~WebrtcAudioPrivateFunction() {
-}
+WebrtcAudioPrivateFunction::~WebrtcAudioPrivateFunction() {}
 
-void WebrtcAudioPrivateFunction::GetOutputDeviceDescriptions() {
-  scoped_refptr<base::SingleThreadTaskRunner> audio_manager_runner =
-      AudioManager::Get()->GetTaskRunner();
-  if (!audio_manager_runner->BelongsToCurrentThread()) {
-    DCHECK_CURRENTLY_ON(BrowserThread::UI);
-    audio_manager_runner->PostTask(
-        FROM_HERE,
-        base::Bind(&WebrtcAudioPrivateFunction::GetOutputDeviceDescriptions,
-                   this));
-    return;
-  }
-
-  std::unique_ptr<AudioDeviceDescriptions> device_descriptions =
-      base::MakeUnique<AudioDeviceDescriptions>();
-  AudioManager::Get()->GetAudioOutputDeviceDescriptions(
-      device_descriptions.get());
-
-  BrowserThread::PostTask(
-      BrowserThread::IO, FROM_HERE,
-      base::Bind(&WebrtcAudioPrivateFunction::OnOutputDeviceDescriptions, this,
-                 base::Passed(&device_descriptions)));
-}
-
-void WebrtcAudioPrivateFunction::OnOutputDeviceDescriptions(
-    std::unique_ptr<AudioDeviceDescriptions> device_descriptions) {
-  NOTREACHED();
-}
-
-void WebrtcAudioPrivateFunction::CalculateHMAC(const std::string& raw_id) {
-  if (!BrowserThread::CurrentlyOn(BrowserThread::IO)) {
-    BrowserThread::PostTask(
-        BrowserThread::IO,
-        FROM_HERE,
-        base::Bind(&WebrtcAudioPrivateFunction::CalculateHMAC, this, raw_id));
-    return;
-  }
-
-  std::string hmac = CalculateHMACImpl(raw_id);
-  BrowserThread::PostTask(
-      BrowserThread::UI,
-      FROM_HERE,
-      base::Bind(&WebrtcAudioPrivateFunction::OnHMACCalculated, this, hmac));
-}
-
-void WebrtcAudioPrivateFunction::OnHMACCalculated(const std::string& hmac) {
-  NOTREACHED();
-}
-
-std::string WebrtcAudioPrivateFunction::CalculateHMACImpl(
+std::string WebrtcAudioPrivateFunction::CalculateHMAC(
     const std::string& raw_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   // We don't hash the default device description, and we always return
   // "default" for the default device. There is code in SetActiveSink
   // that transforms "default" to the empty string, and code in
   // GetActiveSink that ensures we return "default" if we get the
   // empty string as the current device ID.
   if (media::AudioDeviceDescription::IsDefaultDevice(raw_id))
     return media::AudioDeviceDescription::kDefaultDeviceId;
 
   url::Origin security_origin(source_url().GetOrigin());
   return content::GetHMACForMediaDeviceID(device_id_salt(), security_origin,
                                           raw_id);
 }
 
 void WebrtcAudioPrivateFunction::InitDeviceIDSalt() {
   device_id_salt_ = GetProfile()->GetResourceContext()->GetMediaDeviceIDSalt();
 }
 
 std::string WebrtcAudioPrivateFunction::device_id_salt() const {
   return device_id_salt_;
 }
 
 bool WebrtcAudioPrivateGetSinksFunction::RunAsync() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-
   InitDeviceIDSalt();
-  GetOutputDeviceDescriptions();
-
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(
+          &WebrtcAudioPrivateGetSinksFunction::GetOutputDeviceDescriptions,
+          this));
   return true;
 }
 
+void WebrtcAudioPrivateGetSinksFunction::GetOutputDeviceDescriptions() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  media::AudioSystem::Get()->GetDeviceDescriptions(
+      base::Bind(
+          &WebrtcAudioPrivateGetSinksFunction::OnOutputDeviceDescriptions,
+          this),
+      false);
+}
+
 void WebrtcAudioPrivateGetSinksFunction::OnOutputDeviceDescriptions(
-    std::unique_ptr<AudioDeviceDescriptions> raw_ids) {
+    media::AudioDeviceDescriptions sink_devices) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
-  std::vector<wap::SinkInfo> results;
-  for (const media::AudioDeviceDescription& description : *raw_ids) {
+  std::unique_ptr<SyncInfoVector> results = base::MakeUnique<SyncInfoVector>();

[Devlin, 2017/04/06 01:42:33]

optional: I sometimes find it cleaner to do:
auto results = base::MakeUnique<SyncInfoVector>();

The type can't be anything other than std::unique_ptr<T> using
base::MakeUnique<T>, so auto just saves a bit of reading. :)

Though maybe moot with below.

[o1ka, 2017/04/06 11:01:22]

Makes sense!
Done.

+  for (const media::AudioDeviceDescription& description : sink_devices) {
     wap::SinkInfo info;
-    info.sink_id = CalculateHMACImpl(description.unique_id);
+    info.sink_id = CalculateHMAC(description.unique_id);
     info.sink_label = description.device_name;
     // TODO(joi): Add other parameters.
-    results.push_back(std::move(info));
+    results->push_back(std::move(info));
   }
-
-  // It's safe to directly set the results here (from a thread other
-  // than the UI thread, on which an AsyncExtensionFunction otherwise
-  // normally runs) because there is one instance of this object per
-  // function call, no actor outside of this object is modifying the
-  // results_ member, and the different method invocations on this
-  // object run strictly in sequence; first RunAsync on the UI thread,
-  // then DoQuery on the audio IO thread, then DoneOnUIThread on the
-  // UI thread.
-  results_ = wap::GetSinks::Results::Create(results);

[Devlin, 2017/04/06 01:42:33]

I think this was indeed safe, if you wanted to avoid currying the vector in the
callback.

[o1ka, 2017/04/06 11:01:22]

Yes, it was safe. I just prefer the code which does not require 8-line comment
explaining its safety (wherever it's possible and reasonable) :) Also, such code
is more future-proof.
I don't have a very strong opinion in this case though.
Do you prefer me to revert it?

[Devlin, 2017/04/06 21:23:27]

I don't have a strong preference.  The only reason to prefer this would be
because it lets us allocate the vector<SinkInfo> on the stack instead of the
heap, which would be a bit faster.  But this shouldn't be very
performance-sensitive code (and already does a ton), so this should be fine. 
(And, as you say, not needing an 8 line comment is its own reward. :))

[o1ka, 2017/04/07 12:10:22]

Acknowledged.

-
   BrowserThread::PostTask(
-      BrowserThread::UI,
-      FROM_HERE,
-      base::Bind(&WebrtcAudioPrivateGetSinksFunction::DoneOnUIThread, this));
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&WebrtcAudioPrivateGetSinksFunction::DoneOnUIThread, this,
+                 base::Passed(&results)));
 }
 
-void WebrtcAudioPrivateGetSinksFunction::DoneOnUIThread() {
+void WebrtcAudioPrivateGetSinksFunction::DoneOnUIThread(
+    std::unique_ptr<SyncInfoVector> results) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  results_ = wap::GetSinks::Results::Create(*results);
   SendResponse(true);
 }
 
 WebrtcAudioPrivateGetAssociatedSinkFunction::
-WebrtcAudioPrivateGetAssociatedSinkFunction() {
-}
+    WebrtcAudioPrivateGetAssociatedSinkFunction() {}
 
 WebrtcAudioPrivateGetAssociatedSinkFunction::
-~WebrtcAudioPrivateGetAssociatedSinkFunction() {
-}
+    ~WebrtcAudioPrivateGetAssociatedSinkFunction() {}
 
 bool WebrtcAudioPrivateGetAssociatedSinkFunction::RunAsync() {
   params_ = wap::GetAssociatedSink::Params::Create(*args_);
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   EXTENSION_FUNCTION_VALIDATE(params_.get());
-
   InitDeviceIDSalt();
 
-  AudioManager::Get()->GetTaskRunner()->PostTask(
-      FROM_HERE,
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
       base::Bind(&WebrtcAudioPrivateGetAssociatedSinkFunction::
-                 GetDevicesOnDeviceThread, this));
+                     GetInputDeviceDescriptions,
+                 this));
 
   return true;
 }
 
-void WebrtcAudioPrivateGetAssociatedSinkFunction::GetDevicesOnDeviceThread() {
-  DCHECK(AudioManager::Get()->GetTaskRunner()->BelongsToCurrentThread());
-  AudioManager::Get()->GetAudioInputDeviceDescriptions(&source_devices_);
-
-  BrowserThread::PostTask(
-      BrowserThread::IO,
-      FROM_HERE,
+void WebrtcAudioPrivateGetAssociatedSinkFunction::GetInputDeviceDescriptions() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  media::AudioSystem::Get()->GetDeviceDescriptions(
       base::Bind(&WebrtcAudioPrivateGetAssociatedSinkFunction::
-                 GetRawSourceIDOnIOThread,
-                 this));
+                     OnInputDeviceDescriptions,
+                 this),
+      true);
 }
 
-void
-WebrtcAudioPrivateGetAssociatedSinkFunction::GetRawSourceIDOnIOThread() {
+void WebrtcAudioPrivateGetAssociatedSinkFunction::OnInputDeviceDescriptions(
+    media::AudioDeviceDescriptions source_devices) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
   url::Origin security_origin(GURL(params_->security_origin));
   std::string source_id_in_origin(params_->source_id_in_origin);
 
   // Find the raw source ID for source_id_in_origin.
   std::string raw_source_id;
-  for (AudioDeviceDescriptions::const_iterator it = source_devices_.begin();
-       it != source_devices_.end(); ++it) {
-    const std::string& id = it->unique_id;
+  for (const auto device : source_devices) {

[Devlin, 2017/04/06 01:42:33]

const auto&

[o1ka, 2017/04/06 11:01:22]

Done.

     if (content::DoesMediaDeviceIDMatchHMAC(device_id_salt(), security_origin,
-                                            source_id_in_origin, id)) {
-      raw_source_id = id;
+                                            source_id_in_origin,
+                                            device.unique_id)) {
+      raw_source_id = device.unique_id;
       DVLOG(2) << "Found raw ID " << raw_source_id
                << " for source ID in origin " << source_id_in_origin;
       break;
     }
   }
-
-  AudioManager::Get()->GetTaskRunner()->PostTask(
-      FROM_HERE,
-      base::Bind(&WebrtcAudioPrivateGetAssociatedSinkFunction::
-                 GetAssociatedSinkOnDeviceThread,
-                 this,
-                 raw_source_id));
+  if (raw_source_id.empty()) {
+    CalculateHMACAndReplyOnUIThread(std::string());
+    return;
+  }
+  media::AudioSystem::Get()->GetAssociatedOutputDeviceID(
+      raw_source_id, base::Bind(&WebrtcAudioPrivateGetAssociatedSinkFunction::
+                                    CalculateHMACAndReplyOnUIThread,
+                                this));
 }
 
-void
-WebrtcAudioPrivateGetAssociatedSinkFunction::GetAssociatedSinkOnDeviceThread(
-    const std::string& raw_source_id) {
-  DCHECK(AudioManager::Get()->GetTaskRunner()->BelongsToCurrentThread());
-
-  // We return an empty string if there is no associated output device.
-  std::string raw_sink_id;
-  if (!raw_source_id.empty()) {
-    raw_sink_id =
-        AudioManager::Get()->GetAssociatedOutputDeviceID(raw_source_id);
-  }
-
-  CalculateHMAC(raw_sink_id);
+void WebrtcAudioPrivateGetAssociatedSinkFunction::
+    CalculateHMACAndReplyOnUIThread(const std::string& raw_sink_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&WebrtcAudioPrivateGetAssociatedSinkFunction::OnHMACCalculated,
+                 this, CalculateHMAC(raw_sink_id)));
 }
 
 void WebrtcAudioPrivateGetAssociatedSinkFunction::OnHMACCalculated(
     const std::string& associated_sink_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-
   if (associated_sink_id == media::AudioDeviceDescription::kDefaultDeviceId) {
     DVLOG(2) << "Got default ID, replacing with empty ID.";
     results_ = wap::GetAssociatedSink::Results::Create("");
   } else {
     results_ = wap::GetAssociatedSink::Results::Create(associated_sink_id);
   }
-
   SendResponse(true);
 }
 
 }  // namespace extensions