Switching AudioInputDeviceManager from using AudioManager interface to AudioSystem one.

content/browser/renderer_host/media/audio_input_device_manager.cc

Index: content/browser/renderer_host/media/audio_input_device_manager.cc
diff --git a/content/browser/renderer_host/media/audio_input_device_manager.cc b/content/browser/renderer_host/media/audio_input_device_manager.cc
index 4ad35755240e59932c8d60e8e8998c3eca558254..001d65c29d9eef47e6ed352b0e75dad4860dea4d 100644
--- a/content/browser/renderer_host/media/audio_input_device_manager.cc
+++ b/content/browser/renderer_host/media/audio_input_device_manager.cc
@@ -13,7 +13,7 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/common/media_stream_request.h"
 #include "media/audio/audio_input_ipc.h"
-#include "media/audio/audio_manager_base.h"
+#include "media/audio/audio_system.h"
 #include "media/base/audio_parameters.h"
 #include "media/base/channel_layout.h"
 #include "media/base/media_switches.h"
@@ -32,13 +32,12 @@ const int kFirstSessionId = AudioInputDeviceManager::kFakeOpenSessionId + 1;
 }
 
 AudioInputDeviceManager::AudioInputDeviceManager(
-    media::AudioManager* audio_manager)
+    media::AudioSystem* audio_system)
     : next_capture_session_id_(kFirstSessionId),
 #if defined(OS_CHROMEOS)
       keyboard_mic_streams_count_(0),
 #endif
-      audio_manager_(audio_manager),
-      device_task_runner_(audio_manager_->GetTaskRunner()) {
+      audio_system_(audio_system) {
 }
 
 AudioInputDeviceManager::~AudioInputDeviceManager() {
@@ -58,7 +57,6 @@ void AudioInputDeviceManager::RegisterListener(
     MediaStreamProviderListener* listener) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(listener);
-  DCHECK(device_task_runner_);
   listeners_.AddObserver(listener);
 }
 
@@ -73,10 +71,31 @@ int AudioInputDeviceManager::Open(const MediaStreamDevice& device) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   // Generate a new id for this device.
   int session_id = next_capture_session_id_++;
-  device_task_runner_->PostTask(
-      FROM_HERE,
-      base::Bind(&AudioInputDeviceManager::OpenOnDeviceThread,
-                 this, session_id, device));
+
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kUseFakeDeviceForMediaStream)) {
+    audio_system_->GetAssociatedOutputDeviceID(
+        device.id,
+        base::Bind(&AudioInputDeviceManager::OpenedOnIOThread, this, session_id,
+                   device, base::TimeTicks::Now(),
+                   media::AudioParameters::UnavailableDeviceParams(),
+                   media::AudioParameters::UnavailableDeviceParams()));
+  } else {
+    // TODO(tommi): As is, we hit this code path when device.type is
+    // MEDIA_TAB_AUDIO_CAPTURE and the device id is not a device that
+    // the AudioManager can know about. This currently does not fail because
+    // the implementation of GetInputStreamParameters returns valid parameters
+    // by default for invalid devices. That behavior is problematic because it
+    // causes other parts of the code to attempt to open truly invalid or
+    // missing devices and falling back on alternate devices (and likely fail
+    // twice in a row). Tab audio capture should not pass through here and
+    // GetInputStreamParameters should return invalid parameters for invalid
+    // devices.
+
+    audio_system_->GetInputDeviceInfo(
+        device.id, base::Bind(&AudioInputDeviceManager::OpenedOnIOThread, this,

[o1ka, 2017/03/28 16:40:16]

Since AudioSystem is accessed by AudioInputDeviceManager IO thread only, which
is stopped before the mail loop destruction, we may want to bind
Unretained(this) here.

guidou@ WDYT?

[DaleCurtis, 2017/03/28 18:22:11]

Whatever you decide, note the same situation exists above.

[o1ka, 2017/03/30 15:11:53]

Right - the question is for both.

[Guido Urdaneta, 2017/04/03 13:04:39]

The AIDM is destroyed on the IO thread during IO Message loop destruction
https://cs.chromium.org/chromium/src/content/browser/renderer_host/media/medi...
Using Unretained here should be OK.

[Guido Urdaneta, 2017/04/03 20:13:08]

I mean, would be destroyed there provided no more reference counting is used.

+                              session_id, device, base::TimeTicks::Now()));
+  }
 
   return session_id;
 }
@@ -135,78 +154,34 @@ void AudioInputDeviceManager::UnregisterKeyboardMicStream() {
 }
 #endif
 
