diff options
author | tommi@chromium.org <tommi@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-12-12 21:50:00 +0000 |
---|---|---|
committer | tommi@chromium.org <tommi@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-12-12 21:50:00 +0000 |
commit | 2e77cb39b4b2760c6aa15fb194355cc429f9cb8a (patch) | |
tree | 5e90a41a0c15409cee0fca52719187b2025a6dc9 /media/audio/linux/audio_manager_linux.h | |
parent | 25db386820026afcaf815b8e9290817146be111e (diff) | |
download | chromium_src-2e77cb39b4b2760c6aa15fb194355cc429f9cb8a.zip chromium_src-2e77cb39b4b2760c6aa15fb194355cc429f9cb8a.tar.gz chromium_src-2e77cb39b4b2760c6aa15fb194355cc429f9cb8a.tar.bz2 |
Remove the AudioManager singleton.
Unit tests now instantiate their own AudioManager and can choose to
use the default one or provide their own mock implementation without
having to worry about conflicting with the singleton.
The teardown sequence of the AudioManager and its thread has been cleaned
up significantly and I don't think it has been completely tested before as
the audio thread was terminated before all objects that belonged to the
thread had a chance to do cleanup. The AudioManager unit tests do not use
the actual audio thread, so this part seems to have been left out.
In Chrome, the AudioManager instance is now owned by BrowserProcessImpl
and always constructed on the UI thread. This instance is then shared
in the same way that several other 'manager' type objects are shared to
'content' code, via content::ResourceContext. Audio specific classes
do though receive a direct pointer to the AudioManager and are required
to do proper reference counting if they need to hold onto the instance.
I chose to use the ResourceContext rather than direct use of g_browser_process
to avoid requiring another singleton when writing relatively simple tests
that touch the AudioManager.
I added a couple of safeguards to guard against future regressions:
- Not more than one instance of the AudioManager should be created.
- The AudioManager should not be addrefed by its own thread. This
can basically become a circular reference and prevent deterministic
shutdown.
Reviewers: Of course you're free to review everything,
but here's the breakdown in terms of the bare minimum from
the standpoint of "Owners approval". I'm asking Henrik to be the
main reviewer of the entire patch (sorry!).
Henrik: Everything minus the below, but it would be great if you could
take a look at the whole thing, specifically media/audio.
Pawel: I'd like you to take a generic look at this approach.
The key areas as far as the singleton itself goes are in
media/audio/audio_manager[_base].* and
chrome/browser/browser_process*.*
Satish: content/browser/speech/*
media/audio/audio_manager_base.* (new reference counting code)
Andrew: content/browser/renderer_host/media/*
content/renderer/media/webrtc_audio_device_unittest.cc (Owner)
Avi: content/browser/renderer_host/render_process_host_impl.cc
content/browser/resource_context.*
William: chrome/browser/profiles/profile_io_data.cc
chrome/browser/browser_process*.*
Robert: This is basically a heads up. I hope that I didn't break the OpenBSD
implementation, but unfortunately I have no way of knowing for sure.
Shijing: Please take a look at AudioManagerLinux. I replaced the set of
active streams with a simple counter.
BUG=105249
TEST=content_unittests, media_unittests, browser_tests.
Review URL: http://codereview.chromium.org/8818012
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@114084 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'media/audio/linux/audio_manager_linux.h')
-rw-r--r-- | media/audio/linux/audio_manager_linux.h | 5 |
1 files changed, 2 insertions, 3 deletions
diff --git a/media/audio/linux/audio_manager_linux.h b/media/audio/linux/audio_manager_linux.h index 5185013..d4d4bb1 100644 --- a/media/audio/linux/audio_manager_linux.h +++ b/media/audio/linux/audio_manager_linux.h @@ -5,8 +5,7 @@ #ifndef MEDIA_AUDIO_LINUX_AUDIO_MANAGER_LINUX_H_ #define MEDIA_AUDIO_LINUX_AUDIO_MANAGER_LINUX_H_ -#include <set> - +#include <string> #include "base/compiler_specific.h" #include "base/memory/ref_counted.h" #include "base/threading/thread.h" @@ -61,7 +60,7 @@ class MEDIA_EXPORT AudioManagerLinux : public AudioManagerBase { scoped_ptr<AlsaWrapper> wrapper_; - std::set<AudioOutputStream*> active_streams_; + size_t active_output_stream_count_; DISALLOW_COPY_AND_ASSIGN(AudioManagerLinux); }; |