Monitor the IO message loop in the AudioDevice classes.

content/renderer/media/audio_device.h

 // 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.
 //
 // Low-latency audio rendering unit utilizing audio output stream provided
 // by browser process through IPC.
 //
 // Relationship of classes.
 //
 //  AudioOutputController                AudioDevice
@@ skipping 48 matching lines @@
 // - Pause() is asynchronous/non-blocking.
 // - The user must call Stop() before deleting the class instance.
 
 #ifndef CONTENT_RENDERER_MEDIA_AUDIO_DEVICE_H_
 #define CONTENT_RENDERER_MEDIA_AUDIO_DEVICE_H_
 #pragma once
 
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/bind.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/message_loop.h"
 #include "base/shared_memory.h"
 #include "base/synchronization/lock.h"
 #include "base/threading/simple_thread.h"
 #include "content/common/content_export.h"
 #include "content/renderer/media/audio_message_filter.h"
 #include "media/audio/audio_parameters.h"
 #include "media/base/audio_renderer_sink.h"
 
-namespace base {
-class WaitableEvent;
-}
+// A common base class for AudioDevice and AudioInputDevice that manages

[scherkus (not reviewing), 2012/02/04 00:27:46]

seeing as this is common code perhaps move to separate .h/cc?

having audio_input_device.h pull in content/renderer/media/audio_device.h is a
bit odd

[tommi (sloooow) - chröme, 2012/02/06 13:38:56]

Done.

+// the IO message loop pointer and monitors it for destruction.
+// When the loop goes away, we automatically stop any running streams.
+template <class T>
+class AudioDeviceIOLoopObserver
+    : public MessageLoop::DestructionObserver {
+ public:
+  explicit AudioDeviceIOLoopObserver(MessageLoop* io_loop)
+      : io_loop_(io_loop->message_loop_proxy()) {
+    // Watch the IO loop and make sure that we stop the audio thread before we
+    // lose the IO thread.
+    ObserveIOLoopDestruction(true, NULL);
+  }
+
+  ~AudioDeviceIOLoopObserver() {

[scherkus (not reviewing), 2012/02/04 00:27:46]

virtual

[tommi (sloooow) - chröme, 2012/02/06 13:38:56]

Done.

+    ObserveIOLoopDestruction(false, NULL);
+  }
+
+ protected:
+  // Accessor to the IO loop that's used by the derived class.
+  base::MessageLoopProxy* loop() { return io_loop_; }

[scherkus (not reviewing), 2012/02/04 00:27:46]

nit: what about io_message_loop()?

Audio{Input}Device are multithreaded so specifying the exact loop here seems
like A Good Thing

[tommi (sloooow) - chröme, 2012/02/06 13:38:56]

I changed it to message_loop() in the end.  The reason is that the new class is
basically just a scoped message loop observer and not specific to io/ui/whatever
type of loop it observes.

+
+ private:
+  // MessageLoop::DestructionObserver implementation.
+  // Called on the IO thread when the thread is going away.
+  virtual void WillDestroyCurrentMessageLoop() OVERRIDE {
+    DCHECK(io_loop_->BelongsToCurrentThread());
+    // The IO thread is terminating.  We must stop the audio thread before the
+    // IO thread goes away.
+    static_cast<T*>(this)->ShutDownOnIOThread(NULL);

[scherkus (not reviewing), 2012/02/04 00:27:46]

ISTM the only reason this class is templated is for this line of code

what if we left this method undefined and had subclasses implement it? that way
we could de-inline this code and avoid the template goo

[tommi (sloooow) - chröme, 2012/02/06 13:38:56]

Done.

+  }
+
+  // Call from the IO thread to add or remove ourselves from the list of
+  // destruction observers for the IO message loop.
+  void ObserveIOLoopDestruction(bool enable, base::WaitableEvent* done) {

[scherkus (not reviewing), 2012/02/04 00:27:46]

nit: what about splitting this method into AddDestructionObserver() and
RemoveDestructionObserver() ?

it took me a while to parse what this method is doing as |done| is only non-NULL
in the removal case and only when the current thread isn't the IO loop

it also might be clearer to inline the thread-checking / waitable event code
inside the ctor/dtor so it's clear that we're actually blocking destruction
(from a cursory glance the dtor is passing in NULL as the waitable event!)

WDYT?

[tommi (sloooow) - chröme, 2012/02/06 13:38:56]

The function is still as it was because it does basically the same thing except
call Remove or Add depending on the enable flag.
Actually, |done| can be non-NULL for the add case as well.

I made the method private so that I could call it with a NULL pointer when
called from outside the method itself and a valid pointer when it posts a task
so that the method is executed on the message loop thread.  That's really all
there is to it. :)
Perhaps I just need to add documentation that explains that the only time |done|
is not-NULL is when the method has to post a task to the message loop so that
the [un]registration can be done there?

+    if (io_loop_->BelongsToCurrentThread()) {
+      MessageLoop* loop = MessageLoop::current();
+      if (enable) {
+        loop->AddDestructionObserver(this);
+      } else {
+        loop->RemoveDestructionObserver(this);
+      }
+    } else {
+      base::WaitableEvent event(false, false);
+      if (io_loop_->PostTask(FROM_HERE,
+              base::Bind(
+                  &AudioDeviceIOLoopObserver<T>::ObserveIOLoopDestruction,
+                  base::Unretained(this), enable, &event))) {
+        event.Wait();
+      } else {
+        // The IO thread has already terminated, so no need to wait.
+      }
+    }
+
+    if (done)
+      done->Signal();
+  }
+
+  // A pointer to the IO thread's message loop. That's here we do control the
+  // stream from.  It is possible during shutdown that the IO thread has gone
+  // away before the user of the AudioDevice instance has called Stop().
+  // So, we need to be prepared to stop and take down the audio thread if
+  // the IO thread ever goes away while we're still playing.
+  // So, when this pointer is NULL, the IO thread has gone away and all methods
+  // that post to it, must check for this first.
+  scoped_refptr<base::MessageLoopProxy> io_loop_;
+};

