Merge 186855 "Stop OnMoreDataConverter during StartStream() fail..."

> Stop OnMoreDataConverter during StartStream() failure.
> 
> On StartStream() failure we will never get a Stop() call, so we end
> up with an OnMoreDataConverter left in the started state.  There was
> a test for this, but a typo caused it to never run for
> AudioOutputResampler :-/
> 
> BUG=179986
> TEST=media_unittests
> 
> Review URL: https://codereview.chromium.org/12544014

TBR=dalecurtis@google.com
Review URL: https://codereview.chromium.org/12632017

git-svn-id: svn://svn.chromium.org/chrome/branches/1410/src@188223 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 19 insertions, 5 deletions

diff --git a/media/audio/audio_output_proxy.cc b/media/audio/audio_output_proxy.cc
index 3609079..b3818e6 100644
--- a/media/audio/audio_output_proxy.cc
+++ b/media/audio/audio_output_proxy.cc
@@ -37,7 +37,11 @@ bool AudioOutputProxy::Open() {
 
 void AudioOutputProxy::Start(AudioSourceCallback* callback) {
   DCHECK(CalledOnValidThread());
-  DCHECK_EQ(state_, kOpened);
+
+  // We need to support both states since the callback may not handle OnError()
+  // immediately (or at all).  It's also possible for subsequent StartStream()
+  // calls to succeed after failing, so we allow it to be called again.
+  DCHECK(state_ == kOpened || state_ == kStartError);
 
   if (!dispatcher_->StartStream(callback, this)) {
     state_ = kStartError;
diff --git a/media/audio/audio_output_proxy_unittest.cc b/media/audio/audio_output_proxy_unittest.cc
index 9b1599a..ba4681d 100644
--- a/media/audio/audio_output_proxy_unittest.cc
+++ b/media/audio/audio_output_proxy_unittest.cc
@@ -414,7 +414,7 @@ class AudioOutputProxyTest : public testing::Test {
     EXPECT_CALL(stream, Close())
         .Times(1);
 
-    AudioOutputProxy* proxy = new AudioOutputProxy(dispatcher_impl_);
+    AudioOutputProxy* proxy = new AudioOutputProxy(dispatcher);
     EXPECT_TRUE(proxy->Open());
 
     // Simulate a delay.
@@ -427,13 +427,19 @@ class AudioOutputProxyTest : public testing::Test {
 
     // |stream| is closed at this point. Start() should reopen it again.
     EXPECT_CALL(manager(), MakeAudioOutputStream(_))
-        .WillOnce(Return(reinterpret_cast<AudioOutputStream*>(NULL)));
+        .Times(2)
+        .WillRepeatedly(Return(reinterpret_cast<AudioOutputStream*>(NULL)));
 
     EXPECT_CALL(callback_, OnError(_, _))
-        .Times(1);
+        .Times(2);
 
     proxy->Start(&callback_);
 
+    // Double Start() in the error case should be allowed since it's possible a
+    // callback may not have had time to process the OnError() in between.
+    proxy->Stop();
+    proxy->Start(&callback_);
+
     Mock::VerifyAndClear(&callback_);
 
     proxy->Close();
diff --git a/media/audio/audio_output_resampler.cc b/media/audio/audio_output_resampler.cc
index 4ad1188..3833b11 100644
--- a/media/audio/audio_output_resampler.cc
+++ b/media/audio/audio_output_resampler.cc
@@ -245,8 +245,12 @@ bool AudioOutputResampler::StartStream(
   } else {
     resampler_callback = it->second;
   }
+
   resampler_callback->Start(callback);
-  return dispatcher_->StartStream(resampler_callback, stream_proxy);
+  bool result = dispatcher_->StartStream(resampler_callback, stream_proxy);
+  if (!result)
+    resampler_callback->Stop();
+  return result;
 }
 
 void AudioOutputResampler::StreamVolumeSet(AudioOutputProxy* stream_proxy,