Rejigger audio capture pipeline to work with separate main+worker thr…

…eads (Mac).

The audio capture pipeline (specifically: WebContentsAudioInputStream, VirtualAudio[In|Out]putStream, and FakeAudioConsumer) was implemented under the assumption that the thread pumping audio data was the same as the thread invoking Open/Start/Stop/Close.  This assumption was correct until SVN rev 204130 (https://codereview.chromium.org/14273018/), where these tasks were split across two separate threads.

This change restores functionality by utilizing the same design assumptions as the normal, "non-virtual" input/output stream implementations.  Data pumping is assumed to occur by one thread, and the implementation must block the control thread on a call to Stop() and not return from Stop() until the data pumping has ceased.

Augmented VirtualAudioInputStream unit testing to run all tests under the one-thread (non-Mac platforms) versus two-thread (Mac only) scenario.

BUG=249089
TEST=media_unittests, content_unittests, manual confirmation

Review URL: https://chromiumcodereview.appspot.com/17122006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@207105 0039d316-1c4b-4281-b951-d872f2087c98

content/browser/renderer_host/media/audio_input_renderer_host.cc

@@ -268,7 +268,7 @@ void AudioInputRendererHost::OnCreateStream(
   entry->writer.reset(writer.release());
   if (WebContentsCaptureUtil::IsWebContentsDeviceId(device_id)) {
     entry->controller = media::AudioInputController::CreateForStream(
-        audio_manager_->GetWorkerLoop(),
+        audio_manager_->GetMessageLoop(),
         this,
         WebContentsAudioInputStream::Create(
             device_id, audio_params, audio_manager_->GetWorkerLoop()),

content/browser/renderer_host/media/web_contents_audio_input_stream.cc

@@ -10,7 +10,7 @@
 #include "base/bind_helpers.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
-#include "base/message_loop/message_loop_proxy.h"
+#include "base/threading/thread_checker.h"
 #include "content/browser/browser_main_loop.h"
 #include "content/browser/renderer_host/media/audio_mirroring_manager.h"
 #include "content/browser/renderer_host/media/web_contents_capture_util.h"
@@ -27,7 +27,6 @@ class WebContentsAudioInputStream::Impl
  public:
   // Takes ownership of |mixer_stream|.  The rest outlive this instance.
   Impl(int render_process_id, int render_view_id,
-       const scoped_refptr<base::MessageLoopProxy>& message_loop,
        AudioMirroringManager* mirroring_manager,
        const scoped_refptr<WebContentsTracker>& tracker,
        media::VirtualAudioInputStream* mixer_stream);
@@ -86,7 +85,6 @@ class WebContentsAudioInputStream::Impl
   void OnTargetChanged(int render_process_id, int render_view_id);
 
   // Injected dependencies.
-  const scoped_refptr<base::MessageLoopProxy> message_loop_;
   AudioMirroringManager* const mirroring_manager_;
   const scoped_refptr<WebContentsTracker> tracker_;
   // The AudioInputStream implementation that handles the audio conversion and
@@ -102,32 +100,36 @@ class WebContentsAudioInputStream::Impl
   // Current callback used to consume the resulting mixed audio data.
   AudioInputCallback* callback_;
 
+  base::ThreadChecker thread_checker_;
+
   DISALLOW_COPY_AND_ASSIGN(Impl);
 };
 
 WebContentsAudioInputStream::Impl::Impl(
     int render_process_id, int render_view_id,
-    const scoped_refptr<base::MessageLoopProxy>& message_loop,
     AudioMirroringManager* mirroring_manager,
     const scoped_refptr<WebContentsTracker>& tracker,
     media::VirtualAudioInputStream* mixer_stream)
-    : message_loop_(message_loop), mirroring_manager_(mirroring_manager),
+    : mirroring_manager_(mirroring_manager),
       tracker_(tracker), mixer_stream_(mixer_stream), state_(CONSTRUCTED),
       target_render_process_id_(render_process_id),
       target_render_view_id_(render_view_id),
       callback_(NULL) {
-  DCHECK(message_loop_.get());
   DCHECK(mirroring_manager_);
   DCHECK(tracker_.get());
   DCHECK(mixer_stream_.get());
+
+  // WAIS::Impl can be constructed on any thread, but will DCHECK that all
+  // its methods from here on are called from the same thread.
+  thread_checker_.DetachFromThread();
 }
 
 WebContentsAudioInputStream::Impl::~Impl() {
   DCHECK(state_ == CONSTRUCTED || state_ == CLOSED);
 }
 
 bool WebContentsAudioInputStream::Impl::Open() {
-  DCHECK(message_loop_->BelongsToCurrentThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   DCHECK_EQ(CONSTRUCTED, state_) << "Illegal to Open more than once.";
 
@@ -144,7 +146,7 @@ bool WebContentsAudioInputStream::Impl::Open() {
 }
 
 void WebContentsAudioInputStream::Impl::Start(AudioInputCallback* callback) {
-  DCHECK(message_loop_->BelongsToCurrentThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(callback);
 
   if (state_ != OPENED)
@@ -164,7 +166,7 @@ void WebContentsAudioInputStream::Impl::Start(AudioInputCallback* callback) {
 }
 
 void WebContentsAudioInputStream::Impl::Stop() {
-  DCHECK(message_loop_->BelongsToCurrentThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   if (state_ != MIRRORING)
     return;
@@ -179,7 +181,7 @@ void WebContentsAudioInputStream::Impl::Stop() {
 }
 
 void WebContentsAudioInputStream::Impl::Close() {
-  DCHECK(message_loop_->BelongsToCurrentThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   Stop();
 
@@ -194,21 +196,21 @@ void WebContentsAudioInputStream::Impl::Close() {
 }
 
 bool WebContentsAudioInputStream::Impl::IsTargetLost() const {
-  DCHECK(message_loop_->BelongsToCurrentThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   return target_render_process_id_ <= 0 || target_render_view_id_ <= 0;
 }
 
 void WebContentsAudioInputStream::Impl::ReportError() {
-  DCHECK(message_loop_->BelongsToCurrentThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   // TODO(miu): Need clean-up of AudioInputCallback interface in a future
   // change, since its only implementation ignores the first argument entirely
   callback_->OnError(NULL);
 }
 
 void WebContentsAudioInputStream::Impl::StartMirroring() {
-  DCHECK(message_loop_->BelongsToCurrentThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   BrowserThread::PostTask(
       BrowserThread::IO,
@@ -220,7 +222,7 @@ void WebContentsAudioInputStream::Impl::StartMirroring() {
 }
 
 void WebContentsAudioInputStream::Impl::StopMirroring() {
-  DCHECK(message_loop_->BelongsToCurrentThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   BrowserThread::PostTask(
       BrowserThread::IO,
@@ -238,7 +240,6 @@ media::AudioOutputStream* WebContentsAudioInputStream::Impl::AddInput(
   // VirtualAudioOutputStream.
   return new media::VirtualAudioOutputStream(
       params,
-      message_loop_.get(),
       mixer_stream_.get(),
       base::Bind(&Impl::ReleaseInput, this));
 }
@@ -250,7 +251,7 @@ void WebContentsAudioInputStream::Impl::ReleaseInput(
 
 void WebContentsAudioInputStream::Impl::OnTargetChanged(int render_process_id,
                                                         int render_view_id) {
-  DCHECK(message_loop_->BelongsToCurrentThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   if (target_render_process_id_ == render_process_id &&
       target_render_view_id_ == render_view_id) {
@@ -281,7 +282,7 @@ void WebContentsAudioInputStream::Impl::OnTargetChanged(int render_process_id,
 WebContentsAudioInputStream* WebContentsAudioInputStream::Create(
     const std::string& device_id,
     const media::AudioParameters& params,
-    const scoped_refptr<base::MessageLoopProxy>& message_loop) {
+    const scoped_refptr<base::MessageLoopProxy>& worker_loop) {
   int render_process_id;
   int render_view_id;
   if (!WebContentsCaptureUtil::ExtractTabCaptureTarget(
@@ -290,21 +291,20 @@ WebContentsAudioInputStream* WebContentsAudioInputStream::Create(
   }
 
   return new WebContentsAudioInputStream(
-      render_process_id, render_view_id, message_loop,
+      render_process_id, render_view_id,
       BrowserMainLoop::GetAudioMirroringManager(),
       new WebContentsTracker(),
       new media::VirtualAudioInputStream(
-          params, message_loop,
+          params, worker_loop,
           media::VirtualAudioInputStream::AfterCloseCallback()));
 }
 
 WebContentsAudioInputStream::WebContentsAudioInputStream(
     int render_process_id, int render_view_id,
-    const scoped_refptr<base::MessageLoopProxy>& message_loop,
     AudioMirroringManager* mirroring_manager,
     const scoped_refptr<WebContentsTracker>& tracker,
     media::VirtualAudioInputStream* mixer_stream)
-    : impl_(new Impl(render_process_id, render_view_id, message_loop,
+    : impl_(new Impl(render_process_id, render_view_id,
                      mirroring_manager, tracker, mixer_stream)) {}
 
 WebContentsAudioInputStream::~WebContentsAudioInputStream() {}

content/browser/renderer_host/media/web_contents_audio_input_stream.h

@@ -55,10 +55,13 @@ class CONTENT_EXPORT WebContentsAudioInputStream
   // WebContentsCaptureUtil::ExtractTabCaptureTarget().  The caller must
   // guarantee Close() is called on the returned object so that it may
   // self-destruct.
+  // |worker_loop| is the loop on which AudioInputCallback methods are called
+  // and may or may not be the single thread that invokes the AudioInputStream
+  // methods.
   static WebContentsAudioInputStream* Create(
       const std::string& device_id,
       const media::AudioParameters& params,
-      const scoped_refptr<base::MessageLoopProxy>& message_loop);
+      const scoped_refptr<base::MessageLoopProxy>& worker_loop);
 
  private:
   friend class WebContentsAudioInputStreamTest;
@@ -72,7 +75,6 @@ class CONTENT_EXPORT WebContentsAudioInputStream
 
   WebContentsAudioInputStream(
       int render_process_id, int render_view_id,
-      const scoped_refptr<base::MessageLoopProxy>& message_loop,
       AudioMirroringManager* mirroring_manager,
       const scoped_refptr<WebContentsTracker>& tracker,
       media::VirtualAudioInputStream* mixer_stream);

content/browser/renderer_host/media/web_contents_audio_input_stream_unittest.cc

@@ -89,10 +89,10 @@ class MockWebContentsTracker : public WebContentsTracker {
 class MockVirtualAudioInputStream : public VirtualAudioInputStream {
  public:
   explicit MockVirtualAudioInputStream(
-      const scoped_refptr<base::MessageLoopProxy>& message_loop)
-      : VirtualAudioInputStream(TestAudioParameters(), message_loop,
+      const scoped_refptr<base::MessageLoopProxy>& worker_loop)
+      : VirtualAudioInputStream(TestAudioParameters(), worker_loop,
                                 VirtualAudioInputStream::AfterCloseCallback()),
-        real_(TestAudioParameters(), message_loop,
+        real_(TestAudioParameters(), worker_loop,
               base::Bind(&MockVirtualAudioInputStream::OnRealStreamHasClosed,
                          base::Unretained(this))),
         real_stream_is_closed_(false) {
@@ -218,7 +218,7 @@ class WebContentsAudioInputStreamTest : public testing::Test {
 
     wcais_ = new WebContentsAudioInputStream(
         current_render_process_id_, current_render_view_id_,
-        audio_thread_.message_loop_proxy(), mock_mirroring_manager_.get(),
+        mock_mirroring_manager_.get(),
         mock_tracker_, mock_vais_);
     wcais_->Open();
   }