Add lock to fix race in AudioRendererMixerInput.

media/base/audio_renderer_mixer_input.cc

Index: media/base/audio_renderer_mixer_input.cc
diff --git a/media/base/audio_renderer_mixer_input.cc b/media/base/audio_renderer_mixer_input.cc
index 5743700f04b0a62c1c3c6eaf0f2d0bfa54aef0f5..d58bff33ed3b272b406cef5b3120c0f9faffefa0 100644
--- a/media/base/audio_renderer_mixer_input.cc
+++ b/media/base/audio_renderer_mixer_input.cc
@@ -38,6 +38,8 @@ AudioRendererMixerInput::~AudioRendererMixerInput() {
 void AudioRendererMixerInput::Initialize(
     const AudioParameters& params,
     AudioRendererSink::RenderCallback* callback) {
+  base::AutoLock auto_lock(lock_);
+
   DCHECK(!mixer_);
   DCHECK(callback);
 
@@ -46,6 +48,8 @@ void AudioRendererMixerInput::Initialize(
 }
 
 void AudioRendererMixerInput::Start() {
+  base::AutoLock auto_lock(lock_);
+
   DCHECK(!started_);
   DCHECK(!mixer_);
   DCHECK(callback_);  // Initialized.
@@ -61,16 +65,23 @@ void AudioRendererMixerInput::Start() {
   mixer_->AddErrorCallback(error_cb_);
 
   if (!pending_switch_callback_.is_null()) {
-    SwitchOutputDevice(pending_switch_device_id_,
-                       pending_switch_security_origin_,
-                       base::ResetAndReturn(&pending_switch_callback_));
+    SwitchOutputDevice_Locked(pending_switch_device_id_,
+                              pending_switch_security_origin_,
+                              base::ResetAndReturn(&pending_switch_callback_));
   }
 }
 
 void AudioRendererMixerInput::Stop() {
+  base::AutoLock auto_lock(lock_);
+  Stop_Locked();
+}
+
+void AudioRendererMixerInput::Stop_Locked() {
+  lock_.AssertAcquired();
+
   // Stop() may be called at any time, if Pause() hasn't been called we need to
   // remove our mixer input before shutdown.
-  Pause();
+  Pause_Locked();
 
   if (mixer_) {
     // TODO(dalecurtis): This is required so that |callback_| isn't called after
@@ -90,6 +101,13 @@ void AudioRendererMixerInput::Stop() {
 }
 
 void AudioRendererMixerInput::Play() {
+  base::AutoLock auto_lock(lock_);
+  Play_Locked();
+}
+
+void AudioRendererMixerInput::Play_Locked() {
+  lock_.AssertAcquired();
+
   if (playing_ || !mixer_)
     return;
 
@@ -98,6 +116,13 @@ void AudioRendererMixerInput::Play() {
 }
 
 void AudioRendererMixerInput::Pause() {
+  base::AutoLock auto_lock(lock_);
+  Pause_Locked();
+}
+
+void AudioRendererMixerInput::Pause_Locked() {
+  lock_.AssertAcquired();
+
   if (!playing_ || !mixer_)
     return;
 
@@ -106,6 +131,8 @@ void AudioRendererMixerInput::Pause() {
 }
 
 bool AudioRendererMixerInput::SetVolume(double volume) {
+  base::AutoLock auto_lock(lock_);
+
   volume_ = volume;
   return true;
 }
@@ -118,6 +145,16 @@ void AudioRendererMixerInput::SwitchOutputDevice(
     const std::string& device_id,
     const url::Origin& security_origin,
     const SwitchOutputDeviceCB& callback) {
+  base::AutoLock auto_lock(lock_);
+  SwitchOutputDevice_Locked(device_id, security_origin, callback);
+}
+
+void AudioRendererMixerInput::SwitchOutputDevice_Locked(
+    const std::string& device_id,
+    const url::Origin& security_origin,
+    const SwitchOutputDeviceCB& callback) {
+  lock_.AssertAcquired();
+
   if (!mixer_) {
     if (pending_switch_callback_.is_null()) {
       pending_switch_callback_ = callback;
@@ -145,7 +182,7 @@ void AudioRendererMixerInput::SwitchOutputDevice(
   }
 
   bool was_playing = playing_;
-  Stop();
+  Stop_Locked();
   device_id_ = device_id;
   security_origin_ = security_origin;
   mixer_ = new_mixer;
@@ -153,18 +190,22 @@ void AudioRendererMixerInput::SwitchOutputDevice(
   started_ = true;
 
   if (was_playing)
-    Play();
+    Play_Locked();
 
   callback.Run(OUTPUT_DEVICE_STATUS_OK);
 }
 
 AudioParameters AudioRendererMixerInput::GetOutputParameters() {
+  base::AutoLock auto_lock(lock_);
+
   if (mixer_)
     return mixer_->GetOutputDevice()->GetOutputParameters();
   return get_hardware_params_cb_.Run(device_id_, security_origin_);
 }
 
 OutputDeviceStatus AudioRendererMixerInput::GetDeviceStatus() {
+  base::AutoLock auto_lock(lock_);
+
   if (mixer_)
     return mixer_->GetOutputDevice()->GetDeviceStatus();
 
@@ -176,6 +217,8 @@ OutputDeviceStatus AudioRendererMixerInput::GetDeviceStatus() {
 
 double AudioRendererMixerInput::ProvideInput(AudioBus* audio_bus,
                                              base::TimeDelta buffer_delay) {
+  base::AutoLock auto_lock(lock_);

[o1ka, 2016/03/03 11:04:02]

ProvideInput runs on real-time "render" thread, so we should avoid any locks
we'll wait on for any significant amount of time; and Start/Stop/Switch
operations are really long ones.

[chcunningham, 2016/03/03 18:40:20]

Ack. I'll remove locks from Start/Stop/Switch and drop in a thread checker.

+
   // TODO(chcunningham): Delete this conversion and change ProvideInput to more
   // precisely describe delay as a count of frames delayed instead of TimeDelta.
   // See http://crbug.com/587522.
@@ -194,6 +237,8 @@ double AudioRendererMixerInput::ProvideInput(AudioBus* audio_bus,
 }
 
 void AudioRendererMixerInput::OnRenderError() {
+  base::AutoLock auto_lock(lock_);

[o1ka, 2016/03/03 11:04:02]

OnRenderError is a callback which can potentially be run during device
start/swtich, i.e. when the lock is already acquired.

[chcunningham, 2016/03/03 18:40:20]

Ack, will remove.

+
   callback_->OnRenderError();
 }