[scherkus (not reviewing), 2012/02/04 00:27:46]

DISALLOW_ etc

[tommi (sloooow) - chröme, 2012/02/06 13:38:56]

Done.

 
 class CONTENT_EXPORT AudioDevice
     : NON_EXPORTED_BASE(public media::AudioRendererSink),
       public AudioMessageFilter::Delegate,
-      public base::DelegateSimpleThread::Delegate {
+      public base::DelegateSimpleThread::Delegate,
+      public AudioDeviceIOLoopObserver<AudioDevice> {
  public:
   // Methods called on main render thread -------------------------------------
 
   // Minimal constructor where Initialize() must be called later.
   AudioDevice();
 
   AudioDevice(size_t buffer_size,
               int channels,
               double sample_rate,
               RenderCallback* callback);
@@ skipping 51 matching lines @@
   friend class base::RefCountedThreadSafe<AudioDevice>;
   virtual ~AudioDevice();
 
   // Methods called on IO thread ----------------------------------------------
   // The following methods are tasks posted on the IO thread that needs to
   // be executed on that thread. They interact with AudioMessageFilter and
   // sends IPC messages on that thread.
   void InitializeOnIOThread(const AudioParameters& params);
   void PlayOnIOThread();
   void PauseOnIOThread(bool flush);
+  // Allow AudioDeviceIOLoopObserver to call ShutDownOnIOThread.
+  friend class AudioDeviceIOLoopObserver<AudioDevice>;
   void ShutDownOnIOThread(base::WaitableEvent* signal);
   void SetVolumeOnIOThread(double volume);
 
   void Send(IPC::Message* message);
 
   // Method called on the audio thread ----------------------------------------
   // Calls the client's callback for rendering audio.
   // Returns actual number of filled frames that callback returned. This length
   // is passed to host at the end of the shared memory (i.e. buffer). In case of
   // continuous stream host just ignores it and assumes buffer is always filled
@@ skipping 54 matching lines @@
   // These variables must only be set on the IO thread while the audio_thread_
   // is not running.
   base::SharedMemoryHandle shared_memory_handle_;
   scoped_ptr<base::CancelableSyncSocket> audio_socket_;
   int memory_length_;
 
   DISALLOW_COPY_AND_ASSIGN(AudioDevice);
 };
 
 #endif  // CONTENT_RENDERER_MEDIA_AUDIO_DEVICE_H_