[omnibox] Use suggestion instead of input for base text

components/omnibox/browser/base_search_provider_unittest.cc

Index: components/omnibox/browser/base_search_provider_unittest.cc
diff --git a/components/omnibox/browser/base_search_provider_unittest.cc b/components/omnibox/browser/base_search_provider_unittest.cc
index 1848c9f1d90e6dc1a377772641a99ae1974d29d6..352925bcc0afeb06b66040d823ac288ddae7bd0c 100644
--- a/components/omnibox/browser/base_search_provider_unittest.cc
+++ b/components/omnibox/browser/base_search_provider_unittest.cc
@@ -8,6 +8,7 @@
 
 #include "base/macros.h"
 #include "base/strings/string16.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/omnibox/browser/autocomplete_match.h"
 #include "components/omnibox/browser/autocomplete_match_type.h"
@@ -63,6 +64,14 @@ class TestBaseSearchProvider : public BaseSearchProvider {
   DISALLOW_COPY_AND_ASSIGN(TestBaseSearchProvider);
 };
 
+class TestSchemeClassifier : public AutocompleteSchemeClassifier {
+ public:
+  metrics::OmniboxInputType::Type GetInputTypeForScheme(
+      const std::string&) const override {
+    return metrics::OmniboxInputType::QUERY;
+  }
+};
+
 class BaseSearchProviderTest : public testing::Test {
  public:
   ~BaseSearchProviderTest() override {}
@@ -169,3 +178,43 @@ TEST_F(BaseSearchProviderTest, PreserveAnswersWhenDeduplicating) {
   EXPECT_EQ(AutocompleteMatchType::SEARCH_SUGGEST, duplicate.type);
   EXPECT_EQ(850, duplicate.relevance);
 }
+
+TEST_F(BaseSearchProviderTest, MatchTailSuggestionProperly) {
+  TemplateURLData data;
+  data.SetURL("http://foo.com/url?bar={searchTerms}");
+  std::unique_ptr<TemplateURL> template_url(new TemplateURL(data));

[Peter Kasting, 2017/05/10 20:38:11]

Nit: Prefer MakeUnique to bare new:

  auto template_url = base::MakeUnique<TemplateURL>(data);

[Kevin Bailey, 2017/05/11 13:27:37]

Done. Fixed the other test too.

+
+  AutocompleteInput autocomplete_input(
+      base::ASCIIToUTF16("weather"), 7, "", GURL(), base::string16(),

[Peter Kasting, 2017/05/10 20:38:11]

Nit: Prefer std::string() to ""

[Kevin Bailey, 2017/05/11 13:27:37]

Done.

+      metrics::OmniboxEventProto::BLANK, false, false, false, false, false,
+      TestSchemeClassifier());
+
+  EXPECT_CALL(*provider_, GetInput(_))
+      .WillRepeatedly(Return(autocomplete_input));
+  EXPECT_CALL(*provider_, GetTemplateURL(_))
+      .WillRepeatedly(Return(template_url.get()));
+
+  base::string16 query = base::ASCIIToUTF16("angeles now");
+  base::string16 suggestion = base::ASCIIToUTF16("weather los ") + query;
+  SearchSuggestionParser::SuggestResult suggest_result(
+      suggestion, AutocompleteMatchType::SEARCH_SUGGEST_TAIL, 0, query,
+      base::ASCIIToUTF16("..."), base::string16(), base::string16(),
+      base::string16(), nullptr, std::string(), std::string(), false, 1300,
+      true, false, query);
+
+  TestBaseSearchProvider::MatchMap map;
+  provider_->AddMatchToMap(suggest_result, std::string(),
+                           TemplateURLRef::NO_SUGGESTION_CHOSEN, false, false,
+                           &map);
+
+  EXPECT_EQ(1UL, map.size());

[Peter Kasting, 2017/05/10 20:38:12]

Nit: Maybe this should be ASSERT, so we can blindly deref the first element
below and eliminate the loop around it?  That would also reduce the amount of
(probably not helpful) error spew in the case where there's more than one
element in the map.

[Kevin Bailey, 2017/05/11 13:27:37]

Done.

+  for (const auto& entry : map) {
+    std::string text =
+        entry.second.GetAdditionalInfo(kACMatchPropertyContentsStartIndex);
+    int length;

[Peter Kasting, 2017/05/10 20:38:12]

Nit: Declare as size_t; then no EXPECT_GE is needed just below, and no
static_cast in the last line of the loop.

[Kevin Bailey, 2017/05/11 13:27:37]

Done. With the high template usage, they could have just had StringTo().

+    EXPECT_TRUE(base::StringToInt(text, &length));
+    EXPECT_GE(length, 0);
+    text = entry.second.GetAdditionalInfo(kACMatchPropertyContentsText);
+    EXPECT_GE(text.length(), static_cast<unsigned>(length));
+  }
+}