Speech refactoring: Reimplemented SpeechRecognitionManagerImpl as a FSM. (CL1.7)

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 "base/memory/singleton.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"
+#include "content/browser/speech/google_one_shot_remote_engine.h"
+#include "content/browser/speech/speech_recognition_engine.h"
+#include "content/browser/speech/speech_recognizer_impl.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
-#include "content/public/browser/speech_recognizer.h"
-#include "content/public/browser/render_view_host_delegate.h"
 #include "content/public/browser/resource_context.h"
+#include "content/public/browser/speech_recognition_event_listener.h"
 #include "content/public/browser/speech_recognition_manager_delegate.h"
-#include "content/public/browser/speech_recognition_preferences.h"
-#include "content/public/common/view_type.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_result.h"
 #include "media/audio/audio_manager.h"
 
+using base::Callback;
+using base::Unretained;
 using content::BrowserMainLoop;
 using content::BrowserThread;
-using content::RenderViewHostImpl;
+using content::SpeechRecognitionError;
+using content::SpeechRecognitionEventListener;
 using content::SpeechRecognitionManager;
-using content::SpeechRecognitionManagerDelegate;
+using content::SpeechRecognitionResult;
+using content::SpeechRecognitionSessionContext;
+using content::SpeechRecognitionSessionConfig;
+using media::AudioManager;

[Satish, 2012/04/19 13:03:19]

same as below

[Primiano Tucci (use gerrit), 2012/04/20 16:06:43]

Done.

+using std::string;

[Satish, 2012/04/19 13:03:19]

could remove this as string is used only in 1 place

[Primiano Tucci (use gerrit), 2012/04/20 16:06:43]

Done.

+
+namespace {
+
+  // A dummy implementation of the SpeechRecognitionManagerDelegate interface

[Satish, 2012/04/19 13:03:19]

no indentation required within namespaces

[Primiano Tucci (use gerrit), 2012/04/20 16:06:43]

Done.

+  // used when no delegate has been passed to the SpeechRecognitionManagerImpl.
+  class VoidRecognitionManagerDelegate :
+      public content::SpeechRecognitionManagerDelegate {
+   public:
+    static VoidRecognitionManagerDelegate* GetInstance() {
+      return Singleton<VoidRecognitionManagerDelegate>::get();
+    }
+    virtual void GetDiagnosticInformation(
+        bool* can_report_metrics, std::string* request_info) OVERRIDE {}
+    virtual bool CheckRecognitionIsAllowed(int session_id) OVERRIDE {
+      return false; }
+    virtual void ShowRecognitionRequested(int session_id) OVERRIDE {}
+    virtual void ShowWarmUp(int session_id) OVERRIDE {}
+    virtual void ShowRecognizing(int session_id) OVERRIDE {}
+    virtual void ShowRecording(int session_id) OVERRIDE {}
+    virtual void ShowInputVolume(
+        int session_id, float volume, float noise_volume) OVERRIDE {}
+    virtual void ShowError(int session_id,
+        const content::SpeechRecognitionError& error) OVERRIDE {}
+    virtual void DoClose(int session_id) OVERRIDE {}
+
+   private:
+    VoidRecognitionManagerDelegate() {}
+    virtual ~VoidRecognitionManagerDelegate() {}
+    friend struct DefaultSingletonTraits<VoidRecognitionManagerDelegate>;
+  };
+} //namespace

[Satish, 2012/04/19 13:03:19]

add 2 spaces before // and 1 space after //

[Primiano Tucci (use gerrit), 2012/04/20 16:06:43]

Done.