-void AudioInputDeviceManager::OpenOnDeviceThread(
+void AudioInputDeviceManager::OpenedOnIOThread(
     int session_id,
-    const MediaStreamDevice& device) {
-  SCOPED_UMA_HISTOGRAM_TIMER(
-      "Media.AudioInputDeviceManager.OpenOnDeviceThreadTime");
-  DCHECK(IsOnDeviceThread());
-
-  StreamDeviceInfo out(device.type, device.name, device.id, 0, 0, 0);
-  out.session_id = session_id;
-
-  MediaStreamDevice::AudioDeviceParameters& input_params = out.device.input;
-
-  // Add preferred output device information if a matching output device
-  // exists.
-  out.device.matched_output_device_id =
-      audio_manager_->GetAssociatedOutputDeviceID(device.id);
-
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kUseFakeDeviceForMediaStream)) {
-    // Don't need to query the hardware information if using fake device.
-    input_params.sample_rate = 44100;
-    input_params.channel_layout = media::CHANNEL_LAYOUT_STEREO;
-    if (!out.device.matched_output_device_id.empty()) {
-      out.device.matched_output.sample_rate = 44100;
-      out.device.matched_output.channel_layout = media::CHANNEL_LAYOUT_STEREO;
-    }
-  } else {
-    // TODO(tommi): As is, we hit this code path when device.type is
-    // MEDIA_TAB_AUDIO_CAPTURE and the device id is not a device that
-    // the AudioManager can know about. This currently does not fail because
-    // the implementation of GetInputStreamParameters returns valid parameters
-    // by default for invalid devices. That behavior is problematic because it
-    // causes other parts of the code to attempt to open truly invalid or
-    // missing devices and falling back on alternate devices (and likely fail
-    // twice in a row). Tab audio capture should not pass through here and
-    // GetInputStreamParameters should return invalid parameters for invalid
-    // devices.
-
-    // Get the preferred sample rate and channel configuration for the
-    // audio device.
-    media::AudioParameters params =
-        audio_manager_->GetInputStreamParameters(device.id);
-    input_params.sample_rate = params.sample_rate();
-    input_params.channel_layout = params.channel_layout();
-    input_params.frames_per_buffer = params.frames_per_buffer();
-    input_params.effects = params.effects();
-    input_params.mic_positions = params.mic_positions();
-    if (!out.device.matched_output_device_id.empty()) {
-      params = audio_manager_->GetOutputStreamParameters(
-          out.device.matched_output_device_id);
-      MediaStreamDevice::AudioDeviceParameters& matched_output_params =
-          out.device.matched_output;
-      matched_output_params.sample_rate = params.sample_rate();
-      matched_output_params.channel_layout = params.channel_layout();
-      matched_output_params.frames_per_buffer = params.frames_per_buffer();
-    }
-  }
-
-  // Return the |session_id| through the listener by posting a task on
-  // IO thread since MediaStreamManager handles the callback asynchronously.
-  BrowserThread::PostTask(BrowserThread::IO,
-                          FROM_HERE,
-                          base::Bind(&AudioInputDeviceManager::OpenedOnIOThread,
-                                     this, session_id, out));
-}
-
-void AudioInputDeviceManager::OpenedOnIOThread(int session_id,
-                                               const StreamDeviceInfo& info) {
+    const MediaStreamDevice& device,
+    base::TimeTicks start_time,
+    const media::AudioParameters& input_params,
+    const media::AudioParameters& matched_output_params,
+    const std::string& matched_output_device_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  DCHECK_EQ(session_id, info.session_id);
   DCHECK(GetDevice(session_id) == devices_.end());
 
+  UMA_HISTOGRAM_TIMES("Media.AudioInputDeviceManager.OpenOnDeviceThreadTime",

[o1ka, 2017/03/28 16:40:16]

Now we record it on IO thread, so IO->Device thread->IO hopping is included into
the time, but it should be insignificant. We want to keep the same metric to be
able to compare with historical data.

+                      base::TimeTicks::Now() - start_time);
+
+  StreamDeviceInfo info(device.type, device.name, device.id);
+  info.session_id = session_id;
+  info.device.input.sample_rate = input_params.sample_rate();
+  info.device.input.channel_layout = input_params.channel_layout();
+  info.device.input.frames_per_buffer = input_params.frames_per_buffer();
+  info.device.input.effects = input_params.effects();
+  info.device.input.mic_positions = input_params.mic_positions();
+  info.device.matched_output_device_id = matched_output_device_id;
+  info.device.matched_output.sample_rate = matched_output_params.sample_rate();
+  info.device.matched_output.channel_layout =
+      matched_output_params.channel_layout();
+  info.device.matched_output.frames_per_buffer =
+      matched_output_params.frames_per_buffer();
+  info.device.matched_output.effects = matched_output_params.effects();
+
   devices_.push_back(info);
 
   for (auto& listener : listeners_)
@@ -220,9 +195,6 @@ void AudioInputDeviceManager::ClosedOnIOThread(MediaStreamType stream_type,
     listener.Closed(stream_type, session_id);
 }
 
-bool AudioInputDeviceManager::IsOnDeviceThread() const {
-  return device_task_runner_->BelongsToCurrentThread();
-}
 
 AudioInputDeviceManager::StreamDeviceList::iterator
 AudioInputDeviceManager::GetDevice(int session_id) {
@@ -243,5 +215,4 @@ void AudioInputDeviceManager::SetKeyboardMicStreamActiveOnUIThread(
 }
 #endif
 
-
 }  // namespace content