[AiS] Added version parameter to search query.

[AiS] Added version parameter to search query.

Answers in Suggest requires a version number on all suggest queries.
Add a gs_rn=42 attribute to requests if Answers is enabled.

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

[groby-ooo-7-16, 2014-05-06 23:55:34]

pkasting: PTAL

mariakhomenko, jdonnelly: FYI

[Peter Kasting, 2014-05-07 00:01:52]

You should add a test that ensure this does what you want, in particular because
I believe this version of you change doesn't (see second comment below)

https://codereview.chromium.org/269293002/diff/1/chrome/browser/search_engine...
File chrome/browser/search_engines/template_url.cc (right):

https://codereview.chromium.org/269293002/diff/1/chrome/browser/search_engine...
chrome/browser/search_engines/template_url.cc:610:
switches::kEnableAnswersInSuggest)) {
Nit: Indent 4, not 8; no {}

https://codereview.chromium.org/269293002/diff/1/chrome/browser/search_engine...
chrome/browser/search_engines/template_url.cc:611: url->insert(start,
"gs_rn=42");
Doesn't this need a trailing '&'?

[groby-ooo-7-16, 2014-05-07 01:32:31]

Fixes done.

The littering with #if defined(OS_ANDROID) bugs me, but I can't see an easy way
around it as long as the switch is Android only.

https://codereview.chromium.org/269293002/diff/1/chrome/browser/search_engine...
File chrome/browser/search_engines/template_url.cc (right):

https://codereview.chromium.org/269293002/diff/1/chrome/browser/search_engine...
chrome/browser/search_engines/template_url.cc:610:
switches::kEnableAnswersInSuggest)) {
On 2014/05/07 00:01:52, Peter Kasting wrote:
> Nit: Indent 4, not 8; no {}

Fixed. 

(Style guide & git cl format disagree, but I don't feel like reformatting the
entire file :)

https://codereview.chromium.org/269293002/diff/1/chrome/browser/search_engine...
chrome/browser/search_engines/template_url.cc:611: url->insert(start,
"gs_rn=42");
On 2014/05/07 00:01:52, Peter Kasting wrote:
> Doesn't this need a trailing '&'?

Indeed, done.

[Peter Kasting, 2014-05-07 01:35:45]

LGTM

https://codereview.chromium.org/269293002/diff/1/chrome/browser/search_engine...
File chrome/browser/search_engines/template_url.cc (right):

https://codereview.chromium.org/269293002/diff/1/chrome/browser/search_engine...
chrome/browser/search_engines/template_url.cc:610:
switches::kEnableAnswersInSuggest)) {
On 2014/05/07 01:32:31, groby wrote:
> On 2014/05/07 00:01:52, Peter Kasting wrote:
> > Nit: Indent 4, not 8; no {}
> 
> Fixed. 
> 
> (Style guide & git cl format disagree, but I don't feel like reformatting the
> entire file :)

The style guide doesn't disagree AFAIK.  clang-format I make no claims about.

[Peter Kasting, 2014-05-07 01:36:41]

Oh, also, you'll need to resync prepopulated_engines.json and bump the version
number to 73 -- a change to 72 got checked in earlier today.

[Peter Kasting, 2014-05-07 17:05:58]

On 2014/05/07 01:36:41, Peter Kasting wrote:
> Oh, also, you'll need to resync prepopulated_engines.json and bump the version
> number to 73 -- a change to 72 got checked in earlier today.

And https://codereview.chromium.org/267333002/ is changing the number to 73, so
you'll probably want to either land manually and un-check the commit button on
that CL so it can be fixed, or wait for it to land, merge, and bump yours to 74.

[groby-ooo-7-16, 2014-05-07 17:27:00]

I was *just* making a joke about this happening, was about to upload.

[groby-ooo-7-16, 2014-05-09 00:42:08]

The CQ bit was checked by groby@chromium.org

[commit-bot: I haz the power, 2014-05-09 00:43:21]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/groby@chromium.org/269293002/40001

[groby-ooo-7-16, 2014-05-09 00:54:25]

The CQ bit was unchecked by groby@chromium.org

[groby-ooo-7-16, 2014-05-09 00:55:27]

The CQ bit was checked by groby@chromium.org

[commit-bot: I haz the power, 2014-05-09 00:57:25]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/groby@chromium.org/269293002/40001

[commit-bot: I haz the power, 2014-05-10 02:15:35]

Change committed as 269486