Switching MediaStreamManager from using AudioManager to AudioSystem

MediaDevicesManager and AudioInputDeviceManager will be addressed in separate follow-up CLs, to keep it simpler.

BUG=672468
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2697033006
Cr-Commit-Position: refs/heads/master@{#457807}

content/browser/browser_main_loop.cc

@@ -1536,7 +1536,7 @@ int BrowserMainLoop::BrowserThreadsStarted() {
   {
     TRACE_EVENT0("startup",
       "BrowserMainLoop::BrowserThreadsStarted:InitMediaStreamManager");
-    media_stream_manager_.reset(new MediaStreamManager(audio_manager_.get()));
+    media_stream_manager_.reset(new MediaStreamManager(audio_system_.get()));
   }
 
   {

content/browser/renderer_host/media/audio_input_renderer_host_unittest.cc

@@ -24,6 +24,7 @@
 #include "content/public/test/test_browser_thread.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "media/audio/audio_device_description.h"
+#include "media/audio/audio_system_impl.h"
 #include "media/audio/fake_audio_log_factory.h"
 #include "media/audio/fake_audio_manager.h"
 #include "media/base/media_switches.h"
@@ -266,8 +267,9 @@ class AudioInputRendererHostTest : public testing::Test {
     audio_manager_.reset(new media::FakeAudioManager(
         base::ThreadTaskRunnerHandle::Get(),
         base::ThreadTaskRunnerHandle::Get(), &log_factory_));
+    audio_system_ = media::AudioSystemImpl::Create(audio_manager_.get());
     media_stream_manager_ =
-        base::MakeUnique<MediaStreamManager>(audio_manager_.get());
+        base::MakeUnique<MediaStreamManager>(audio_system_.get());
     airh_ = new AudioInputRendererHostWithInterception(
         kRenderProcessId, kRendererPid, media::AudioManager::Get(),
         media_stream_manager_.get(), AudioMirroringManager::GetInstance(),
@@ -315,6 +317,7 @@ class AudioInputRendererHostTest : public testing::Test {
   std::unique_ptr<MediaStreamManager> media_stream_manager_;
   TestBrowserThreadBundle thread_bundle_;
   media::ScopedAudioManagerPtr audio_manager_;
+  std::unique_ptr<media::AudioSystem> audio_system_;
   StrictMock<MockRenderer> renderer_;
   scoped_refptr<AudioInputRendererHost> airh_;
 

content/browser/renderer_host/media/audio_output_authorization_handler_unittest.cc

@@ -69,7 +69,7 @@ class AudioOutputAuthorizationHandlerTest : public testing::Test {
         &log_factory_));
     audio_system_ = media::AudioSystemImpl::Create(audio_manager_.get());
     media_stream_manager_ =
-        base::MakeUnique<MediaStreamManager>(audio_manager_.get());
+        base::MakeUnique<MediaStreamManager>(audio_system_.get());
     // Make sure everything is done initializing:
     SyncWithAllThreads();
   }

content/browser/renderer_host/media/audio_output_delegate_impl_unittest.cc

@@ -21,6 +21,7 @@
 #include "content/public/browser/media_observer.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "media/audio/audio_output_controller.h"
+#include "media/audio/audio_system_impl.h"
 #include "media/audio/fake_audio_log_factory.h"
 #include "media/audio/fake_audio_manager.h"
 #include "media/base/media_switches.h"
@@ -115,8 +116,9 @@ class AudioOutputDelegateTest : public testing::Test {
     audio_manager_.reset(new media::FakeAudioManager(
         audio_thread_->task_runner(), audio_thread_->worker_task_runner(),
         &log_factory_));
+    audio_system_ = media::AudioSystemImpl::Create(audio_manager_.get());
     media_stream_manager_ =
-        base::MakeUnique<MediaStreamManager>(audio_manager_.get());
+        base::MakeUnique<MediaStreamManager>(audio_system_.get());
   }
 
   // Test bodies are here, so that we can run them on the IO thread.
@@ -450,6 +452,7 @@ class AudioOutputDelegateTest : public testing::Test {
   std::unique_ptr<TestBrowserThreadBundle> thread_bundle_;
   std::unique_ptr<AudioManagerThread> audio_thread_;
   media::ScopedAudioManagerPtr audio_manager_;
+  std::unique_ptr<media::AudioSystem> audio_system_;
   StrictMock<MockAudioMirroringManager> mirroring_manager_;
   StrictMock<MockEventHandler> event_handler_;
   StrictMock<MockObserver> media_observer_;

content/browser/renderer_host/media/audio_renderer_host_unittest.cc

@@ -242,7 +242,8 @@ class AudioRendererHostTest : public testing::Test {
         render_process_host_(&browser_context_, &auth_run_loop_) {
     base::CommandLine::ForCurrentProcess()->AppendSwitch(
         switches::kUseFakeDeviceForMediaStream);
-    media_stream_manager_.reset(new MediaStreamManager(audio_manager_.get()));
+    media_stream_manager_ =
+        base::MakeUnique<MediaStreamManager>(audio_system_.get());
     host_ = new MockAudioRendererHost(
         &auth_run_loop_, render_process_host_.GetID(), audio_manager_.get(),
         audio_system_.get(), &mirroring_manager_, media_stream_manager_.get(),

content/browser/renderer_host/media/media_devices_dispatcher_host_unittest.cc

@@ -25,6 +25,7 @@
 #include "content/public/test/test_browser_context.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "media/audio/audio_device_description.h"
+#include "media/audio/audio_system_impl.h"
 #include "media/audio/mock_audio_manager.h"
 #include "media/base/media_switches.h"
 #include "media/capture/video/fake_video_capture_device_factory.h"
@@ -81,7 +82,9 @@ class MediaDevicesDispatcherHostTest : public testing::Test {
                            kNumFakeVideoDevices, kDefaultVideoDeviceID));
     audio_manager_.reset(
         new media::MockAudioManager(base::ThreadTaskRunnerHandle::Get()));
-    media_stream_manager_.reset(new MediaStreamManager(audio_manager_.get()));
+    audio_system_ = media::AudioSystemImpl::Create(audio_manager_.get());
+    media_stream_manager_ =
+        base::MakeUnique<MediaStreamManager>(audio_system_.get());
 
     MockResourceContext* mock_resource_context =
         static_cast<MockResourceContext*>(
@@ -286,6 +289,7 @@ class MediaDevicesDispatcherHostTest : public testing::Test {
 
   std::unique_ptr<media::AudioManager, media::AudioManagerDeleter>
       audio_manager_;
+  std::unique_ptr<media::AudioSystem> audio_system_;
   content::TestBrowserContext browser_context_;
   MediaDeviceEnumeration physical_devices_;
   url::Origin origin_;

content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc

@@ -35,6 +35,7 @@
 #include "content/test/test_content_client.h"
 #include "ipc/ipc_message_macros.h"
 #include "media/audio/audio_device_description.h"
+#include "media/audio/audio_system_impl.h"
 #include "media/audio/mock_audio_manager.h"
 #include "media/base/media_switches.h"
 #include "media/capture/video/fake_video_capture_device_factory.h"
@@ -235,11 +236,13 @@ class MediaStreamDispatcherHostTest : public testing::Test {
         origin_(GURL("https://test.com")) {
     audio_manager_.reset(
         new media::MockAudioManager(base::ThreadTaskRunnerHandle::Get()));
+    audio_system_ = media::AudioSystemImpl::Create(audio_manager_.get());
     // Make sure we use fake devices to avoid long delays.
     base::CommandLine::ForCurrentProcess()->AppendSwitch(
         switches::kUseFakeDeviceForMediaStream);
     // Create our own MediaStreamManager.
-    media_stream_manager_.reset(new MediaStreamManager(audio_manager_.get()));
+    media_stream_manager_ =
+        base::MakeUnique<MediaStreamManager>(audio_system_.get());
     video_capture_device_factory_ =
         static_cast<media::FakeVideoCaptureDeviceFactory*>(
             media_stream_manager_->video_capture_manager()
@@ -426,6 +429,7 @@ class MediaStreamDispatcherHostTest : public testing::Test {
   content::TestBrowserThreadBundle thread_bundle_;
   std::unique_ptr<media::AudioManager, media::AudioManagerDeleter>
       audio_manager_;
+  std::unique_ptr<media::AudioSystem> audio_system_;
   MockMediaStreamUIProxy* stream_ui_;
   ContentBrowserClient* old_browser_client_;
   std::unique_ptr<ContentClient> content_client_;

content/browser/renderer_host/media/media_stream_manager.cc

@@ -45,7 +45,7 @@
 #include "content/public/common/media_stream_request.h"
 #include "crypto/hmac.h"
 #include "media/audio/audio_device_description.h"
-#include "media/audio/audio_manager.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"
@@ -392,14 +392,14 @@ void MediaStreamManager::SendMessageToNativeLog(const std::string& message) {
   msm->AddLogMessageOnIOThread(message);
 }
 
-MediaStreamManager::MediaStreamManager(media::AudioManager* audio_manager)
-    : audio_manager_(audio_manager),
+MediaStreamManager::MediaStreamManager(media::AudioSystem* audio_system)
+    : audio_system_(audio_system),
 #if defined(OS_WIN)
       video_capture_thread_("VideoCaptureThread"),
 #endif
       use_fake_ui_(base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kUseFakeUIForMediaStream)) {
-  DCHECK(audio_manager_);
+  DCHECK(audio_system_);
 
   // Some unit tests create the MSM in the IO thread and assumes the
   // initialization is done synchronously.
@@ -423,7 +423,6 @@ MediaStreamManager::~MediaStreamManager() {
   DCHECK(!BrowserThread::IsThreadInitialized(BrowserThread::IO));
   DVLOG(1) << "~MediaStreamManager";
   DCHECK(requests_.empty());
-  DCHECK(!device_task_runner_.get());
 
   base::PowerMonitor* power_monitor = base::PowerMonitor::Get();
   // The PowerMonitor instance owned by BrowserMainLoops always outlives the
@@ -909,14 +908,11 @@ void MediaStreamManager::ReadOutputParamsAndPostRequestToUI(
   // TODO(guidou): MEDIA_TAB_AUDIO_CAPTURE should not be a special case. See
   // crbug.com/584287.
   if (request->audio_type() == MEDIA_TAB_AUDIO_CAPTURE) {
-    // Read output parameters on the correct thread for native audio OS calls.
-    // Using base::Unretained is safe since |audio_manager_| is deleted after
-    // its task runner, and MediaStreamManager is deleted on the UI thread,
-    // after the IO thread has been stopped.
-    base::PostTaskAndReplyWithResult(
-        audio_manager_->GetTaskRunner(), FROM_HERE,
-        base::Bind(&media::AudioManager::GetDefaultOutputStreamParameters,
-                   base::Unretained(audio_manager_)),
+    // Using base::Unretained is safe: |audio_system_| will post
+    // PostRequestToUI() to IO thread, and MediaStreamManager is deleted on the
+    // UI thread, after the IO thread has been stopped.
+    audio_system_->GetOutputStreamParameters(
+        media::AudioDeviceDescription::kDefaultDeviceId,
         base::Bind(&MediaStreamManager::PostRequestToUI, base::Unretained(this),
                    label, request, enumeration));
   } else {
@@ -1233,15 +1229,14 @@ void MediaStreamManager::InitializeDeviceManagersOnIOThread() {
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "457525 MediaStreamManager::InitializeDeviceManagersOnIOThread 1"));
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  DCHECK(!device_task_runner_.get());
-  device_task_runner_ = audio_manager_->GetTaskRunner();
 
   // TODO(dalecurtis): Remove ScopedTracker below once crbug.com/457525 is
   // fixed.
   tracked_objects::ScopedTracker tracking_profile2(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "457525 MediaStreamManager::InitializeDeviceManagersOnIOThread 2"));
-  audio_input_device_manager_ = new AudioInputDeviceManager(audio_manager_);
+  audio_input_device_manager_ =
+      new AudioInputDeviceManager(audio_system_->GetAudioManager());
   audio_input_device_manager_->RegisterListener(this);
 
   // TODO(dalecurtis): Remove ScopedTracker below once crbug.com/457525 is
@@ -1271,13 +1266,13 @@ void MediaStreamManager::InitializeDeviceManagersOnIOThread() {
   video_capture_manager_ = new VideoCaptureManager(
       media::VideoCaptureDeviceFactory::CreateFactory(
           BrowserThread::GetTaskRunnerForThread(BrowserThread::UI)),
-      device_task_runner_);
+      audio_system_->GetTaskRunner());
 #endif
 
   video_capture_manager_->RegisterListener(this);
 
-  media_devices_manager_.reset(
-      new MediaDevicesManager(audio_manager_, video_capture_manager_, this));
+  media_devices_manager_.reset(new MediaDevicesManager(
+      audio_system_->GetAudioManager(), video_capture_manager_, this));
 }
 
 void MediaStreamManager::Opened(MediaStreamType stream_type,
@@ -1560,7 +1555,6 @@ void MediaStreamManager::WillDestroyCurrentMessageLoop() {
   if (audio_input_device_manager_)
     audio_input_device_manager_->UnregisterListener(this);
 
-  device_task_runner_ = nullptr;
   audio_input_device_manager_ = nullptr;
   video_capture_manager_ = nullptr;
   media_devices_manager_ = nullptr;

content/browser/renderer_host/media/media_stream_manager.h

@@ -48,7 +48,7 @@
 #include "media/base/video_facing.h"
 
 namespace media {
-class AudioManager;
+class AudioSystem;
 }
 
 namespace url {
@@ -85,7 +85,7 @@ class CONTENT_EXPORT MediaStreamManager
   // logging from webrtcLoggingPrivate API. Safe to call from any thread.
   static void SendMessageToNativeLog(const std::string& message);
 
-  explicit MediaStreamManager(media::AudioManager* audio_manager);
+  explicit MediaStreamManager(media::AudioSystem* audio_system);
 
   ~MediaStreamManager() override;
 
@@ -403,13 +403,7 @@ class CONTENT_EXPORT MediaStreamManager
       MediaStreamType stream_type,
       const MediaDeviceInfoArray& device_infos);
 
-  // Task runner shared by VideoCaptureManager and AudioInputDeviceManager and
-  // used for enumerating audio output devices.
-  // Note: Enumeration tasks may take seconds to complete so must never be run
-  // on any of the BrowserThreads (UI, IO, etc).  See http://crbug.com/256945.
-  scoped_refptr<base::SingleThreadTaskRunner> device_task_runner_;
-
-  media::AudioManager* const audio_manager_;  // not owned
+  media::AudioSystem* const audio_system_;  // not owned
   scoped_refptr<AudioInputDeviceManager> audio_input_device_manager_;
   scoped_refptr<VideoCaptureManager> video_capture_manager_;
 #if defined(OS_WIN)

content/browser/renderer_host/media/media_stream_manager_unittest.cc

@@ -23,6 +23,7 @@
 #include "content/public/common/content_switches.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "media/audio/audio_device_description.h"
+#include "media/audio/audio_system_impl.h"
 #include "media/audio/fake_audio_log_factory.h"
 #include "media/base/media_switches.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -166,7 +167,9 @@ class MediaStreamManagerTest : public ::testing::Test {
   MediaStreamManagerTest()
       : thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP) {
     audio_manager_.reset(new MockAudioManager());
-    media_stream_manager_.reset(new MediaStreamManager(audio_manager_.get()));
+    audio_system_ = media::AudioSystemImpl::Create(audio_manager_.get());
+    media_stream_manager_ =
+        base::MakeUnique<MediaStreamManager>(audio_system_.get());
     base::RunLoop().RunUntilIdle();
   }
 
@@ -202,6 +205,7 @@ class MediaStreamManagerTest : public ::testing::Test {
   std::unique_ptr<MediaStreamManager> media_stream_manager_;
   content::TestBrowserThreadBundle thread_bundle_;
   std::unique_ptr<MockAudioManager, media::AudioManagerDeleter> audio_manager_;
+  std::unique_ptr<media::AudioSystem> audio_system_;
   base::RunLoop run_loop_;
 
  private:

content/browser/renderer_host/media/video_capture_unittest.cc

@@ -28,6 +28,7 @@
 #include "content/public/test/test_browser_context.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "content/test/test_content_browser_client.h"
+#include "media/audio/audio_system_impl.h"
 #include "media/audio/mock_audio_manager.h"
 #include "media/base/media_switches.h"
 #include "media/capture/video_capture_types.h"
@@ -116,6 +117,7 @@ class VideoCaptureTest : public testing::Test,
       : thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP),
         audio_manager_(
             new media::MockAudioManager(base::ThreadTaskRunnerHandle::Get())),
+        audio_system_(media::AudioSystemImpl::Create(audio_manager_.get())),
         task_runner_(base::ThreadTaskRunnerHandle::Get()),
         opened_session_id_(kInvalidMediaCaptureSessionId),
         observer_binding_(this) {}
@@ -127,7 +129,8 @@ class VideoCaptureTest : public testing::Test,
         switches::kUseFakeDeviceForMediaStream);
     base::CommandLine::ForCurrentProcess()->AppendSwitch(
         switches::kUseFakeUIForMediaStream);
-    media_stream_manager_.reset(new MediaStreamManager(audio_manager_.get()));
+    media_stream_manager_ =
+        base::MakeUnique<MediaStreamManager>(audio_system_.get());
 
     // Create a Host and connect it to a simulated IPC channel.
     host_.reset(new VideoCaptureHost(media_stream_manager_.get()));
@@ -311,6 +314,7 @@ class VideoCaptureTest : public testing::Test,
   // |audio_manager_| needs to outlive |thread_bundle_| because it uses the
   // underlying message loop.
   media::ScopedAudioManagerPtr audio_manager_;
+  std::unique_ptr<media::AudioSystem> audio_system_;
   content::TestBrowserContext browser_context_;
   content::TestContentBrowserClient browser_client_;
   const scoped_refptr<base::SingleThreadTaskRunner> task_runner_;

media/audio/audio_system.h

@@ -9,6 +9,10 @@
 #include "media/base/audio_parameters.h"
 #include "media/base/media_export.h"
 
+namespace base {
+class SingleThreadTaskRunner;
+}
+
 namespace media {
 class AudioManager;
 
@@ -17,7 +21,9 @@ class AudioManager;
 // to Mojo audio service.
 class MEDIA_EXPORT AudioSystem {
  public:
-  // Replies are asynchronously sent to the thread the call is issued on.
+  // Replies are asynchronously sent from audio system thread to the thread the
+  // call is issued on. Attention! Since audio system thread may outlive all the
+  // others, callbacks must always be bound to weak pointers!
   using OnAudioParamsCallback = base::Callback<void(const AudioParameters&)>;
   using OnBoolCallback = base::Callback<void(bool)>;
 
@@ -47,6 +53,8 @@ class MEDIA_EXPORT AudioSystem {
 
   virtual void HasInputDevices(OnBoolCallback on_has_devices_cb) const = 0;
 
+  virtual base::SingleThreadTaskRunner* GetTaskRunner() const = 0;
+
   // Must not be used for anything but stream creation.
   virtual AudioManager* GetAudioManager() const = 0;
 

media/audio/audio_system_impl.h

@@ -31,14 +31,14 @@ class MEDIA_EXPORT AudioSystemImpl : public AudioSystem {
 
   void HasInputDevices(OnBoolCallback on_has_devices_cb) const override;
 
+  base::SingleThreadTaskRunner* GetTaskRunner() const override;
+
   AudioManager* GetAudioManager() const override;
 
  protected:
   AudioSystemImpl(AudioManager* audio_manager);
 
  private:
-  base::SingleThreadTaskRunner* GetTaskRunner() const;
-
   AudioManager* const audio_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(AudioSystemImpl);