Remove the AudioManager singleton.

media/audio/audio_manager.h

Index: media/audio/audio_manager.h
===================================================================
--- media/audio/audio_manager.h	(revision 113173)
+++ media/audio/audio_manager.h	(working copy)
@@ -5,7 +5,9 @@
 #ifndef MEDIA_AUDIO_AUDIO_MANAGER_H_
 #define MEDIA_AUDIO_AUDIO_MANAGER_H_
 
+#include <string>

[scherkus (not reviewing), 2011/12/09 22:47:30]

nit: blank lines between <system includes.h> and "regular chrome includes.h"

[tommi (sloooow) - chröme, 2011/12/10 00:11:14]

Done.

 #include "base/basictypes.h"
+#include "base/memory/ref_counted.h"
 #include "base/string16.h"
 #include "base/task.h"
 #include "media/audio/audio_device_name.h"
@@ -15,14 +17,27 @@
 class AudioOutputStream;
 class MessageLoop;
 
-// TODO(sergeyu): In this interface and some other places AudioParameters struct
-// is passed by value. It is better to change it to const reference.
-
 // Manages all audio resources. In particular it owns the AudioOutputStream
 // objects. Provides some convenience functions that avoid the need to provide
 // iterators over the existing streams.
-class MEDIA_EXPORT AudioManager {
+class MEDIA_EXPORT AudioManager
+    : public base::RefCountedThreadSafe<AudioManager> {

[scherkus (not reviewing), 2011/12/09 22:47:30]

we really shouldn't require refcounting but let's save that for another day

there's some detangling of object lifetimes and cleanup of stream
creation/deletion that has to happen first before we can tackling that beast

mind filing a bug + referencing it in this comment?

[tommi (sloooow) - chröme, 2011/12/10 00:11:14]

Will do. crbug seems to be down at the moment so I'll file one tomorrow.

I started out without reference counting (hence all the audio_manager.get()'s
that were still in there) but as the cl grew, I saw that lifetime, thread
management, shutdown etc were undefined and using scoped_ptr<> + non-ref
pointers, all over the place would be risky.  However, the threading model,
usage and teardown is a lot more defined now, so switching from refcounted
shouldn't be too hard moving forward.

[scherkus (not reviewing), 2011/12/10 00:16:52]

agree 100%

[tommi (sloooow) - chröme, 2011/12/10 09:59:57]

bug filed and comment added: crbug.com/107087

[willchan no longer on Chromium, 2011/12/12 16:44:05]

This plan sounds great to me! It looks like as long as the BrowserProcessImpl
destroys it after the other threads are destroyed, then we shouldn't have any
dangling pointers.

  public:
+  AudioManager();
+
+#ifndef NDEBUG
+  // Allow base classes in debug builds to override the reference counting
+  // functions.  This allows us to protect against regressions and enforce
+  // correct usage.  The default implementation just calls the base class.
+  virtual void AddRef() const;
+  virtual void Release() const;
+#endif
+
+  // Use to construct the audio manager.
+  // NOTE: There should only be one instance.  If you try to create more than
+  // one instance, it will hit a CHECK().
+  static scoped_refptr<AudioManager> Create();
+
   // Returns true if the OS reports existence of audio devices. This does not
   // guarantee that the existing devices support all formats and sample rates.
   virtual bool HasAudioOutputDevices() = 0;
@@ -106,27 +121,15 @@
   // Returns message loop used for audio IO.
   virtual MessageLoop* GetMessageLoop() = 0;
 
-  // Get AudioManager singleton.
-  // TODO(cpu): Define threading requirements for interacting with AudioManager.
-  static AudioManager* GetAudioManager();
-
  protected:
-  virtual ~AudioManager() {}
-
-  // Called from GetAudioManager() to initialiaze the instance.
+  // Called from Create() to initialize the instance.
   virtual void Init() = 0;
 
-  // Called by Destroy() to cleanup the instance before destruction.
-  virtual void Cleanup() = 0;
+  friend class base::RefCountedThreadSafe<AudioManager>;
+  virtual ~AudioManager();
 
  private:
-  // Allow unit tests to delete and recreate the singleton.
-  friend class AutoAudioManagerCleanup;
-  static void Destroy(void*);
-  static bool SingletonExists();
-  static void Resurrect();
-
-  // Called by GetAudioManager() to create platform-specific audio manager.
+  // Called by Create() to create platform-specific audio manager.
   static AudioManager* CreateAudioManager();

[scherkus (not reviewing), 2011/12/09 22:47:30]

could we move this to the .cc or do implementors require access to private
methods inside AudioManager?

[tommi (sloooow) - chröme, 2011/12/10 00:11:14]

Done.

 };