Moving tray icon and notification balloon for background speech recognition sessions to ChromeSpeec…

Moving tray icon and notification balloon for background speech recognition sessions to ChromeSpeechRecognitionManagerDelegate. (Speech CL1.13)

The ChromeSpeechRecognitionManagerDelegate now keeps, within profile preferences, a flag that indicates whether a security notification has been shown for each speech recognition context. This applies only to speech recognition sessions initiated by extensions or JS.

BUG=116954
TEST=none


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=142399

[Primiano Tucci (use gerrit), 2012-05-24 13:16:42]

Note for reviewers: review after Speech CL1.12

[jochen (gone - plz use gerrit), 2012-05-24 13:22:31]

On 2012/05/24 13:16:42, Primiano Tucci wrote:
> Note for reviewers: review after Speech CL1.12

content/ changes look good

[hans, 2012-06-11 15:52:15]

lgtm

http://codereview.chromium.org/10407120/diff/14003/chrome/browser/speech/chro...
File chrome/browser/speech/chrome_speech_recognition_manager_delegate.h (right):

http://codereview.chromium.org/10407120/diff/14003/chrome/browser/speech/chro...
chrome/browser/speech/chrome_speech_recognition_manager_delegate.h:65: //
profile associated to the renderer that initiated the recognition.
s/to/with/

http://codereview.chromium.org/10407120/diff/14003/content/browser/speech/spe...
File content/browser/speech/speech_recognition_dispatcher_host.cc (right):

http://codereview.chromium.org/10407120/diff/14003/content/browser/speech/spe...
content/browser/speech/speech_recognition_dispatcher_host.cc:10: #include
"base/utf_string_conversions.h"
i can't see where this is used..

[Primiano Tucci (use gerrit), 2012-06-11 17:03:21]

http://codereview.chromium.org/10407120/diff/14003/chrome/browser/speech/chro...
File chrome/browser/speech/chrome_speech_recognition_manager_delegate.h (right):

http://codereview.chromium.org/10407120/diff/14003/chrome/browser/speech/chro...
chrome/browser/speech/chrome_speech_recognition_manager_delegate.h:65: //
profile associated to the renderer that initiated the recognition.
On 2012/06/11 15:52:15, hans wrote:
> s/to/with/

Done.

http://codereview.chromium.org/10407120/diff/14003/content/browser/speech/spe...
File content/browser/speech/speech_recognition_dispatcher_host.cc (right):

http://codereview.chromium.org/10407120/diff/14003/content/browser/speech/spe...
content/browser/speech/speech_recognition_dispatcher_host.cc:10: #include
"base/utf_string_conversions.h"
On 2012/06/11 15:52:15, hans wrote:
> i can't see where this is used..
Right, it was part some experiments later aborted.
Also removed gurl include (and GURL() code below) because it seems that the
origin_url passed by WebKit is already clean (i.e. without the /request/path).

[Satish, 2012-06-13 12:56:59]

lgtm

http://codereview.chromium.org/10407120/diff/15018/chrome/app/generated_resou...
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/10407120/diff/15018/chrome/app/generated_resou...
chrome/app/generated_resources.grd:15920: +      "<ph
name="EXTENSION_NAME">$1<ex>Speech Recognizer</ex></ph>" is currently capturing
audio for speech recognition.
are you sure that the quotes around <ph> are needed? Looks like a syntax error
to use quotes inside quotes as well, I wonder how this gets built

http://codereview.chromium.org/10407120/diff/15018/chrome/browser/speech/chro...
File chrome/browser/speech/chrome_speech_recognition_manager_delegate.cc
(right):

http://codereview.chromium.org/10407120/diff/15018/chrome/browser/speech/chro...
chrome/browser/speech/chrome_speech_recognition_manager_delegate.cc:408: if
(extension)
do we need a DCHECK here since all the id should match an installed extension?

[Primiano Tucci (use gerrit), 2012-06-13 13:29:38]

http://codereview.chromium.org/10407120/diff/15018/chrome/app/generated_resou...
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/10407120/diff/15018/chrome/app/generated_resou...
chrome/app/generated_resources.grd:15920: +      "<ph
name="EXTENSION_NAME">$1<ex>Speech Recognizer</ex></ph>" is currently capturing
audio for speech recognition.
On 2012/06/13 12:56:59, Satish wrote:
> are you sure that the quotes around <ph> are needed? Looks like a syntax error
> to use quotes inside quotes as well, I wonder how this gets built

