Remove task_runner from audio_input_device, as the ipc object is threadsafe now

Bug: chromium:672469
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I1e24e99df4a6d9dc0480aeb460115bafdceeafd5
Reviewed-on: https://chromium-review.googlesource.com/1018475
Commit-Queue: Jonas Olsson <jonasolsson@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Max Morin <maxmorin@chromium.org>
Reviewed-by: Olga Sharonova <olka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553499}

base/threading/thread_restrictions.h

@@ -74,6 +74,7 @@ namespace leveldb {
 class LevelDBMojoProxy;
 }
 namespace media {
+class AudioInputDevice;
 class BlockingUrlProtocol;
 }
 namespace midi {
@@ -217,6 +218,7 @@ class BASE_EXPORT ScopedAllowBlocking {
   friend class content::BrowserProcessSubThread;
   friend class cronet::CronetPrefsManager;
   friend class cronet::CronetURLRequestContext;
+  friend class media::AudioInputDevice;
   friend class mojo::CoreLibraryInitializer;
   friend class resource_coordinator::TabManagerDelegate;  // crbug.com/778703
   friend class ui::MaterialDesignController;

content/renderer/media/audio_device_factory.cc

@@ -165,8 +165,7 @@ AudioDeviceFactory::NewAudioCapturerSource(int render_frame_id) {
   }
 
   return base::MakeRefCounted<media::AudioInputDevice>(
-      AudioInputIPCFactory::get()->CreateAudioInputIPC(render_frame_id),
-      AudioInputIPCFactory::get()->io_task_runner());
+      AudioInputIPCFactory::get()->CreateAudioInputIPC(render_frame_id));
 }
 
 // static

media/audio/audio_input_device.h

@@ -27,25 +27,17 @@
 //
 // State sequences:
 //
-// Start -> InitializeOnIOThread -> CreateStream ->
+// Start -> CreateStream ->
 //       <- OnStreamCreated <-
-//       -> StartOnIOThread -> PlayStream ->
-//
+//       -> RecordStream ->
 //
 // AudioInputDevice::Capture => low latency audio transport on audio thread =>
-//                               |
-// Stop --> ShutDownOnIOThread ------>  CloseStream -> Close
 //
-// This class depends on two threads to function:
+// Stop ->  CloseStream -> Close
 //
-// 1. An IO thread.
-//    This thread is used to asynchronously process Start/Stop etc operations
-//    that are available via the public interface.  The public methods are
-//    asynchronous and simply post a task to the IO thread to actually perform
-//    the work.
-// 2. Audio transport thread.
-//    Responsible for calling the CaptureCallback and feed audio samples from
-//    the server side audio layer using a socket and shared memory.
+// This class depends on the audio transport thread. That thread is responsible
+// for calling the CaptureCallback and feeding it audio samples from the server
+// side audio layer using a socket and shared memory.
 //
 // Implementation notes:
 // - The user must call Stop() before deleting the class instance.
@@ -59,32 +51,22 @@
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "base/memory/shared_memory.h"
+#include "base/sequence_checker.h"
 #include "base/time/time.h"
-#include "base/timer/timer.h"
 #include "media/audio/alive_checker.h"
 #include "media/audio/audio_device_thread.h"
 #include "media/audio/audio_input_ipc.h"
