Defer input stream start around suspend and resume.

As with output streams on OSX, we should defer input stream starts
around suspend and resume.

UMA statistic rates for M33 -> M34 show us going from 0.54% to 0.30%
failures! Lets see if we can go even further.

BUG=160920,361999
TEST=none

Review URL: https://codereview.chromium.org/233823003

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

9 files changed, 81 insertions, 26 deletions

diff --git a/media/audio/audio_input_unittest.cc b/media/audio/audio_input_unittest.cc
index 1573abc..f972e44 100644
--- a/media/audio/audio_input_unittest.cc
+++ b/media/audio/audio_input_unittest.cc
@@ -6,6 +6,7 @@
 #include "base/environment.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/threading/platform_thread.h"
 #include "media/audio/audio_io.h"
@@ -52,9 +53,13 @@ class AudioInputTest : public testing::Test {
       message_loop_(base::MessageLoop::TYPE_UI),
       audio_manager_(AudioManager::CreateForTesting()),
       audio_input_stream_(NULL) {
+    // Wait for the AudioManager to finish any initialization on the audio loop.
+    base::RunLoop().RunUntilIdle();
   }
 
-  virtual ~AudioInputTest() {}
+  virtual ~AudioInputTest() {
+    base::RunLoop().RunUntilIdle();
+  }
 
  protected:
   AudioManager* audio_manager() { return audio_manager_.get(); }
diff --git a/media/audio/mac/audio_auhal_mac.cc b/media/audio/mac/audio_auhal_mac.cc
index abbb4bf..fc44b4e 100644
--- a/media/audio/mac/audio_auhal_mac.cc
+++ b/media/audio/mac/audio_auhal_mac.cc
@@ -152,7 +152,7 @@ void AUHALStream::Start(AudioSourceCallback* callback) {
   }
 
   // Check if we should defer Start() for http://crbug.com/160920.
-  if (manager_->ShouldDeferOutputStreamStart()) {
+  if (manager_->ShouldDeferStreamStart()) {
     // Use a cancellable closure so that if Stop() is called before Start()
     // actually runs, we can cancel the pending start.
     DCHECK(deferred_start_cb_.IsCancelled());
diff --git a/media/audio/mac/audio_input_mac.cc b/media/audio/mac/audio_input_mac.cc
index 8f98eed..aeeac43 100644
--- a/media/audio/mac/audio_input_mac.cc
+++ b/media/audio/mac/audio_input_mac.cc
@@ -9,13 +9,12 @@
 #include "base/basictypes.h"
 #include "base/logging.h"
 #include "base/mac/mac_logging.h"
-#include "media/audio/audio_manager_base.h"
-
+#include "media/audio/mac/audio_manager_mac.h"
 
 namespace media {
 
 PCMQueueInAudioInputStream::PCMQueueInAudioInputStream(
-    AudioManagerBase* manager, const AudioParameters& params)
+    AudioManagerMac* manager, const AudioParameters& params)
     : manager_(manager),
       callback_(NULL),
       audio_queue_(NULL),
@@ -65,6 +64,22 @@ void PCMQueueInAudioInputStream::Start(AudioInputCallback* callback) {
   DLOG_IF(ERROR, !audio_queue_) << "Open() has not been called successfully";
   if (callback_ || !audio_queue_)
     return;
+
+  // Check if we should defer Start() for http://crbug.com/160920.
+  if (manager_->ShouldDeferStreamStart()) {
+    // Use a cancellable closure so that if Stop() is called before Start()
+    // actually runs, we can cancel the pending start.
+    DCHECK(deferred_start_cb_.IsCancelled());
+    deferred_start_cb_.Reset(base::Bind(
+        &PCMQueueInAudioInputStream::Start, base::Unretained(this), callback));
+    manager_->GetTaskRunner()->PostDelayedTask(
+        FROM_HERE,
+        deferred_start_cb_.callback(),
+        base::TimeDelta::FromSeconds(
+            AudioManagerMac::kStartDelayInSecsForPowerEvents));
+    return;
+  }
+
   callback_ = callback;
   OSStatus err = AudioQueueStart(audio_queue_, NULL);
   if (err != noErr) {
@@ -75,6 +90,7 @@ void PCMQueueInAudioInputStream::Start(AudioInputCallback* callback) {
 }
 
 void PCMQueueInAudioInputStream::Stop() {
+  deferred_start_cb_.Cancel();
   if (!audio_queue_ || !started_)
     return;
 
diff --git a/media/audio/mac/audio_input_mac.h b/media/audio/mac/audio_input_mac.h
index 77eb65b..e8f77c7 100644
--- a/media/audio/mac/audio_input_mac.h
+++ b/media/audio/mac/audio_input_mac.h
@@ -8,6 +8,7 @@
 #include <AudioToolbox/AudioFormat.h>
 #include <AudioToolbox/AudioQueue.h>
 
+#include "base/cancelable_callback.h"
 #include "base/compiler_specific.h"
 #include "base/time/time.h"
 #include "media/audio/audio_io.h"
@@ -15,14 +16,14 @@
 
 namespace media {
 
-class AudioManagerBase;
+class AudioManagerMac;
 
 // Implementation of AudioInputStream for Mac OS X using the audio queue service
 // present in OS 10.5 and later. Design reflects PCMQueueOutAudioOutputStream.
 class PCMQueueInAudioInputStream : public AudioInputStream {
  public:
   // Parameters as per AudioManager::MakeAudioInputStream.
-  PCMQueueInAudioInputStream(AudioManagerBase* manager,
+  PCMQueueInAudioInputStream(AudioManagerMac* manager,
                              const AudioParameters& params);
   virtual ~PCMQueueInAudioInputStream();
 
@@ -66,7 +67,7 @@ class PCMQueueInAudioInputStream : public AudioInputStream {
   static const int kNumberBuffers = 3;
 
   // Manager that owns this stream, used for closing down.
-  AudioManagerBase* manager_;
+  AudioManagerMac* manager_;
   // We use the callback mostly to periodically supply the recorded audio data.
   AudioInputCallback* callback_;
   // Structure that holds the stream format details such as bitrate.
@@ -79,6 +80,8 @@ class PCMQueueInAudioInputStream : public AudioInputStream {
   bool started_;
   // Used to determine if we need to slow down |callback_| calls.
   base::TimeTicks last_fill_;
+  // Used to defer Start() to workaround http://crbug.com/160920.
+  base::CancelableClosure deferred_start_cb_;
 
   DISALLOW_COPY_AND_ASSIGN(PCMQueueInAudioInputStream);
 };
diff --git a/media/audio/mac/audio_low_latency_input_mac.cc b/media/audio/mac/audio_low_latency_input_mac.cc
index 5623bce..5b78a80 100644
--- a/media/audio/mac/audio_low_latency_input_mac.cc
+++ b/media/audio/mac/audio_low_latency_input_mac.cc
@@ -274,6 +274,22 @@ void AUAudioInputStream::Start(AudioInputCallback* callback) {
   DLOG_IF(ERROR, !audio_unit_) << "Open() has not been called successfully";
   if (started_ || !audio_unit_)
     return;
+
+  // Check if we should defer Start() for http://crbug.com/160920.
+  if (manager_->ShouldDeferStreamStart()) {
+    // Use a cancellable closure so that if Stop() is called before Start()
+    // actually runs, we can cancel the pending start.
+    DCHECK(deferred_start_cb_.IsCancelled());
+    deferred_start_cb_.Reset(base::Bind(
+        &AUAudioInputStream::Start, base::Unretained(this), callback));
+    manager_->GetTaskRunner()->PostDelayedTask(
+        FROM_HERE,
+        deferred_start_cb_.callback(),
+        base::TimeDelta::FromSeconds(
+            AudioManagerMac::kStartDelayInSecsForPowerEvents));
+    return;
+  }
+
   sink_ = callback;
   StartAgc();
   OSStatus result = AudioOutputUnitStart(audio_unit_);
diff --git a/media/audio/mac/audio_low_latency_input_mac.h b/media/audio/mac/audio_low_latency_input_mac.h
index 04592d2..ae0c405 100644
--- a/media/audio/mac/audio_low_latency_input_mac.h
+++ b/media/audio/mac/audio_low_latency_input_mac.h
@@ -39,7 +39,7 @@
 #include <AudioUnit/AudioUnit.h>
 #include <CoreAudio/CoreAudio.h>
 
-#include "base/atomicops.h"
+#include "base/cancelable_callback.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/synchronization/lock.h"
 #include "media/audio/agc_audio_stream.h"
@@ -162,6 +162,9 @@ class AUAudioInputStream : public AgcAudioStream<AudioInputStream> {
   // OnData() callbacks where each callback contains this amount of bytes.
   int requested_size_bytes_;
 
+  // Used to defer Start() to workaround http://crbug.com/160920.
+  base::CancelableClosure deferred_start_cb_;
+
   DISALLOW_COPY_AND_ASSIGN(AUAudioInputStream);
 };
 
diff --git a/media/audio/mac/audio_low_latency_input_mac_unittest.cc b/media/audio/mac/audio_low_latency_input_mac_unittest.cc
index e80cbcd..fdd7c05 100644
--- a/media/audio/mac/audio_low_latency_input_mac_unittest.cc
+++ b/media/audio/mac/audio_low_latency_input_mac_unittest.cc
@@ -5,6 +5,7 @@
 #include "base/basictypes.h"
 #include "base/environment.h"
 #include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
 #include "base/test/test_timeouts.h"
 #include "base/threading/platform_thread.h"
 #include "media/audio/audio_io.h"
@@ -22,9 +23,9 @@ using ::testing::NotNull;
 
 namespace media {
 
-ACTION_P3(CheckCountAndPostQuitTask, count, limit, loop) {
+ACTION_P4(CheckCountAndPostQuitTask, count, limit, loop, closure) {
   if (++*count >= limit) {
-    loop->PostTask(FROM_HERE, base::MessageLoop::QuitClosure());
+    loop->PostTask(FROM_HERE, closure);
   }
 }
 
@@ -93,8 +94,16 @@ class WriteToFileAudioSink : public AudioInputStream::AudioInputCallback {
 
 class MacAudioInputTest : public testing::Test {
  protected:
-  MacAudioInputTest() : audio_manager_(AudioManager::CreateForTesting()) {}
-  virtual ~MacAudioInputTest() {}
+  MacAudioInputTest()
+      : message_loop_(base::MessageLoop::TYPE_UI),
+        audio_manager_(AudioManager::CreateForTesting()) {
+    // Wait for the AudioManager to finish any initialization on the audio loop.
+    base::RunLoop().RunUntilIdle();
+  }
+
+  virtual ~MacAudioInputTest() {
+    base::RunLoop().RunUntilIdle();
+  }
 
   // Convenience method which ensures that we are not running on the build
   // bots and that at least one valid input device can be found.
@@ -132,6 +141,7 @@ class MacAudioInputTest : public testing::Test {
     return ais;
   }
 
+  base::MessageLoop message_loop_;
   scoped_ptr<AudioManager> audio_manager_;
 };
 
@@ -209,7 +219,6 @@ TEST_F(MacAudioInputTest, AUAudioInputStreamVerifyMonoRecording) {
     return;
 
   int count = 0;
-  base::MessageLoopForUI loop;
 
   // Create an audio input stream which records in mono.
   AudioInputStream* ais = CreateAudioInputStream(CHANNEL_LAYOUT_MONO);
@@ -225,11 +234,13 @@ TEST_F(MacAudioInputTest, AUAudioInputStreamVerifyMonoRecording) {
   // We use 10ms packets and will run the test until ten packets are received.
   // All should contain valid packets of the same size and a valid delay
   // estimate.
+  base::RunLoop run_loop;
   EXPECT_CALL(sink, OnData(ais, NotNull(), bytes_per_packet, _, _))
       .Times(AtLeast(10))
-      .WillRepeatedly(CheckCountAndPostQuitTask(&count, 10, &loop));
+      .WillRepeatedly(CheckCountAndPostQuitTask(
+          &count, 10, &message_loop_, run_loop.QuitClosure()));
   ais->Start(&sink);
-  loop.Run();
+  run_loop.Run();
   ais->Stop();
   ais->Close();
 }
@@ -240,7 +251,6 @@ TEST_F(MacAudioInputTest, AUAudioInputStreamVerifyStereoRecording) {
     return;
 
   int count = 0;
-  base::MessageLoopForUI loop;
 
   // Create an audio input stream which records in stereo.
   AudioInputStream* ais = CreateAudioInputStream(CHANNEL_LAYOUT_STEREO);
@@ -263,11 +273,13 @@ TEST_F(MacAudioInputTest, AUAudioInputStreamVerifyStereoRecording) {
   // parameter #4 does no longer pass. I am removing this restriction here to
   // ensure that we can land the patch but will revisit this test again when
   // more analysis of the delay estimates are done.
+  base::RunLoop run_loop;
   EXPECT_CALL(sink, OnData(ais, NotNull(), bytes_per_packet, _, _))
       .Times(AtLeast(10))
-      .WillRepeatedly(CheckCountAndPostQuitTask(&count, 10, &loop));
+      .WillRepeatedly(CheckCountAndPostQuitTask(
+          &count, 10, &message_loop_, run_loop.QuitClosure()));
   ais->Start(&sink);
-  loop.Run();
+  run_loop.Run();
   ais->Stop();
   ais->Close();
 }
diff --git a/media/audio/mac/audio_manager_mac.cc b/media/audio/mac/audio_manager_mac.cc
index 60c8a3e..9d27d1a 100644
--- a/media/audio/mac/audio_manager_mac.cc
+++ b/media/audio/mac/audio_manager_mac.cc
@@ -242,7 +242,7 @@ class AudioManagerMac::AudioPowerObserver : public base::PowerObserver {
     base::PowerMonitor::Get()->RemoveObserver(this);
   }
 
-  bool ShouldDeferOutputStreamStart() {
+  bool ShouldDeferStreamStart() {
     DCHECK(thread_checker_.CalledOnValidThread());
     // Start() should be deferred if the system is in the middle of a suspend or
     // has recently started the process of resuming.
@@ -763,9 +763,9 @@ int AudioManagerMac::ChooseBufferSize(int output_sample_rate) {
   return buffer_size;
 }
 
-bool AudioManagerMac::ShouldDeferOutputStreamStart() {
+bool AudioManagerMac::ShouldDeferStreamStart() {
   DCHECK(GetTaskRunner()->BelongsToCurrentThread());
-  return power_observer_->ShouldDeferOutputStreamStart();
+  return power_observer_->ShouldDeferStreamStart();
 }
 
 void AudioManagerMac::ReleaseOutputStream(AudioOutputStream* stream) {
diff --git a/media/audio/mac/audio_manager_mac.h b/media/audio/mac/audio_manager_mac.h
index 7cc1a63f..490b0b6 100644
--- a/media/audio/mac/audio_manager_mac.h
+++ b/media/audio/mac/audio_manager_mac.h
@@ -69,11 +69,11 @@ class MEDIA_EXPORT AudioManagerMac : public AudioManagerBase {
   // As a workaround we delay Start() when it occurs after suspend and for a
   // small amount of time after resume.
   //
-  // Output streams should consult ShouldDeferOutputStreamStart() and if true
-  // check the value again after |kStartDelayInSecsForPowerEvents| has elapsed.
-  // If false, the stream may be started immediately.
+  // Streams should consult ShouldDeferStreamStart() and if true check the value
+  // again after |kStartDelayInSecsForPowerEvents| has elapsed. If false, the
+  // stream may be started immediately.
   enum { kStartDelayInSecsForPowerEvents = 1 };
-  bool ShouldDeferOutputStreamStart();
+  bool ShouldDeferStreamStart();
 
  protected:
   virtual ~AudioManagerMac();