chrome/browser/speech/chrome_speech_recognition_manager_delegate.cc - Issue 636863003: Make SpeechRecognition per RenderFrame instead of per RenderView.

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: review comments Created 6 years 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

« no previous file with comments | « chrome/browser/speech/chrome_speech_recognition_manager_delegate.h ('k') | chrome/browser/ui/app_list/speech_recognizer.cc » ('j') | content/browser/speech/speech_recognition_dispatcher_host.h » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

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..db3c0a79866ab03b0554ff2f4a4cc387f507769c 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,9 @@ namespace speech {

namespace {

-void TabClosedCallbackOnIOThread(int render_process_id, int render_view_id) {

- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));

+void AbortSpeechSessionsForFrameOnIO(int render_process_id,

+ int render_frame_id) {

+ DCHECK_CURRENTLY_ON(BrowserThread::IO);

SpeechRecognitionManager* manager = SpeechRecognitionManager::GetInstance();

// |manager| becomes NULL if a browser shutdown happens between the post of

@@ -59,7 +62,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(&AbortSpeechSessionsForFrameOnIO,

+ render_frame_host->GetProcess()->GetID(),

+ render_frame_host->GetRoutingID()));

}

} // namespace

@@ -140,7 +153,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 +161,14 @@ class ChromeSpeechRecognitionManagerDelegate::TabWatcher

}

// Starts monitoring the WebContents corresponding to the given

- // 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 +179,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 +206,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 +217,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,32 +228,16 @@ 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_;

// Keeps track of which WebContent(s) have been registered, in order to avoid

- // double registrations on |registrar_| and to pass the correct render

- // process id and render view id to |tab_closed_callback_| after the process

- // has gone away.

- std::vector<WebContentsInfo> registered_web_contents_;

+ // double registrations on |registrar_|.

+ std::set<WebContents*> registered_web_contents_;

- // Callback used to notify, on the thread specified by |callback_thread_| the

- // closure of a registered tab.

+ // Callback used to notify, on the the UI thread the closure of a registered

+ // tab.

TabClosedCallback tab_closed_callback_;

DISALLOW_COPY_AND_ASSIGN(TabWatcher);

@@ -275,13 +252,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 +270,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 +336,10 @@ void ChromeSpeechRecognitionManagerDelegate::CheckRecognitionIsAllowed(

const content::SpeechRecognitionSessionContext& context =

SpeechRecognitionManager::GetInstance()->GetSessionContext(session_id);

- // Make sure that initiators (extensions/web pages) properly set the

Charlie Reis 2014/12/09 00:52:33 Let's preserve this comment, since it gives the mo

mlamouri (slow - plz ping) 2015/01/19 11:20:23 Done. Moved where the check happens.

- // |render_process_id| field, which is needed later to retrieve the profile.

- DCHECK_NE(context.render_process_id, 0);

- 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 +361,25 @@ 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);

+ DCHECK_NE(context.render_process_id, 0);

+ DCHECK_NE(context.render_frame_id, MSG_ROUTING_NONE);

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 +388,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() &&