-#include "media/audio/scoped_task_runner_observer.h"
 #include "media/base/audio_capturer_source.h"
 #include "media/base/audio_parameters.h"
 #include "media/base/media_export.h"
 
 namespace media {
 
-// TODO(henrika): This class is based on the AudioOutputDevice class and it has
-// many components in common. Investigate potential for re-factoring.
-// See http://crbug.com/179597.
-// TODO(henrika): Add support for event handling (e.g. OnStateChanged,
-// OnCaptureStopped etc.) and ensure that we can deliver these notifications
-// to any clients using this class.
 class MEDIA_EXPORT AudioInputDevice : public AudioCapturerSource,
-                                      public AudioInputIPCDelegate,
-                                      public ScopedTaskRunnerObserver {
+                                      public AudioInputIPCDelegate {
  public:
   // NOTE: Clients must call Initialize() before using.
-  AudioInputDevice(
-      std::unique_ptr<AudioInputIPC> ipc,
-      const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner);
+  AudioInputDevice(std::unique_ptr<AudioInputIPC> ipc);
 
   // AudioCapturerSource implementation.
   void Initialize(const AudioParameters& params,
@@ -111,7 +93,6 @@ class MEDIA_EXPORT AudioInputDevice : public AudioCapturerSource,
 
   ~AudioInputDevice() override;
 
-  // Methods called on IO thread ----------------------------------------------
   // AudioInputIPCDelegate implementation.
   void OnStreamCreated(base::SharedMemoryHandle handle,
                        base::SyncSocket::Handle socket_handle,
@@ -120,24 +101,8 @@ class MEDIA_EXPORT AudioInputDevice : public AudioCapturerSource,
   void OnMuted(bool is_muted) override;
   void OnIPCClosed() override;
 
-  // Methods called on IO thread ----------------------------------------------
-  // The following methods are tasks posted on the IO thread that needs to
-  // be executed on that thread.
-  void InitializeOnIOThread(const AudioParameters& params,
-                            CaptureCallback* callback,
-                            int session_id);
-  void StartUpOnIOThread();
-  void ShutDownOnIOThread();
-  void SetVolumeOnIOThread(double volume);
-  void SetAutomaticGainControlOnIOThread(bool enabled);
-
-  // base::MessageLoopCurrent::DestructionObserver implementation for the IO
-  // loop. If the IO loop dies before we do, we shut down the audio thread from
-  // here.
-  void WillDestroyCurrentMessageLoop() override;
-
   // This is called by |alive_checker_| if it detects that the input stream is
-  // dead. Called on the IO thread.
+  // dead.
   void DetectedDeadInputStream();
 
   AudioParameters audio_parameters_;
@@ -145,46 +110,30 @@ class MEDIA_EXPORT AudioInputDevice : public AudioCapturerSource,
   CaptureCallback* callback_;
 
   // A pointer to the IPC layer that takes care of sending requests over to
-  // the stream implementation.  Only valid when state_ != IPC_CLOSED and must
-  // only be accessed on the IO thread.
+  // the stream implementation.  Only valid when state_ != IPC_CLOSED.
   std::unique_ptr<AudioInputIPC> ipc_;
 
-  // Current state (must only be accessed from the IO thread).  See comments for
-  // State enum above.
+  // Current state. See comments for State enum above.
   State state_;
 
-  // For UMA stats. May only be accessed on the IO thread.
+  // For UMA stats.
   bool had_callback_error_ = false;
 
   // The media session ID used to identify which input device to be started.
-  // Only modified in Initialize() and ShutDownOnIOThread().
+  // Only modified in Initialize().
   int session_id_;
 
   // Stores the Automatic Gain Control state. Default is false.
-  // Only modified on the IO thread.
   bool agc_is_enabled_;
 
-  // In order to avoid a race between OnStreamCreated and Stop(), we use this
-  // guard to control stopping and starting the audio thread.
-  base::Lock audio_thread_lock_;
-
   // Checks regularly that the input stream is alive and notifies us if it
-  // isn't by calling DetectedDeadInputStream(). Created and deleted on the IO
-  // thread. Must outlive |audio_callback_|.
+  // isn't by calling DetectedDeadInputStream(). Must outlive |audio_callback_|.
   std::unique_ptr<AliveChecker> alive_checker_;
 
-  // Created and deleted on the IO thread, with the exception of in Stop(),
-  // where |audio_thread_| is reset (see comment on |audio_thread_lock_| above).
   std::unique_ptr<AudioInputDevice::AudioThreadCallback> audio_callback_;
   std::unique_ptr<AudioDeviceThread> audio_thread_;
 
-  // Temporary hack to ignore OnStreamCreated() due to the user calling Stop()
-  // so we don't start the audio thread pointing to a potentially freed
-  // |callback_|.
-  //
-  // TODO(miu): Replace this by changing AudioCapturerSource to accept the
-  // callback via Start(). See http://crbug.com/151051 for details.
-  bool stopping_hack_;
+  SEQUENCE_CHECKER(sequence_checker_);
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(AudioInputDevice);
 };

media/audio/audio_input_device_unittest.cc

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "media/audio/audio_input_device.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/shared_memory.h"
 #include "base/message_loop/message_loop.h"
@@ -9,7 +10,7 @@
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
 #include "base/sync_socket.h"
-#include "media/audio/audio_input_device.h"
+#include "base/test/scoped_task_environment.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gmock_mutant.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -59,12 +60,6 @@ class MockCaptureCallback : public AudioCapturerSource::CaptureCallback {
   MOCK_METHOD1(OnCaptureMuted, void(bool is_muted));
 };
 
-// Used to terminate a loop from a different thread than the loop belongs to.
-// |task_runner| should be a SingleThreadTaskRunner.
-ACTION_P(QuitLoop, task_runner) {
-  task_runner->PostTask(FROM_HERE, base::MessageLoop::QuitWhenIdleClosure());
-}
-
 }  // namespace.
 
 // Regular construction.
@@ -72,7 +67,7 @@ TEST(AudioInputDeviceTest, Noop) {
   base::MessageLoopForIO io_loop;
   MockAudioInputIPC* input_ipc = new MockAudioInputIPC();
   scoped_refptr<AudioInputDevice> device(
-      new AudioInputDevice(base::WrapUnique(input_ipc), io_loop.task_runner()));
+      new AudioInputDevice(base::WrapUnique(input_ipc)));
 }
 
 ACTION_P(ReportStateChange, device) {
@@ -84,20 +79,16 @@ TEST(AudioInputDeviceTest, FailToCreateStream) {
   AudioParameters params(AudioParameters::AUDIO_PCM_LOW_LATENCY,
                          CHANNEL_LAYOUT_STEREO, 48000, 16, 480);
 
-  base::MessageLoopForIO io_loop;
   MockCaptureCallback callback;
   MockAudioInputIPC* input_ipc = new MockAudioInputIPC();
   scoped_refptr<AudioInputDevice> device(
-      new AudioInputDevice(base::WrapUnique(input_ipc), io_loop.task_runner()));
+      new AudioInputDevice(base::WrapUnique(input_ipc)));
   device->Initialize(params, &callback, 1);
-  device->Start();
   EXPECT_CALL(*input_ipc, CreateStream(_, _, _, _, _))
       .WillOnce(ReportStateChange(device.get()));
-  EXPECT_CALL(callback, OnCaptureError(_))
-      .WillOnce(QuitLoop(io_loop.task_runner()));
-  base::RunLoop().Run();
+  EXPECT_CALL(callback, OnCaptureError(_));
+  device->Start();
   device->Stop();
-  base::RunLoop().RunUntilIdle();
 }
 
 ACTION_P3(ReportOnStreamCreated, device, handle, socket) {
@@ -127,24 +118,23 @@ TEST(AudioInputDeviceTest, CreateStream) {
       shared_memory.handle().Duplicate();
   ASSERT_TRUE(duplicated_memory_handle.IsValid());
 
-  base::MessageLoopForIO io_loop;
+  base::test::ScopedTaskEnvironment ste;
   MockCaptureCallback callback;
   MockAudioInputIPC* input_ipc = new MockAudioInputIPC();
   scoped_refptr<AudioInputDevice> device(
-      new AudioInputDevice(base::WrapUnique(input_ipc), io_loop.task_runner()));
+      new AudioInputDevice(base::WrapUnique(input_ipc)));
   device->Initialize(params, &callback, 1);
-  device->Start();
 
   EXPECT_CALL(*input_ipc, CreateStream(_, _, _, _, _))
       .WillOnce(ReportOnStreamCreated(
           device.get(), duplicated_memory_handle,
           SyncSocket::UnwrapHandle(audio_device_socket_descriptor)));
   EXPECT_CALL(*input_ipc, RecordStream());
-  EXPECT_CALL(callback, OnCaptureStarted())
-      .WillOnce(QuitLoop(io_loop.task_runner()));
-  base::RunLoop().Run();
+
+  EXPECT_CALL(callback, OnCaptureStarted());
+  device->Start();
+  EXPECT_CALL(*input_ipc, CloseStream());
   device->Stop();
-  base::RunLoop().RunUntilIdle();
   duplicated_memory_handle.Close();
 }