Moved instantiation of SpeechRecognitionManager inside browser_main_loop, instead of Singleton. (Speech CL1.10)

Compared to the Singleton solution, this allows the SpeechRecognitionManager to instantiate and cleanup the delegate at the right time.
Also, cleaned-up speech_recognition_browsertest.cc so that the FakeSpeechRecognitionManager extends only the SRM interface, and not the Impl class.

BUG=116954
TEST=none


Review URL: https://chromiumcodereview.appspot.com/10399025

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

9 files changed, 68 insertions, 40 deletions

diff --git a/content/browser/browser_main_loop.cc b/content/browser/browser_main_loop.cc
index 9e14737..3a92507 100644
--- a/content/browser/browser_main_loop.cc
+++ b/content/browser/browser_main_loop.cc
@@ -23,6 +23,7 @@
 #include "content/browser/net/browser_online_state_observer.h"
 #include "content/browser/plugin_service_impl.h"
 #include "content/browser/renderer_host/resource_dispatcher_host_impl.h"
+#include "content/browser/speech/speech_recognition_manager_impl.h"
 #include "content/browser/trace_controller_impl.h"
 #include "content/common/hi_res_timer_manager.h"
 #include "content/public/browser/browser_main_parts.h"
@@ -480,6 +481,8 @@ void BrowserMainLoop::ShutdownThreadsAndCleanUp() {
   if (resource_dispatcher_host_.get())
     resource_dispatcher_host_.get()->Shutdown();
 
+  speech_recognition_manager_.reset();
+
 #if defined(USE_AURA)
   ImageTransportFactory::Terminate();
 #endif
@@ -593,6 +596,10 @@ void BrowserMainLoop::BrowserThreadsStarted() {
   // RDH needs the IO thread to be created.
   resource_dispatcher_host_.reset(new ResourceDispatcherHostImpl());
 
+#if defined(ENABLE_INPUT_SPEECH)
+  speech_recognition_manager_.reset(new speech::SpeechRecognitionManagerImpl());
+#endif
+
   // Start the GpuDataManager before we set up the MessageLoops because
   // otherwise we'll trigger the assertion about doing IO on the UI thread.
   content::GpuDataManager::GetInstance();
diff --git a/content/browser/browser_main_loop.h b/content/browser/browser_main_loop.h
index 514ddf0..6905431 100644
--- a/content/browser/browser_main_loop.h
+++ b/content/browser/browser_main_loop.h
@@ -32,6 +32,10 @@ namespace net {
 class NetworkChangeNotifier;
 }
 
+namespace speech {
+class SpeechRecognitionManagerImpl;
+}
+
 namespace content {
 
 class BrowserMainParts;
@@ -108,6 +112,7 @@ class BrowserMainLoop {
 
   // Members initialized in |BrowserThreadsStarted()| --------------------------
   scoped_ptr<ResourceDispatcherHostImpl> resource_dispatcher_host_;
+  scoped_ptr<speech::SpeechRecognitionManagerImpl> speech_recognition_manager_;
 
   // Members initialized in |RunMainMessageLoopParts()| ------------------------
   scoped_ptr<BrowserProcessSubThread> db_thread_;
diff --git a/content/browser/speech/input_tag_speech_dispatcher_host.cc b/content/browser/speech/input_tag_speech_dispatcher_host.cc
index cf604829..0fa54be 100644
--- a/content/browser/speech/input_tag_speech_dispatcher_host.cc
+++ b/content/browser/speech/input_tag_speech_dispatcher_host.cc
@@ -14,6 +14,7 @@
 #include "content/public/browser/speech_recognition_session_context.h"
 
 using content::BrowserThread;
+using content::SpeechRecognitionManager;
 using content::SpeechRecognitionSessionConfig;
 using content::SpeechRecognitionSessionContext;
 
@@ -29,11 +30,11 @@ bool IsSameContext(int render_process_id,
 }  // namespace
 
 namespace speech {
-SpeechRecognitionManagerImpl* InputTagSpeechDispatcherHost::manager_;
+SpeechRecognitionManager* InputTagSpeechDispatcherHost::manager_for_tests_;
 
-void InputTagSpeechDispatcherHost::set_manager(
-    SpeechRecognitionManagerImpl* manager) {
-  manager_ = manager;
+void InputTagSpeechDispatcherHost::SetManagerForTests(
+    SpeechRecognitionManager* manager) {
+  manager_for_tests_ = manager;
 }
 
 InputTagSpeechDispatcherHost::InputTagSpeechDispatcherHost(
@@ -59,11 +60,11 @@ InputTagSpeechDispatcherHost::~InputTagSpeechDispatcherHost() {
     manager()->AbortAllSessionsForListener(this);
 }
 
-SpeechRecognitionManagerImpl* InputTagSpeechDispatcherHost::manager() {
-  if (manager_)
-    return manager_;
+SpeechRecognitionManager* InputTagSpeechDispatcherHost::manager() {
+  if (manager_for_tests_)
+    return manager_for_tests_;
 #if defined(ENABLE_INPUT_SPEECH)
-  return SpeechRecognitionManagerImpl::GetInstance();
+  return SpeechRecognitionManager::GetInstance();
 #else
   return NULL;
 #endif
@@ -111,7 +112,7 @@ void InputTagSpeechDispatcherHost::OnStartRecognition(
   config.event_listener = this;
 
   int session_id = manager()->CreateSession(config);
-  if (session_id == content::SpeechRecognitionManager::kSessionIDInvalid)
+  if (session_id == SpeechRecognitionManager::kSessionIDInvalid)
     return;
 
    manager()->StartSession(session_id);
@@ -125,7 +126,7 @@ void InputTagSpeechDispatcherHost::OnCancelRecognition(int render_view_id,
                  render_process_id_,
                  render_view_id,
                  request_id));
-  if (session_id != content::SpeechRecognitionManager::kSessionIDInvalid)
+  if (session_id != SpeechRecognitionManager::kSessionIDInvalid)
     manager()->AbortSession(session_id);
 }
 
@@ -137,7 +138,7 @@ void InputTagSpeechDispatcherHost::OnStopRecording(int render_view_id,
                  render_process_id_,
                  render_view_id,
                  request_id));
-  DCHECK_NE(session_id, content::SpeechRecognitionManager::kSessionIDInvalid);
+  DCHECK_NE(session_id, SpeechRecognitionManager::kSessionIDInvalid);
   manager()->StopAudioCaptureForSession(session_id);
 }
 
diff --git a/content/browser/speech/input_tag_speech_dispatcher_host.h b/content/browser/speech/input_tag_speech_dispatcher_host.h
index b98d9e5..637ca12 100644
--- a/content/browser/speech/input_tag_speech_dispatcher_host.h
+++ b/content/browser/speech/input_tag_speech_dispatcher_host.h
@@ -15,14 +15,13 @@
 struct InputTagSpeechHostMsg_StartRecognition_Params;
 
 namespace content {
+class SpeechRecognitionManager;
 class SpeechRecognitionPreferences;
 struct SpeechRecognitionResult;
 }
 
 namespace speech {
 
-class SpeechRecognitionManagerImpl;
-
 // InputTagSpeechDispatcherHost is a delegate for Speech API messages used by
 // RenderMessageFilter. Basically it acts as a proxy, relaying the events coming
 // from the SpeechRecognitionManager to IPC messages (and vice versa).
@@ -56,7 +55,7 @@ class CONTENT_EXPORT InputTagSpeechDispatcherHost
                                  bool* message_was_ok) OVERRIDE;
 
   // Singleton manager setter useful for tests.
-  static void set_manager(SpeechRecognitionManagerImpl* manager);
+  static void SetManagerForTests(content::SpeechRecognitionManager* manager);
 
  private:
   virtual ~InputTagSpeechDispatcherHost();
@@ -68,7 +67,7 @@ class CONTENT_EXPORT InputTagSpeechDispatcherHost
 
   // Returns the speech recognition manager to forward events to, creating one
   // if needed.
-  SpeechRecognitionManagerImpl* manager();
+  content::SpeechRecognitionManager* manager();
 
   int render_process_id_;
   bool may_have_pending_requests_;  // Set if we received any speech IPC request
@@ -76,7 +75,7 @@ class CONTENT_EXPORT InputTagSpeechDispatcherHost
   scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_;
   scoped_refptr<content::SpeechRecognitionPreferences> recognition_preferences_;
 
-  static SpeechRecognitionManagerImpl* manager_;
+  static content::SpeechRecognitionManager* manager_for_tests_;
 
   DISALLOW_COPY_AND_ASSIGN(InputTagSpeechDispatcherHost);
 };
diff --git a/content/browser/speech/speech_recognition_browsertest.cc b/content/browser/speech/speech_recognition_browsertest.cc
index 6415640..a939fb6 100644
--- a/content/browser/speech/speech_recognition_browsertest.cc
+++ b/content/browser/speech/speech_recognition_browsertest.cc
@@ -14,9 +14,9 @@
 #include "chrome/test/base/ui_test_utils.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/speech_recognition_manager_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/public/browser/notification_types.h"
+#include "content/public/browser/speech_recognition_manager.h"
 #include "content/public/browser/speech_recognition_session_config.h"
 #include "content/public/browser/speech_recognition_session_context.h"
 #include "content/public/common/content_switches.h"
@@ -25,6 +25,7 @@
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebInputEvent.h"
 
 using content::SpeechRecognitionEventListener;
+using content::SpeechRecognitionSessionConfig;
 using content::SpeechRecognitionSessionContext;
 using content::NavigationController;
 using content::WebContents;
@@ -37,7 +38,7 @@ namespace speech {
 
 const char kTestResult[] = "Pictures of the moon";
 
-class FakeSpeechRecognitionManager : public SpeechRecognitionManagerImpl {
+class FakeSpeechRecognitionManager : public content::SpeechRecognitionManager {
  public:
   FakeSpeechRecognitionManager()
       : session_id_(0),
@@ -77,6 +78,7 @@ class FakeSpeechRecognitionManager : public SpeechRecognitionManagerImpl {
     if (config.grammars.size() > 0)
       grammar_ = config.grammars[0].url;
     session_ctx_ = config.initial_context;
+    session_config_ = config;
     session_id_ = 1;
     return session_id_;
   }
@@ -136,6 +138,12 @@ class FakeSpeechRecognitionManager : public SpeechRecognitionManagerImpl {
     return matched ? session_id_ : 0;
   }
 
+  virtual const SpeechRecognitionSessionConfig& GetSessionConfig(
+      int session_id) const OVERRIDE {
+    EXPECT_EQ(session_id, session_id_);
+    return session_config_;
+  }
+
   virtual content::SpeechRecognitionSessionContext GetSessionContext(
       int session_id) const OVERRIDE {
     EXPECT_EQ(session_id, session_id_);
@@ -160,6 +168,7 @@ class FakeSpeechRecognitionManager : public SpeechRecognitionManagerImpl {
 
   int session_id_;
   SpeechRecognitionEventListener* listener_;
+  SpeechRecognitionSessionConfig session_config_;
   SpeechRecognitionSessionContext session_ctx_;
   std::string grammar_;
   bool did_cancel_all_;
@@ -226,7 +235,8 @@ class SpeechRecognitionBrowserTest : public InProcessBrowserTest {
 
     // Inject the fake manager factory so that the test result is returned to
     // the web page.
-    InputTagSpeechDispatcherHost::set_manager(speech_recognition_manager_);
+    InputTagSpeechDispatcherHost::SetManagerForTests(
+        speech_recognition_manager_);
   }
 
   virtual void TearDownInProcessBrowserTestFixture() {
@@ -237,10 +247,10 @@ class SpeechRecognitionBrowserTest : public InProcessBrowserTest {
 
   // This is used by the static |fakeManager|, and it is a pointer rather than a
   // direct instance per the style guide.
-  static SpeechRecognitionManagerImpl* speech_recognition_manager_;
+  static content::SpeechRecognitionManager* speech_recognition_manager_;
 };
 
-SpeechRecognitionManagerImpl*
+content::SpeechRecognitionManager*
     SpeechRecognitionBrowserTest::speech_recognition_manager_ = NULL;
 
 // TODO(satish): Once this flakiness has been fixed, add a second test here to
diff --git a/content/browser/speech/speech_recognition_manager_impl.cc b/content/browser/speech/speech_recognition_manager_impl.cc
index 9301d9d..7a0d5c8 100644
--- a/content/browser/speech/speech_recognition_manager_impl.cc
+++ b/content/browser/speech/speech_recognition_manager_impl.cc
@@ -5,7 +5,6 @@
 #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/speech/google_one_shot_remote_engine.h"
 #include "content/browser/speech/speech_recognition_engine.h"
@@ -39,22 +38,30 @@ SpeechRecognitionManager* SpeechRecognitionManager::GetInstance() {
 }
 }  // namespace content
 
+namespace {
+speech::SpeechRecognitionManagerImpl* g_speech_recognition_manager_impl;
+}  // namespace
+
 namespace speech {
 
 SpeechRecognitionManagerImpl* SpeechRecognitionManagerImpl::GetInstance() {
-  return Singleton<SpeechRecognitionManagerImpl,
-                   LeakySingletonTraits<SpeechRecognitionManagerImpl> >::get();
+  DCHECK(g_speech_recognition_manager_impl);
+  return g_speech_recognition_manager_impl;
 }
 
 SpeechRecognitionManagerImpl::SpeechRecognitionManagerImpl()
     : session_id_capturing_audio_(kSessionIDInvalid),
       last_session_id_(kSessionIDInvalid),
-      is_dispatching_event_(false) {
-  delegate_ = content::GetContentClient()->browser()->
-      GetSpeechRecognitionManagerDelegate();
+      is_dispatching_event_(false),
+      delegate_(content::GetContentClient()->browser()->
+                    GetSpeechRecognitionManagerDelegate()) {
+  DCHECK(!g_speech_recognition_manager_impl);
+  g_speech_recognition_manager_impl = this;
 }
 
 SpeechRecognitionManagerImpl::~SpeechRecognitionManagerImpl() {
+  DCHECK(g_speech_recognition_manager_impl);
+  g_speech_recognition_manager_impl = NULL;
   // Recognition sessions will be aborted by the corresponding destructors.
   sessions_.clear();
 }
@@ -73,7 +80,7 @@ int SpeechRecognitionManagerImpl::CreateSession(
 
   std::string hardware_info;
   bool can_report_metrics = false;
-  if (delegate_)
+  if (delegate_.get())
     delegate_->GetDiagnosticInformation(&can_report_metrics, &hardware_info);
 
   SpeechRecognitionEngineConfig remote_engine_config;
@@ -108,7 +115,7 @@ void SpeechRecognitionManagerImpl::StartSession(int session_id) {
     AbortSession(session_id_capturing_audio_);
   }
 
-  if (delegate_)
+  if (delegate_.get())
     delegate_->CheckRecognitionIsAllowed(
         session_id,
         base::Bind(&SpeechRecognitionManagerImpl::RecognitionAllowedCallback,
@@ -472,7 +479,7 @@ SpeechRecognitionEventListener* SpeechRecognitionManagerImpl::GetListener(
 
 SpeechRecognitionEventListener*
 SpeechRecognitionManagerImpl::GetDelegateListener() const {
-  return delegate_ ? delegate_->GetEventListener() : NULL;
+  return delegate_.get() ? delegate_->GetEventListener() : NULL;
 }
 
 const SpeechRecognitionSessionConfig&
diff --git a/content/browser/speech/speech_recognition_manager_impl.h b/content/browser/speech/speech_recognition_manager_impl.h
index 6088a5a..5aab48f 100644
--- a/content/browser/speech/speech_recognition_manager_impl.h
+++ b/content/browser/speech/speech_recognition_manager_impl.h
@@ -12,7 +12,6 @@
 #include "base/basictypes.h"
 #include "base/callback.h"
 #include "base/compiler_specific.h"
-#include "base/memory/singleton.h"
 #include "content/public/browser/speech_recognition_event_listener.h"
 #include "content/public/browser/speech_recognition_manager.h"
 #include "content/public/browser/speech_recognition_session_config.h"
@@ -20,6 +19,7 @@
 #include "content/public/common/speech_recognition_error.h"
 
 namespace content {
+class BrowserMainLoop;
 class SpeechRecognitionManagerDelegate;
 }
 
@@ -27,7 +27,7 @@ namespace speech {
 
 class SpeechRecognizerImpl;
 
-// This is the manager for speech recognition. It is a singleton instance in
+// This is the manager for speech recognition. It is a single instance in
 // the browser process and can serve several requests. Each recognition request
 // corresponds to a session, initiated via |CreateSession|.
 // In every moment the manager has at most one session capturing audio, which
@@ -51,6 +51,8 @@ class CONTENT_EXPORT SpeechRecognitionManagerImpl :
     public NON_EXPORTED_BASE(content::SpeechRecognitionManager),
     public content::SpeechRecognitionEventListener {
  public:
+  // Returns the current SpeechRecognitionManagerImpl. Can be called only after
+  // the RecognitionMnager has been created (by BrowserMainLoop).
   static SpeechRecognitionManagerImpl* GetInstance();
 
   // SpeechRecognitionManager implementation.
@@ -90,8 +92,9 @@ class CONTENT_EXPORT SpeechRecognitionManagerImpl :
                                    float noise_volume) OVERRIDE;
 
  protected:
-  // Private constructor to enforce singleton.
-  friend struct DefaultSingletonTraits<SpeechRecognitionManagerImpl>;
+  // BrowserMainLoop is the only one allowed to istantiate and free us.
+  friend class content::BrowserMainLoop;
+  friend class scoped_ptr<SpeechRecognitionManagerImpl>;  // Needed for dtor.
   SpeechRecognitionManagerImpl();
   virtual ~SpeechRecognitionManagerImpl();
 
@@ -158,7 +161,7 @@ class CONTENT_EXPORT SpeechRecognitionManagerImpl :
   int session_id_capturing_audio_;
   int last_session_id_;
   bool is_dispatching_event_;
-  content::SpeechRecognitionManagerDelegate* delegate_;
+  scoped_ptr<content::SpeechRecognitionManagerDelegate> delegate_;
 };
 
 }  // namespace speech
diff --git a/content/public/browser/speech_recognition_session_config.cc b/content/public/browser/speech_recognition_session_config.cc
index 1f3cd4c..8a21f33 100644
--- a/content/public/browser/speech_recognition_session_config.cc
+++ b/content/public/browser/speech_recognition_session_config.cc
@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 #include "content/public/browser/speech_recognition_session_config.h"
-#include "net/url_request/url_request_context_getter.h"
 
 namespace content {
 
diff --git a/content/public/browser/speech_recognition_session_config.h b/content/public/browser/speech_recognition_session_config.h
index 461cec2..8f44374 100644
--- a/content/public/browser/speech_recognition_session_config.h
+++ b/content/public/browser/speech_recognition_session_config.h
@@ -11,10 +11,7 @@
 #include "content/common/content_export.h"
 #include "content/public/browser/speech_recognition_session_context.h"
 #include "content/public/common/speech_recognition_grammar.h"
-
-namespace net {
-class URLRequestContextGetter;
-}
+#include "net/url_request/url_request_context_getter.h"
 
 namespace content {