+
+namespace content {
+const int SpeechRecognitionManager::kSessionIDInvalid = 0;
 
 SpeechRecognitionManager* SpeechRecognitionManager::GetInstance() {
   return speech::SpeechRecognitionManagerImpl::GetInstance();
 }
+}  // namespace content
 
 namespace speech {
 
-struct SpeechRecognitionManagerImpl::SpeechRecognitionParams {
-  SpeechRecognitionParams(
-      InputTagSpeechDispatcherHost* delegate,
-      int session_id,
-      int render_process_id,
-      int render_view_id,
-      const gfx::Rect& element_rect,
-      const std::string& language,
-      const std::string& grammar,
-      const std::string& origin_url,
-      net::URLRequestContextGetter* context_getter,
-      content::SpeechRecognitionPreferences* recognition_prefs)
-      : delegate(delegate),
-        session_id(session_id),
-        render_process_id(render_process_id),
-        render_view_id(render_view_id),
-        element_rect(element_rect),
-        language(language),
-        grammar(grammar),
-        origin_url(origin_url),
-        context_getter(context_getter),
-        recognition_prefs(recognition_prefs) {
-  }
-
-  InputTagSpeechDispatcherHost* delegate;
-  int session_id;
-  int render_process_id;
-  int render_view_id;
-  gfx::Rect element_rect;
-  std::string language;
-  std::string grammar;
-  std::string origin_url;
-  net::URLRequestContextGetter* context_getter;
-  content::SpeechRecognitionPreferences* recognition_prefs;
-};
-
 SpeechRecognitionManagerImpl* SpeechRecognitionManagerImpl::GetInstance() {
   return Singleton<SpeechRecognitionManagerImpl>::get();
 }
 
 SpeechRecognitionManagerImpl::SpeechRecognitionManagerImpl()
-    : can_report_metrics_(false),
-      recording_session_id_(0) {
-  delegate_.reset(content::GetContentClient()->browser()->
-                  GetSpeechRecognitionManagerDelegate());
+    : interactive_session_id_(0),
+      last_session_id_(0),
+      is_dispatching_event_(false) {
+  delegate_ = content::GetContentClient()->browser()->
+      GetSpeechRecognitionManagerDelegate();
+  // In lack of one being provided, instantiate a void delegate so we can avoid
+  // unaesthetic "if (delegate_ != NULL)" statements.
+  if (delegate_ == NULL)

[Satish, 2012/04/19 13:03:19]

use 'if (!delegate_)'

[Primiano Tucci (use gerrit), 2012/04/20 16:06:43]

Hmm I'd prefer the == / != NULL form since:
1. It is IMHO easier to read.
2. It works seamlessly with both naked and smart pointers.
3. I like only boolean conditions to be evaluated inside an if statement (even
if, from the C viewpoint, it would work anyway)

+    delegate_ = VoidRecognitionManagerDelegate::GetInstance();
 }
 
 SpeechRecognitionManagerImpl::~SpeechRecognitionManagerImpl() {
-  while (requests_.begin() != requests_.end())
-    CancelRecognition(requests_.begin()->first);
-}
+  // Recognition sessions will be aborted by the corresponding destructors.

[Satish, 2012/04/19 13:03:19]

I see the destructor of Session not explicitly aborting ongoing recording etc.
Probably that happens automatically when the SpeechRecognizer object gets
destroyed implicitly? Even then we should be informing the listener of that
session about the event (e.g. what SessionAbort does). So looks like we need to
explicitly abort all ongoing sessions here?

[Primiano Tucci (use gerrit), 2012/04/20 16:06:43]

Yes, I meant, by destructor calls of the sessions_ container dtor.
Well, the point is that since SpeechRecognitionManagerImpl is a singleton, the
only case in which the destructor is called is the browser shutdown. Do we
really need to do all these graceful aborts? Even though, calling Abort on
sessions upon destructor will not really gracefully abort them, rather it will
just post a task, which will never be executed.

[Satish, 2012/04/23 10:25:17]

That is a bit worrying because the session could be capturing audio and the
connection may need to be gracefully closed. Since this isn't a self contained
class not interfacing with anyone else, I think a graceful shutdown is better.
We could bypass the FSM, not post a task and get this closed immediately since
the object is going away anyway

+  sessions_.clear();
+}
+
+int SpeechRecognitionManagerImpl::CreateSession(
+    SpeechRecognitionSessionConfig& config,

[Satish, 2012/04/19 13:03:19]

can this be changed to a const reference?

[Primiano Tucci (use gerrit), 2012/04/20 16:06:43]

Done.

+    SpeechRecognitionEventListener* event_listener) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  const int session_id = GetNextSessionID();
+  DCHECK(!SessionExists(session_id));
+  // Set-up the new session.
+  Session& session = sessions_[session_id];
+  session.id = session_id;
+  session.event_listener = event_listener;
+  session.context = config.initial_context;
+
+  // TODO(primiano) Is this check enough just on creation or shall we move/copy
+  // it on SessionStart in order to repeat the check every time?.
+  if (!delegate_->CheckRecognitionIsAllowed(session_id)) {
+    sessions_.erase(session_id);
+    return kSessionIDInvalid;
+  }
+
+  string hardware_info;
+  bool can_report_metrics;
+  delegate_->GetDiagnosticInformation(&can_report_metrics, &hardware_info);
+
+  GoogleOneShotRemoteEngineConfig remote_engine_config;
+  remote_engine_config.language = config.language;
+  remote_engine_config.grammar = config.grammar;
+  remote_engine_config.audio_sample_rate =
+      SpeechRecognizerImpl::kAudioSampleRate;
+  remote_engine_config.audio_num_bits_per_sample =
+     SpeechRecognizerImpl::kNumBitsPerAudioSample;
+  remote_engine_config.filter_profanities = config.filter_profanities;
+  remote_engine_config.hardware_info = hardware_info;
+  remote_engine_config.origin_url = can_report_metrics ? config.origin_url : "";
+
+  GoogleOneShotRemoteEngine* google_remote_engine =
+      new GoogleOneShotRemoteEngine(config.url_context_getter);
+  google_remote_engine->SetConfig(remote_engine_config);
+
+  session.recognizer = new SpeechRecognizerImpl(this,
+                                                session_id,
+                                                google_remote_engine);
+  return session_id;
+}
+
+void SpeechRecognitionManagerImpl::StartSession(int session_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK(SessionExists(session_id));
+
+  // If there is another interactive session, detach prior to start the new one.
+  if (interactive_session_id_ > 0 && interactive_session_id_ != session_id) {

[Satish, 2012/04/19 13:03:19]

could remove braces

[Primiano Tucci (use gerrit), 2012/04/20 16:06:43]

Done.

[Primiano Tucci (use gerrit), 2012/04/20 16:06:43]

Oops, right!

+    DetachSession(interactive_session_id_);
+  }
+
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+      base::Bind(&SpeechRecognitionManagerImpl::DispatchEvent, Unretained(this),
+                 session_id, FSMEventArgs(EVENT_START)));
+}
+
+void SpeechRecognitionManagerImpl::AbortSession(int session_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK(SessionExists(session_id));
+
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+      base::Bind(&SpeechRecognitionManagerImpl::DispatchEvent, Unretained(this),
+                 session_id, FSMEventArgs(EVENT_ABORT)));
+}
+
+void SpeechRecognitionManagerImpl::StopAudioCaptureForSession(int session_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK(SessionExists(session_id));
+
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+      base::Bind(&SpeechRecognitionManagerImpl::DispatchEvent, Unretained(this),
+                 session_id, FSMEventArgs(EVENT_STOP_CAPTURE)));
+}
+
+void SpeechRecognitionManagerImpl::DetachSession(int session_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK(SessionExists(session_id));
+
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+      base::Bind(&SpeechRecognitionManagerImpl::DispatchEvent, Unretained(this),
+                 session_id, FSMEventArgs(EVENT_DETACH)));
+}
+
+// Here begins the SpeechRecognitionEventListener interface implementation,
+// which will simply relay the events to the proper listener registered for the
+// particular session (most likely InputTagSpeechDispatcherHost) and intercept
+// some of them to provide UI notifications.
+
+void SpeechRecognitionManagerImpl::OnRecognitionStart(int session_id) {
+  if (!SessionExists(session_id))
+    return;
+  DCHECK_EQ(interactive_session_id_, session_id);
+  delegate_->ShowWarmUp(session_id);
+  GetListener(session_id)->OnRecognitionStart(session_id);
+}
+
+void SpeechRecognitionManagerImpl::OnAudioStart(int session_id) {
+  if (!SessionExists(session_id))
+    return;
+  DCHECK_EQ(interactive_session_id_, session_id);
+  delegate_->ShowRecording(session_id);
+  GetListener(session_id)->OnAudioStart(session_id);
+}
+
+void SpeechRecognitionManagerImpl::OnEnvironmentEstimationComplete(
+    int session_id) {
+  if (!SessionExists(session_id))
+    return;
+  DCHECK_EQ(interactive_session_id_, session_id);
+  GetListener(session_id)->OnEnvironmentEstimationComplete(session_id);
+}
+
+void SpeechRecognitionManagerImpl::OnSoundStart(int session_id) {
+  if (!SessionExists(session_id))
+    return;
+  DCHECK_EQ(interactive_session_id_, session_id);
+  GetListener(session_id)->OnSoundStart(session_id);
+}
+
+void SpeechRecognitionManagerImpl::OnSoundEnd(int session_id) {
+  if (!SessionExists(session_id))
+    return;
+  GetListener(session_id)->OnSoundEnd(session_id);
+}
+
+void SpeechRecognitionManagerImpl::OnAudioEnd(int session_id) {
+  if (!SessionExists(session_id))
+    return;
+
+  // OnAudioEnd can also be raised after an abort request, when the session is
+  // not interactive anymore.
+  if (interactive_session_id_ == session_id)
+    delegate_->ShowRecognizing(session_id);
+
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));

