Adds the speech recognition button to the app-list searchbox.

Adds the speech recognition button to the app-list searchbox.

BUG=313904
R=xiyuan@chromium.org
TBR=jamescook@chromium.org
TEST=manually

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

[xiyuan, 2013-11-12 03:09:24]

LGTM with nits

https://codereview.chromium.org/69813002/diff/1/ui/app_list/app_list_view_del...
File ui/app_list/app_list_view_delegate.h (right):

https://codereview.chromium.org/69813002/diff/1/ui/app_list/app_list_view_del...
ui/app_list/app_list_view_delegate.h:119: virtual void OnRightButtonClicked() =
0;
nit: Rename to ToggleVoiceRecognition to be more clear about what it does.

https://codereview.chromium.org/69813002/diff/1/ui/app_list/search_box_model.cc
File ui/app_list/search_box_model.cc (right):

https://codereview.chromium.org/69813002/diff/1/ui/app_list/search_box_model....
ui/app_list/search_box_model.cc:39:
right_button_prop_.reset(right_button_prop.release());
nit: right_button_prop_ = right_button_prop.Pass();

https://codereview.chromium.org/69813002/diff/1/ui/app_list/search_box_model.h
File ui/app_list/search_box_model.h (right):

https://codereview.chromium.org/69813002/diff/1/ui/app_list/search_box_model....
ui/app_list/search_box_model.h:27: struct RightButtonProperty {
nit: Can we use a more specific name? RightButton is vague. Prefer to call this
ToggleButtonProperty and call the RightButton instance below (in SetRightButton
etc) as VoiceRecogButton or something along the line.

https://codereview.chromium.org/69813002/diff/1/ui/app_list/search_box_model....
ui/app_list/search_box_model.h:37: ~RightButtonProperty();
nit: Move ctor and dtor before the member variables.

[Jun Mukai, 2013-11-12 19:24:08]

https://codereview.chromium.org/69813002/diff/1/ui/app_list/app_list_view_del...
File ui/app_list/app_list_view_delegate.h (right):

https://codereview.chromium.org/69813002/diff/1/ui/app_list/app_list_view_del...
ui/app_list/app_list_view_delegate.h:119: virtual void OnRightButtonClicked() =
0;
On 2013/11/12 03:09:25, xiyuan wrote:
> nit: Rename to ToggleVoiceRecognition to be more clear about what it does.

Done.

https://codereview.chromium.org/69813002/diff/1/ui/app_list/search_box_model.cc
File ui/app_list/search_box_model.cc (right):

https://codereview.chromium.org/69813002/diff/1/ui/app_list/search_box_model....
ui/app_list/search_box_model.cc:39:
right_button_prop_.reset(right_button_prop.release());
On 2013/11/12 03:09:25, xiyuan wrote:
> nit: right_button_prop_ = right_button_prop.Pass();

Done.

https://codereview.chromium.org/69813002/diff/1/ui/app_list/search_box_model.h
File ui/app_list/search_box_model.h (right):

https://codereview.chromium.org/69813002/diff/1/ui/app_list/search_box_model....
ui/app_list/search_box_model.h:27: struct RightButtonProperty {
On 2013/11/12 03:09:25, xiyuan wrote:
> nit: Can we use a more specific name? RightButton is vague. Prefer to call
this
> ToggleButtonProperty and call the RightButton instance below (in
SetRightButton
> etc) as VoiceRecogButton or something along the line. 

Done.

https://codereview.chromium.org/69813002/diff/1/ui/app_list/search_box_model....
ui/app_list/search_box_model.h:37: ~RightButtonProperty();
On 2013/11/12 03:09:25, xiyuan wrote:
> nit: Move ctor and dtor before the member variables.

Done.

[xiyuan, 2013-11-12 19:32:59]

Can you also update the CL description? We sometime use "speech recognition" and
sometime use "voice recognition", we probably should unify the name. Either one
is fine. I don't have preference. :p

[Jun Mukai, 2013-11-12 20:05:55]

On 2013/11/12 19:32:59, xiyuan wrote:
> Can you also update the CL description? We sometime use "speech recognition"
and
> sometime use "voice recognition", we probably should unify the name. Either
one
> is fine. I don't have preference. :p

Updated. Note that this depends on crrev.com/55433002, cannot land without it.

[xiyuan, 2013-11-12 20:42:40]

On 2013/11/12 20:05:55, Jun Mukai wrote:
> On 2013/11/12 19:32:59, xiyuan wrote:
> > Can you also update the CL description? We sometime use "speech recognition"
> and
> > sometime use "voice recognition", we probably should unify the name. Either
> one
> > is fine. I don't have preference. :p
> 
> Updated. Note that this depends on crrev.com/55433002, cannot land without it.

What about unifying the names? e.g. we have both ToggleSpeechRecognition (in
StartPageService) and ToggleVoiceRecognition (In AppListViewDelegate). Can we
use just one?

[Jun Mukai, 2013-11-12 22:53:42]

On 2013/11/12 20:42:40, xiyuan wrote:
> On 2013/11/12 20:05:55, Jun Mukai wrote:
> > On 2013/11/12 19:32:59, xiyuan wrote:
> > > Can you also update the CL description? We sometime use "speech
recognition"
> > and
> > > sometime use "voice recognition", we probably should unify the name.
Either
> > one
> > > is fine. I don't have preference. :p
> > 
> > Updated. Note that this depends on crrev.com/55433002, cannot land without
it.
> 
> What about unifying the names? e.g. we have both ToggleSpeechRecognition (in
> StartPageService) and ToggleVoiceRecognition (In AppListViewDelegate). Can we
> use just one?

Good catch, thanks. Use "speech recognition" because my existing code (like JS
files)
uses 'speech'.

[xiyuan, 2013-11-12 23:19:39]

LGTM++

[commit-bot: I haz the power, 2013-11-13 01:13:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mukai@chromium.org/69813002/310001

[commit-bot: I haz the power, 2013-11-13 01:49:02]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[Jun Mukai, 2013-11-13 02:09:28]

On 2013/11/13 01:49:02, I haz the power (commit-bot) wrote:
> Retried try job too often on chromium_presubmit for step(s) presubmit
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

TBR=jamescook since I have a trivial change for ash.

[commit-bot: I haz the power, 2013-11-13 02:16:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mukai@chromium.org/69813002/310001

[commit-bot: I haz the power, 2013-11-13 03:54:15]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_x64_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-11-13 07:38:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mukai@chromium.org/69813002/310001

[commit-bot: I haz the power, 2013-11-13 08:31:04]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_x64_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-11-13 18:05:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mukai@chromium.org/69813002/560003

[commit-bot: I haz the power, 2013-11-13 21:11:09]

Change committed as 234902