Speech JavaScript API: Use the MediaStreamManager to ask for user permission.

content/browser/speech/speech_recognition_manager_impl.cc

Index: content/browser/speech/speech_recognition_manager_impl.cc
diff --git a/content/browser/speech/speech_recognition_manager_impl.cc b/content/browser/speech/speech_recognition_manager_impl.cc
index 06b1d8809730482943747757297ee1fd8483cd04..0fdf0c5e86532f8c59f882b442d8e1b9211c0b5d 100644
--- a/content/browser/speech/speech_recognition_manager_impl.cc
+++ b/content/browser/speech/speech_recognition_manager_impl.cc
@@ -6,6 +6,7 @@
 
 #include "base/bind.h"
 #include "content/browser/browser_main_loop.h"
+#include "content/browser/renderer_host/media/media_stream_manager.h"
 #include "content/browser/speech/google_one_shot_remote_engine.h"
 #include "content/browser/speech/google_streaming_remote_engine.h"
 #include "content/browser/speech/speech_recognition_engine.h"
@@ -17,13 +18,13 @@
 #include "content/public/browser/speech_recognition_manager_delegate.h"
 #include "content/public/browser/speech_recognition_session_config.h"
 #include "content/public/browser/speech_recognition_session_context.h"
+#include "content/public/common/speech_recognition_error.h"
 #include "content/public/common/speech_recognition_result.h"
 #include "media/audio/audio_manager.h"
 
 using base::Callback;
 using content::BrowserMainLoop;
 using content::BrowserThread;
-using content::SpeechRecognitionError;
 using content::SpeechRecognitionEventListener;
 using content::SpeechRecognitionManager;
 using content::SpeechRecognitionResult;
@@ -55,7 +56,7 @@ SpeechRecognitionManagerImpl* SpeechRecognitionManagerImpl::GetInstance() {
 }
 
 SpeechRecognitionManagerImpl::SpeechRecognitionManagerImpl()
-    : session_id_capturing_audio_(kSessionIDInvalid),
+    : primary_session_id_(kSessionIDInvalid),
       last_session_id_(kSessionIDInvalid),
       is_dispatching_event_(false),
       delegate_(content::GetContentClient()->browser()->
@@ -124,28 +125,62 @@ void SpeechRecognitionManagerImpl::StartSession(int session_id) {
     return;
 
   // If there is another active session, abort that.
-  if (session_id_capturing_audio_ != kSessionIDInvalid &&
-      session_id_capturing_audio_ != session_id) {
-    AbortSession(session_id_capturing_audio_);
+  if (primary_session_id_ != kSessionIDInvalid &&
+      primary_session_id_ != session_id) {
+    AbortSession(primary_session_id_);
   }
 
-  if (delegate_.get())
+  primary_session_id_ = session_id;
+
+  if (delegate_.get()) {
+    sessions_[session_id].is_waiting_for_permission = true;
     delegate_->CheckRecognitionIsAllowed(
         session_id,
         base::Bind(&SpeechRecognitionManagerImpl::RecognitionAllowedCallback,
-                   weak_factory_.GetWeakPtr()));
+                   weak_factory_.GetWeakPtr(),
+                   session_id));
+  }
 }
 
 void SpeechRecognitionManagerImpl::RecognitionAllowedCallback(int session_id,
+                                                              bool ask_user,
                                                               bool is_allowed) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DCHECK(SessionExists(session_id));

[Satish, 2012/09/12 06:15:09]

this is not new code, but is it possible that in the time control goes to the UI
thread and comes back the session had got deleted, either due to JS API aborting
the start call or something else? If it is a valid scenario lets check and do an
early return instead of a dcheck

[hans, 2012/09/12 09:46:58]

Done.

+
+  if (ask_user) {
+    media_stream::MediaStreamManager* media_stream_manager =
+        BrowserMainLoop::GetMediaStreamManager();
+
+    const SpeechRecognitionSessionContext& context =
+        GetSessionContext(session_id);
+
+    permission_request_.reset(new PermissionRequest(
+        media_stream_manager,
+        base::Bind(&SpeechRecognitionManagerImpl::RecognitionAllowedCallback,
+                   weak_factory_.GetWeakPtr(),
+                   session_id,
+                   false)));
+
+    permission_request_->Start(context.render_process_id,
+                               context.render_view_id,
+                               GURL(context.context_name));
+
+    return;
+  }
+
+  sessions_[session_id].is_waiting_for_permission = false;
+
   if (is_allowed) {
     MessageLoop::current()->PostTask(FROM_HERE,
         base::Bind(&SpeechRecognitionManagerImpl::DispatchEvent,
                    weak_factory_.GetWeakPtr(), session_id, EVENT_START));
   } else {
-    sessions_.erase(session_id);
+    OnRecognitionError(session_id, content::SpeechRecognitionError(
+        content::SPEECH_RECOGNITION_ERROR_NOT_ALLOWED));
+    MessageLoop::current()->PostTask(FROM_HERE,
+        base::Bind(&SpeechRecognitionManagerImpl::DispatchEvent,
+                   weak_factory_.GetWeakPtr(), session_id, EVENT_ABORT));
   }
 }
 
