Add ctxsl_alternate_term to the contextual search request and update the API version to 2.

Add ctxsl_alternate_term to the contextual search request and update the API version to 2.

BUG=397738, 397734
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289561

[jeremycho, 2014-08-13 01:50:28]

This is ready for review.  Thanks!

[donnd, 2014-08-13 02:03:34]

LGTM -- Thanks for doing this!

Nit: add this to the description too?
BUG=397734

[Peter Kasting, 2014-08-13 02:04:16]

LGTM

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

https://codereview.chromium.org/465183002/diff/1/chrome/browser/search_engine...
chrome/browser/search_engines/template_url_service_android.cc:25: using
base::android::ConvertUTF8ToJavaString;
Nit: Avoid using statements unless they significantly improve code readability
(I'd remove all these)

https://codereview.chromium.org/465183002/diff/1/chrome/browser/search_engine...
chrome/browser/search_engines/template_url_service_android.cc:213: JNIEnv* env,
jobject obj, jstring jquery, jstring jalternate_term) {
Nit: One arg per line

https://codereview.chromium.org/465183002/diff/1/chrome/browser/search_engine...
chrome/browser/search_engines/template_url_service_android.cc:226:
ConvertJavaStringToUTF8(env, jalternate_term));
Nit: Is it safe to run this if !jalternate_term?  Because if so it seems like
the conditional below would catch that case.

[jeremycho, 2014-08-13 02:20:07]

David - can you review for TemplateUrlService.java?

[David Trainor- moved to gerrit, 2014-08-13 04:09:28]

lgtm

[jeremycho, 2014-08-13 20:29:27]

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

https://codereview.chromium.org/465183002/diff/1/chrome/browser/search_engine...
chrome/browser/search_engines/template_url_service_android.cc:25: using
base::android::ConvertUTF8ToJavaString;
On 2014/08/13 02:04:16, pkasting (away thru Aug 20) wrote:
> Nit: Avoid using statements unless they significantly improve code readability
> (I'd remove all these)

Done.

https://codereview.chromium.org/465183002/diff/1/chrome/browser/search_engine...
chrome/browser/search_engines/template_url_service_android.cc:213: JNIEnv* env,
jobject obj, jstring jquery, jstring jalternate_term) {
On 2014/08/13 02:04:16, pkasting (away thru Aug 20) wrote:
> Nit: One arg per line

Done.

https://codereview.chromium.org/465183002/diff/1/chrome/browser/search_engine...
chrome/browser/search_engines/template_url_service_android.cc:226:
ConvertJavaStringToUTF8(env, jalternate_term));
On 2014/08/13 02:04:15, pkasting (away thru Aug 20) wrote:
> Nit: Is it safe to run this if !jalternate_term?  Because if so it seems like
> the conditional below would catch that case.

A null is being passed from Java, so I think having the check is good to avoid a
warning from ConvertJavaStringToUTF8.

[jeremycho, 2014-08-13 20:31:03]

The CQ bit was checked by jeremycho@chromium.org

[commit-bot: I haz the power, 2014-08-13 20:33:24]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jeremycho@chromium.org/465183002/60001

[commit-bot: I haz the power, 2014-08-14 12:35:40]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)

[commit-bot: I haz the power, 2014-08-14 14:27:57]

Committed patchset #4 (60001) as 289561