Adding audio unit tests for Android

media/audio/android/opensles_input.cc

Index: media/audio/android/opensles_input.cc
diff --git a/media/audio/android/opensles_input.cc b/media/audio/android/opensles_input.cc
index 15c3eac3726389bb7f8d0a7f37ee4b8d35a82f91..815c92f71adbb41ec985227069ac2d091eff9b7d 100644
--- a/media/audio/android/opensles_input.cc
+++ b/media/audio/android/opensles_input.cc
@@ -27,6 +27,7 @@ OpenSLESInputStream::OpenSLESInputStream(AudioManagerAndroid* audio_manager,
       active_queue_(0),
       buffer_size_bytes_(0),
       started_(false) {
+  DVLOG(2) << "OpenSLESInputStream::OpenSLESInputStream()";
   format_.formatType = SL_DATAFORMAT_PCM;
   format_.numChannels = static_cast<SLuint32>(params.channels());
   // Provides sampling rate in milliHertz to OpenSLES.
@@ -47,6 +48,8 @@ OpenSLESInputStream::OpenSLESInputStream(AudioManagerAndroid* audio_manager,
 }
 
 OpenSLESInputStream::~OpenSLESInputStream() {
+  DVLOG(2) << "OpenSLESInputStream::~OpenSLESInputStream()";
+  DCHECK(thread_checker_.CalledOnValidThread());

[tommi (sloooow) - chröme, 2013/08/29 09:56:27]

nice to have the thread checker in here.

[henrika (OOO until Aug 14), 2013/08/29 11:59:07]

Thanks. It was a really smart guy who suggested it ;-)

   DCHECK(!recorder_object_.Get());
   DCHECK(!engine_object_.Get());
   DCHECK(!recorder_);
@@ -55,6 +58,8 @@ OpenSLESInputStream::~OpenSLESInputStream() {
 }
 
 bool OpenSLESInputStream::Open() {
+  DVLOG(2) << "OpenSLESInputStream::Open()";
+  DCHECK(thread_checker_.CalledOnValidThread());
   if (engine_object_.Get())
     return false;
 
@@ -67,16 +72,20 @@ bool OpenSLESInputStream::Open() {
 }
 
 void OpenSLESInputStream::Start(AudioInputCallback* callback) {
+  DVLOG(2) << "OpenSLESInputStream::Start()";
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(callback);
   DCHECK(recorder_);
   DCHECK(simple_buffer_queue_);
   if (started_)

[tommi (sloooow) - chröme, 2013/08/29 09:56:27]

this needs to be checked under the lock

[henrika (OOO until Aug 14), 2013/08/29 11:59:07]

Done.

     return;
-
-  // Enable the flags before streaming.
-  callback_ = callback;
-  active_queue_ = 0;
-  started_ = true;
+  {
+    base::AutoLock lock(lock_);
+    // Enable the flags before streaming.
+    callback_ = callback;
+    active_queue_ = 0;
+    started_ = true;
+  }
 
   SLresult err = SL_RESULT_UNKNOWN_ERROR;
   // Enqueues |kNumOfQueuesInBuffer| zero buffers to get the ball rolling.
@@ -98,6 +107,8 @@ void OpenSLESInputStream::Start(AudioInputCallback* callback) {
 }
 
 void OpenSLESInputStream::Stop() {
+  DVLOG(2) << "OpenSLESInputStream::Stop()";
+  DCHECK(thread_checker_.CalledOnValidThread());
   if (!started_)

[tommi (sloooow) - chröme, 2013/08/29 09:56:27]

need to hold lock.

[henrika (OOO until Aug 14), 2013/08/29 11:59:07]

Done.

     return;
 
@@ -105,23 +116,38 @@ void OpenSLESInputStream::Stop() {
   LOG_ON_FAILURE_AND_RETURN(
       (*recorder_)->SetRecordState(recorder_,
                                    SL_RECORDSTATE_STOPPED));
+  {
+    base::AutoLock lock(lock_);
 
-  // Clear the buffer queue to get rid of old data when resuming recording.
-  LOG_ON_FAILURE_AND_RETURN(
-      (*simple_buffer_queue_)->Clear(simple_buffer_queue_));
+    // Clear the buffer queue to get rid of old data when resuming recording.
+    LOG_ON_FAILURE_AND_RETURN(
+        (*simple_buffer_queue_)->Clear(simple_buffer_queue_));
 
-  started_ = false;
+    started_ = false;
+  }
 }
 
 void OpenSLESInputStream::Close() {
+  DVLOG(2) << "OpenSLESInputStream::Close()";
+  DCHECK(thread_checker_.CalledOnValidThread());
   // Stop the stream if it is still recording.
   Stop();
+  {
+    base::AutoLock lock(lock_);
+    if (callback_) {
+      callback_->OnClose(this);
+      callback_ = NULL;
+    }
+  }
 
   // Explicitly free the player objects and invalidate their associated
   // interfaces. They have to be done in the correct order.
   recorder_object_.Reset();
   engine_object_.Reset();

[tommi (sloooow) - chröme, 2013/08/29 09:56:27]

If I understand things correctly, resetting the engine should be done last since
it actually deletes the object that implements the interfaces.  As is, you've
here deleted the engine object but simple_buffer_queue_ is still non-NULL but
points to a deleted object (i.e. use after free problem).

[henrika (OOO until Aug 14), 2013/08/29 11:59:07]

Good point, thanks. Analyzed how things are created and rewrote.

-  simple_buffer_queue_ = NULL;
+  {
+    base::AutoLock lock(lock_);
+    simple_buffer_queue_ = NULL;

[tommi (sloooow) - chröme, 2013/08/29 09:56:27]

here you actually do grab the lock before changing the value... this doesn't
mean anything though unless you do the same in all other places that use the
pointer.

[henrika (OOO until Aug 14), 2013/08/29 11:59:07]

Done.

+  }
   recorder_ = NULL;
 
   ReleaseAudioBuffer();
@@ -153,6 +179,7 @@ bool OpenSLESInputStream::GetAutomaticGainControl() {
 }
 
 bool OpenSLESInputStream::CreateRecorder() {
+  DCHECK(thread_checker_.CalledOnValidThread());

[tommi (sloooow) - chröme, 2013/08/29 09:56:27]

We should also dcheck that all interface and object pointers are NULL when this
is called.

If this is considered part of the object's construction sequence, it is OK to
not grab the lock (since no other threads should have access to them) but DCHECK
that all member variables are in their newly-constructed state.

[henrika (OOO until Aug 14), 2013/08/29 11:59:07]

Done.

   // Initializes the engine object with specific option. After working with the
   // object, we need to free the object and its resources.
   SLEngineOption option[] = {
@@ -265,6 +292,7 @@ void OpenSLESInputStream::SimpleBufferQueueCallback(
 }
 
 void OpenSLESInputStream::ReadBufferQueue() {
+  base::AutoLock lock(lock_);
   if (!started_)
     return;
 
@@ -287,6 +315,7 @@ void OpenSLESInputStream::ReadBufferQueue() {
 }
 
 void OpenSLESInputStream::SetupAudioBuffer() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!audio_data_[0]);
   for (int i = 0; i < kNumOfQueuesInBuffer; ++i) {
     audio_data_[i] = new uint8[buffer_size_bytes_];
@@ -294,6 +323,7 @@ void OpenSLESInputStream::SetupAudioBuffer() {
 }
 
 void OpenSLESInputStream::ReleaseAudioBuffer() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   if (audio_data_[0]) {
     for (int i = 0; i < kNumOfQueuesInBuffer; ++i) {
       delete [] audio_data_[i];