@@ -154,6 +189,11 @@ void SpeechRecognitionManagerImpl::AbortSession(int session_id) {
   if (!SessionExists(session_id))
     return;
 
+  if (GetSession(session_id).is_waiting_for_permission) {
+    DCHECK(permission_request_.get());
+    permission_request_->Abort();
+  }
+
   MessageLoop::current()->PostTask(FROM_HERE,
       base::Bind(&SpeechRecognitionManagerImpl::DispatchEvent,
                  weak_factory_.GetWeakPtr(), session_id, EVENT_ABORT));
@@ -164,6 +204,11 @@ void SpeechRecognitionManagerImpl::StopAudioCaptureForSession(int session_id) {
   if (!SessionExists(session_id))
     return;
 
+  if (GetSession(session_id).is_waiting_for_permission) {
+    DCHECK(permission_request_.get());
+    permission_request_->Abort();
+  }
+
   MessageLoop::current()->PostTask(FROM_HERE,
       base::Bind(&SpeechRecognitionManagerImpl::DispatchEvent,
                  weak_factory_.GetWeakPtr(), session_id, EVENT_STOP_CAPTURE));
@@ -179,7 +224,7 @@ void SpeechRecognitionManagerImpl::OnRecognitionStart(int session_id) {
   if (!SessionExists(session_id))
     return;
 
-  DCHECK_EQ(session_id_capturing_audio_, session_id);
+  DCHECK_EQ(primary_session_id_, session_id);
   if (SpeechRecognitionEventListener* delegate_listener = GetDelegateListener())
     delegate_listener->OnRecognitionStart(session_id);
   if (SpeechRecognitionEventListener* listener = GetListener(session_id))
@@ -191,7 +236,7 @@ void SpeechRecognitionManagerImpl::OnAudioStart(int session_id) {
   if (!SessionExists(session_id))
     return;
 
-  DCHECK_EQ(session_id_capturing_audio_, session_id);
+  DCHECK_EQ(primary_session_id_, session_id);
   if (SpeechRecognitionEventListener* delegate_listener = GetDelegateListener())
     delegate_listener->OnAudioStart(session_id);
   if (SpeechRecognitionEventListener* listener = GetListener(session_id))
@@ -204,7 +249,7 @@ void SpeechRecognitionManagerImpl::OnEnvironmentEstimationComplete(
   if (!SessionExists(session_id))
     return;
 
-  DCHECK_EQ(session_id_capturing_audio_, session_id);
+  DCHECK_EQ(primary_session_id_, session_id);
   if (SpeechRecognitionEventListener* delegate_listener = GetDelegateListener())
     delegate_listener->OnEnvironmentEstimationComplete(session_id);
   if (SpeechRecognitionEventListener* listener = GetListener(session_id))
@@ -216,7 +261,7 @@ void SpeechRecognitionManagerImpl::OnSoundStart(int session_id) {
   if (!SessionExists(session_id))
     return;
 
-  DCHECK_EQ(session_id_capturing_audio_, session_id);
+  DCHECK_EQ(primary_session_id_, session_id);
   if (SpeechRecognitionEventListener* delegate_listener = GetDelegateListener())
     delegate_listener->OnSoundStart(session_id);
   if (SpeechRecognitionEventListener* listener = GetListener(session_id))
@@ -396,9 +441,11 @@ void SpeechRecognitionManagerImpl::ExecuteTransitionAndGetNextState(
         case EVENT_START:
           return SessionStart(session);
         case EVENT_ABORT:
+          return SessionAbort(session);
         case EVENT_RECOGNITION_ENDED:
           return SessionDelete(session);
         case EVENT_STOP_CAPTURE:
+          return SessionStopAudioCapture(session);
         case EVENT_AUDIO_ENDED:
           return;
       }
@@ -448,33 +495,33 @@ SpeechRecognitionManagerImpl::GetSessionState(int session_id) const {
 //  - Are guaranteed to be not reentrant (themselves and each other);
 
 void SpeechRecognitionManagerImpl::SessionStart(const Session& session) {
-  session_id_capturing_audio_ = session.id;
+  DCHECK_EQ(primary_session_id_, session.id);
   session.recognizer->StartRecognition();
 }
 
 void SpeechRecognitionManagerImpl::SessionAbort(const Session& session) {
-  if (session_id_capturing_audio_ == session.id)
-    session_id_capturing_audio_ = kSessionIDInvalid;
-  DCHECK(session.recognizer.get() && session.recognizer->IsActive());
+  if (primary_session_id_ == session.id)
+    primary_session_id_ = kSessionIDInvalid;
+  DCHECK(session.recognizer.get());
   session.recognizer->AbortRecognition();
 }
 
 void SpeechRecognitionManagerImpl::SessionStopAudioCapture(
     const Session& session) {
-  DCHECK(session.recognizer.get() && session.recognizer->IsCapturingAudio());

[Satish, 2012/09/12 06:15:09]

why was the second condition removed?

[hans, 2012/09/12 09:46:58]

Because it's possible to stop or abort a session while it's waiting for
permission, in which case it hasn't started capturing audio yet.

+  DCHECK(session.recognizer.get());
   session.recognizer->StopAudioCapture();
 }
 
 void SpeechRecognitionManagerImpl::ResetCapturingSessionId(
     const Session& session) {
-  DCHECK_EQ(session_id_capturing_audio_, session.id);
-  session_id_capturing_audio_ = kSessionIDInvalid;
+  DCHECK_EQ(primary_session_id_, session.id);
+  primary_session_id_ = kSessionIDInvalid;
 }
 
 void SpeechRecognitionManagerImpl::SessionDelete(const Session& session) {
   DCHECK(session.recognizer == NULL || !session.recognizer->IsActive());
-  if (session_id_capturing_audio_ == session.id)
-    session_id_capturing_audio_ = kSessionIDInvalid;
+  if (primary_session_id_ == session.id)
+    primary_session_id_ = kSessionIDInvalid;
   sessions_.erase(session.id);
 }
 
@@ -542,10 +589,57 @@ void SpeechRecognitionManagerImpl::ShowAudioInputSettings() {
 
 SpeechRecognitionManagerImpl::Session::Session()
   : id(kSessionIDInvalid),
-    listener_is_active(true) {
+    listener_is_active(true),
+    is_waiting_for_permission(false) {
 }
 
 SpeechRecognitionManagerImpl::Session::~Session() {
 }
 
+SpeechRecognitionManagerImpl::PermissionRequest::PermissionRequest(
+    media_stream::MediaStreamManager* media_stream_manager,
+    const base::Callback<void(bool is_allowed)>& callback)
+  : media_stream_manager_(media_stream_manager),

[Satish, 2012/09/12 06:15:09]

indent to right by 2 spaces

[hans, 2012/09/12 09:46:58]

This line got moved, and I think I got it right in the new place.

+    callback_(callback) {
+}
+
+SpeechRecognitionManagerImpl::PermissionRequest::~PermissionRequest() {
+}
+
+void SpeechRecognitionManagerImpl::PermissionRequest::Start(
+    int render_process_id, int render_view_id, const GURL& origin) {
+  media_stream_manager_->GenerateStream(

[Satish, 2012/09/12 06:15:09]

these method names don't seem to make sense when called for bringing up an
infobar for speech reco permission. Perhaps they need to be renamed to be more
generic?

Hmm this actually feels like we are using a class in MediaStream that is a bit
higher than what we actually need.. i.e. a manager class that shows infobars at
the right time than the infobar class itself. Looking at the class declaration
that seems to be the case. Can just the infobar and permissions management code
be extracted out to a smaller class and that can be reused by speech
recognition?

[hans, 2012/09/12 09:46:58]

MediaStream does not have a convenient way of just accessing the infobar and
permission stuff. It's tightly integrated with the process of opening a new
stream. I think it would be hard to extract it and make it part of two different
flows. Maybe the MediaStream people CC'd can comment on this.

Though I admit that opening a stream just to check if we have permission is
slightly overkill, I do think that it solves our problem quite nicely for now.

[Satish, 2012/09/12 13:51:38]

xians/tommyw, any thoughts on this? Can the infobar/permissions code in
MediaStreamManager be factored out into a separate class that we can use?

+      this,
+      render_process_id,
+      render_view_id,
+      media_stream::StreamOptions(/*audio=*/true, /*video=*/false),
+      origin,
+      &label_);
+}
+
+void SpeechRecognitionManagerImpl::PermissionRequest::Abort() {
+  media_stream_manager_->CancelGenerateStream(label_);

[Satish, 2012/09/12 06:15:09]

ditto

+}
+
+void SpeechRecognitionManagerImpl::PermissionRequest::StreamGenerated(
+    const std::string& label,
+    const media_stream::StreamDeviceInfoArray& audio_devices,
+    const media_stream::StreamDeviceInfoArray& video_devices) {
+

[Satish, 2012/09/12 06:15:09]

remove empty line

[hans, 2012/09/12 09:46:58]

Done.

+  // TODO(hans): One day it would be nice to actually use the generated stream,
+  // but right now we only use it to request permission, and then we dump it.
+  BrowserThread::PostTask(
+      BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(&media_stream::MediaStreamManager::StopGeneratedStream,
+                 base::Unretained(media_stream_manager_),
+                 label));
+  callback_.Run(true);
+}
+
+void SpeechRecognitionManagerImpl::PermissionRequest::StreamGenerationFailed(
+    const std::string& label) {
+  callback_.Run(false);
+}
+
 }  // namespace speech