Allow AudioRendererMixerInputs to be restarted after stopped.

We probably shouldn't allow this, but it's currently relied upon by the
WebAudio code and cleaning it up is a big deal: http://crbug.com/151051.

The AudioRendererSink interface does not specify that after Stop() the
sink can no longer be used.  All other sinks allow restart, so this is
just keeping with the pace.

In http://crrev.com/280502 I conflated the one-stop-only mechanics of
AudioRenderer:Stop() with those of AudioRendererSink::Stop().  This
patch reverts that mistake while keeping the fixes for the original
issue.

BUG=389204,390977
TEST=new unittest.

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

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

content/renderer/media/audio_renderer_mixer_manager_unittest.cc

@@ -133,20 +133,22 @@ TEST_F(AudioRendererMixerManagerTest, CreateInput) {
 
   // Create two mixer inputs and ensure this doesn't instantiate any mixers yet.
   EXPECT_EQ(mixer_count(), 0);
+  media::FakeAudioRenderCallback callback(0);
   scoped_refptr<media::AudioRendererMixerInput> input(
       manager_->CreateInput(kRenderViewId, kRenderFrameId));
+  input->Initialize(params, &callback);
   EXPECT_EQ(mixer_count(), 0);
+  media::FakeAudioRenderCallback another_callback(1);
   scoped_refptr<media::AudioRendererMixerInput> another_input(
       manager_->CreateInput(kAnotherRenderViewId, kAnotherRenderFrameId));
+  another_input->Initialize(params, &another_callback);
   EXPECT_EQ(mixer_count(), 0);
 
   // Implicitly test that AudioRendererMixerInput was provided with the expected
   // callbacks needed to acquire an AudioRendererMixer and remove it.
-  media::FakeAudioRenderCallback callback(0);
-  input->Initialize(params, &callback);
+  input->Start();
   EXPECT_EQ(mixer_count(), 1);
-  media::FakeAudioRenderCallback another_callback(1);
-  another_input->Initialize(params, &another_callback);
+  another_input->Start();
   EXPECT_EQ(mixer_count(), 2);
 
   // Destroying the inputs should destroy the mixers.

media/base/audio_renderer_mixer_input.cc

@@ -24,7 +24,6 @@ AudioRendererMixerInput::AudioRendererMixerInput(
 }
 
 AudioRendererMixerInput::~AudioRendererMixerInput() {
-  DCHECK(!callback_);
   DCHECK(!playing_);
   DCHECK(!mixer_);
 }
@@ -38,16 +37,16 @@ void AudioRendererMixerInput::Initialize(
   params_ = params;
   callback_ = callback;
   initialized_ = true;
-  mixer_ = get_mixer_cb_.Run(params_);
-
-  // Note: OnRenderError() may be called immediately after this call completes,
-  // so ensure |callback_| has been set first.
-  mixer_->AddErrorCallback(error_cb_);
 }
 
 void AudioRendererMixerInput::Start() {
   DCHECK(initialized_);
   DCHECK(!playing_);
+  DCHECK(!mixer_);
+  mixer_ = get_mixer_cb_.Run(params_);
+
+  // Note: OnRenderError() may be called immediately after this call returns.
+  mixer_->AddErrorCallback(error_cb_);
 }
 
 void AudioRendererMixerInput::Stop() {
@@ -58,17 +57,19 @@ void AudioRendererMixerInput::Stop() {
     playing_ = false;
   }
 
-  // Once Stop() is called the input can no longer be used.
-  if (callback_) {
+  if (mixer_) {
+    // TODO(dalecurtis): This is required so that |callback_| isn't called after
+    // Stop() by an error event since it may outlive this ref-counted object. We
+    // should instead have sane ownership semantics: http://crbug.com/151051
     mixer_->RemoveErrorCallback(error_cb_);
     remove_mixer_cb_.Run(params_);
     mixer_ = NULL;
-    callback_ = NULL;
   }
 }
 
 void AudioRendererMixerInput::Play() {
   DCHECK(initialized_);
+  DCHECK(mixer_);
 
   if (playing_)
     return;
@@ -79,6 +80,7 @@ void AudioRendererMixerInput::Play() {
 
 void AudioRendererMixerInput::Pause() {
   DCHECK(initialized_);
+  DCHECK(mixer_);
 
   if (!playing_)
     return;

media/base/audio_renderer_mixer_input_unittest.cc

@@ -28,7 +28,6 @@ class AudioRendererMixerInputTest : public testing::Test {
     CreateMixerInput();
     fake_callback_.reset(new FakeAudioRenderCallback(0));
     mixer_input_->Initialize(audio_parameters_, fake_callback_.get());
-    EXPECT_CALL(*this, RemoveMixer(testing::_));
     audio_bus_ = AudioBus::Create(audio_parameters_);
   }
 
@@ -49,6 +48,7 @@ class AudioRendererMixerInputTest : public testing::Test {
       mixer_.reset(new AudioRendererMixer(
           audio_parameters_, audio_parameters_, sink));
     }
+    EXPECT_CALL(*this, RemoveMixer(testing::_));
     return mixer_.get();
   }
 
@@ -109,4 +109,12 @@ TEST_F(AudioRendererMixerInputTest, StopBeforeInitializeOrStart) {
   mixer_input_->Stop();
 }
 
+// Test that Start() can be called after Stop().
+// TODO(dalecurtis): We shouldn't allow this.  See http://crbug.com/151051
+TEST_F(AudioRendererMixerInputTest, StartAfterStop) {
+  mixer_input_->Stop();
+  mixer_input_->Start();
+  mixer_input_->Stop();
+}
+
 }  // namespace media

media/base/audio_renderer_mixer_unittest.cc

@@ -396,8 +396,10 @@ TEST_P(AudioRendererMixerBehavioralTest, OnRenderError) {
 TEST_P(AudioRendererMixerBehavioralTest, OnRenderErrorPausedInput) {
   InitializeInputs(kMixerInputs);
 
-  for (size_t i = 0; i < mixer_inputs_.size(); ++i)
+  for (size_t i = 0; i < mixer_inputs_.size(); ++i) {
+    mixer_inputs_[i]->Start();
     EXPECT_CALL(*fake_callbacks_[i], OnRenderError()).Times(1);
+  }
 
   // Fire the error before attaching any inputs.  Ensure an error is recieved
   // even if the input is not connected.