Uh, honestly I didn't noticed at all, I just removed the first piece. It seems
to work but it looks weird also to me. Don't know honestly how and when <ph>
tags are replaced and if we can use HTML codes (&quot;) for example

http://codereview.chromium.org/10407120/diff/15018/chrome/browser/speech/chro...
File chrome/browser/speech/chrome_speech_recognition_manager_delegate.cc
(right):

http://codereview.chromium.org/10407120/diff/15018/chrome/browser/speech/chro...
chrome/browser/speech/chrome_speech_recognition_manager_delegate.cc:408: if
(extension)
On 2012/06/13 12:56:59, Satish wrote:
> do we need a DCHECK here since all the id should match an installed extension?
Honestly I tried to be conservative since the purpose of this method is just
lookup a better name.
Don't know if there might be cases in which an extensions cannot be looked-up
back.
Should I introduce a DCHECK anyway?

[Primiano Tucci (use gerrit), 2012-06-13 14:57:17]

http://codereview.chromium.org/10407120/diff/15018/chrome/app/generated_resou...
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/10407120/diff/15018/chrome/app/generated_resou...
chrome/app/generated_resources.grd:15920: +      "<ph
name="EXTENSION_NAME">$1<ex>Speech Recognizer</ex></ph>" is currently capturing
audio for speech recognition.
On 2012/06/13 13:29:38, Primiano Tucci wrote:
> On 2012/06/13 12:56:59, Satish wrote:
> > are you sure that the quotes around <ph> are needed? Looks like a syntax
error
> > to use quotes inside quotes as well, I wonder how this gets built
> 
> Uh, honestly I didn't noticed at all, I just removed the first piece. It seems
> to work but it looks weird also to me. Don't know honestly how and when <ph>
> tags are replaced and if we can use HTML codes (&quot;) for example

It seems a recurring pattern. E.g., IDS_PROFILES_DELETE_MESSAGE -> Are you sure
you want to delete "<ph name="PROFILE_NAME">$1<ex>New User</ex></ph>"

http://codereview.chromium.org/10407120/diff/15018/chrome/browser/speech/chro...
File chrome/browser/speech/chrome_speech_recognition_manager_delegate.cc
(right):

http://codereview.chromium.org/10407120/diff/15018/chrome/browser/speech/chro...
chrome/browser/speech/chrome_speech_recognition_manager_delegate.cc:408: if
(extension)
On 2012/06/13 13:29:38, Primiano Tucci wrote:
> On 2012/06/13 12:56:59, Satish wrote:
> > do we need a DCHECK here since all the id should match an installed
extension?
> Honestly I tried to be conservative since the purpose of this method is just
> lookup a better name.
> Don't know if there might be cases in which an extensions cannot be looked-up
> back.
> Should I introduce a DCHECK anyway?

Done.

[Primiano Tucci (use gerrit), 2012-06-13 15:01:49]

+avi for owners approval on /content/public and /content/renderer

[Avi (use Gerrit), 2012-06-14 14:35:38]

LGTM if you fix the include noted.

http://codereview.chromium.org/10407120/diff/12023/content/public/browser/spe...
File content/public/browser/speech_recognition_session_context.h (right):

http://codereview.chromium.org/10407120/diff/12023/content/public/browser/spe...
content/public/browser/speech_recognition_session_context.h:9: #include
"base/basictypes.h"
What do you use from this? If you need std::string, #include <string>.

[commit-bot: I haz the power, 2012-06-14 14:51:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/primiano@chromium.org/10407120/11013

[Primiano Tucci (use gerrit), 2012-06-14 14:52:01]

http://codereview.chromium.org/10407120/diff/12023/content/public/browser/spe...
File content/public/browser/speech_recognition_session_context.h (right):

http://codereview.chromium.org/10407120/diff/12023/content/public/browser/spe...
content/public/browser/speech_recognition_session_context.h:9: #include
"base/basictypes.h"
On 2012/06/14 14:35:39, Avi wrote:
> What do you use from this? If you need std::string, #include <string>.

Done.

[commit-bot: I haz the power, 2012-06-14 15:26:08]

Try job failure for 10407120-11013 (retry) on win_rel for step "runhooks"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-06-14 15:27:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/primiano@chromium.org/10407120/11013

[commit-bot: I haz the power, 2012-06-14 16:51:41]

Try job failure for 10407120-11013 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-06-15 14:55:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/primiano@chromium.org/10407120/21001

[commit-bot: I haz the power, 2012-06-15 16:07:56]

Change committed as 142399