[Satish, 2012/04/19 13:03:19]

could move this check to the very beginning (same for all methods below).

Also this is probably something to add in all the listener methods

[Primiano Tucci (use gerrit), 2012/04/20 16:06:43]

Right. Added in all public methods.

+  GetListener(session_id)->OnAudioEnd(session_id);
+  DispatchEvent(session_id, FSMEventArgs(EVENT_AUDIO_ENDED));
+}
+
+void SpeechRecognitionManagerImpl::OnRecognitionResult(
+    int session_id, const content::SpeechRecognitionResult& result) {
+  if (!SessionExists(session_id))
+    return;
+
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  GetListener(session_id)->OnRecognitionResult(session_id, result);
+  FSMEventArgs event_args(EVENT_RECOGNITION_RESULT);
+  event_args.speech_result = &result;

[Satish, 2012/04/19 13:03:19]

can this be passed by value to avoid any use-after-free type situations in
future?

[Primiano Tucci (use gerrit), 2012/04/20 16:06:43]

Removed. Was not used at all.

+  DispatchEvent(session_id, event_args);
+}
+
+void SpeechRecognitionManagerImpl::OnRecognitionError(
+    int session_id, const content::SpeechRecognitionError& error) {
+  if (!SessionExists(session_id))
+    return;
+
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  GetListener(session_id)->OnRecognitionError(session_id, error);
+  FSMEventArgs event_args(EVENT_RECOGNITION_ERROR);
+  event_args.speech_error = &error;

[Satish, 2012/04/19 13:03:19]

ditto

[Primiano Tucci (use gerrit), 2012/04/20 16:06:43]

Done.

+  DispatchEvent(session_id, event_args);
+}
+
+void SpeechRecognitionManagerImpl::OnAudioLevelsChange(
+    int session_id, float volume, float noise_volume) {
+  if (!SessionExists(session_id))
+    return;
+  delegate_->ShowInputVolume(session_id, volume, noise_volume);
+  GetListener(session_id)->OnAudioLevelsChange(session_id, volume,
+                                               noise_volume);
+}
+
+void SpeechRecognitionManagerImpl::OnRecognitionEnd(int session_id) {
+  if (!SessionExists(session_id))
+    return;
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  GetListener(session_id)->OnRecognitionEnd(session_id);
+  DispatchEvent(session_id, FSMEventArgs(EVENT_RECOGNITION_ENDED));
+}
+
+int SpeechRecognitionManagerImpl::LookupSessionByContext(

[Satish, 2012/04/19 13:03:19]

Since this class is the broker for SessionContext data between the dispatcher
host and delegates, I think it is fine for it to know and access the members
instead of treating it as an opaque entity. So this method can be changed to a
simpler one where caller passes in a SessionContext and it just matches the
fields before returning the right session id

[Primiano Tucci (use gerrit), 2012/04/20 16:06:43]

Can we keep this as a TODO and do it after CL2 and WebKit patch is stabilized? I
am not 100% sure that the (future) speech_recognition_dispatcher_host
(equivalent of input_tag_dispatcher_host for JS APIs) will perform the lookup on
the basis of the same information.

[Satish, 2012/04/23 10:25:17]

I'd prefer we make this simpler now and change it in a future CL if required.
Its cheap to change when required but leaving it as is now and we may
forget/accumulate cruft over time. For the current usage using base::Bind and a
matcher is complicated compared to passing in the 3 fields to check

+      Callback<bool(const SpeechRecognitionSessionContext&)> matcher) const {

[Satish, 2012/04/19 13:03:19]

indent by 4 spaces only

[Primiano Tucci (use gerrit), 2012/04/20 16:06:43]

Done.

+  SessionsTable::const_iterator iter;
+  // Note: the callback (matcher) must NEVER perform non-const calls on us.
+  for(iter = sessions_.begin(); iter != sessions_.end(); iter++) {
+    const int session_id = iter->first;
+    const Session& session = iter->second;
+    bool matches = matcher.Run(session.context);
+    if (matches)
+      return session_id;
+  }
+  return 0;
+}
+
+// TODO(primiano) are we really sure that iterator->second will always give us

