Renamed speech input implementation from to speech_recognition_*. The namespace has been renamed fr…

Renamed speech recognition code in preparation for introducing a new speech implementation.

- The general-purpose speech recognition classes (that will be needed also by future code) have been renamed to SpeechRecognition*;
- The IPC Messages and dispatcher(host) classes that are only related to the current input tag (x-webkit-speech) support have been renamed to InputTagSpeech*;
- The namespace has been renamed from speech_input to speech;

BUG=116954
TEST=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=125378

[hans, 2012-03-05 13:26:33]

This looks good! A couple of nits and a question below.

I also think the CL "Description" (which will become the commit message) could
be better. There should normally be a short first line that summarizes what the
CL does, then a blank line, and then a longer explanation.

http://codereview.chromium.org/9568002/diff/1/chrome/chrome_browser.gypi
File chrome/chrome_browser.gypi (right):

http://codereview.chromium.org/9568002/diff/1/chrome/chrome_browser.gypi#newc...
chrome/chrome_browser.gypi:2329:
'browser/speech/speech_recognition_bubble_views.cc',
these needs to be sorted after "speech_input_extension_*" below

http://codereview.chromium.org/9568002/diff/1/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

http://codereview.chromium.org/9568002/diff/1/chrome/chrome_tests.gypi#newcod...
chrome/chrome_tests.gypi:2739:
'browser/speech/speech_recognition_bubble_browsertest.cc',
sort

http://codereview.chromium.org/9568002/diff/1/chrome/common/pref_names.h
File chrome/common/pref_names.h (right):

http://codereview.chromium.org/9568002/diff/1/chrome/common/pref_names.h#newc...
chrome/common/pref_names.h:253: extern const char
kSpeechRecognitionFilterProfanities[];
hmm, this list looks like it's semi-sorted, so maybe move this one step down

http://codereview.chromium.org/9568002/diff/1/content/browser/speech/speech_r...
File content/browser/speech/speech_recognition_manager_impl.cc (right):

http://codereview.chromium.org/9568002/diff/1/content/browser/speech/speech_r...
content/browser/speech/speech_recognition_manager_impl.cc:24:
content::SpeechRecognitionManager*
if you put a "using content::SpeechRecognitionManager" above, so you wouldn't
need the "content::" prefixes here, maybe we wouldn't need the line-break?

http://codereview.chromium.org/9568002/diff/1/content/public/common/speech_re...
File content/public/common/speech_recognition_traits.h (right):

