Make AudioOutputDevice restartable and reinitializable

media/audio/audio_output_device.h

Index: media/audio/audio_output_device.h
diff --git a/media/audio/audio_output_device.h b/media/audio/audio_output_device.h
index 532f45fe4a13eeb6b8b2884292e28652da3690f1..ef286de0a141462ce0678bef46ffcb668c016973 100644
--- a/media/audio/audio_output_device.h
+++ b/media/audio/audio_output_device.h
@@ -45,8 +45,6 @@
 // 1. Creating thread.
 //    Must be the main render thread.
 // 2. Control thread (may be the main render thread or another thread).
-//    The methods: Start(), Stop(), Play(), Pause(), SetVolume()
-//    must be called on the same thread.
 // 3. IO thread (internal implementation detail - not exposed to public API)
 //    The thread within which this class receives all the IPC messages and
 //    IPC communications can only happen in this thread.
@@ -56,8 +54,7 @@
 //    the audio layer in the browser process using sync sockets and shared
 //    memory.
 //
-// Implementation notes:
-// - The user must call Stop() before deleting the class instance.
+// NOTE: The user must call Stop() before deleting the class instance.
 
 #ifndef MEDIA_AUDIO_AUDIO_OUTPUT_DEVICE_H_
 #define MEDIA_AUDIO_AUDIO_OUTPUT_DEVICE_H_
@@ -80,7 +77,8 @@
 namespace media {
 
 class MEDIA_EXPORT AudioOutputDevice
-    : NON_EXPORTED_BASE(public AudioRendererSink),
+    : NON_EXPORTED_BASE(public RestartableAudioRendererSink),
+      NON_EXPORTED_BASE(public AudioRendererSink::RenderCallback),
       NON_EXPORTED_BASE(public AudioOutputIPCDelegate),
       NON_EXPORTED_BASE(public ScopedTaskRunnerObserver) {
  public:
@@ -97,7 +95,7 @@ class MEDIA_EXPORT AudioOutputDevice
 
   // AudioRendererSink implementation.
   void Initialize(const AudioParameters& params,
-                  RenderCallback* callback) override;
+                  AudioRendererSink::RenderCallback* callback) override;
   void Start() override;
   void Stop() override;
   void Play() override;
@@ -134,15 +132,24 @@ class MEDIA_EXPORT AudioOutputDevice
     PLAYING,  // Playing back.  Can Pause()/Stop().
   };
 
+  // media::AudioRendererSink::RenderCallback implementation.

[DaleCurtis, 2016/06/09 22:37:27]

I don't understand why you have this? Isn't this the same as
AudioOutputDevice::AudioThreadCallback::Process() ?

I'm a fan of making an OnRenderError() method, but you don't need to inherit
RenderCallback for this. The Render() method I would remove since you can do
that in ::Process() (it's only called there).

[Henrik Grunell, 2016/06/13 13:43:43]

The reason is to gate the callback here. After Stop() has returned we must
guarantee that there is no more callbacks. And to be able to serialize the
Initialize, Start and Stop, which they must be to make it restartable, the
stopping hack must go away. This makes the actual stopping of the thread
asynchronous and this is used to handle the new situation.

[DaleCurtis, 2016/06/13 18:52:32]

I'm sorry I don't understand. I don't see any reason why AudioOutputDevice needs
to implement the RenderCallback interface. You just need to protect callback_
with a lock.

+  // These two functions are called on the audio worker thread and passes calls
+  // through to |render_callback_| if it exists.
+  int Render(media::AudioBus* audio_bus,
+             uint32_t frames_delayed,
+             uint32_t frames_skipped) override;
+  void OnRenderError() override;
+
   // Methods called on IO thread ----------------------------------------------
   // The following methods are tasks posted on the IO thread that need to
   // be executed on that thread.  They use AudioOutputIPC to send IPC messages
   // upon state changes.
   void RequestDeviceAuthorizationOnIOThread();
-  void CreateStreamOnIOThread(const AudioParameters& params);
+  void InitializeOnIOThread(const AudioParameters& params);
+  void CreateStreamOnIOThread();
   void PlayOnIOThread();
   void PauseOnIOThread();
-  void ShutDownOnIOThread();
+  void StopOnIOThread();
   void SetVolumeOnIOThread(double volume);
 
   // base::MessageLoop::DestructionObserver implementation for the IO loop.
@@ -151,7 +158,11 @@ class MEDIA_EXPORT AudioOutputDevice
 
   AudioParameters audio_parameters_;
 
-  RenderCallback* callback_;
+  // |callback_lock_| protects |render_callback_|. When held, only accessing
+  // |render_callback_| is allowed. In particular, no calls must be made on
+  // |audio_thread_| for the risk of deadlocks.
+  base::Lock render_callback_lock_;
+  AudioRendererSink::RenderCallback* render_callback_;
 
   // A pointer to the IPC layer that takes care of sending requests over to
   // the AudioRendererHost.  Only valid when state_ != IPC_CLOSED and must only
@@ -183,19 +194,10 @@ class MEDIA_EXPORT AudioOutputDevice
   // Our audio thread callback class.  See source file for details.
   class AudioThreadCallback;
 
-  // 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_;
+  // The audio thread implementation.
   AudioDeviceThread audio_thread_;
-  std::unique_ptr<AudioOutputDevice::AudioThreadCallback> audio_callback_;
-
-  // 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(scherkus): Replace this by changing AudioRendererSink to either accept
-  // the callback via Start(). See http://crbug.com/151051 for details.
-  bool stopping_hack_;
+  std::unique_ptr<AudioOutputDevice::AudioThreadCallback>
+      audio_thread_callback_;
 
   base::WaitableEvent did_receive_auth_;
   AudioParameters output_params_;