Stopping the audio thread before destroying the AudioManager<Platform>.

The destruction of the AudioManager family happens in order of: AudioManager<Platform>, AudioManagerBase, AudioManager.
So before getting into the destruction of AudioManagerBase, we have make sure the audio thread has been stopped before AudioManager<Platform> is gone, otherwise it will end up into unexpected behavior, for example, crash because of pure virtual function.


BUG=117470
TEST=media_unittests, Address Sanitizer

Review URL: http://codereview.chromium.org/9692038

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@126635 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 108 insertions, 83 deletions

diff --git a/media/audio/android/audio_manager_android.cc b/media/audio/android/audio_manager_android.cc
index 9d17934..81264b5 100644
--- a/media/audio/android/audio_manager_android.cc
+++ b/media/audio/android/audio_manager_android.cc
@@ -21,7 +21,7 @@ AudioManagerAndroid::AudioManagerAndroid() {
 }
 
 AudioManagerAndroid::~AudioManagerAndroid() {
-  audio_thread_->Stop();
+  Shutdown();
 }
 
 bool AudioManagerAndroid::HasAudioOutputDevices() {
@@ -55,11 +55,11 @@ AudioOutputStream* AudioManagerAndroid::MakeLowLatencyOutputStream(
 AudioInputStream* AudioManagerAndroid::MakeLinearInputStream(
     const AudioParameters& params, const std::string& device_id) {
   DCHECK_EQ(AudioParameters::AUDIO_PCM_LINEAR, params.format);
-  return FakeAudioInputStream::MakeFakeStream(params);
+  return FakeAudioInputStream::MakeFakeStream(this, params);
 }
 
 AudioInputStream* AudioManagerAndroid::MakeLowLatencyInputStream(
     const AudioParameters& params, const std::string& device_id) {
   DCHECK_EQ(AudioParameters::AUDIO_PCM_LOW_LATENCY, params.format);
-  return FakeAudioInputStream::MakeFakeStream(params);
+  return FakeAudioInputStream::MakeFakeStream(this, params);
 }
diff --git a/media/audio/audio_manager_base.cc b/media/audio/audio_manager_base.cc
index 05ed044..abe31dc 100644
--- a/media/audio/audio_manager_base.cc
+++ b/media/audio/audio_manager_base.cc
@@ -16,7 +16,11 @@ static const int kStreamCloseDelaySeconds = 5;
 
 // Default maximum number of output streams that can be open simultaneously
 // for all platforms.
-static const int kDefaultMaxOutputStreams = 15;
+static const int kDefaultMaxOutputStreams = 16;
+
+// Default maximum number of input streams that can be open simultaneously
+// for all platforms.
+static const int kDefaultMaxInputStreams = 16;
 
 static const int kMaxInputChannels = 2;
 
@@ -26,13 +30,22 @@ const char AudioManagerBase::kDefaultDeviceId[] = "default";
 AudioManagerBase::AudioManagerBase()
     : num_active_input_streams_(0),
       max_num_output_streams_(kDefaultMaxOutputStreams),
-      num_output_streams_(0) {
+      max_num_input_streams_(kDefaultMaxInputStreams),
+      num_output_streams_(0),
+      num_input_streams_(0) {
 }
 
 AudioManagerBase::~AudioManagerBase() {
-  Shutdown();
+  // The platform specific AudioManager implementation must have already
+  // stopped the audio thread. Otherwise, we may destroy audio streams before
+  // stopping the thread, resulting an unexpected behavior.
+  // This way we make sure activities of the audio streams are all stopped
+  // before we destroy them.
+  CHECK(!audio_thread_.get());
   // All the output streams should have been deleted.
   DCHECK_EQ(0, num_output_streams_);
+  // All the input streams should have been deleted.
+  DCHECK_EQ(0, num_input_streams_);
 }
 
 void AudioManagerBase::Init() {
@@ -63,22 +76,25 @@ AudioOutputStream* AudioManagerBase::MakeAudioOutputStream(
   // importantly it prevents instability on certain systems.
   // See bug: http://crbug.com/30242.
   if (num_output_streams_ >= max_num_output_streams_) {
-    DLOG(ERROR) << "Number of opened audio streams " << num_output_streams_
-                << " exceed the max allowed number " << max_num_output_streams_;
+    DLOG(ERROR) << "Number of opened output audio streams "
+                << num_output_streams_
+                << " exceed the max allowed number "
+                << max_num_output_streams_;
     return NULL;
   }
 
   AudioOutputStream* stream = NULL;
   if (params.format == AudioParameters::AUDIO_MOCK) {
-    stream = FakeAudioOutputStream::MakeFakeStream(params);
+    stream = FakeAudioOutputStream::MakeFakeStream(this, params);
   } else if (params.format == AudioParameters::AUDIO_PCM_LINEAR) {
-    num_output_streams_++;
     stream = MakeLinearOutputStream(params);
   } else if (params.format == AudioParameters::AUDIO_PCM_LOW_LATENCY) {
-    num_output_streams_++;
     stream = MakeLowLatencyOutputStream(params);
   }
 
+  if (stream)
+    ++num_output_streams_;
+
   return stream;
 }
 
@@ -90,15 +106,25 @@ AudioInputStream* AudioManagerBase::MakeAudioInputStream(
     return NULL;
   }
 
+  if (num_input_streams_ >= max_num_input_streams_) {
+    DLOG(ERROR) << "Number of opened input audio streams "
+                << num_input_streams_
+                << " exceed the max allowed number " << max_num_input_streams_;
+    return NULL;
+  }
+
   AudioInputStream* stream = NULL;
   if (params.format == AudioParameters::AUDIO_MOCK) {
-    stream = FakeAudioInputStream::MakeFakeStream(params);
+    stream = FakeAudioInputStream::MakeFakeStream(this, params);
   } else if (params.format == AudioParameters::AUDIO_PCM_LINEAR) {
     stream = MakeLinearInputStream(params, device_id);
   } else if (params.format == AudioParameters::AUDIO_PCM_LOW_LATENCY) {
     stream = MakeLowLatencyInputStream(params, device_id);
   }
 
+  if (stream)
+    ++num_input_streams_;
+
   return stream;
 }
 
@@ -137,6 +163,7 @@ void AudioManagerBase::ReleaseOutputStream(AudioOutputStream* stream) {
 void AudioManagerBase::ReleaseInputStream(AudioInputStream* stream) {
   DCHECK(stream);
   // TODO(xians) : Have a clearer destruction path for the AudioInputStream.
+  num_input_streams_--;
   delete stream;
 }
 
diff --git a/media/audio/audio_manager_base.h b/media/audio/audio_manager_base.h
index ea9c78a..ba27221 100644
--- a/media/audio/audio_manager_base.h
+++ b/media/audio/audio_manager_base.h
@@ -59,11 +59,6 @@ class MEDIA_EXPORT AudioManagerBase : public AudioManager {
   void IncreaseActiveInputStreamCount();
   void DecreaseActiveInputStreamCount();
 
-  // Shuts down the audio thread and releases all the audio output dispatchers
-  // on the audio thread.  All AudioOutputProxy instances should be freed before
-  // Shutdown is called.
-  void Shutdown();
-
   // Creates the output stream for the |AUDIO_PCM_LINEAR| format. The legacy
   // name is also from |AUDIO_PCM_LINEAR|.
   virtual AudioOutputStream* MakeLinearOutputStream(
@@ -89,6 +84,12 @@ class MEDIA_EXPORT AudioManagerBase : public AudioManager {
                    AudioParameters::Compare>
       AudioOutputDispatchersMap;
 
+  // Shuts down the audio thread and releases all the audio output dispatchers
+  // on the audio thread.  All audio streams should be freed before
+  // Shutdown is called.
+  // This must be called in the destructor of the AudioManager<Platform>.
+  void Shutdown();
+
   void ShutdownOnAudioThread();
 
   void SetMaxOutputStreamsAllowed(int max) { max_num_output_streams_ = max; }
@@ -111,9 +112,15 @@ class MEDIA_EXPORT AudioManagerBase : public AudioManager {
   // SetMaxOutputStreamsAllowed().
   int max_num_output_streams_;
 
+  // Max number of open input streams.
+  int max_num_input_streams_;
+
   // Number of currently open output streams.
   int num_output_streams_;
 
+  // Number of currently open input streams.
+  int num_input_streams_;
+
   DISALLOW_COPY_AND_ASSIGN(AudioManagerBase);
 };
 
diff --git a/media/audio/fake_audio_input_stream.cc b/media/audio/fake_audio_input_stream.cc
index b9b271b..f1284f1 100644
--- a/media/audio/fake_audio_input_stream.cc
+++ b/media/audio/fake_audio_input_stream.cc
@@ -5,26 +5,26 @@
 #include "media/audio/fake_audio_input_stream.h"
 
 #include "base/bind.h"
+#include "media/audio/audio_manager_base.h"
 
 using base::Time;
 using base::TimeDelta;
 
 AudioInputStream* FakeAudioInputStream::MakeFakeStream(
+    AudioManagerBase* manager,
     const AudioParameters& params) {
-  return new FakeAudioInputStream(params);
+  return new FakeAudioInputStream(manager, params);
 }
 
-FakeAudioInputStream::FakeAudioInputStream(const AudioParameters& params)
-    : callback_(NULL),
+FakeAudioInputStream::FakeAudioInputStream(AudioManagerBase* manager,
+                                           const AudioParameters& params)
+    : audio_manager_(manager),
+      callback_(NULL),
       buffer_size_((params.channels * params.bits_per_sample *
                     params.samples_per_packet) / 8),
       thread_("FakeAudioRecordingThread"),
       callback_interval_(base::TimeDelta::FromMilliseconds(
           (params.samples_per_packet * 1000) / params.sample_rate)) {
-  // This object is ref counted (so that it can be used with Thread, PostTask)
-  // but the caller expects a plain pointer. So we take a reference here and
-  // will Release() ourselves in Close().
-  AddRef();
 }
 
 FakeAudioInputStream::~FakeAudioInputStream() {}
@@ -42,7 +42,7 @@ void FakeAudioInputStream::Start(AudioInputCallback* callback)  {
   thread_.Start();
   thread_.message_loop()->PostDelayedTask(
       FROM_HERE,
-      base::Bind(&FakeAudioInputStream::DoCallback, this),
+      base::Bind(&FakeAudioInputStream::DoCallback, base::Unretained(this)),
       callback_interval_);
 }
 
@@ -62,7 +62,7 @@ void FakeAudioInputStream::DoCallback() {
   last_callback_time_ = now;
   thread_.message_loop()->PostDelayedTask(
       FROM_HERE,
-      base::Bind(&FakeAudioInputStream::DoCallback, this),
+      base::Bind(&FakeAudioInputStream::DoCallback, base::Unretained(this)),
       next_callback_time);
 }
 
@@ -75,7 +75,7 @@ void FakeAudioInputStream::Close() {
     callback_->OnClose(this);
     callback_ = NULL;
   }
-  Release();  // Destoys this object.
+  audio_manager_->ReleaseInputStream(this);
 }
 
 double FakeAudioInputStream::GetMaxVolume() {
diff --git a/media/audio/fake_audio_input_stream.h b/media/audio/fake_audio_input_stream.h
index 1619374..99668e8b 100644
--- a/media/audio/fake_audio_input_stream.h
+++ b/media/audio/fake_audio_input_stream.h
@@ -9,18 +9,19 @@
 
 #include <vector>
 
-#include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/threading/thread.h"
 #include "base/time.h"
 #include "media/audio/audio_io.h"
 #include "media/audio/audio_parameters.h"
 
+class AudioManagerBase;
+
 class MEDIA_EXPORT FakeAudioInputStream
-    : public AudioInputStream,
-      public base::RefCountedThreadSafe<FakeAudioInputStream> {
+    : public AudioInputStream {
  public:
-  static AudioInputStream* MakeFakeStream(const AudioParameters& params);
+  static AudioInputStream* MakeFakeStream(AudioManagerBase* manager,
+                                          const AudioParameters& params);
 
   virtual bool Open() OVERRIDE;
   virtual void Start(AudioInputCallback* callback) OVERRIDE;
@@ -31,14 +32,14 @@ class MEDIA_EXPORT FakeAudioInputStream
   virtual double GetVolume() OVERRIDE;
 
  private:
-  // Give RefCountedThreadSafe access our destructor.
-  friend class base::RefCountedThreadSafe<FakeAudioInputStream>;
+  FakeAudioInputStream(AudioManagerBase* manager,
+                       const AudioParameters& params);
 
-  FakeAudioInputStream(const AudioParameters& params);
   virtual ~FakeAudioInputStream();
 
   void DoCallback();
 
+  AudioManagerBase* audio_manager_;
   AudioInputCallback* callback_;
   scoped_array<uint8> buffer_;
   int buffer_size_;
diff --git a/media/audio/fake_audio_output_stream.cc b/media/audio/fake_audio_output_stream.cc
index fe0d857..1831626 100644
--- a/media/audio/fake_audio_output_stream.cc
+++ b/media/audio/fake_audio_output_stream.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -6,33 +6,21 @@
 
 #include "base/at_exit.h"
 #include "base/logging.h"
+#include "media/audio/audio_manager_base.h"
 
-bool FakeAudioOutputStream::has_created_fake_stream_ = false;
-FakeAudioOutputStream* FakeAudioOutputStream::last_fake_stream_ = NULL;
+FakeAudioOutputStream* FakeAudioOutputStream::current_fake_stream_ = NULL;
 
 // static
 AudioOutputStream* FakeAudioOutputStream::MakeFakeStream(
+    AudioManagerBase* manager,
     const AudioParameters& params) {
-  if (!has_created_fake_stream_)
-    base::AtExitManager::RegisterCallback(&DestroyLastFakeStream, NULL);
-  has_created_fake_stream_ = true;
-
-  FakeAudioOutputStream* new_stream = new FakeAudioOutputStream(params);
-
-  if (last_fake_stream_) {
-    DCHECK(last_fake_stream_->closed_);
-    delete last_fake_stream_;
-  }
-  last_fake_stream_ = new_stream;
-
+  FakeAudioOutputStream* new_stream = new FakeAudioOutputStream(manager,
+                                                                params);
+  DCHECK(current_fake_stream_ == NULL);
+  current_fake_stream_ = new_stream;
   return new_stream;
 }
 
-// static
-FakeAudioOutputStream* FakeAudioOutputStream::GetLastFakeStream() {
-  return last_fake_stream_;
-}
-
 bool FakeAudioOutputStream::Open() {
   if (packet_size_ < sizeof(int16))
     return false;
@@ -40,6 +28,11 @@ bool FakeAudioOutputStream::Open() {
   return true;
 }
 
+// static
+FakeAudioOutputStream* FakeAudioOutputStream::GetCurrentFakeStream() {
+  return current_fake_stream_;
+}
+
 void FakeAudioOutputStream::Start(AudioSourceCallback* callback)  {
   callback_ = callback;
   memset(buffer_.get(), 0, packet_size_);
@@ -61,21 +54,19 @@ void FakeAudioOutputStream::GetVolume(double* volume) {
 
 void FakeAudioOutputStream::Close() {
   closed_ = true;
+  audio_manager_->ReleaseOutputStream(this);
 }
 
-FakeAudioOutputStream::FakeAudioOutputStream(const AudioParameters& params)
-    : volume_(0),
+FakeAudioOutputStream::FakeAudioOutputStream(AudioManagerBase* manager,
+                                             const AudioParameters& params)
+    : audio_manager_(manager),
+      volume_(0),
       callback_(NULL),
       packet_size_(params.GetPacketSize()),
       closed_(false) {
 }
 
-FakeAudioOutputStream::~FakeAudioOutputStream() {}
-
-// static
-void FakeAudioOutputStream::DestroyLastFakeStream(void* param) {
-  if (last_fake_stream_) {
-    DCHECK(last_fake_stream_->closed_);
-    delete last_fake_stream_;
-  }
+FakeAudioOutputStream::~FakeAudioOutputStream() {
+  if (current_fake_stream_ == this)
+    current_fake_stream_ = NULL;
 }
diff --git a/media/audio/fake_audio_output_stream.h b/media/audio/fake_audio_output_stream.h
index 3ea0f8b..d0a7e82 100644
--- a/media/audio/fake_audio_output_stream.h
+++ b/media/audio/fake_audio_output_stream.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 //
@@ -15,10 +15,14 @@
 #include "media/audio/audio_io.h"
 #include "media/audio/audio_parameters.h"
 
+class AudioManagerBase;
+
 class MEDIA_EXPORT FakeAudioOutputStream : public AudioOutputStream {
  public:
-  static AudioOutputStream* MakeFakeStream(const AudioParameters& params);
-  static FakeAudioOutputStream* GetLastFakeStream();
+  static AudioOutputStream* MakeFakeStream(AudioManagerBase* manager,
+                                           const AudioParameters& params);
+
+  static FakeAudioOutputStream* GetCurrentFakeStream();
 
   virtual bool Open() OVERRIDE;
   virtual void Start(AudioSourceCallback* callback) OVERRIDE;
@@ -31,13 +35,14 @@ class MEDIA_EXPORT FakeAudioOutputStream : public AudioOutputStream {
   double volume() { return volume_; }
 
  private:
-  explicit FakeAudioOutputStream(const AudioParameters& params);
+  explicit FakeAudioOutputStream(AudioManagerBase* manager,
+                                 const AudioParameters& params);
+
   virtual ~FakeAudioOutputStream();
 
-  static void DestroyLastFakeStream(void* param);
-  static bool has_created_fake_stream_;
-  static FakeAudioOutputStream* last_fake_stream_;
+  static FakeAudioOutputStream* current_fake_stream_;
 
+  AudioManagerBase* audio_manager_;
   double volume_;
   AudioSourceCallback* callback_;
   scoped_array<uint8> buffer_;
diff --git a/media/audio/mac/audio_manager_mac.cc b/media/audio/mac/audio_manager_mac.cc
index fdaf5a1..31b4d6d 100644
--- a/media/audio/mac/audio_manager_mac.cc
+++ b/media/audio/mac/audio_manager_mac.cc
@@ -221,6 +221,7 @@ AudioManagerMac::AudioManagerMac() {
 }
 
 AudioManagerMac::~AudioManagerMac() {
+  Shutdown();
 }
 
 bool AudioManagerMac::HasAudioOutputDevices() {
diff --git a/media/audio/openbsd/audio_manager_openbsd.cc b/media/audio/openbsd/audio_manager_openbsd.cc
index 1ef9398..552d528 100644
--- a/media/audio/openbsd/audio_manager_openbsd.cc
+++ b/media/audio/openbsd/audio_manager_openbsd.cc
@@ -46,15 +46,7 @@ AudioManagerOpenBSD::AudioManagerOpenBSD() {
 }
 
 AudioManagerOpenBSD::~AudioManagerOpenBSD() {
-  // Make sure we stop the thread first. If we allow the default destructor to
-  // destroy the members, we may destroy audio streams before stopping the
-  // thread, resulting an unexpected behavior.
-  // This way we make sure activities of the audio streams are all stopped
-  // before we destroy them.
-  audio_thread_.Stop();
-
-  // Free output dispatchers, closing all remaining open streams.
-  output_dispatchers_.clear();
+  Shutdown();
 }
 
 void AudioManagerOpenBSD::Init() {
diff --git a/media/audio/simple_sources_unittest.cc b/media/audio/simple_sources_unittest.cc
index ef00ed7..09e4ce9 100644
--- a/media/audio/simple_sources_unittest.cc
+++ b/media/audio/simple_sources_unittest.cc
@@ -78,12 +78,11 @@ TEST(SimpleSources, SineWaveAudio16MonoTest) {
 
   oas->Start(&source);
   oas->Stop();
-  oas->Close();
 
-  ASSERT_TRUE(FakeAudioOutputStream::GetLastFakeStream());
+  ASSERT_TRUE(FakeAudioOutputStream::GetCurrentFakeStream());
   const int16* last_buffer =
       reinterpret_cast<int16*>(
-          FakeAudioOutputStream::GetLastFakeStream()->buffer());
+          FakeAudioOutputStream::GetCurrentFakeStream()->buffer());
   ASSERT_TRUE(NULL != last_buffer);
 
   uint32 half_period = AudioParameters::kTelephoneSampleRate / (freq * 2);
@@ -98,4 +97,5 @@ TEST(SimpleSources, SineWaveAudio16MonoTest) {
   EXPECT_EQ(-5126, last_buffer[half_period + 1]);
   EXPECT_TRUE(last_buffer[half_period + 1] > last_buffer[half_period + 2]);
   EXPECT_TRUE(last_buffer[half_period + 2] > last_buffer[half_period + 3]);
+  oas->Close();
 }
diff --git a/media/audio/win/audio_manager_win.cc b/media/audio/win/audio_manager_win.cc
index 2827d11..66f4a55 100644
--- a/media/audio/win/audio_manager_win.cc
+++ b/media/audio/win/audio_manager_win.cc
@@ -108,6 +108,7 @@ AudioManagerWin::AudioManagerWin() {
 }
 
 AudioManagerWin::~AudioManagerWin() {
+  Shutdown();
 }
 
 bool AudioManagerWin::HasAudioOutputDevices() {