Simplify shutdown of AudioDevice's audio thread.

content/renderer/media/audio_device.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/media/audio_device.h"
 
 #include "base/bind.h"
 #include "base/debug/trace_event.h"
 #include "base/message_loop.h"
 #include "base/time.h"
@@ skipping 88 matching lines @@
   params.bits_per_sample = bits_per_sample_;
   params.samples_per_packet = buffer_size_;
 
   ChildProcess::current()->io_message_loop()->PostTask(
       FROM_HERE,
       base::Bind(&AudioDevice::InitializeOnIOThread, this, params));
 }
 
 void AudioDevice::Stop() {
   DCHECK(MessageLoop::current() != ChildProcess::current()->io_message_loop());
-  base::WaitableEvent completion(false, false);
 
+  // Stop and shutdown the audio thread from the IO thread.
   ChildProcess::current()->io_message_loop()->PostTask(
       FROM_HERE,
-      base::Bind(&AudioDevice::ShutDownOnIOThread, this, &completion));
-
-  // We wait here for the IO task to be completed to remove race conflicts
-  // with OnLowLatencyCreated() and to ensure that Stop() acts as a synchronous
-  // function call.
-  completion.Wait();
-  ShutDownAudioThread();
+      base::Bind(&AudioDevice::ShutDownOnIOThread, this));
 }
 
 void AudioDevice::Play() {
   ChildProcess::current()->io_message_loop()->PostTask(
       FROM_HERE,
       base::Bind(&AudioDevice::PlayOnIOThread, this));
 }
 
 void AudioDevice::Pause(bool flush) {
   ChildProcess::current()->io_message_loop()->PostTask(
@@ skipping 13 matching lines @@
 
   return true;
 }
 
 void AudioDevice::GetVolume(double* volume) {
   // Return a locally cached version of the current scaling factor.
   *volume = volume_;
 }
 
 void AudioDevice::InitializeOnIOThread(const AudioParameters& params) {
-  DCHECK(MessageLoop::current() == ChildProcess::current()->io_message_loop());
+  DCHECK_EQ(MessageLoop::current(), ChildProcess::current()->io_message_loop());
   // Make sure we don't create the stream more than once.
   DCHECK_EQ(0, stream_id_);
   if (stream_id_)
     return;
 
   stream_id_ = filter_->AddDelegate(this);
   Send(new AudioHostMsg_CreateStream(stream_id_, params, true));
 }
 
 void AudioDevice::PlayOnIOThread() {
-  DCHECK(MessageLoop::current() == ChildProcess::current()->io_message_loop());
+  DCHECK_EQ(MessageLoop::current(), ChildProcess::current()->io_message_loop());
   if (stream_id_ && is_started_)
     Send(new AudioHostMsg_PlayStream(stream_id_));
   else
     play_on_start_ = true;
 }
 
 void AudioDevice::PauseOnIOThread(bool flush) {
-  DCHECK(MessageLoop::current() == ChildProcess::current()->io_message_loop());
+  DCHECK_EQ(MessageLoop::current(), ChildProcess::current()->io_message_loop());
   if (stream_id_ && is_started_) {
     Send(new AudioHostMsg_PauseStream(stream_id_));
     if (flush)
       Send(new AudioHostMsg_FlushStream(stream_id_));
   } else {
     // Note that |flush| isn't relevant here since this is the case where
     // the stream is first starting.
     play_on_start_ = false;
   }
 }
 
-void AudioDevice::ShutDownOnIOThread(base::WaitableEvent* completion) {
-  DCHECK(MessageLoop::current() == ChildProcess::current()->io_message_loop());
-  is_started_ = false;
+void AudioDevice::ShutDownOnIOThread() {
+  DCHECK_EQ(MessageLoop::current(), ChildProcess::current()->io_message_loop());
 
   // Make sure we don't call shutdown more than once.
-  if (!stream_id_) {
-    if (completion)
-      completion->Signal();
+  if (!stream_id_)
     return;
-  }
+
+  is_started_ = false;
 
   filter_->RemoveDelegate(stream_id_);
   Send(new AudioHostMsg_CloseStream(stream_id_));
   stream_id_ = 0;
 
-  if (completion)
-    completion->Signal();
+  ShutDownAudioThread();
 }
 
 void AudioDevice::SetVolumeOnIOThread(double volume) {
-  DCHECK(MessageLoop::current() == ChildProcess::current()->io_message_loop());
+  DCHECK_EQ(MessageLoop::current(), ChildProcess::current()->io_message_loop());
   if (stream_id_)
     Send(new AudioHostMsg_SetVolume(stream_id_, volume));
 }
 
 void AudioDevice::OnRequestPacket(AudioBuffersState buffers_state) {
   // This method does not apply to the low-latency system.
 }
 
 void AudioDevice::OnStateChanged(AudioStreamState state) {
   if (state == kAudioStreamError) {
     DLOG(WARNING) << "AudioDevice::OnStateChanged(kError)";
   }
 }
 
 void AudioDevice::OnCreated(
     base::SharedMemoryHandle handle, uint32 length) {
   // Not needed in this simple implementation.
 }
 
 void AudioDevice::OnLowLatencyCreated(
     base::SharedMemoryHandle handle,
     base::SyncSocket::Handle socket_handle,
     uint32 length) {
-  DCHECK(MessageLoop::current() == ChildProcess::current()->io_message_loop());
+  DCHECK_EQ(MessageLoop::current(), ChildProcess::current()->io_message_loop());
+  DCHECK_GE(length, buffer_size_ * sizeof(int16) * channels_);
 #if defined(OS_WIN)
   DCHECK(handle);
   DCHECK(socket_handle);
 #else
   DCHECK_GE(handle.fd, 0);
   DCHECK_GE(socket_handle, 0);
 #endif
-  DCHECK(length);
-  DCHECK(!audio_thread_.get());
 
   // Takes care of the case when Stop() is called before OnLowLatencyCreated().
   if (!stream_id_) {
     base::SharedMemory::CloseHandle(handle);
     // Close the socket handler.
     base::SyncSocket socket(socket_handle);
     return;
   }
 
+  // In case the audio thread is still shutting down from an async Stop(),
+  // we have to wait for it to be completely stopped.  We must do this first
+  // before setting to the member variables below.
+  ShutDownAudioThread();

[enal1, 2012/01/20 23:09:46]

I don't fully understand the comment. Do you mean "if there was shut down task
posted, shut down now, and let the task that would be executed in the future to
shut down this stream, even if this stream has nothing to do with stream it was
supposed to stop?"

That is why I felt uneasy with making Stop() asynchronous...

[tommi (sloooow) - chröme, 2012/01/23 10:17:48]

You're right, I don't need to do this and I shouldn't.

(mostly repeating for my own sanity:)
Since both Start() and Stop() are now serialized via the IO thread, we don't
really have this problem.
If Start() is called while a shutdown is taking place, then the actual
"InitializeOnIOThread" task, won't run until the AudioDevice has been completely
shut down.  So, when we get the OnLowLatencyCreated notification, it can only be
for a new stream and if there was a previous one, it has already been shut down
correctly.

Thanks for catching this.  I removed this call.

+
   shared_memory_handle_ = handle;
   memory_length_ = length;
+  audio_socket_ = new AudioSocket(socket_handle);
 
-  DCHECK_GE(length, buffer_size_ * sizeof(int16) * channels_);
-
-  audio_socket_ = new AudioSocket(socket_handle);
-  {
-    // Synchronize with ShutDownAudioThread().
-    base::AutoLock auto_lock(lock_);
-
-    DCHECK(!audio_thread_.get());
-    audio_thread_.reset(
-        new base::DelegateSimpleThread(this, "renderer_audio_thread"));
-    audio_thread_->Start();
-  }
+  audio_thread_.reset(
+      new base::DelegateSimpleThread(this, "renderer_audio_thread"));
+  audio_thread_->Start();
 
   // We handle the case where Play() and/or Pause() may have been called
   // multiple times before OnLowLatencyCreated() gets called.
   is_started_ = true;
   if (play_on_start_)
     PlayOnIOThread();
 }
 
 void AudioDevice::OnVolume(double volume) {
   NOTIMPLEMENTED();
@@ skipping 54 matching lines @@
     // to the browser process as float, so we don't lose precision for
     // audio hardware which has better than 16bit precision.
     media::InterleaveFloatToInt16(audio_data_,
                                   data,
                                   buffer_size_);
   }
   return num_frames;
 }
 
 void AudioDevice::ShutDownAudioThread() {
-  // Synchronize with OnLowLatencyCreated().
-  base::AutoLock auto_lock(lock_);
+  DCHECK_EQ(MessageLoop::current(), ChildProcess::current()->io_message_loop());
+
   if (audio_thread_.get()) {
     // Close the socket to terminate the main thread function in the
     // audio thread.
     audio_socket_->Close();
+    audio_socket_ = NULL;
     audio_thread_->Join();
     audio_thread_.reset(NULL);
   }
 }