Commit 6df4ab90 authored by primiano@chromium.org's avatar primiano@chromium.org

Web Speech API: Fix a race condition causing renderer crash.

The introduction of the MediaRequestPermission (i.e. infobar) has
opened a race window which causes a recognition session to be aborted
twice, causing a consequent crash on the renderer.
The race window is the following:
1) Session 1 is started (SpeechRecognitionManagerImpl::StartSession)
2) Security checks for session 1 are started asynchronously
   (delegate_->CheckRecognitionIsAllowed).
3) Session 2 is started. This causes an immediate abort of session 1.
4) The oustanding security check for session 1 completes and returns
   a nack. The nack causes an abort of session 1 (in
   RecognitionAllowedCallback). However, session 1 was already aborted
   in 3).
5) The double abort is not tolerated by the renderer, which crashes.

This CL closes the race window with a not-so-elegant fix.
A refactoring of the speech_recognition_manager_impl.cc is STRONGLY
adviced (Hint: use and extend the already existing FSM to keep track
of the asynchronous completion of security checks. Don't introduce
extra state with extra variables).

BUG=296690,116954

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@226523 0039d316-1c4b-4281-b951-d872f2087c98
parent 10c76b37
...@@ -184,10 +184,15 @@ void SpeechRecognitionManagerImpl::RecognitionAllowedCallback(int session_id, ...@@ -184,10 +184,15 @@ void SpeechRecognitionManagerImpl::RecognitionAllowedCallback(int session_id,
if (!SessionExists(session_id)) if (!SessionExists(session_id))
return; return;
SessionsTable::iterator iter = sessions_.find(session_id);
DCHECK(iter != sessions_.end());
Session* session = iter->second;
if (session->abort_requested)
return;
if (ask_user) { if (ask_user) {
SessionsTable::iterator iter = sessions_.find(session_id); SpeechRecognitionSessionContext& context = session->context;
DCHECK(iter != sessions_.end());
SpeechRecognitionSessionContext& context = iter->second->context;
context.label = media_stream_manager_->MakeMediaAccessRequest( context.label = media_stream_manager_->MakeMediaAccessRequest(
context.render_process_id, context.render_process_id,
context.render_view_id, context.render_view_id,
...@@ -253,6 +258,11 @@ void SpeechRecognitionManagerImpl::AbortSession(int session_id) { ...@@ -253,6 +258,11 @@ void SpeechRecognitionManagerImpl::AbortSession(int session_id) {
SessionsTable::iterator iter = sessions_.find(session_id); SessionsTable::iterator iter = sessions_.find(session_id);
iter->second->ui.reset(); iter->second->ui.reset();
if (iter->second->abort_requested)
return;
iter->second->abort_requested = true;
base::MessageLoop::current()->PostTask( base::MessageLoop::current()->PostTask(
FROM_HERE, FROM_HERE,
base::Bind(&SpeechRecognitionManagerImpl::DispatchEvent, base::Bind(&SpeechRecognitionManagerImpl::DispatchEvent,
...@@ -676,6 +686,7 @@ void SpeechRecognitionManagerImpl::ShowAudioInputSettings() { ...@@ -676,6 +686,7 @@ void SpeechRecognitionManagerImpl::ShowAudioInputSettings() {
SpeechRecognitionManagerImpl::Session::Session() SpeechRecognitionManagerImpl::Session::Session()
: id(kSessionIDInvalid), : id(kSessionIDInvalid),
abort_requested(false),
listener_is_active(true) { listener_is_active(true) {
} }
......
...@@ -127,6 +127,7 @@ class CONTENT_EXPORT SpeechRecognitionManagerImpl : ...@@ -127,6 +127,7 @@ class CONTENT_EXPORT SpeechRecognitionManagerImpl :
~Session(); ~Session();
int id; int id;
bool abort_requested;
bool listener_is_active; bool listener_is_active;
SpeechRecognitionSessionConfig config; SpeechRecognitionSessionConfig config;
SpeechRecognitionSessionContext context; SpeechRecognitionSessionContext context;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment