Add contextual search to the template url system

chrome/browser/search_engines/template_url_unittest.cc

Index: chrome/browser/search_engines/template_url_unittest.cc
diff --git a/chrome/browser/search_engines/template_url_unittest.cc b/chrome/browser/search_engines/template_url_unittest.cc
index 97ea55b75885c1b9f13593baa03f767f15ac64fa..e3bc8457b029595d5f7c737ecd1e4b71ebab75e7 100644
--- a/chrome/browser/search_engines/template_url_unittest.cc
+++ b/chrome/browser/search_engines/template_url_unittest.cc
@@ -1357,3 +1357,44 @@ TEST_F(TemplateURLTest, SessionToken) {
   result = url.url_ref().ReplaceSearchTerms(search_terms_args);
   EXPECT_EQ("http://bar/search?q=foo&xssi=t", result);
 }
+
+TEST_F(TemplateURLTest, ContextualSearchParameters) {
+  TemplateURLData data;
+  UIThreadSearchTermsData::SetGoogleBaseURL("http://bar/");
+  data.SetURL("http://bar/_/contextualsearch?"
+              "{google:contextualSearchVersion}"
+              "{google:contextualSearchContextData}");
+
+  TemplateURL url(NULL, data);
+  TemplateURLRef::SearchTermsArgs search_terms_args(ASCIIToUTF16("foo"));
+  search_terms_args.contextual_search_params.version = 1;

[Peter Kasting, 2014/06/06 17:51:34]

Nit: I still think the presence of these long lists of args in this unittest
suggests that you should add a six-arg constructor to ContextualSearchParams. 
You could even make it #ifdef UNIT_TEST if you really wanted non-test code to
not use it.

[jeremycho, 2014/06/06 21:51:45]

Done.

+  search_terms_args.contextual_search_params.start = 6;
+  search_terms_args.contextual_search_params.end = 11;
+  search_terms_args.contextual_search_params.selection = "allen";
+  search_terms_args.contextual_search_params.content = "woody+allen+movies";
+  search_terms_args.contextual_search_params.base_page_url =
+      "www.wikipedia.org";
+  search_terms_args.contextual_search_params.encoding = "utf-8";
+  std::string result = url.url_ref().ReplaceSearchTerms(search_terms_args);
+
+  EXPECT_EQ("http://bar/_/contextualsearch?"

[Peter Kasting, 2014/06/06 17:51:34]

Nit: Consider indenting like this:

  EXPECT_EQ("http://bar/_/contextualsearch?"
                "ctxs=1&"
                ...,
            result);

...to emphasize that the strings continued the first arg, and then |result| is a
separate arg.

[jeremycho, 2014/06/06 21:51:45]

Done.

+            "ctxs=1&"
+            "ctxs_start=6&"
+            "ctxs_end=11&"
+            "q=allen&"
+            "ctxs_content=woody+allen+movies&"
+            "ctxs_url=www.wikipedia.org&"
+            "ctxs_encoding=utf-8&", result);
+
+  TemplateURL url2(NULL, data);
+  search_terms_args.contextual_search_params.version = -1;

[Peter Kasting, 2014/06/06 17:51:34]

Nit: Should we do this test first?  Then there'd be no need to set all these
members, they'd be in this state already from construction.

[jeremycho, 2014/06/06 21:51:45]

Done.

+  search_terms_args.contextual_search_params.start = base::string16::npos;
+  search_terms_args.contextual_search_params.end = base::string16::npos;
+  search_terms_args.contextual_search_params.selection = "";

[Peter Kasting, 2014/06/06 17:51:33]

Nit: Use .clear(), or in the limit "= std::string()", rather than "".

[jeremycho, 2014/06/06 21:51:45]

Removed.

+  search_terms_args.contextual_search_params.content = "";
+  search_terms_args.contextual_search_params.base_page_url = "";
+  search_terms_args.contextual_search_params.encoding = "";
+  result = url.url_ref().ReplaceSearchTerms(search_terms_args);
+
+  EXPECT_EQ("http://bar/_/contextualsearch?", result);
+}