Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(3055)

Unified Diff: chrome/browser/speech/chrome_speech_recognition_manager_delegate.cc

Issue 636863003: Make SpeechRecognition per RenderFrame instead of per RenderView. Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: fixes threading issues Created 6 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/speech/chrome_speech_recognition_manager_delegate.cc
diff --git a/chrome/browser/speech/chrome_speech_recognition_manager_delegate.cc b/chrome/browser/speech/chrome_speech_recognition_manager_delegate.cc
index 267b9d2938f4f0d0aa44c008603912cd37f16e6b..5a136ab99e87d329bfc9acf1381cefad95f1523f 100644
--- a/chrome/browser/speech/chrome_speech_recognition_manager_delegate.cc
+++ b/chrome/browser/speech/chrome_speech_recognition_manager_delegate.cc
@@ -21,6 +21,7 @@
#include "content/public/browser/notification_registrar.h"
#include "content/public/browser/notification_source.h"
#include "content/public/browser/notification_types.h"
+#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/render_view_host.h"
#include "content/public/browser/resource_context.h"
@@ -42,6 +43,7 @@
#endif
using content::BrowserThread;
+using content::RenderFrameHost;
using content::SpeechRecognitionManager;
using content::WebContents;
@@ -49,8 +51,8 @@ namespace speech {
namespace {
-void TabClosedCallbackOnIOThread(int render_process_id, int render_view_id) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+void AbortSpeechSessionsForFrameIO(int render_process_id, int render_frame_id) {
Charlie Reis 2014/12/01 23:49:43 nit: OnIO
mlamouri (slow - plz ping) 2014/12/08 17:41:04 Done.
+ DCHECK_CURRENTLY_ON(BrowserThread::IO);
SpeechRecognitionManager* manager = SpeechRecognitionManager::GetInstance();
// |manager| becomes NULL if a browser shutdown happens between the post of
@@ -59,7 +61,17 @@ void TabClosedCallbackOnIOThread(int render_process_id, int render_view_id) {
if (!manager)
return;
- manager->AbortAllSessionsForRenderView(render_process_id, render_view_id);
+ manager->AbortAllSessionsForRenderFrame(render_process_id, render_frame_id);
+}
+
+void AbortSpeechSessionsForFrame(RenderFrameHost* render_frame_host) {
+ DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+ BrowserThread::PostTask(
+ BrowserThread::IO, FROM_HERE,
+ base::Bind(&AbortSpeechSessionsForFrameIO,
+ render_frame_host->GetProcess()->GetID(),
+ render_frame_host->GetRoutingID()));
}
} // namespace
@@ -140,7 +152,7 @@ class ChromeSpeechRecognitionManagerDelegate::TabWatcher
: public base::RefCountedThreadSafe<TabWatcher>,
public content::NotificationObserver {
public:
- typedef base::Callback<void(int render_process_id, int render_view_id)>
+ typedef base::Callback<void(WebContents* web_contents)>
TabClosedCallback;
explicit TabWatcher(TabClosedCallback tab_closed_callback)
@@ -148,17 +160,14 @@ class ChromeSpeechRecognitionManagerDelegate::TabWatcher
}
// Starts monitoring the WebContents corresponding to the given
- // |render_process_id|, |render_view_id| pair, invoking |tab_closed_callback_|
- // if closed/unloaded.
- void Watch(int render_process_id, int render_view_id) {
- if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
- BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, base::Bind(
- &TabWatcher::Watch, this, render_process_id, render_view_id));
- return;
- }
+ // |render_process_id|, |render_frame_id| pair, invoking
+ // |tab_closed_callback_| if closed/unloaded.
+ void Watch(int render_process_id, int render_frame_id) {
+ DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+ WebContents* web_contents = WebContents::FromRenderFrameHost(
+ RenderFrameHost::FromID(render_process_id, render_frame_id));
- WebContents* web_contents = tab_util::GetWebContentsByID(render_process_id,
- render_view_id);
// Sessions initiated by speech input extension APIs will end up in a NULL
// WebContent here, but they are properly managed by the
// chrome::SpeechInputExtensionManager. However, sessions initiated within a
@@ -169,11 +178,11 @@ class ChromeSpeechRecognitionManagerDelegate::TabWatcher
return;
// Avoid multiple registrations on |registrar_| for the same |web_contents|.
- if (FindWebContents(web_contents) != registered_web_contents_.end()) {
+ if (registered_web_contents_.find(web_contents) !=
+ registered_web_contents_.end()) {
return;
}
- registered_web_contents_.push_back(
- WebContentsInfo(web_contents, render_process_id, render_view_id));
+ registered_web_contents_.insert(web_contents);
// Lazy initialize the registrar.
if (!registrar_.get())
@@ -196,11 +205,9 @@ class ChromeSpeechRecognitionManagerDelegate::TabWatcher
type == content::NOTIFICATION_RENDER_VIEW_HOST_CHANGED);
WebContents* web_contents = content::Source<WebContents>(source).ptr();
- std::vector<WebContentsInfo>::iterator iter = FindWebContents(web_contents);
- DCHECK(iter != registered_web_contents_.end());
- int render_process_id = iter->render_process_id;
- int render_view_id = iter->render_view_id;
- registered_web_contents_.erase(iter);
+ DCHECK(registered_web_contents_.find(web_contents) !=
+ registered_web_contents_.end());
+ registered_web_contents_.erase(web_contents);
registrar_->Remove(this,
content::NOTIFICATION_RENDER_VIEW_HOST_CHANGED,
@@ -209,25 +216,10 @@ class ChromeSpeechRecognitionManagerDelegate::TabWatcher
content::NOTIFICATION_WEB_CONTENTS_DISCONNECTED,
content::Source<WebContents>(web_contents));
- tab_closed_callback_.Run(render_process_id, render_view_id);
+ tab_closed_callback_.Run(web_contents);
}
private:
- struct WebContentsInfo {
- WebContentsInfo(content::WebContents* web_contents,
- int render_process_id,
- int render_view_id)
- : web_contents(web_contents),
- render_process_id(render_process_id),
- render_view_id(render_view_id) {}
-
- ~WebContentsInfo() {}
-
- content::WebContents* web_contents;
- int render_process_id;
- int render_view_id;
- };
-
friend class base::RefCountedThreadSafe<TabWatcher>;
~TabWatcher() override {
@@ -235,20 +227,6 @@ class ChromeSpeechRecognitionManagerDelegate::TabWatcher
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
}
- // Helper function to find the iterator in |registered_web_contents_| which
- // contains |web_contents|.
- std::vector<WebContentsInfo>::iterator FindWebContents(
- content::WebContents* web_contents) {
- for (std::vector<WebContentsInfo>::iterator i(
- registered_web_contents_.begin());
- i != registered_web_contents_.end(); ++i) {
- if (i->web_contents == web_contents)
- return i;
- }
-
- return registered_web_contents_.end();
- }
-
// Lazy-initialized and used on the UI thread to handle web contents
// notifications (tab closing).
scoped_ptr<content::NotificationRegistrar> registrar_;
@@ -257,7 +235,7 @@ class ChromeSpeechRecognitionManagerDelegate::TabWatcher
// double registrations on |registrar_| and to pass the correct render
// process id and render view id to |tab_closed_callback_| after the process
Charlie Reis 2014/12/01 23:49:42 This comment looks stale.
mlamouri (slow - plz ping) 2014/12/08 17:41:04 Done.
// has gone away.
- std::vector<WebContentsInfo> registered_web_contents_;
+ std::set<WebContents*> registered_web_contents_;
// Callback used to notify, on the thread specified by |callback_thread_| the
Charlie Reis 2014/12/01 23:49:42 callback_thread_ looks stale. This gets called on
mlamouri (slow - plz ping) 2014/12/08 17:41:04 Done.
// closure of a registered tab.
@@ -275,13 +253,10 @@ ChromeSpeechRecognitionManagerDelegate
}
void ChromeSpeechRecognitionManagerDelegate::TabClosedCallback(
- int render_process_id, int render_view_id) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ WebContents* web_contents) {
+ DCHECK_CURRENTLY_ON(BrowserThread::UI);
- // Tell the S.R. Manager (which lives on the IO thread) to abort all the
- // sessions for the given renderer view.
- BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, base::Bind(
- &TabClosedCallbackOnIOThread, render_process_id, render_view_id));
+ web_contents->ForEachFrame(base::Bind(&AbortSpeechSessionsForFrame));
}
void ChromeSpeechRecognitionManagerDelegate::OnRecognitionStart(
@@ -296,7 +271,12 @@ void ChromeSpeechRecognitionManagerDelegate::OnRecognitionStart(
base::Bind(&ChromeSpeechRecognitionManagerDelegate::TabClosedCallback,
base::Unretained(this)));
}
- tab_watcher_->Watch(context.render_process_id, context.render_view_id);
+
+ BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+ base::Bind(&TabWatcher::Watch,
+ tab_watcher_,
+ context.render_process_id,
+ context.render_frame_id));
}
void ChromeSpeechRecognitionManagerDelegate::OnAudioStart(int session_id) {
@@ -357,26 +337,10 @@ void ChromeSpeechRecognitionManagerDelegate::CheckRecognitionIsAllowed(
const content::SpeechRecognitionSessionContext& context =
SpeechRecognitionManager::GetInstance()->GetSessionContext(session_id);
- // Make sure that initiators (extensions/web pages) properly set the
- // |render_process_id| field, which is needed later to retrieve the profile.
- DCHECK_NE(context.render_process_id, 0);
Charlie Reis 2014/12/01 23:49:42 I see that the rest of this block has been moved i
mlamouri (slow - plz ping) 2014/12/08 17:41:04 I removed that because the check sounded pretty ar
Charlie Reis 2014/12/08 20:02:57 Looks like it's initialized to 0 in the context's
mlamouri (slow - plz ping) 2014/12/08 21:07:59 Done.
-
- int render_process_id = context.render_process_id;
- int render_view_id = context.render_view_id;
- if (context.embedder_render_process_id) {
- // If this is a request originated from a guest, we need to re-route the
- // permission check through the embedder (app).
- render_process_id = context.embedder_render_process_id;
- render_view_id = context.embedder_render_view_id;
- }
-
- // Check that the render view type is appropriate, and whether or not we
- // need to request permission from the user.
+ // Check that the context is appropriate, and whether or not we need to
+ // request permission from the user.
BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
- base::Bind(&CheckRenderViewType,
- callback,
- render_process_id,
- render_view_id));
+ base::Bind(&CheckRenderViewType, callback, context));
}
content::SpeechRecognitionEventListener*
@@ -398,16 +362,23 @@ bool ChromeSpeechRecognitionManagerDelegate::FilterProfanities(
// static.
void ChromeSpeechRecognitionManagerDelegate::CheckRenderViewType(
base::Callback<void(bool ask_user, bool is_allowed)> callback,
- int render_process_id,
- int render_view_id) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- const content::RenderViewHost* render_view_host =
- content::RenderViewHost::FromID(render_process_id, render_view_id);
+ const content::SpeechRecognitionSessionContext& context) {
+ DCHECK_CURRENTLY_ON(BrowserThread::UI);
bool allowed = false;
bool check_permission = false;
- if (!render_view_host) {
+ RenderFrameHost* render_frame_host = RenderFrameHost::FromID(
+ context.render_process_id, context.render_frame_id);
+
+ if (context.embedder_render_process_id) {
+ // If this is a request originated from a guest, we need to re-route the
+ // permission check through the embedder (app).
+ render_frame_host = RenderFrameHost::FromID(
+ context.embedder_render_process_id, context.embedder_render_frame_id);
+ }
+
+ if (!render_frame_host) {
// This happens for extensions. Manifest should be checked for permission.
allowed = true;
check_permission = false;
@@ -416,7 +387,8 @@ void ChromeSpeechRecognitionManagerDelegate::CheckRenderViewType(
return;
}
- WebContents* web_contents = WebContents::FromRenderViewHost(render_view_host);
+ WebContents* web_contents =
+ WebContents::FromRenderFrameHost(render_frame_host);
// chrome://app-list/ uses speech recognition.
if (web_contents->GetCommittedWebUI() &&

Powered by Google App Engine
This is Rietveld 408576698