Refactoring of chrome speech recognition architecture (CL1.3)

content/browser/speech/speech_recognition_manager_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/speech/speech_recognition_manager_impl.h"
 
 #include "base/bind.h"
 #include "content/browser/browser_main_loop.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/speech/input_tag_speech_dispatcher_host.h"
@@ skipping 185 matching lines @@
     NOTREACHED();
     return;
   }
 
   // We should not currently be recording for the caller.
   CHECK(recording_caller_id_ != caller_id);
 
   // If we are currently recording audio for another caller, abort that cleanly.
   if (recording_caller_id_)
     CancelRecognitionAndInformDelegate(recording_caller_id_);
-
-  if (!HasAudioInputDevices()) {
-    if (delegate_) {
-      delegate_->ShowMicError(caller_id,
-          SpeechRecognitionManagerDelegate::MIC_ERROR_NO_DEVICE_AVAILABLE);
-    }
-  } else if (IsCapturingAudio()) {
-    if (delegate_) {
-      delegate_->ShowMicError(
-          caller_id, SpeechRecognitionManagerDelegate::MIC_ERROR_DEVICE_IN_USE);
-    }
-  } else {
-    recording_caller_id_ = caller_id;
-    requests_[caller_id].is_active = true;
-    requests_[caller_id].recognizer->StartRecognition();
-    if (delegate_)
-      delegate_->ShowWarmUp(caller_id);
-  }
+  recording_caller_id_ = caller_id;
+  requests_[caller_id].is_active = true;
+  requests_[caller_id].recognizer->StartRecognition();

[Satish, 2012/03/21 13:29:48]

I see the checks present here before have now moved to OnRecognitionError. Does
this preserve the callbacks as they work today? In particular are these no-mic
and mic-in-use errors handled transparently allowing user to retry without
issuing callbacks to the listeners/webkit/javascript as they are today?

[Primiano Tucci (use gerrit), 2012/03/22 11:20:41]

Yes. Errors are not propagated to the dispatcher.

+  if (delegate_)
+    delegate_->ShowWarmUp(caller_id);
 }
 
 void SpeechRecognitionManagerImpl::CancelRecognitionForRequest(int caller_id) {
   // Ignore if the caller id was not in our active recognizers list because the
   // user might have clicked more than once, or recognition could have been
   // ended due to other reasons before the user click was processed.
   if (!HasPendingRequest(caller_id))
     return;
 
   CancelRecognitionAndInformDelegate(caller_id);
@@ skipping 43 matching lines @@
     return;
 
   requests_[caller_id].recognizer->StopAudioCapture();
 }
 
 // -------- SpeechRecognitionEventListener interface implementation. ---------
 
 void SpeechRecognitionManagerImpl::OnRecognitionResult(
     int caller_id, const content::SpeechRecognitionResult& result) {
   DCHECK(HasPendingRequest(caller_id));
+  DCHECK(result.error == content::SPEECH_RECOGNITION_ERROR_NONE);
   GetDelegate(caller_id)->SetRecognitionResult(caller_id, result);
 }
 
 void SpeechRecognitionManagerImpl::OnAudioEnd(int caller_id) {
+  if (recording_caller_id_ != caller_id)

[Satish, 2012/03/21 13:29:48]

this check shouldn't be here as we have a DCHECK for the same below

[Primiano Tucci (use gerrit), 2012/03/22 11:20:41]

Basically this checks were introduced because the new recognizer implementation
issues the *End events also in case of errors (in that case recording_caller_id
becomes 0).
This code is, however, transitional and will be much more clean in the next CLs.

+    return;
   DCHECK_EQ(recording_caller_id_, caller_id);
   DCHECK(HasPendingRequest(caller_id));
+  if (!requests_[caller_id].is_active)

[Satish, 2012/03/21 13:29:48]

any reason why this check was added? seems like an error case.. may be a DCHECK
is more appropriate, but then if caller_id is the same as recording_caller_id_
then it should be active anyway so are we doing two checks for the same time?

[Primiano Tucci (use gerrit), 2012/03/22 11:20:41]

Same as above. We could receive an AudioEnd of a recognition that ended with an
error, just a while after we started a new recognition (this happens basically
if the user clicks very fast multiple times on the UI).
As above, this is just a transitional, not-so-clean patch to mix the new
recognizer code while keeping the current manager (that will be replaced in the
next CLs).

+    return;
   recording_caller_id_ = 0;
   GetDelegate(caller_id)->DidCompleteRecording(caller_id);
   if (delegate_)
     delegate_->ShowRecognizing(caller_id);
 }
 
 void SpeechRecognitionManagerImpl::OnRecognitionEnd(int caller_id) {
+  if (!HasPendingRequest(caller_id) || !requests_[caller_id].is_active)

[Satish, 2012/03/21 13:29:48]

ditto, could be a DCHECK if you really want to check for this

[Primiano Tucci (use gerrit), 2012/03/22 11:20:41]

Same as above.

+    return;
   GetDelegate(caller_id)->DidCompleteRecognition(caller_id);
   requests_.erase(caller_id);
   if (delegate_)
     delegate_->DoClose(caller_id);
 }
 
 void SpeechRecognitionManagerImpl::OnSoundStart(int caller_id) {
 }
 
 void SpeechRecognitionManagerImpl::OnSoundEnd(int caller_id) {
 }
 
 void SpeechRecognitionManagerImpl::OnRecognitionError(
-    int caller_id, const content::SpeechRecognitionErrorCode& error) {
+    int caller_id, const content::SpeechRecognitionError& error) {
+  DCHECK(HasPendingRequest(caller_id));
   if (caller_id == recording_caller_id_)
     recording_caller_id_ = 0;
   requests_[caller_id].is_active = false;
-  if (delegate_)
-    delegate_->ShowRecognizerError(caller_id, error);
+  if (delegate_) {
+    if (error.code == content::SPEECH_RECOGNITION_ERROR_AUDIO &&
+        error.details == content::AUDIO_ERROR_NO_MIC) {
+      delegate_->ShowMicError(caller_id,
+          SpeechRecognitionManagerDelegate::MIC_ERROR_NO_DEVICE_AVAILABLE);
+    } else if (error.code == content::SPEECH_RECOGNITION_ERROR_AUDIO &&
+               error.details == content::AUDIO_ERROR_MIC_IN_USE) {
+      delegate_->ShowMicError(
+          caller_id, SpeechRecognitionManagerDelegate::MIC_ERROR_DEVICE_IN_USE);
+    } else {
+      delegate_->ShowRecognizerError(caller_id, error.code);
+    }
+  }
 }
 
 void SpeechRecognitionManagerImpl::OnAudioStart(int caller_id) {
   DCHECK(HasPendingRequest(caller_id));
   DCHECK_EQ(recording_caller_id_, caller_id);
   if (delegate_)
     delegate_->ShowRecording(caller_id);
 }
 
 void SpeechRecognitionManagerImpl::OnRecognitionStart(int caller_id) {
@@ skipping 23 matching lines @@
 
 SpeechRecognitionManagerImpl::Request::Request()
     : delegate(NULL),
       is_active(false) {
 }
 
 SpeechRecognitionManagerImpl::Request::~Request() {
 }
 
 }  // namespace speech