[Satish, 2012/04/19 13:03:19]

I've seen more code rely on that behavior so could remove this todo

[Primiano Tucci (use gerrit), 2012/04/20 16:06:43]

Done.

+// a reference to the inner contained object, and not a copy? Couldn't find any
+// strong indication in the STL doc.
+SpeechRecognitionSessionContext&

[Satish, 2012/04/19 13:03:19]

could return this by value or if you prefer pass in a pointer where the contents
get copied to

[Primiano Tucci (use gerrit), 2012/04/20 16:06:43]

Done.

+    SpeechRecognitionManagerImpl::GetSessionContext(int session_id) const {
+  SessionsTable::const_iterator iter = sessions_.find(session_id);
+  DCHECK(iter != sessions_.end());
+  return const_cast<SpeechRecognitionSessionContext&>(iter->second.context);
+}
+
+void SpeechRecognitionManagerImpl::AbortAllSessionsForListener(
+    SpeechRecognitionEventListener* listener) {
+  SessionsTable::iterator it = sessions_.begin();
+  // AbortSession is asyncrhonous and the session will not be removed from the

[Satish, 2012/04/19 13:03:19]

asyncrhonous -> asynchronous

[Primiano Tucci (use gerrit), 2012/04/20 16:06:43]

Done.

+  // collection while we are iterating over it.
+  while (it != sessions_.end()) {
+    if (it->second.event_listener == listener)
+      AbortSession(it->first);
+    ++it;
+  }
+}
+
+// -----------------------  Core FSM implementation ---------------------------
+void SpeechRecognitionManagerImpl::DispatchEvent(int session_id,
+                                                 FSMEventArgs event_args) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  if (!SessionExists(session_id))
+    return;
+
+  Session& session = sessions_[session_id];
+  DCHECK_LE(session.state, STATE_MAX_VALUE);
+  DCHECK_LE(event_args.event, EVENT_MAX_VALUE);
+
+  // Event dispatching must be sequential, otherwise it will break all the rules
+  // and the assumptions of the finite state automata model.
+  DCHECK(!is_dispatching_event_);
+  is_dispatching_event_ = true;
+
+  // Pedantic consistency checks.
+  if (session.state == STATE_INTERACTIVE ||
+      session.state == STATE_DETACHABLE ||
+      session.state == STATE_ENDED_WITH_ERROR)

[Satish, 2012/04/19 13:03:19]

suggest braces for multiline if statements like these

[Primiano Tucci (use gerrit), 2012/04/20 16:06:43]

Done.

