Trying to solve crash in AudioInputController related to invalid EventHandler.

The AIC now NULLs out the media::AudioInputController::EventHandler in a thread safe manner to avoid any chance of accessing and invalid event handler. 

BUG=363990
TEST=WebRTC and WebSpeech demo applications.

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

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

5 files changed, 45 insertions, 22 deletions

diff --git a/content/browser/renderer_host/media/audio_input_renderer_host.cc b/content/browser/renderer_host/media/audio_input_renderer_host.cc
index 3e683fb..b536cf3 100644
--- a/content/browser/renderer_host/media/audio_input_renderer_host.cc
+++ b/content/browser/renderer_host/media/audio_input_renderer_host.cc
@@ -65,6 +65,7 @@ AudioInputRendererHost::AudioInputRendererHost(
           media::AudioLogFactory::AUDIO_INPUT_CONTROLLER)) {}
 
 AudioInputRendererHost::~AudioInputRendererHost() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DCHECK(audio_entries_.empty());
 }
 
@@ -131,8 +132,8 @@ void AudioInputRendererHost::DoCompleteCreation(
     return;
   }
 
-  if (!entry->controller->LowLatencyMode()) {
-    NOTREACHED() << "Only low-latency mode is supported.";
+  if (!entry->controller->SharedMemoryAndSyncSocketMode()) {
+    NOTREACHED() << "Only shared-memory/sync-socket mode is supported.";
     DeleteEntryOnError(entry, INVALID_LATENCY_MODE);
     return;
   }
diff --git a/content/browser/speech/speech_recognizer_impl.cc b/content/browser/speech/speech_recognizer_impl.cc
index b296cc0..3ba1f08 100644
--- a/content/browser/speech/speech_recognizer_impl.cc
+++ b/content/browser/speech/speech_recognizer_impl.cc
@@ -254,6 +254,7 @@ SpeechRecognizerImpl::recognition_engine() const {
 }
 
 SpeechRecognizerImpl::~SpeechRecognizerImpl() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   endpointer_.EndSession();
   if (audio_controller_.get()) {
     audio_controller_->Close(
diff --git a/media/audio/audio_input_controller.cc b/media/audio/audio_input_controller.cc
index 1e2180d..4cf948b 100644
--- a/media/audio/audio_input_controller.cc
+++ b/media/audio/audio_input_controller.cc
@@ -189,14 +189,16 @@ void AudioInputController::DoCreateForStream(
   stream_ = stream_to_control;
 
   if (!stream_) {
-    handler_->OnError(this, STREAM_CREATE_ERROR);
+    if (handler_)
+      handler_->OnError(this, STREAM_CREATE_ERROR);
     return;
   }
 
   if (stream_ && !stream_->Open()) {
     stream_->Close();
     stream_ = NULL;
-    handler_->OnError(this, STREAM_OPEN_ERROR);
+    if (handler_)
+      handler_->OnError(this, STREAM_OPEN_ERROR);
     return;
   }
 
@@ -222,7 +224,8 @@ void AudioInputController::DoCreateForStream(
   }
 
   state_ = CREATED;
-  handler_->OnCreated(this);
+  if (handler_)
+    handler_->OnCreated(this);
 
   if (user_input_monitor_) {
     user_input_monitor_->EnableKeyPressMonitoring();
@@ -249,7 +252,8 @@ void AudioInputController::DoRecord() {
   }
 
   stream_->Start(this);
-  handler_->OnRecording(this);
+  if (handler_)
+    handler_->OnRecording(this);
 }
 
 void AudioInputController::DoClose() {
@@ -262,10 +266,10 @@ void AudioInputController::DoClose() {
   // Delete the timer on the same thread that created it.
   no_data_timer_.reset();
 
-  DoStopCloseAndClearStream(NULL);
+  DoStopCloseAndClearStream();
   SetDataIsActive(false);
 
-  if (LowLatencyMode())
+  if (SharedMemoryAndSyncSocketMode())
     sync_writer_->Close();
 
   if (user_input_monitor_)
@@ -276,7 +280,8 @@ void AudioInputController::DoClose() {
 
 void AudioInputController::DoReportError() {
   DCHECK(task_runner_->BelongsToCurrentThread());
-  handler_->OnError(this, STREAM_ERROR);
+  if (handler_)
+    handler_->OnError(this, STREAM_ERROR);
 }
 
 void AudioInputController::DoSetVolume(double volume) {
@@ -320,7 +325,8 @@ void AudioInputController::DoCheckForNoData() {
     // The data-is-active marker will be false only if it has been more than
     // one second since a data packet was recorded. This can happen if a
     // capture device has been removed or disabled.
-    handler_->OnError(this, NO_DATA_ERROR);
+    if (handler_)
+      handler_->OnError(this, NO_DATA_ERROR);
   }
 
   // Mark data as non-active. The flag will be re-enabled in OnData() each
@@ -359,14 +365,30 @@ void AudioInputController::OnData(AudioInputStream* stream,
   // DoCheckForNoData() does not report an error to the event handler.
   SetDataIsActive(true);
 
-  // Use SyncSocket if we are in a low-latency mode.
-  if (LowLatencyMode()) {
+  // Use SharedMemory and SyncSocket if the client has created a SyncWriter.
+  // Used by all low-latency clients except WebSpeech.
+  if (SharedMemoryAndSyncSocketMode()) {
     sync_writer_->Write(data, size, volume, key_pressed);
     sync_writer_->UpdateRecordedBytes(hardware_delay_bytes);
     return;
   }
 
-  handler_->OnData(this, data, size);
+  // TODO(henrika): Investigate if we can avoid the extra copy here.
+  // (see http://crbug.com/249316 for details). AFAIK, this scope is only
+  // active for WebSpeech clients.
+  scoped_ptr<uint8[]> audio_data(new uint8[size]);
+  memcpy(audio_data.get(), data, size);
+
+  // Ownership of the audio buffer will be with the callback until it is run,
+  // when ownership is passed to the callback function.
+  task_runner_->PostTask(FROM_HERE, base::Bind(
+      &AudioInputController::DoOnData, this, base::Passed(&audio_data), size));
+}
+
+void AudioInputController::DoOnData(scoped_ptr<uint8[]> data, uint32 size) {
+  DCHECK(task_runner_->BelongsToCurrentThread());
+  if (handler_)
+    handler_->OnData(this, data.get(), size);
 }
 
 void AudioInputController::OnError(AudioInputStream* stream) {
@@ -375,8 +397,7 @@ void AudioInputController::OnError(AudioInputStream* stream) {
       &AudioInputController::DoReportError, this));
 }
 
-void AudioInputController::DoStopCloseAndClearStream(
-    base::WaitableEvent* done) {
+void AudioInputController::DoStopCloseAndClearStream() {
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   // Allow calling unconditionally and bail if we don't have a stream to close.
@@ -386,9 +407,8 @@ void AudioInputController::DoStopCloseAndClearStream(
     stream_ = NULL;
   }
 
-  // Should be last in the method, do not touch "this" from here on.
-  if (done != NULL)
-    done->Signal();
+  // The event handler should not be touched after the stream has been closed.
+  handler_ = NULL;
 }
 
 void AudioInputController::SetDataIsActive(bool enabled) {
diff --git a/media/audio/audio_input_controller.h b/media/audio/audio_input_controller.h
index 566e2a6..ac1a690 100644
--- a/media/audio/audio_input_controller.h
+++ b/media/audio/audio_input_controller.h
@@ -224,7 +224,7 @@ class MEDIA_EXPORT AudioInputController
                       uint32 hardware_delay_bytes, double volume) OVERRIDE;
   virtual void OnError(AudioInputStream* stream) OVERRIDE;
 
-  bool LowLatencyMode() const { return sync_writer_ != NULL; }
+  bool SharedMemoryAndSyncSocketMode() const { return sync_writer_ != NULL; }
 
  protected:
   friend class base::RefCountedThreadSafe<AudioInputController>;
@@ -251,14 +251,14 @@ class MEDIA_EXPORT AudioInputController
   void DoReportError();
   void DoSetVolume(double volume);
   void DoSetAutomaticGainControl(bool enabled);
+  void DoOnData(scoped_ptr<uint8[]> data, uint32 size);
 
   // Method which ensures that OnError() is triggered when data recording
   // times out. Called on the audio thread.
   void DoCheckForNoData();
 
   // Helper method that stops, closes, and NULL:s |*stream_|.
-  // Signals event when done if the event is not NULL.
-  void DoStopCloseAndClearStream(base::WaitableEvent* done);
+  void DoStopCloseAndClearStream();
 
   void SetDataIsActive(bool enabled);
   bool GetDataIsActive();
diff --git a/media/audio/win/wavein_input_win.cc b/media/audio/win/wavein_input_win.cc
index 0577125..43a356f 100644
--- a/media/audio/win/wavein_input_win.cc
+++ b/media/audio/win/wavein_input_win.cc
@@ -224,7 +224,8 @@ bool PCMWaveInAudioInputStream::GetAutomaticGainControl() {
 
 void PCMWaveInAudioInputStream::HandleError(MMRESULT error) {
   DLOG(WARNING) << "PCMWaveInAudio error " << error;
-  callback_->OnError(this);
+  if (callback_)
+    callback_->OnError(this);
 }
 
 void PCMWaveInAudioInputStream::QueueNextPacket(WAVEHDR *buffer) {