Switch fake audio output streams over to CancelableClosure.

Prevents multiple audio callback chains from running concurrently.

BUG=159049
TEST=multiple audio streams w/o audio hardware. media_unittests.


Review URL: https://chromiumcodereview.appspot.com/11365053

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

3 files changed, 90 insertions, 20 deletions

diff --git a/media/audio/fake_audio_output_stream.cc b/media/audio/fake_audio_output_stream.cc
index 38b5437..c21026d 100644
--- a/media/audio/fake_audio_output_stream.cc
+++ b/media/audio/fake_audio_output_stream.cc
@@ -25,8 +25,7 @@ FakeAudioOutputStream::FakeAudioOutputStream(AudioManagerBase* manager,
       audio_bus_(AudioBus::Create(params)),
       frames_per_millisecond_(
           params.sample_rate() / static_cast<float>(
-              base::Time::kMillisecondsPerSecond)),
-      ALLOW_THIS_IN_INITIALIZER_LIST(weak_this_(this)) {
+              base::Time::kMillisecondsPerSecond)) {
 }
 
 FakeAudioOutputStream::~FakeAudioOutputStream() {
@@ -41,19 +40,21 @@ bool FakeAudioOutputStream::Open() {
 void FakeAudioOutputStream::Start(AudioSourceCallback* callback)  {
   DCHECK(audio_manager_->GetMessageLoop()->BelongsToCurrentThread());
   callback_ = callback;
-  audio_manager_->GetMessageLoop()->PostTask(FROM_HERE, base::Bind(
-      &FakeAudioOutputStream::OnMoreDataTask, weak_this_.GetWeakPtr()));
+  on_more_data_cb_.Reset(base::Bind(
+      &FakeAudioOutputStream::OnMoreDataTask, base::Unretained(this)));
+  audio_manager_->GetMessageLoop()->PostTask(
+      FROM_HERE, on_more_data_cb_.callback());
 }
 
 void FakeAudioOutputStream::Stop() {
   DCHECK(audio_manager_->GetMessageLoop()->BelongsToCurrentThread());
   callback_ = NULL;
+  on_more_data_cb_.Cancel();
 }
 
 void FakeAudioOutputStream::Close() {
   DCHECK(!callback_);
   DCHECK(audio_manager_->GetMessageLoop()->BelongsToCurrentThread());
-  weak_this_.InvalidateWeakPtrs();
   audio_manager_->ReleaseOutputStream(this);
 }
 
@@ -65,20 +66,16 @@ void FakeAudioOutputStream::GetVolume(double* volume) {
 
 void FakeAudioOutputStream::OnMoreDataTask() {
   DCHECK(audio_manager_->GetMessageLoop()->BelongsToCurrentThread());
+  DCHECK(callback_);
 
   audio_bus_->Zero();
-  int frames_received = audio_bus_->frames();
-  if (callback_) {
-    frames_received = callback_->OnMoreData(
-        audio_bus_.get(), AudioBuffersState());
-  }
+  int frames_received = callback_->OnMoreData(
+      audio_bus_.get(), AudioBuffersState());
 
   // Calculate our sleep duration for simulated playback.  Sleep for at least
   // one millisecond so we don't spin the CPU.
-  MessageLoop::current()->PostDelayedTask(
-      FROM_HERE, base::Bind(
-          &FakeAudioOutputStream::OnMoreDataTask, weak_this_.GetWeakPtr()),
-      base::TimeDelta::FromMilliseconds(
+  audio_manager_->GetMessageLoop()->PostDelayedTask(
+      FROM_HERE, on_more_data_cb_.callback(), base::TimeDelta::FromMilliseconds(
           std::max(1.0f, frames_received / frames_per_millisecond_)));
 }
 
diff --git a/media/audio/fake_audio_output_stream.h b/media/audio/fake_audio_output_stream.h
index 6d4f19b..d188b9f 100644
--- a/media/audio/fake_audio_output_stream.h
+++ b/media/audio/fake_audio_output_stream.h
@@ -5,8 +5,8 @@
 #ifndef MEDIA_AUDIO_FAKE_AUDIO_OUTPUT_STREAM_H_
 #define MEDIA_AUDIO_FAKE_AUDIO_OUTOUT_STREAM_H_
 
+#include "base/cancelable_callback.h"
 #include "base/memory/scoped_ptr.h"
-#include "base/memory/weak_ptr.h"
 #include "media/audio/audio_io.h"
 #include "media/audio/audio_parameters.h"
 
@@ -45,7 +45,7 @@ class MEDIA_EXPORT FakeAudioOutputStream : public AudioOutputStream {
   float frames_per_millisecond_;
 
   // Used to post delayed tasks to the AudioThread that we can cancel.
-  base::WeakPtrFactory<FakeAudioOutputStream> weak_this_;
+  base::CancelableClosure on_more_data_cb_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeAudioOutputStream);
 };
diff --git a/media/audio/fake_audio_output_stream_unittest.cc b/media/audio/fake_audio_output_stream_unittest.cc
index 8318c2b..6838e3f 100644
--- a/media/audio/fake_audio_output_stream_unittest.cc
+++ b/media/audio/fake_audio_output_stream_unittest.cc
@@ -5,6 +5,7 @@
 #include "base/bind.h"
 #include "base/message_loop.h"
 #include "base/synchronization/waitable_event.h"
+#include "base/time.h"
 #include "media/audio/fake_audio_output_stream.h"
 #include "media/audio/audio_manager.h"
 #include "media/audio/simple_sources.h"
@@ -22,6 +23,10 @@ class FakeAudioOutputStreamTest : public testing::Test {
         done_(false, false) {
     stream_ = audio_manager_->MakeAudioOutputStream(AudioParameters(params_));
     CHECK(stream_);
+
+    time_between_callbacks_ = base::TimeDelta::FromMilliseconds(
+        params_.frames_per_buffer() * base::Time::kMillisecondsPerSecond /
+        static_cast<float>(params_.sample_rate()));
   }
 
   virtual ~FakeAudioOutputStreamTest() {}
@@ -30,17 +35,48 @@ class FakeAudioOutputStreamTest : public testing::Test {
     ASSERT_TRUE(audio_manager_->GetMessageLoop()->BelongsToCurrentThread());
     ASSERT_TRUE(stream_->Open());
     stream_->Start(&source_);
+  }
+
+  void RunOnceOnAudioThread() {
+    ASSERT_TRUE(audio_manager_->GetMessageLoop()->BelongsToCurrentThread());
+    RunOnAudioThread();
     // Start() should immediately post a task to run the source callback, so we
     // should end up with only a single callback being run.
     audio_manager_->GetMessageLoop()->PostTask(FROM_HERE, base::Bind(
-        &FakeAudioOutputStreamTest::EndTest, base::Unretained(this)));
+        &FakeAudioOutputStreamTest::EndTest, base::Unretained(this), 1));
   }
 
-  void EndTest() {
+  void StopStartOnAudioThread() {
+    ASSERT_TRUE(audio_manager_->GetMessageLoop()->BelongsToCurrentThread());
+    stream_->Stop();
+    stream_->Start(&source_);
+  }
+
+  void TimeCallbacksOnAudioThread(int callbacks) {
+    ASSERT_TRUE(audio_manager_->GetMessageLoop()->BelongsToCurrentThread());
+
+    if (source_.callbacks() == 0) {
+      RunOnAudioThread();
+      start_time_ = base::Time::Now();
+    }
+
+    // Keep going until we've seen the requested number of callbacks.
+    if (source_.callbacks() < callbacks) {
+      audio_manager_->GetMessageLoop()->PostDelayedTask(FROM_HERE, base::Bind(
+          &FakeAudioOutputStreamTest::TimeCallbacksOnAudioThread,
+          base::Unretained(this), callbacks), time_between_callbacks_);
+    } else {
+      audio_manager_->GetMessageLoop()->PostTask(FROM_HERE, base::Bind(
+          &FakeAudioOutputStreamTest::EndTest, base::Unretained(this),
+          callbacks));
+    }
+  }
+
+  void EndTest(int callbacks) {
     ASSERT_TRUE(audio_manager_->GetMessageLoop()->BelongsToCurrentThread());
     stream_->Stop();
     stream_->Close();
-    EXPECT_EQ(1, source_.callbacks());
+    EXPECT_EQ(callbacks, source_.callbacks());
     EXPECT_EQ(0, source_.errors());
     done_.Signal();
   }
@@ -51,6 +87,8 @@ class FakeAudioOutputStreamTest : public testing::Test {
   AudioOutputStream* stream_;
   SineWaveAudioSource source_;
   base::WaitableEvent done_;
+  base::Time start_time_;
+  base::TimeDelta time_between_callbacks_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(FakeAudioOutputStreamTest);
@@ -60,8 +98,43 @@ class FakeAudioOutputStreamTest : public testing::Test {
 // callbacks to the AudioSourceCallback.
 TEST_F(FakeAudioOutputStreamTest, FakeStreamBasicCallback) {
   audio_manager_->GetMessageLoop()->PostTask(FROM_HERE, base::Bind(
-      &FakeAudioOutputStreamTest::RunOnAudioThread, base::Unretained(this)));
+      &FakeAudioOutputStreamTest::RunOnceOnAudioThread,
+      base::Unretained(this)));
+  done_.Wait();
+}
+
+// Ensure the time between callbacks is sane.
+TEST_F(FakeAudioOutputStreamTest, TimeBetweenCallbacks) {
+  static const int kTestCallbacks = 5;
+
+  audio_manager_->GetMessageLoop()->PostTask(FROM_HERE, base::Bind(
+      &FakeAudioOutputStreamTest::TimeCallbacksOnAudioThread,
+      base::Unretained(this), kTestCallbacks));
+
+  // Let the loop run for a second or two then issue Stop() / Start().
+  audio_manager_->GetMessageLoop()->PostDelayedTask(FROM_HERE, base::Bind(
+      &FakeAudioOutputStreamTest::StopStartOnAudioThread,
+      base::Unretained(this)), time_between_callbacks_);
+
   done_.Wait();
+
+  base::TimeDelta elapsed = base::Time::Now() - start_time_;
+
+  // There are only (kTestCallbacks - 1) intervals between kTestCallbacks.
+  float actual_time_between_callbacks_ms =
+      elapsed.InMillisecondsF() / (kTestCallbacks - 1);
+  float expected_time_between_callbacks_ms =
+      time_between_callbacks_.InMillisecondsF();
+
+  // Ensure callback time is no faster than the expected time between callbacks.
+  EXPECT_GE(actual_time_between_callbacks_ms,
+            expected_time_between_callbacks_ms);
+
+  // Softly check if the callback time is no slower than twice the expected time
+  // between callbacks.  Since this test runs on the bots we can't be too strict
+  // with the bounds.
+  if (actual_time_between_callbacks_ms > 2 * expected_time_between_callbacks_ms)
+    LOG(ERROR) << "Time between fake audio callbacks is too large!";
 }
 
 }  // namespace media