+    DCHECK_EQ(interactive_session_id_, session_id);
+  if (session.state == STATE_IDLE ||
+      session.state == STATE_DETACHED ||
+      session.state == STATE_ABORTING)
+    DCHECK_NE(interactive_session_id_, session_id);
+
+  session.state = ExecuteTransitionAndGetNextState(session, event_args);
+
+  is_dispatching_event_ = false;
+}
+
+// This FSM handles the evolution of each session only from the UI viewpoint,
+// coordinating the UI bubbles and the user input actions.
+// All the events received by the SpeechRecognizerImpl instances (one for each
+// session) are always routed to the SpeechRecognitionEventListener(s)
+// regardless the choices taken in this FSM.
+SpeechRecognitionManagerImpl::FSMState
+SpeechRecognitionManagerImpl::ExecuteTransitionAndGetNextState(
+    Session& session, const FSMEventArgs& event_args) {
+  const FSMEvent event = event_args.event;
+  switch (session.state) {
+    case STATE_IDLE:
+      switch (event) {
+        case EVENT_ABORT:
+          return SessionDelete(session, event_args);
+        case EVENT_START:
+          return SessionStart(session, event_args);
+        case EVENT_STOP_CAPTURE:
+        case EVENT_DETACH:
+          return DoNothing(session, event_args);
+        case EVENT_AUDIO_ENDED:
+        case EVENT_RECOGNITION_ENDED:
+        case EVENT_RECOGNITION_RESULT:
+        case EVENT_RECOGNITION_ERROR:
+          return NotFeasible(session, event_args);
+      }
+      break;
+    case STATE_INTERACTIVE:
+      switch (event) {
+        case EVENT_ABORT:
+          return SessionAbort(session, event_args);
+        case EVENT_START:
+          return DoNothing(session, event_args);

[Satish, 2012/04/19 13:03:19]

should be calling NotFeasible instead of DoNothing

[Primiano Tucci (use gerrit), 2012/04/20 16:06:43]

This code is changed. Btw the idea is that in this class I consider the caller
as untrusted (since this class will be almost directly visible to the future JS
APIs). Therefore I use DoNothing for actions that are possible but don't make
sense.
NotFeasible, instead, is reserved for events that structurally cannot happen
(highlighting the presence of a bug in the code)

+        case EVENT_STOP_CAPTURE:
+          return SessionStopAudioCapture(session, event_args);
+        case EVENT_DETACH:
+          return SessionAbort(session, event_args);
+        case EVENT_AUDIO_ENDED:
+          return SessionSetDetachable(session, event_args);
+        case EVENT_RECOGNITION_ENDED:
+        case EVENT_RECOGNITION_RESULT:
+          // TODO(primiano) Valid only in single shot mode.
+          return NotFeasible(session, event_args);
+        case EVENT_RECOGNITION_ERROR:
+          return SessionReportError(session, event_args);
+      }
+      break;
+    case STATE_DETACHABLE:

[Satish, 2012/04/19 13:03:19]

it would be clearer to understand if 'detachable' can be renamed to
'waiting_for_result' or something similar.

[Primiano Tucci (use gerrit), 2012/04/20 16:06:43]

Simlpified the FSM. Now should be clearer.

+      // STATE_DETACHABLE implies that the session is still interactive, but it
+      // is not capturing audio anymore.
+      switch (event) {
+        case EVENT_ABORT:
+          return SessionAbort(session, event_args);
+        case EVENT_START:
+        case EVENT_STOP_CAPTURE:
+          return DoNothing(session, event_args);
+        case EVENT_DETACH:
+          return SessionDetach(session, event_args);
+        case EVENT_AUDIO_ENDED:
+          return NotFeasible(session, event_args);
+        case EVENT_RECOGNITION_ENDED:
+          return SessionReportNoMatch(session, event_args);
+        case EVENT_RECOGNITION_RESULT:
+          // TODO(primiano) Valid only in single shot mode.
+          return SessionDetach(session, event_args);
+        case EVENT_RECOGNITION_ERROR:
+          return SessionReportError(session, event_args);
+      }
+      break;
+    case STATE_DETACHED:

[Satish, 2012/04/19 13:03:19]

since this seems to be used only in one shot recognition, you could move to
STATE_IDLE and handle EVENT_RECOGNITION_ENDED there to call SessionDelete. That
way we don't need a separate STATE_DETACHED.
Same suggestion for the ABORTING & ENDED_WITH_ERROR states, perhaps they all can
be the same as IDLE so after the session ends it goes back to IDLE

[Primiano Tucci (use gerrit), 2012/04/20 16:06:43]

Ditto.

+      switch (event) {
+        case EVENT_ABORT:
+          return SessionAbort(session, event_args);
+        case EVENT_START:
+        case EVENT_STOP_CAPTURE:
+        case EVENT_DETACH:
+          return DoNothing(session, event_args);
+        case EVENT_AUDIO_ENDED:
+          return NotFeasible(session, event_args);
+        case EVENT_RECOGNITION_ENDED:
+          return SessionDelete(session, event_args);
+        case EVENT_RECOGNITION_RESULT:
+        case EVENT_RECOGNITION_ERROR:
+          return DoNothing(session, event_args);
+      }
+      break;
+    case STATE_ABORTING:
+      // STATE_ABORTING implies that the session is already detached.
+      switch (event) {
+        case EVENT_ABORT:
+        case EVENT_START:
+        case EVENT_STOP_CAPTURE:
+        case EVENT_DETACH:
+        case EVENT_AUDIO_ENDED:
+          return DoNothing(session, event_args);
+        case EVENT_RECOGNITION_ENDED:
+          return SessionDelete(session, event_args);
+        case EVENT_RECOGNITION_RESULT:
+        case EVENT_RECOGNITION_ERROR:
+          return DoNothing(session, event_args);
+      }
+      break;
+    case STATE_ENDED_WITH_ERROR:
+      switch (event) {
+        case EVENT_ABORT:
+          return SessionDelete(session, event_args);
+        case EVENT_START:
+          return SessionStart(session, event_args);
+        case EVENT_STOP_CAPTURE:
+          return DoNothing(session, event_args);
+        case EVENT_DETACH:
+          return SessionDelete(session, event_args);
+        case EVENT_AUDIO_ENDED:
+          return NotFeasible(session, event_args);
+        case EVENT_RECOGNITION_ENDED:
+          return DoNothing(session, event_args);
+        case EVENT_RECOGNITION_RESULT:
+          return NotFeasible(session, event_args);
+        case EVENT_RECOGNITION_ERROR:
+          return DoNothing(session, event_args);
+      }
+      break;
+  }
+  return NotFeasible(session, event_args);
+}
+
+// ----------- Contract for all the FSM evolution functions below -------------
+//  - Are guaranteed to be executed in the IO thread;
+//  - Are guaranteed to be not reentrant (themselves and each other);
+//  - event_args members are guaranteed to be stable during the call;
+
+SpeechRecognitionManagerImpl::FSMState
+SpeechRecognitionManagerImpl::SessionStart(Session& session,
+                                           const FSMEventArgs& event_args) {
+  if (interactive_session_id_ != 0)

[Satish, 2012/04/19 13:03:19]

replace 0 with kSessionIDInvalid here and other places

[Primiano Tucci (use gerrit), 2012/04/20 16:06:43]

Done.

+    delegate_->DoClose(interactive_session_id_);
+  interactive_session_id_ = session.id;
+  delegate_->ShowRecognitionRequested(session.id);
+  session.error_occurred = false;
+  session.recognizer->StartRecognition();
+  return STATE_INTERACTIVE;
+}
+
+SpeechRecognitionManagerImpl::FSMState
+SpeechRecognitionManagerImpl::SessionAbort(Session& session,
+                                           const FSMEventArgs& event_args) {
+  DCHECK(session.recognizer.get() != NULL);
+  DCHECK(session.recognizer->IsActive());
+  session.recognizer->AbortRecognition();
+  if (interactive_session_id_ == session.id) {
+    interactive_session_id_ = 0;
+    delegate_->DoClose(session.id);
+  }
+  return STATE_ABORTING;
+}
+
+SpeechRecognitionManagerImpl::FSMState
+SpeechRecognitionManagerImpl::SessionStopAudioCapture(
+    Session& session, const FSMEventArgs& event_args) {
+  DCHECK(session.recognizer.get() != NULL);
+  DCHECK(session.recognizer->IsActive());
+  session.recognizer->StopAudioCapture();
+  return STATE_DETACHABLE;
+}
+
+SpeechRecognitionManagerImpl::FSMState
+SpeechRecognitionManagerImpl::SessionDetach(Session& session,
+                                            const FSMEventArgs& event_args) {
+  DCHECK_EQ(interactive_session_id_, session.id);
+  interactive_session_id_ = 0;
+  delegate_->DoClose(session.id);
+  return STATE_DETACHED;
+}
+
+SpeechRecognitionManagerImpl::FSMState
+SpeechRecognitionManagerImpl::SessionReportError(
+    Session& session, const FSMEventArgs& event_args) {
+  DCHECK_EQ(interactive_session_id_, session.id);
+  DCHECK(event_args.speech_error != NULL);
+  delegate_->ShowError(session.id, *event_args.speech_error);
+  session.error_occurred = true;
+  return STATE_ENDED_WITH_ERROR;
+}
+
+SpeechRecognitionManagerImpl::FSMState
+SpeechRecognitionManagerImpl::SessionReportNoMatch(
+    Session& session, const FSMEventArgs& event_args) {
+  DCHECK_EQ(interactive_session_id_, session.id);
+  DCHECK(!session.error_occurred);
+  delegate_->ShowError(
+      session.id,
+      SpeechRecognitionError(content::SPEECH_RECOGNITION_ERROR_NO_MATCH));
+  return STATE_ENDED_WITH_ERROR;
+}
+
+SpeechRecognitionManagerImpl::FSMState
+SpeechRecognitionManagerImpl::SessionDelete(Session& session,
+                                            const FSMEventArgs& event_args) {
+  if (interactive_session_id_ == session.id) {
+    interactive_session_id_ = 0;
+    delegate_->DoClose(session.id);
+  }
+  sessions_.erase(session.id);
+  // Next state is ininfluent, the session will be deleted afterwards.
+  return STATE_ENDED_WITH_ERROR;
+}
+
+SpeechRecognitionManagerImpl::FSMState
+SpeechRecognitionManagerImpl::SessionSetDetachable(
+    Session& session, const FSMEventArgs& event_args) {
+  return STATE_DETACHABLE;
+}
+
+SpeechRecognitionManagerImpl::FSMState
+SpeechRecognitionManagerImpl::DoNothing(Session& session,
+                                        const FSMEventArgs& event_args) {
+  return session.state;
+}
+
+SpeechRecognitionManagerImpl::FSMState
+SpeechRecognitionManagerImpl::NotFeasible(Session& session,
+                                          const FSMEventArgs& event_args) {
+  NOTREACHED() << "Unfeasible event " << event_args.event
+               << " in state " << session.state
+               << " for session " << session.id;
+  return session.state;
+}
+
+int SpeechRecognitionManagerImpl::GetNextSessionID() {
+  ++last_session_id_;
+  // Deal with wrapping of last_session_id_. (How civilized).
+  if (last_session_id_ <= 0)
+    last_session_id_ = 1;
+  return last_session_id_;
+}
+
+bool SpeechRecognitionManagerImpl::SessionExists(int session_id) const {
+  return sessions_.find(session_id) != sessions_.end();
+}
+
+SpeechRecognitionEventListener* SpeechRecognitionManagerImpl::GetListener(
+    int session_id) const {
+  return sessions_.find(session_id)->second.event_listener;
+}
+
 
 bool SpeechRecognitionManagerImpl::HasAudioInputDevices() {
   return BrowserMainLoop::GetAudioManager()->HasAudioInputDevices();
 }
 
 bool SpeechRecognitionManagerImpl::IsCapturingAudio() {
   return BrowserMainLoop::GetAudioManager()->IsRecordingInProcess();
 }
 
 string16 SpeechRecognitionManagerImpl::GetAudioInputDeviceModel() {
   return BrowserMainLoop::GetAudioManager()->GetAudioInputDeviceModel();
 }
 
