about:omnibox - add "in keyword mode" checkbox

about:omnibox - add "in keyword mode" checkbox

BTW, I changed the order of parameters in the message that gets
sent in order to correspond with the order in AutocompleteInput.

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

[Mark P, 2013-01-25 21:22:24]

Evan,

Can you glance at this follow-up changelist?

Bart: I need this to help me debug my coming change to make the SearchProvider
generate the keyword mode verbatim suggestion.  I'm sorry that it might make you
have to do slightly more work to make the AutocompleteInput changes you
eventually want.

--mark

[Bart N., 2013-01-25 21:52:04]

Feel free to ignore my comment, as I'm not really reviewing this CL. I just
couldn't resist to comment on something I saw last time, but decided not to vent
out :)

>Bart: I need this to help me debug my coming change to make the >SearchProvider
>generate the keyword mode verbatim suggestion.  I'm sorry that it might make
>you
>have to do slightly more work to make the AutocompleteInput changes you
>eventually want.
Don't worry about that :) and thanks for the heads-up!

https://codereview.chromium.org/12081002/diff/1/chrome/browser/ui/webui/omnib...
File chrome/browser/ui/webui/omnibox/omnibox_ui_handler.h (right):

https://codereview.chromium.org/12081002/diff/1/chrome/browser/ui/webui/omnib...
chrome/browser/ui/webui/omnibox/omnibox_ui_handler.h:52: //
|four_element_input_string| is expected to be a four-element list:
Honestly, I've never seen this style (naming vars to include number of elements)
as it's too verbose and error prone (as you can see in this very example, see my
comment below). I think comment is sufficient that we expect 4 elements.

How about: "input_string"?

https://codereview.chromium.org/12081002/diff/1/chrome/browser/ui/webui/omnib...
chrome/browser/ui/webui/omnibox/omnibox_ui_handler.h:58: void
StartOmniboxQuery(const base::ListValue* two_element_input_string);
s/two/four

[Mark P, 2013-01-25 22:24:58]

https://codereview.chromium.org/12081002/diff/1/chrome/browser/ui/webui/omnib...
File chrome/browser/ui/webui/omnibox/omnibox_ui_handler.h (right):

https://codereview.chromium.org/12081002/diff/1/chrome/browser/ui/webui/omnib...
chrome/browser/ui/webui/omnibox/omnibox_ui_handler.h:52: //
|four_element_input_string| is expected to be a four-element list:
On 2013/01/25 21:52:04, Bart N. wrote:
> Honestly, I've never seen this style (naming vars to include number of
elements)
> as it's too verbose and error prone (as you can see in this very example, see
my
> comment below). I think comment is sufficient that we expect 4 elements.
> 
> How about: "input_string"?

I always felt silly updating this code from two to three to four.  It never
occurred to me to rename the variable!  Crazy.  Now done.

https://codereview.chromium.org/12081002/diff/1/chrome/browser/ui/webui/omnib...
chrome/browser/ui/webui/omnibox/omnibox_ui_handler.h:58: void
StartOmniboxQuery(const base::ListValue* two_element_input_string);
On 2013/01/25 21:52:04, Bart N. wrote:
> s/two/four

Done.

[Mark P, 2013-01-29 21:38:00]

Evan,

Can you please glance at this?

thanks,
mark

[Bart N., 2013-01-29 21:39:42]

On 2013/01/29 21:38:00, Mark P wrote:
> Evan,
> 
> Can you please glance at this?
> 
> thanks,
> mark
lgtm 

Mark, I don't think you are waiting for me, but if you do, lgtm.

[Mark P, 2013-02-04 22:02:58]

Evan?

A new week, a new seeking of a LGTM to submit code. :)

thanks,
mark

[Evan Stade, 2013-02-04 22:10:48]

sorry, I have been OOO since January 25th. LGTM

https://codereview.chromium.org/12081002/diff/4001/chrome/browser/ui/webui/om...
File chrome/browser/ui/webui/omnibox/omnibox_ui_handler.h (right):

https://codereview.chromium.org/12081002/diff/4001/chrome/browser/ui/webui/om...
chrome/browser/ui/webui/omnibox/omnibox_ui_handler.h:57: // - forth element:
boolean indicating whether we should set prefer_keyword
forth->fourth

[Mark P, 2013-02-04 22:12:49]

Thanks for the prompt review!

https://codereview.chromium.org/12081002/diff/4001/chrome/browser/ui/webui/om...
File chrome/browser/ui/webui/omnibox/omnibox_ui_handler.h (right):

https://codereview.chromium.org/12081002/diff/4001/chrome/browser/ui/webui/om...
chrome/browser/ui/webui/omnibox/omnibox_ui_handler.h:57: // - forth element:
boolean indicating whether we should set prefer_keyword
On 2013/02/04 22:10:48, Evan Stade wrote:
> forth->fourth

Done.

[commit-bot: I haz the power, 2013-02-04 22:13:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mpearson@chromium.org/12081002/12001

[commit-bot: I haz the power, 2013-02-05 00:57:37]

Retried try job too often on mac_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2013-02-05 19:09:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mpearson@chromium.org/12081002/12001

[commit-bot: I haz the power, 2013-02-05 21:06:41]

Change committed as 180782