http://codereview.chromium.org/9568002/diff/1/content/public/common/speech_re...
content/public/common/speech_recognition_traits.h:49: struct CONTENT_EXPORT
SpeechInputResult {
looking at this file's name, I would expect it to declare a class
SpeechRecognitionTraits?

[Satish, 2012-03-05 13:32:33]

https://chromiumcodereview.appspot.com/9568002/diff/1/content/public/common/s...
File content/public/common/speech_recognition_traits.h (left):

https://chromiumcodereview.appspot.com/9568002/diff/1/content/public/common/s...
content/public/common/speech_recognition_traits.h:5: #ifndef
CONTENT_PUBLIC_COMMON_SPEECH_INPUT_RESULT_H_
suggest naming this file as 'speech_recognition_result.h' and adjusting these
appropriately

https://chromiumcodereview.appspot.com/9568002/diff/1/content/public/common/s...
content/public/common/speech_recognition_traits.h:16: struct
SpeechInputHypothesis {
could also rename these 'Input' to 'Recognition', same for other similar classes
in this file

https://chromiumcodereview.appspot.com/9568002/diff/1/content/renderer/speech...
File content/renderer/speech_recognition_dispatcher.cc (right):

https://chromiumcodereview.appspot.com/9568002/diff/1/content/renderer/speech...
content/renderer/speech_recognition_dispatcher.cc:39:
IPC_MESSAGE_HANDLER(SpeechRecognitionMsg_ResultRetrieved,
as we discussed offline lets separate the old and new speech recognition
implementations cleanly so the old can be deprecated when the new one is ready.
With that in mind may be revert these messages to what they were before but just
rename Input=>Recognition ?

[Primiano Tucci (use gerrit), 2012-03-05 17:37:52]

https://chromiumcodereview.appspot.com/9568002/diff/1/chrome/chrome_browser.gypi
File chrome/chrome_browser.gypi (right):

https://chromiumcodereview.appspot.com/9568002/diff/1/chrome/chrome_browser.g...
chrome/chrome_browser.gypi:2329:
'browser/speech/speech_recognition_bubble_views.cc',
On 2012/03/05 13:26:33, hans wrote:
> these needs to be sorted after "speech_input_extension_*" below

Done.

https://chromiumcodereview.appspot.com/9568002/diff/1/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://chromiumcodereview.appspot.com/9568002/diff/1/chrome/chrome_tests.gyp...
chrome/chrome_tests.gypi:2739:
'browser/speech/speech_recognition_bubble_browsertest.cc',
On 2012/03/05 13:26:33, hans wrote:
> sort

Done.

https://chromiumcodereview.appspot.com/9568002/diff/1/chrome/common/pref_names.h
File chrome/common/pref_names.h (right):

https://chromiumcodereview.appspot.com/9568002/diff/1/chrome/common/pref_name...
chrome/common/pref_names.h:253: extern const char
kSpeechRecognitionFilterProfanities[];
On 2012/03/05 13:26:33, hans wrote:
> hmm, this list looks like it's semi-sorted, so maybe move this one step down

Done.

https://chromiumcodereview.appspot.com/9568002/diff/1/content/browser/speech/...
File content/browser/speech/speech_recognition_manager_impl.cc (right):

https://chromiumcodereview.appspot.com/9568002/diff/1/content/browser/speech/...
content/browser/speech/speech_recognition_manager_impl.cc:24:
content::SpeechRecognitionManager*
On 2012/03/05 13:26:33, hans wrote:
> if you put a "using content::SpeechRecognitionManager" above, so you wouldn't
> need the "content::" prefixes here, maybe we wouldn't need the line-break?

Done.

https://chromiumcodereview.appspot.com/9568002/diff/1/content/public/common/s...
File content/public/common/speech_recognition_traits.h (left):

https://chromiumcodereview.appspot.com/9568002/diff/1/content/public/common/s...
content/public/common/speech_recognition_traits.h:5: #ifndef
CONTENT_PUBLIC_COMMON_SPEECH_INPUT_RESULT_H_
On 2012/03/05 13:32:33, Satish wrote:
> suggest naming this file as 'speech_recognition_result.h' and adjusting these
> appropriately

Done.

https://chromiumcodereview.appspot.com/9568002/diff/1/content/public/common/s...
content/public/common/speech_recognition_traits.h:16: struct
SpeechInputHypothesis {
On 2012/03/05 13:32:33, Satish wrote:
> could also rename these 'Input' to 'Recognition', same for other similar
classes
> in this file

Done.

https://chromiumcodereview.appspot.com/9568002/diff/1/content/public/common/s...
File content/public/common/speech_recognition_traits.h (right):

https://chromiumcodereview.appspot.com/9568002/diff/1/content/public/common/s...
content/public/common/speech_recognition_traits.h:49: struct CONTENT_EXPORT
SpeechInputResult {
On 2012/03/05 13:26:33, hans wrote:
> looking at this file's name, I would expect it to declare a class
> SpeechRecognitionTraits?

Done.

https://chromiumcodereview.appspot.com/9568002/diff/1/content/renderer/speech...
File content/renderer/speech_recognition_dispatcher.cc (right):

https://chromiumcodereview.appspot.com/9568002/diff/1/content/renderer/speech...
content/renderer/speech_recognition_dispatcher.cc:39:
IPC_MESSAGE_HANDLER(SpeechRecognitionMsg_ResultRetrieved,
As we re-discussed offline I renamed the messages to InputTagSpeechMsg* in order
to clearly highlight their scope related only to the x-webkit-speech input tag
support.

[hans, 2012-03-06 10:05:17]

lgtm

[Satish, 2012-03-06 10:05:31]

Some more comments..

https://chromiumcodereview.appspot.com/9568002/diff/4021/chrome/common/pref_n...
File chrome/common/pref_names.h (right):

https://chromiumcodereview.appspot.com/9568002/diff/4021/chrome/common/pref_n...
chrome/common/pref_names.h:253: extern const char
kSpeechInputTrayNotificationShown[];
this reminds me that it would be useful to open a new bug to track renaming
'experimental.speechInput' extension api to 'experimental.speechRecognition' as
well

https://chromiumcodereview.appspot.com/9568002/diff/4021/content/browser/spee...
File content/browser/speech/input_tag_speech_dispatcher_host.cc (right):

https://chromiumcodereview.appspot.com/9568002/diff/4021/content/browser/spee...
content/browser/speech/input_tag_speech_dispatcher_host.cc:60: int
render_process_id,
suggest moving this to previous line and aligning the other 2 parameters with
that as done before, that is easier to read when space is available

https://chromiumcodereview.appspot.com/9568002/diff/4021/content/browser/spee...
content/browser/speech/input_tag_speech_dispatcher_host.cc:160:
IPC_MESSAGE_HANDLER(InputTagSpeechHostMsg_StartRequest,
since the purpose of this CL is to only rename SpeechInput to SpeechRecognition
and the likes, we should probably keep the IPC message names as they were before
- X_StartRecognition, X_CancelRecognition and so on.

https://chromiumcodereview.appspot.com/9568002/diff/4021/content/browser/spee...
content/browser/speech/input_tag_speech_dispatcher_host.cc:212: Send(new
InputTagSpeechMsg_ResultRetrieved(caller_render_view_id,
same as above, suggest maintaining the IPC message names as they were except for
switching the prefix

https://chromiumcodereview.appspot.com/9568002/diff/4021/content/renderer/inp...
File content/renderer/input_tag_speech_dispatcher.h (right):

https://chromiumcodereview.appspot.com/9568002/diff/4021/content/renderer/inp...
content/renderer/input_tag_speech_dispatcher.h:1: // Copyright (c) 2011 The
Chromium Authors. All rights reserved.
2011 -> 2012, same for other files in this CL

[Primiano Tucci (use gerrit), 2012-03-06 10:40:44]

https://chromiumcodereview.appspot.com/9568002/diff/4021/content/browser/spee...
File content/browser/speech/input_tag_speech_dispatcher_host.cc (right):

https://chromiumcodereview.appspot.com/9568002/diff/4021/content/browser/spee...
content/browser/speech/input_tag_speech_dispatcher_host.cc:60: int
render_process_id,
On 2012/03/06 10:05:36, Satish wrote:
> suggest moving this to previous line and aligning the other 2 parameters with
> that as done before, that is easier to read when space is available

Done.

https://chromiumcodereview.appspot.com/9568002/diff/4021/content/browser/spee...
content/browser/speech/input_tag_speech_dispatcher_host.cc:160:
IPC_MESSAGE_HANDLER(InputTagSpeechHostMsg_StartRequest,
On 2012/03/06 10:05:36, Satish wrote:
> since the purpose of this CL is to only rename SpeechInput to
SpeechRecognition
> and the likes, we should probably keep the IPC message names as they were
before
> - X_StartRecognition, X_CancelRecognition and so on.

Done.

https://chromiumcodereview.appspot.com/9568002/diff/4021/content/browser/spee...
content/browser/speech/input_tag_speech_dispatcher_host.cc:212: Send(new
InputTagSpeechMsg_ResultRetrieved(caller_render_view_id,
On 2012/03/06 10:05:36, Satish wrote:
> same as above, suggest maintaining the IPC message names as they were except
for
> switching the prefix

Done.

https://chromiumcodereview.appspot.com/9568002/diff/4021/content/renderer/inp...
File content/renderer/input_tag_speech_dispatcher.h (right):

https://chromiumcodereview.appspot.com/9568002/diff/4021/content/renderer/inp...
content/renderer/input_tag_speech_dispatcher.h:1: // Copyright (c) 2011 The
Chromium Authors. All rights reserved.
On 2012/03/06 10:05:36, Satish wrote:
> 2011 -> 2012, same for other files in this CL

Done.

[Satish, 2012-03-06 10:55:28]

lgtm

[darin (slow to review), 2012-03-06 17:35:33]

OWNERS LGTM

[commit-bot: I haz the power, 2012-03-07 10:43:17]

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

[commit-bot: I haz the power, 2012-03-07 10:44:02]

Presubmit check for 9568002-18255 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Messages **
If this change requires manual test instructions to QA team, add
TEST=[instructions].

** Presubmit ERRORS **
Missing LGTM from an OWNER for:
chrome/browser/profiles/profile.cc,chrome/browser/profiles/profile.h,content/public/common/speech_recognition_result.cc,chrome/browser/profiles/profile_impl.h,content/public/browser/speech_recognition_manager.h,content/public/browser/speech_recognizer.h,content/public/common/speech_recognition_result.h,chrome/browser/profiles/off_the_record_profile_impl.cc,chrome/browser/profiles/off_the_record_profile_impl.h,content/public/browser/speech_recognition_preferences.h,chrome/browser/ui/cocoa/speech_recognition_window_controller.mm,chrome/browser/ui/cocoa/speech_recognition_window_controller.h,chrome/browser/profiles/profile_impl.cc,content/public/browser/browser_context.h,content/public/browser/speech_recognizer_delegate.h,content/public/browser/content_browser_client.h,content/public/browser/speech_recognition_manager_delegate.h

Presubmit checks took 6.8s to calculate.