-bool SpeechRecognitionManagerImpl::HasPendingRequest(int session_id) const {
-  return requests_.find(session_id) != requests_.end();
-}
-
-InputTagSpeechDispatcherHost* SpeechRecognitionManagerImpl::GetDelegate(
-    int session_id) const {
-  return requests_.find(session_id)->second.delegate;
-}
-
 void SpeechRecognitionManagerImpl::ShowAudioInputSettings() {
   // Since AudioManager::ShowAudioInputSettings can potentially launch external
   // processes, do that in the FILE thread to not block the calling threads.
   if (!BrowserThread::CurrentlyOn(BrowserThread::FILE)) {
     BrowserThread::PostTask(
         BrowserThread::FILE, FROM_HERE,
         base::Bind(&SpeechRecognitionManagerImpl::ShowAudioInputSettings,
                    base::Unretained(this)));
     return;
   }
 
-  media::AudioManager* audio_manager = BrowserMainLoop::GetAudioManager();
+  AudioManager* audio_manager = BrowserMainLoop::GetAudioManager();
   DCHECK(audio_manager->CanShowAudioInputSettings());
   if (audio_manager->CanShowAudioInputSettings())
     audio_manager->ShowAudioInputSettings();
 }
 
-void SpeechRecognitionManagerImpl::StartRecognition(
-    InputTagSpeechDispatcherHost* delegate,
-    int session_id,
-    int render_process_id,
-    int render_view_id,
-    const gfx::Rect& element_rect,
-    const std::string& language,
-    const std::string& grammar,
-    const std::string& origin_url,
-    net::URLRequestContextGetter* context_getter,
-    content::SpeechRecognitionPreferences* recognition_prefs) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      base::Bind(
-          &SpeechRecognitionManagerImpl::CheckRenderViewTypeAndStartRecognition,
-          base::Unretained(this),
-          SpeechRecognitionParams(
-              delegate, session_id, render_process_id, render_view_id,
-              element_rect, language, grammar, origin_url, context_getter,
-              recognition_prefs)));
-}
-
-void SpeechRecognitionManagerImpl::CheckRenderViewTypeAndStartRecognition(
-    const SpeechRecognitionParams& params) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
-  RenderViewHostImpl* render_view_host = RenderViewHostImpl::FromID(
-      params.render_process_id, params.render_view_id);
-  if (!render_view_host || !render_view_host->GetDelegate())
-    return;
-
-  // For host delegates other than TabContents we can't reliably show a popup,
-  // including the speech input bubble. In these cases for privacy reasons we
-  // don't want to start recording if the user can't be properly notified.
-  // An example of this is trying to show the speech input bubble within an
-  // extension popup: http://crbug.com/92083. In these situations the speech
-  // input extension API should be used instead.
-  if (render_view_host->GetDelegate()->GetRenderViewType() ==
-      content::VIEW_TYPE_TAB_CONTENTS) {
-    BrowserThread::PostTask(
-        BrowserThread::IO, FROM_HERE,
-        base::Bind(&SpeechRecognitionManagerImpl::ProceedStartingRecognition,
-        base::Unretained(this), params));
-  }
-}
-
-void SpeechRecognitionManagerImpl::ProceedStartingRecognition(
-    const SpeechRecognitionParams& params) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  DCHECK(!HasPendingRequest(params.session_id));
-
-  if (delegate_.get()) {
-    delegate_->ShowRecognitionRequested(
-        params.session_id, params.render_process_id, params.render_view_id,
-        params.element_rect);
-    delegate_->GetRequestInfo(&can_report_metrics_, &request_info_);
-  }
-
-  Request* request = &requests_[params.session_id];
-  request->delegate = params.delegate;
-  request->recognizer = content::SpeechRecognizer::Create(
-      this, params.session_id, params.language, params.grammar,
-      params.context_getter, params.recognition_prefs->FilterProfanities(),
-      request_info_, can_report_metrics_ ? params.origin_url : "");
-  request->is_active = false;
-
-  StartRecognitionForRequest(params.session_id);
-}
-
-void SpeechRecognitionManagerImpl::StartRecognitionForRequest(int session_id) {
-  SpeechRecognizerMap::iterator request = requests_.find(session_id);
-  if (request == requests_.end()) {
-    NOTREACHED();
-    return;
-  }
-
-  // We should not currently be recording for the session.
-  CHECK(recording_session_id_ != session_id);
-
-  // If we are currently recording audio for another session, abort it cleanly.
-  if (recording_session_id_)
-    CancelRecognitionAndInformDelegate(recording_session_id_);
-  recording_session_id_ = session_id;
-  requests_[session_id].is_active = true;
-  requests_[session_id].recognizer->StartRecognition();
-  if (delegate_.get())
-    delegate_->ShowWarmUp(session_id);
-}
-
-void SpeechRecognitionManagerImpl::CancelRecognitionForRequest(int session_id) {
-  // Ignore if the session 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(session_id))
-    return;
-
-  CancelRecognitionAndInformDelegate(session_id);
-}
-
-void SpeechRecognitionManagerImpl::FocusLostForRequest(int session_id) {
-  // See above comment.
-  if (!HasPendingRequest(session_id))
-    return;
-
-  // If this is an ongoing recording or if we were displaying an error message
-  // to the user, abort it since user has switched focus. Otherwise
-  // recognition has started and keep that going so user can start speaking to
-  // another element while this gets the results in parallel.
-  if (recording_session_id_ == session_id || !requests_[session_id].is_active)
-    CancelRecognitionAndInformDelegate(session_id);
-}
-
-void SpeechRecognitionManagerImpl::CancelRecognition(int session_id) {
-  DCHECK(HasPendingRequest(session_id));
-  if (requests_[session_id].is_active)
-    requests_[session_id].recognizer->AbortRecognition();
-  requests_.erase(session_id);
-  if (recording_session_id_ == session_id)
-    recording_session_id_ = 0;
-  if (delegate_.get())
-    delegate_->DoClose(session_id);
-}
-
-void SpeechRecognitionManagerImpl::CancelAllRequestsWithDelegate(
-    InputTagSpeechDispatcherHost* delegate) {
-  SpeechRecognizerMap::iterator it = requests_.begin();
-  while (it != requests_.end()) {
-    if (it->second.delegate == delegate) {
-      CancelRecognition(it->first);
-      // This map will have very few elements so it is simpler to restart.
-      it = requests_.begin();
-    } else {
-      ++it;
-    }
-  }
-}
-
-void SpeechRecognitionManagerImpl::StopRecording(int session_id) {
-  // No pending requests on extension popups.
-  if (!HasPendingRequest(session_id))
-    return;
-
-  requests_[session_id].recognizer->StopAudioCapture();
-}
-
-// -------- SpeechRecognitionEventListener interface implementation. ---------
-
-void SpeechRecognitionManagerImpl::OnRecognitionResult(
-    int session_id, const content::SpeechRecognitionResult& result) {
-  DCHECK(HasPendingRequest(session_id));
-  GetDelegate(session_id)->SetRecognitionResult(session_id, result);
-}
-
-void SpeechRecognitionManagerImpl::OnAudioEnd(int session_id) {
-  if (recording_session_id_ != session_id)
-    return;
-  DCHECK_EQ(recording_session_id_, session_id);
-  DCHECK(HasPendingRequest(session_id));
-  if (!requests_[session_id].is_active)
-    return;
-  recording_session_id_ = 0;
-  GetDelegate(session_id)->DidCompleteRecording(session_id);
-  if (delegate_.get())
-    delegate_->ShowRecognizing(session_id);
-}
-
-void SpeechRecognitionManagerImpl::OnRecognitionEnd(int session_id) {
-  if (!HasPendingRequest(session_id) || !requests_[session_id].is_active)
-    return;
-  GetDelegate(session_id)->DidCompleteRecognition(session_id);
-  requests_.erase(session_id);
-  if (delegate_.get())
-    delegate_->DoClose(session_id);
-}
-
-void SpeechRecognitionManagerImpl::OnSoundStart(int session_id) {
-}
-
-void SpeechRecognitionManagerImpl::OnSoundEnd(int session_id) {
-}
-
-void SpeechRecognitionManagerImpl::OnRecognitionError(
-    int session_id, const content::SpeechRecognitionError& error) {
-  DCHECK(HasPendingRequest(session_id));
-  if (session_id == recording_session_id_)
-    recording_session_id_ = 0;
-  requests_[session_id].is_active = false;
-  if (delegate_.get()) {
-    if (error.code == content::SPEECH_RECOGNITION_ERROR_AUDIO &&
-        error.details == content::SPEECH_AUDIO_ERROR_DETAILS_NO_MIC) {
-      delegate_->ShowMicError(session_id,
-          SpeechRecognitionManagerDelegate::MIC_ERROR_NO_DEVICE_AVAILABLE);
-    } else if (error.code == content::SPEECH_RECOGNITION_ERROR_AUDIO &&
-               error.details == content::SPEECH_AUDIO_ERROR_DETAILS_IN_USE) {
-      delegate_->ShowMicError(session_id,
-          SpeechRecognitionManagerDelegate::MIC_ERROR_DEVICE_IN_USE);
-    } else {
-      delegate_->ShowRecognizerError(session_id, error.code);
-    }
-  }
-}
-
-void SpeechRecognitionManagerImpl::OnAudioStart(int session_id) {
-  DCHECK(HasPendingRequest(session_id));
-  DCHECK_EQ(recording_session_id_, session_id);
-  if (delegate_.get())
-    delegate_->ShowRecording(session_id);
-}
-
-void SpeechRecognitionManagerImpl::OnRecognitionStart(int session_id) {
-}
-
-void SpeechRecognitionManagerImpl::OnEnvironmentEstimationComplete(
-    int session_id) {
-  DCHECK(HasPendingRequest(session_id));
-  DCHECK_EQ(recording_session_id_, session_id);
-}
-
-void SpeechRecognitionManagerImpl::OnAudioLevelsChange(
-    int session_id, float volume, float noise_volume) {
-  DCHECK(HasPendingRequest(session_id));
-  DCHECK_EQ(recording_session_id_, session_id);
-  if (delegate_.get())
-    delegate_->ShowInputVolume(session_id, volume, noise_volume);
-}
-
-void SpeechRecognitionManagerImpl::CancelRecognitionAndInformDelegate(
-    int session_id) {
-  InputTagSpeechDispatcherHost* cur_delegate = GetDelegate(session_id);
-  CancelRecognition(session_id);
-  cur_delegate->DidCompleteRecording(session_id);
-  cur_delegate->DidCompleteRecognition(session_id);
-}
-
-SpeechRecognitionManagerImpl::Request::Request()
-    : is_active(false) {
-}
-
-SpeechRecognitionManagerImpl::Request::~Request() {
+SpeechRecognitionManagerImpl::FSMEventArgs::FSMEventArgs(FSMEvent event_value)
+  : event(event_value),
+    speech_result(NULL),
+    speech_error(NULL) {
+}
+
+SpeechRecognitionManagerImpl::FSMEventArgs::~FSMEventArgs() {
+}
+
+SpeechRecognitionManagerImpl::Session::Session()
+  : id(0),
+    event_listener(NULL),
+    state(STATE_IDLE),
+    error_occurred(false) {
+}
+
+SpeechRecognitionManagerImpl::Session::~Session() {
 }
 
 }  // namespace speech