Remove the AudioManager singleton.

content/browser/resource_context.cc

Index: content/browser/resource_context.cc
===================================================================
--- content/browser/resource_context.cc	(revision 113173)
+++ content/browser/resource_context.cc	(working copy)
@@ -21,7 +21,9 @@
       quota_manager_(NULL),
       host_zoom_map_(NULL),
       media_observer_(NULL),
-      media_stream_manager_(NULL) {
+      download_id_factory_(NULL),
+      media_stream_manager_(NULL),
+      audio_manager_(NULL) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 }
 
@@ -158,6 +160,7 @@
   EnsureInitialized();
   return download_id_factory_;
 }
+
 void ResourceContext::set_download_id_factory(
     DownloadIdFactory* download_id_factory) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
@@ -177,4 +180,18 @@
   media_stream_manager_ = media_stream_manager;
 }
 
+AudioManager* ResourceContext::audio_manager() const {
+  // The AudioManager isn't necessarily initialized with other

[willchan no longer on Chromium, 2011/12/06 17:02:12]

Can you explain this comment? When is AudioManager initialized then? And why
isn't there a DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO))?

[tommi (sloooow) - chröme, 2011/12/06 20:28:48]

Sorry, this comment isn't correct. I must have forgotten to update it since an
early version of the cl.  I'll fix it.

When Chrome runs, the audio manager instance is created and owned by the
BrowserProcessImpl class.  This always happens on the IO thread and (as you no
doubt know) there is a check inside EnsureInitialized() for that.
What's different here though, and the reason why there isn't a dcheck here like
with the other accessors, is that the audio_manager accessor can be called on
other threads than the IO thread.  I suspect that the reason for this is
historical since the AudioManager was a singleton with an undefined threading
model and anyone could call the instance method at any time and on any thread.

[willchan no longer on Chromium, 2011/12/07 15:29:20]

I see. While it's not strictly a problem, it's different from the existing
threading model and we should be careful here. ResourceContext is created and
owned by ProfileIOData, which will be deleted on the IO thread and thus
ResourceContext will get deleted on the IO thread. We need to think carefully
about the shutdown case here. Note the shutdown order indicated by
http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/content/public/browse....
If ResourceContext is used on any thread which outlasts the IO thread, then we
have a problem, because ResourceContext may be deleted, and we'll get a use
after free.

This is the reason we have always just forced the accessors to be non-threadsafe
and only accessible on the IO thread.

+  // resource context variables, so if we already have one, we can still
+  // delay initializing the other ones.
+  if (!audio_manager_)
+    EnsureInitialized();
+  return audio_manager_;
+}
+
+void ResourceContext::set_audio_manager(AudioManager* audio_manager) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  audio_manager_ = audio_manager;
+}
+
 }  // namespace content