[omnibox] Use suggestion instead of input for base text

components/omnibox/browser/base_search_provider_unittest.cc

 // Copyright 2014 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 "components/omnibox/browser/base_search_provider.h"
 
 #include <utility>
 
 #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"
 #include "components/omnibox/browser/autocomplete_scheme_classifier.h"
 #include "components/omnibox/browser/mock_autocomplete_provider_client.h"
 #include "components/omnibox/browser/search_suggestion_parser.h"
 #include "components/omnibox/browser/suggestion_answer.h"
 #include "components/search_engines/search_terms_data.h"
 #include "components/search_engines/template_url_service.h"
 #include "components/search_engines/template_url_service_client.h"
@@ skipping 35 matching lines @@
                                       map);
   }
 
  protected:
   virtual ~TestBaseSearchProvider() {}
 
  private:
   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 {}
 
  protected:
   void SetUp() override {
     std::unique_ptr<TemplateURLService> template_url_service(
         new TemplateURLService(
             nullptr, std::unique_ptr<SearchTermsData>(new SearchTermsData),
             nullptr, std::unique_ptr<TemplateURLServiceClient>(), nullptr,
@@ skipping 86 matching lines @@
   EXPECT_TRUE(answer2->Equals(*match.answer));
   EXPECT_EQ(AutocompleteMatchType::SEARCH_HISTORY, match.type);
   EXPECT_EQ(1300, match.relevance);
 
   EXPECT_EQ(answer_contents, duplicate.answer_contents);
   EXPECT_EQ(answer_type, duplicate.answer_type);
   EXPECT_TRUE(answer->Equals(*duplicate.answer));
   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));
+  }
+}