Make SpeechRecognition per RenderFrame instead of per RenderView.

content/browser/speech/speech_recognition_dispatcher_host.h

 // 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.
 
 #ifndef CONTENT_BROWSER_SPEECH_SPEECH_RECOGNITION_DISPATCHER_HOST_H_
 #define CONTENT_BROWSER_SPEECH_SPEECH_RECOGNITION_DISPATCHER_HOST_H_
 
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "content/common/content_export.h"
-#include "content/public/browser/browser_message_filter.h"
 #include "content/public/browser/speech_recognition_event_listener.h"
-#include "net/url_request/url_request_context_getter.h"
+#include "content/public/browser/web_contents_observer.h"
 
 struct SpeechRecognitionHostMsg_StartRequest_Params;
 
+namespace net {
+class URLRequestContextGetter;
+} // namespace net
+
 namespace content {
 
 class SpeechRecognitionManager;
+class WebContents;
 struct SpeechRecognitionResult;
+struct SpeechRecognitionSessionContext;
+struct SpeechRecognitionSessionConfig;
 
 // SpeechRecognitionDispatcherHost 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).
-// It's the complement of SpeechRecognitionDispatcher (owned by RenderView).
+// WebContents. Basically it acts as a proxy, relaying the events coming from
+// the SpeechRecognitionManager to IPC messages (and vice versa).
+// It's the complement of SpeechRecognitionDispatcher (owned by RenderFrame).
+// It MUST be deleted from the BrowserThread::IO thread.
 class CONTENT_EXPORT SpeechRecognitionDispatcherHost
-    : public BrowserMessageFilter,
+    : public WebContentsObserver,

[Charlie Reis, 2014/12/01 23:49:43]

What benefits are we getting from making this a WebContentsObserver rather than
a BrowserMessageFilter?

I ask because it feels a bit awkward for IPC messages to arrive on the IO
thread, get posted to the UI thread (by ChannelProxy) for routing (via
WebContentsObserver), then get posted back to the IO thread here to do work.

Would it be less complex to leave it as a BrowserMessageFilter and always pass
the RenderFrame's routing ID instead, similar to how it used to work with
RenderViews?  Or am I missing something?

[mlamouri (slow - plz ping), 2014/12/08 17:41:04]

The only real benefit from using WebContentsObserver is |RenderFrameDeleted|
which allows us to abort the requests started by a frame after it is deleted. We
might be able to have the same result by other means.

WDYT?

[Charlie Reis, 2014/12/08 20:02:57]

Ah, I see.

Yes, I think we can handle that more cleanly-- we can have a separate
WebContentsObserver class (on the UI thread) whose RenderFrameDeleted method
posts a task to call OnAbortAllRequests on the IO thread.  Then the rest of this
class can stay on the IO thread as a BrowserMessageFilter, which avoids the IO
-> UI -> IO overhead for the majority of its behavior.

[mlamouri (slow - plz ping), 2014/12/08 21:08:00]

Done. I haven't done anything with RenderFrameDeleted actually because
ChromeSpeechRecognitionManagerDelegate already closes the sessions when a tab is
closed and closing a session as soon as the frame is closed isn't something that
was done already so I guess it doesn't have to be done in this CL.

[Charlie Reis, 2014/12/09 00:52:33]

Is this a bug, though?  We're now tracking things per-frame, and a RenderFrame
might be deleted if we navigate cross-process (rather than closing the tab). 
What does it mean to leave the session around in that case?

       public SpeechRecognitionEventListener {
  public:
-  SpeechRecognitionDispatcherHost(
-      int render_process_id,
-      net::URLRequestContextGetter* context_getter);
+  explicit SpeechRecognitionDispatcherHost(WebContents* web_contents);
+  virtual ~SpeechRecognitionDispatcherHost();
 
   base::WeakPtr<SpeechRecognitionDispatcherHost> AsWeakPtr();
 
   // SpeechRecognitionEventListener methods.
   void OnRecognitionStart(int session_id) override;
   void OnAudioStart(int session_id) override;
   void OnEnvironmentEstimationComplete(int session_id) override;
   void OnSoundStart(int session_id) override;
   void OnSoundEnd(int session_id) override;
   void OnAudioEnd(int session_id) override;
   void OnRecognitionEnd(int session_id) override;
   void OnRecognitionResults(int session_id,
                             const SpeechRecognitionResults& results) override;
   void OnRecognitionError(int session_id,
                           const SpeechRecognitionError& error) override;
   void OnAudioLevelsChange(int session_id,
                            float volume,
                            float noise_volume) override;
 
-  // BrowserMessageFilter implementation.
-  void OnDestruct() const override;
-  bool OnMessageReceived(const IPC::Message& message) override;
-  void OverrideThreadForMessage(const IPC::Message& message,
-                                BrowserThread::ID* thread) override;
-
-  void OnChannelClosing() override;
+  // WebContentsObserver implementation.
+  bool OnMessageReceived(const IPC::Message& message,
+                         RenderFrameHost* render_frame_host) override;
+  void RenderFrameDeleted(RenderFrameHost* render_frame_host) override;
 
  private:
-  friend class base::DeleteHelper<SpeechRecognitionDispatcherHost>;
-  friend class BrowserThread;
-
-  ~SpeechRecognitionDispatcherHost() override;
-
   void OnStartRequest(
+      RenderFrameHost* render_frame_host,
       const SpeechRecognitionHostMsg_StartRequest_Params& params);
-  void OnStartRequestOnIO(
-      int embedder_render_process_id,
-      int embedder_render_view_id,
+  void StartSession(
       const SpeechRecognitionHostMsg_StartRequest_Params& params,
-      int params_render_frame_id,
+      const SpeechRecognitionSessionContext& context,
+      net::URLRequestContextGetter* url_request_context_getter,
       bool filter_profanities);
-  void OnAbortRequest(int render_view_id, int request_id);
-  void OnStopCaptureRequest(int render_view_id, int request_id);
-  void OnAbortAllRequests(int render_view_id);
-
-  int render_process_id_;
-  scoped_refptr<net::URLRequestContextGetter> context_getter_;
+  void OnAbortRequest(RenderFrameHost* render_frame_host, int request_id);
+  void OnAbortRequestIO(int render_process_id,

[Charlie Reis, 2014/12/01 23:49:43]

nit: AbortRequestOnIO (similar below)

+                        int render_frame_id,
+                        int request_id);
+  void OnStopCaptureRequest(RenderFrameHost* render_frame_host, int request_id);
+  void OnStopCaptureRequestIO(int render_process_id,
+                              int render_frame_id,
+                              int request_id);
+  void OnAbortAllRequests(RenderFrameHost* render_frame_host);
+  void OnAbortAllRequestsIO(int render_process_id, int render_frame_id);
 
   // Used for posting asynchronous tasks (on the IO thread) without worrying
   // about this class being destroyed in the meanwhile (due to browser shutdown)
   // since tasks pending on a destroyed WeakPtr are automatically discarded.
   base::WeakPtrFactory<SpeechRecognitionDispatcherHost> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(SpeechRecognitionDispatcherHost);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_SPEECH_SPEECH_RECOGNITION_DISPATCHER_HOST_H_