Fixed crash in AudioOutputController.

AudioOutputController must not call EventHandler after it has been
closed. Changing it so that lock_ is always locked whenever we call
handler_ to ensure that state_ != kClosed.
BUG=54939
TEST=unittests

Review URL: http://codereview.chromium.org/3308025

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

2 files changed, 20 insertions, 12 deletions

diff --git a/media/audio/audio_output_controller.cc b/media/audio/audio_output_controller.cc
index 66c91a3..6ff02aa 100644
--- a/media/audio/audio_output_controller.cc
+++ b/media/audio/audio_output_controller.cc
@@ -187,28 +187,28 @@ void AudioOutputController::DoCreate(AudioParameters params,
 void AudioOutputController::DoPlay() {
   DCHECK_EQ(message_loop_, MessageLoop::current());
 
-  State old_state;
-  // Update the |state_| to kPlaying.
   {
     AutoLock auto_lock(lock_);
     // We can start from created or paused state.
     if (state_ != kCreated && state_ != kPaused)
       return;
-    old_state = state_;
     state_ = kPlaying;
   }
 
   // We start the AudioOutputStream lazily.
   stream_->Start(this);
 
-  // Tell the event handler that we are now playing.
-  handler_->OnPlaying(this);
+  {
+    AutoLock auto_lock(lock_);
+    // Tell the event handler that we are now playing.
+    if (state_ != kClosed)
+      handler_->OnPlaying(this);
+  }
 }
 
 void AudioOutputController::DoPause() {
   DCHECK_EQ(message_loop_, MessageLoop::current());
 
-  // Sets the |state_| to kPaused so we don't draw more audio data.
   {
     AutoLock auto_lock(lock_);
     // We can pause from started state.
@@ -227,7 +227,11 @@ void AudioOutputController::DoPause() {
     sync_reader_->UpdatePendingBytes(kPauseMark);
   }
 
-  handler_->OnPaused(this);
+  {
+    AutoLock auto_lock(lock_);
+    if (state_ != kClosed)
+      handler_->OnPaused(this);
+  }
 }
 
 void AudioOutputController::DoFlush() {
@@ -274,7 +278,11 @@ void AudioOutputController::DoSetVolume(double volume) {
 
 void AudioOutputController::DoReportError(int code) {
   DCHECK_EQ(message_loop_, MessageLoop::current());
-  handler_->OnError(this, code);
+  {
+    AutoLock auto_lock(lock_);
+    if (state_ != kClosed)
+      handler_->OnError(this, code);
+  }
 }
 
 uint32 AudioOutputController::OnMoreData(AudioOutputStream* stream,
@@ -335,10 +343,6 @@ void AudioOutputController::SubmitOnMoreData_Locked() {
   uint32 pending_bytes = hardware_pending_bytes_ +
       push_source_.UnProcessedBytes();
 
-  // If we need more data then call the event handler to ask for more data.
-  // It is okay that we don't lock in this block because the parameters are
-  // correct and in the worst case we are just asking more data than needed.
-  AutoUnlock auto_unlock(lock_);
   handler_->OnMoreData(this, timestamp, pending_bytes);
 }
 
diff --git a/media/audio/audio_output_controller.h b/media/audio/audio_output_controller.h
index c260e58..e9ed321 100644
--- a/media/audio/audio_output_controller.h
+++ b/media/audio/audio_output_controller.h
@@ -175,6 +175,7 @@ class AudioOutputController
   // Helper method to submit a OnMoreData() call to the event handler.
   void SubmitOnMoreData_Locked();
 
+  // |handler_| may be called only if |state_| is not kClosed.
   EventHandler* handler_;
   AudioOutputStream* stream_;
 
@@ -188,6 +189,9 @@ class AudioOutputController
 
   uint32 hardware_pending_bytes_;
   base::Time last_callback_time_;
+
+  // The |lock_| must be acquired whenever we access |state_| or call
+  // |handler_|.
   Lock lock_;
 
   // PushSource role is to buffer and it's only used in regular latency mode.