Add contextual search to the template url system

chrome/browser/search_engines/template_url_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/base_paths.h"
 #include "base/command_line.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
@@ skipping 1339 matching lines @@
   TemplateURLRef::SearchTermsArgs search_terms_args(ASCIIToUTF16("foo"));
   search_terms_args.session_token = "SESSIONTOKENGOESHERE";
   std::string result = url.url_ref().ReplaceSearchTerms(search_terms_args);
   EXPECT_EQ("http://bar/search?q=foo&psi=SESSIONTOKENGOESHERE&xssi=t", result);
 
   TemplateURL url2(NULL, data);
   search_terms_args.session_token = "";
   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);
+}