Initialization of audio manager for Android is now done on the audio thread

media/audio/android/audio_android_unittest.cc

Index: media/audio/android/audio_android_unittest.cc
diff --git a/media/audio/android/audio_android_unittest.cc b/media/audio/android/audio_android_unittest.cc
index 688a940bfbc11d0dc13390ac0bde8ec295f688d3..49e6de42e22ea753e896295b8b0d6627349d0e6d 100644
--- a/media/audio/android/audio_android_unittest.cc
+++ b/media/audio/android/audio_android_unittest.cc
@@ -8,6 +8,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/path_service.h"
+#include "base/run_loop.h"
 #include "base/strings/stringprintf.h"
 #include "base/synchronization/lock.h"
 #include "base/synchronization/waitable_event.h"
@@ -421,16 +422,18 @@ class FullDuplexAudioSinkSource
 // Test fixture class for tests which only exercise the output path.
 class AudioAndroidOutputTest : public testing::Test {
  public:
-  AudioAndroidOutputTest() {}
-
- protected:
-  virtual void SetUp() {
-    audio_manager_.reset(AudioManager::CreateForTesting());
-    loop_.reset(new base::MessageLoopForUI());
+  AudioAndroidOutputTest()
+      : loop_(new base::MessageLoopForUI()),
+        audio_manager_(AudioManager::CreateForTesting()) {
+    // Wait for the AudioManager to finish any initialization on the audio loop.
+    base::RunLoop().RunUntilIdle();

[tommi (sloooow) - chröme, 2014/01/31 14:06:59]

what exactly is it that happens during this loop?

[henrika (OOO until Aug 14), 2014/01/31 14:19:05]

Sorry, my bad. Copied from test that Dale had just modified. Must admit that I
don't know the exact benefit. Will remove.

[DaleCurtis, 2014/01/31 19:18:12]

This is only needed on platforms which post initialization to the audio thread.

   }
 
-  virtual void TearDown() {}
+  virtual ~AudioAndroidOutputTest() {
+    base::RunLoop().RunUntilIdle();

[tommi (sloooow) - chröme, 2014/01/31 14:06:59]

add a comment for why we need this?

[henrika (OOO until Aug 14), 2014/01/31 14:19:05]

Will remove.

+  }
 
+ protected:
   AudioManager* audio_manager() { return audio_manager_.get(); }
   base::MessageLoopForUI* loop() { return loop_.get(); }
 
@@ -508,9 +511,11 @@ std::vector<bool> RunAudioRecordInputPathTests() {
 class AudioAndroidInputTest : public AudioAndroidOutputTest,
                               public testing::WithParamInterface<bool> {
  public:
-  AudioAndroidInputTest() {}
+  AudioAndroidInputTest() : audio_input_stream_(NULL) {}
 
  protected:
+  AudioInputStream* ais() { return audio_input_stream_; }

[tommi (sloooow) - chröme, 2014/01/31 14:06:59]

I would keep audio_input_stream() in the spirit of "Function names, variable
names, and filenames should be descriptive; eschew abbreviation."

[henrika (OOO until Aug 14), 2014/01/31 14:19:05]

Will fix.

+
   AudioParameters GetInputStreamParameters() {
     AudioParameters input_params = audio_manager()->GetInputStreamParameters(
         AudioManagerBase::kDefaultDeviceId);
@@ -528,32 +533,58 @@ class AudioAndroidInputTest : public AudioAndroidOutputTest,
     return params;
   }
 
+  void MakeAISOnAudioThread(const AudioParameters& params) {
+    RunOnAudioThread(
+        base::Bind(&AudioAndroidInputTest::MakeInputStream,
+                   base::Unretained(this),
+                   params));
+  }
+
+  void OpenAndCloseAISOnAudioThread() {
+    RunOnAudioThread(
+        base::Bind(&AudioAndroidInputTest::OpenAndClose,
+                   base::Unretained(this)));
+  }
+
+  void OpenAndStartAISOnAudioThread(
+      AudioInputStream::AudioInputCallback* source) {
+    RunOnAudioThread(
+        base::Bind(&AudioAndroidInputTest::OpenAndStart,
+                   base::Unretained(this),
+                   source));
+  }
+
+  void StopAndCloseAISOnAudioThread() {
+    RunOnAudioThread(
+        base::Bind(&AudioAndroidInputTest::StopAndClose,
+                   base::Unretained(this)));
+  }
+
   void StartInputStreamCallbacks(const AudioParameters& params) {
     double expected_time_between_callbacks_ms =
         ExpectedTimeBetweenCallbacks(params);
     const int num_callbacks =
         (kCallbackTestTimeMs / expected_time_between_callbacks_ms);
-    AudioInputStream* stream = audio_manager()->MakeAudioInputStream(
-        params, AudioManagerBase::kDefaultDeviceId);
-    EXPECT_TRUE(stream);
+
+    MakeAISOnAudioThread(params);
 
     int count = 0;
     MockAudioInputCallback sink;
 
     EXPECT_CALL(sink,
-                OnData(stream, NotNull(), params.GetBytesPerBuffer(), _, _))
+                OnData(ais(), NotNull(), params.GetBytesPerBuffer(), _, _))
         .Times(AtLeast(num_callbacks))
         .WillRepeatedly(
              CheckCountAndPostQuitTask(&count, num_callbacks, loop()));
-    EXPECT_CALL(sink, OnError(stream)).Times(0);
+    EXPECT_CALL(sink, OnError(ais())).Times(0);
+
+    OpenAndStartAISOnAudioThread(&sink);
 
-    EXPECT_TRUE(stream->Open());
-    stream->Start(&sink);
     start_time_ = base::TimeTicks::Now();
     loop()->Run();
     end_time_ = base::TimeTicks::Now();
-    stream->Stop();
-    stream->Close();
+
+    StopAndCloseAISOnAudioThread();
 
     double average_time_between_callbacks_ms =
         AverageTimeBetweenCallbacks(num_callbacks);
@@ -567,8 +598,56 @@ class AudioAndroidInputTest : public AudioAndroidOutputTest,
               1.30 * expected_time_between_callbacks_ms);
   }
 
+  void MakeInputStream(const AudioParameters& params) {
+    DCHECK(audio_manager()->GetTaskRunner()->BelongsToCurrentThread());
+    audio_input_stream_ = audio_manager()->MakeAudioInputStream(
+        params, AudioManagerBase::kDefaultDeviceId);
+    EXPECT_TRUE(audio_input_stream_);
+  }
+
+  void OpenAndStart(AudioInputStream::AudioInputCallback* source) {
+    DCHECK(audio_manager()->GetTaskRunner()->BelongsToCurrentThread());
+    EXPECT_TRUE(ais()->Open());
+    ais()->Start(source);
+  }
+
+  void StopAndClose() {
+    DCHECK(audio_manager()->GetTaskRunner()->BelongsToCurrentThread());
+    ais()->Stop();
+    ais()->Close();
+  }
+
+  void OpenAndClose() {
+    DCHECK(audio_manager()->GetTaskRunner()->BelongsToCurrentThread());
+    EXPECT_TRUE(ais()->Open());
+    ais()->Close();
+  }
+
+  // Synchronously runs the provided callback/closure on the audio thread.
+  void RunOnAudioThread(const base::Closure& closure) {
+    if (!audio_manager()->GetTaskRunner()->BelongsToCurrentThread()) {
+      base::WaitableEvent event(false, false);
+      audio_manager()->GetTaskRunner()->PostTask(
+          FROM_HERE,
+          base::Bind(&AudioAndroidInputTest::RunOnAudioThreadImpl,
+                     base::Unretained(this),
+                     closure,
+                     &event));
+      event.Wait();
+    } else {
+      closure.Run();
+    }
+  }
+
+  void RunOnAudioThreadImpl(const base::Closure& closure,
+                            base::WaitableEvent* event) {
+    DCHECK(audio_manager()->GetTaskRunner()->BelongsToCurrentThread());
+    closure.Run();
+    event->Signal();
+  }
 
  private:
+  AudioInputStream* audio_input_stream_;
   DISALLOW_COPY_AND_ASSIGN(AudioAndroidInputTest);
 };
 
@@ -599,6 +678,9 @@ TEST_F(AudioAndroidOutputTest, IsAudioLowLatencySupported) {
 }
 
 // Verify input device enumeration.
+// GetAudioInputDeviceNames() is mainly called from a dedicated device thread
+// in Chrome but perform the test on the main thread here. The call will be
+// sent to the audio thread in AudioManagerAndroid::GetAudioInputDeviceNames().
 TEST_F(AudioAndroidInputTest, GetAudioInputDeviceNames) {
   if (!audio_manager()->HasAudioInputDevices())
     return;
@@ -619,10 +701,10 @@ TEST_F(AudioAndroidOutputTest, GetAudioOutputDeviceNames) {
 // Ensure that a default input stream can be created and closed.
 TEST_P(AudioAndroidInputTest, CreateAndCloseInputStream) {
   AudioParameters params = GetInputStreamParameters();
-  AudioInputStream* ais = audio_manager()->MakeAudioInputStream(
-      params, AudioManagerBase::kDefaultDeviceId);
-  EXPECT_TRUE(ais);
-  ais->Close();
+  MakeAISOnAudioThread(params);
+  RunOnAudioThread(
+      base::Bind(&AudioInputStream::Close,
+                 base::Unretained(ais())));
 }
 
 // Ensure that a default output stream can be created and closed.
@@ -640,11 +722,8 @@ TEST_F(AudioAndroidOutputTest, CreateAndCloseOutputStream) {
 // Ensure that a default input stream can be opened and closed.
 TEST_P(AudioAndroidInputTest, OpenAndCloseInputStream) {
   AudioParameters params = GetInputStreamParameters();
-  AudioInputStream* ais = audio_manager()->MakeAudioInputStream(
-      params, AudioManagerBase::kDefaultDeviceId);
-  EXPECT_TRUE(ais);
-  EXPECT_TRUE(ais->Open());
-  ais->Close();
+  MakeAISOnAudioThread(params);
+  OpenAndCloseAISOnAudioThread();
 }
 
 // Ensure that a default output stream can be opened and closed.
@@ -659,18 +738,15 @@ TEST_F(AudioAndroidOutputTest, OpenAndCloseOutputStream) {
 
 // Start input streaming using default input parameters and ensure that the
 // callback sequence is sane.
-// Disabled per crbug/337867
-TEST_P(AudioAndroidInputTest, DISABLED_StartInputStreamCallbacks) {
-  AudioParameters params = GetInputStreamParameters();
-  StartInputStreamCallbacks(params);
+TEST_P(AudioAndroidInputTest, StartInputStreamCallbacks) {
+  AudioParameters native_params = GetInputStreamParameters();
+  StartInputStreamCallbacks(native_params);
 }
 
 // Start input streaming using non default input parameters and ensure that the
 // callback sequence is sane. The only change we make in this test is to select
 // a 10ms buffer size instead of the default size.
-// TODO(henrika): possibly add support for more variations.
-// Disabled per crbug/337867
-TEST_P(AudioAndroidInputTest, DISABLED_StartInputStreamCallbacksNonDefaultParameters) {
+TEST_P(AudioAndroidInputTest, StartInputStreamCallbacksNonDefaultParameters) {
   AudioParameters native_params = GetInputStreamParameters();
   AudioParameters params(native_params.format(),
                          native_params.channel_layout(),
@@ -748,9 +824,7 @@ TEST_F(AudioAndroidOutputTest, DISABLED_RunOutputStreamWithFileAsSource) {
 TEST_P(AudioAndroidInputTest, DISABLED_RunSimplexInputStreamWithFileAsSink) {
   AudioParameters params = GetInputStreamParameters();
   VLOG(1) << params;
-  AudioInputStream* ais = audio_manager()->MakeAudioInputStream(
-      params, AudioManagerBase::kDefaultDeviceId);
-  EXPECT_TRUE(ais);
+  MakeAISOnAudioThread(params);
 
   std::string file_name = base::StringPrintf("out_simplex_%d_%d_%d.pcm",
                                              params.sample_rate(),
@@ -760,12 +834,10 @@ TEST_P(AudioAndroidInputTest, DISABLED_RunSimplexInputStreamWithFileAsSink) {
   base::WaitableEvent event(false, false);
   FileAudioSink sink(&event, params, file_name);
 
-  EXPECT_TRUE(ais->Open());
-  ais->Start(&sink);
+  OpenAndStartAISOnAudioThread(&sink);
   VLOG(0) << ">> Speak into the microphone to record audio...";
   EXPECT_TRUE(event.TimedWait(TestTimeouts::action_max_timeout()));
-  ais->Stop();
-  ais->Close();
+  StopAndCloseAISOnAudioThread();
 }
 
 // Same test as RunSimplexInputStreamWithFileAsSink but this time output