Factor out AutocompleteMatch creation from BookmarkProvider

components/omnibox/browser/autocomplete_provider_utils_unittest.cc

+// Copyright (c) 2016 The Chromium Authors. All rights reserved.

[Mark P, 2016/12/23 22:38:50]

Thank you again for writing good, comprehensive tests.  You have a fan in me!
:-)

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/omnibox/browser/autocomplete_provider_utils.h"
+
+#include "base/memory/ptr_util.h"
+#include "base/strings/utf_string_conversions.h"
+#include "components/bookmarks/browser/titled_url_match.h"
+#include "components/bookmarks/browser/titled_url_node.h"
+#include "components/metrics/proto/omnibox_event.pb.h"
+#include "components/omnibox/browser/autocomplete_input.h"
+#include "components/omnibox/browser/autocomplete_match.h"
+#include "components/omnibox/browser/autocomplete_provider.h"
+#include "components/omnibox/browser/test_scheme_classifier.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
+
+namespace {
+
+// A simple AutocompleteProvider that does nothing.
+class MockAutocompleteProvider : public AutocompleteProvider {
+ public:
+  MockAutocompleteProvider(Type type) : AutocompleteProvider(type) {}
+
+  void Start(const AutocompleteInput& input, bool minimal_changes) override {}
+
+ private:
+  ~MockAutocompleteProvider() override {}
+};
+
+class MockTitledUrlNode : public bookmarks::TitledUrlNode {
+ public:
+  MockTitledUrlNode(const base::string16& title, const GURL& url)
+      : title_(title), url_(url) {}
+
+  // TitledUrlNode
+  const base::string16& GetTitledUrlNodeTitle() const override {
+    return title_;
+  }
+  const GURL& GetTitledUrlNodeUrl() const override { return url_; }
+
+ private:
+  base::string16 title_;
+  GURL url_;
+};
+
+}  // namespace
+
+bool operator==(const ACMatchClassification& lhs,
+                const ACMatchClassification& rhs) {
+  return lhs.offset == rhs.offset && lhs.style == rhs.style;

[Mark P, 2016/12/23 22:38:49]

nit: parens around binary operators (==)

[mattreynolds, 2017/01/04 00:58:03]

Done.

+}
+
+class AutocompleteProviderUtilsTest : public testing::Test {
+ public:
+  AutocompleteProviderUtilsTest() {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(AutocompleteProviderUtilsTest);
+};
+
+TEST_F(AutocompleteProviderUtilsTest, TitledUrlMatchToAutocompleteMatch) {
+  base::string16 input_text(base::ASCIIToUTF16("goo"));
+  base::string16 match_title(base::ASCIIToUTF16("Google Search"));
+  base::string16 match_url_string(base::ASCIIToUTF16("https://www.google.com"));
+  GURL match_url(match_url_string);
+  bookmarks::TitledUrlMatch::MatchPositions title_match_positions = {{0, 3}};
+  bookmarks::TitledUrlMatch::MatchPositions url_match_positions = {{12, 15}};
+  AutocompleteMatchType::Type type = AutocompleteMatchType::BOOKMARK_TITLE;
+  int relevance = 123;
+
+  MockTitledUrlNode node(match_title, match_url);
+  bookmarks::TitledUrlMatch titled_url_match;
+  titled_url_match.node = &node;
+  titled_url_match.title_match_positions = title_match_positions;
+  titled_url_match.url_match_positions = url_match_positions;
+
+  scoped_refptr<MockAutocompleteProvider> provider =
+      new MockAutocompleteProvider(AutocompleteProvider::Type::TYPE_BOOKMARK);
+  TestSchemeClassifier classifier;
+  AutocompleteInput input(input_text, base::string16::npos,
+                          std::string(), GURL(),
+                          metrics::OmniboxEventProto::INVALID_SPEC,
+                          false, false, false, true, false, classifier);
+
+  AutocompleteMatch autocomplete_match =
+      TitledUrlMatchToAutocompleteMatch(provider.get(), classifier, input,
+                                        input_text, titled_url_match, type,
+                                        relevance);
+
+  ACMatchClassifications expected_contents_class = {
+    {0, ACMatchClassification::URL},
+    {12, ACMatchClassification::URL | ACMatchClassification::MATCH},
+    {15, ACMatchClassification::URL},
+  };
+  ACMatchClassifications expected_description_class = {
+    {0, ACMatchClassification::MATCH},
+    {3, ACMatchClassification::NONE},
+  };
+  base::string16 expected_inline_autocompletion(base::ASCIIToUTF16("gle.com"));
+
+  EXPECT_EQ(provider.get(), autocomplete_match.provider);
+  EXPECT_EQ(type, autocomplete_match.type);
+  EXPECT_EQ(relevance, autocomplete_match.relevance);
+  EXPECT_EQ(match_url, autocomplete_match.destination_url);
+  EXPECT_EQ(match_url_string, autocomplete_match.contents);
+  EXPECT_TRUE(std::equal(expected_contents_class.begin(),
+                         expected_contents_class.end(),
+                         autocomplete_match.contents_class.begin()));
+  EXPECT_EQ(match_title, autocomplete_match.description);
+  EXPECT_TRUE(std::equal(expected_description_class.begin(),
+                         expected_description_class.end(),
+                         autocomplete_match.description_class.begin()));
+  EXPECT_TRUE(autocomplete_match.allowed_to_be_default_match);
+  EXPECT_EQ(match_url_string, autocomplete_match.fill_into_edit);
+  EXPECT_EQ(expected_inline_autocompletion,
+            autocomplete_match.inline_autocompletion);
+}
+
+TEST_F(AutocompleteProviderUtilsTest, EmptyInlineAutocompletion) {
+  // The search term matches the title but not the URL. Since there is no URL
+  // match, the inline autocompletion string will be empty.
+  base::string16 input_text(base::ASCIIToUTF16("goo"));
+  base::string16 match_title(base::ASCIIToUTF16("Email by Google"));
+  base::string16 match_url_string(base::ASCIIToUTF16("https://www.gmail.com"));

[Mark P, 2016/12/23 22:38:49]

nit: add trailing slash to make this a proper URL.
ditto above

(Sorry, I reviewed this test case first.)

[mattreynolds, 2017/01/04 00:58:03]

Done.

+  GURL match_url(match_url_string);
+  bookmarks::TitledUrlMatch::MatchPositions title_match_positions = {{9, 12}};
+  bookmarks::TitledUrlMatch::MatchPositions url_match_positions;
+  AutocompleteMatchType::Type type = AutocompleteMatchType::BOOKMARK_TITLE;
+  int relevance = 123;
+
+  MockTitledUrlNode node(match_title, match_url);
+  bookmarks::TitledUrlMatch titled_url_match;
+  titled_url_match.node = &node;
+  titled_url_match.title_match_positions = title_match_positions;
+  titled_url_match.url_match_positions = url_match_positions;
+
+  scoped_refptr<MockAutocompleteProvider> provider =
+      new MockAutocompleteProvider(AutocompleteProvider::Type::TYPE_BOOKMARK);
+  TestSchemeClassifier classifier;
+  AutocompleteInput input(input_text, base::string16::npos,
+                          std::string(), GURL(),
+                          metrics::OmniboxEventProto::INVALID_SPEC,

[Mark P, 2016/12/23 22:38:50]

Please use a valid page classifier like NTP.
ditto above

[mattreynolds, 2017/01/04 00:58:02]

Done.

+                          false, false, false, true, false, classifier);

[Mark P, 2016/12/23 22:38:50]

Please make the third false to true to better align with typical calls.
ditto above

[mattreynolds, 2017/01/04 00:58:02]

Done.

+
+  AutocompleteMatch autocomplete_match =

[Mark P, 2016/12/23 22:38:49]

Maybe say something here about using input_text as your fixed up input?  Or just
declared a fixed up input variable and set it to input_text?
ditto above

[mattreynolds, 2017/01/04 00:58:02]

Done.

+      TitledUrlMatchToAutocompleteMatch(provider.get(), classifier, input,
+                                        input_text, titled_url_match, type,
+                                        relevance);
+
+  ACMatchClassifications expected_contents_class = {
+    {0, ACMatchClassification::URL},
+  };
+  ACMatchClassifications expected_description_class = {
+    {0, ACMatchClassification::NONE},
+    {9, ACMatchClassification::MATCH},
+    {12, ACMatchClassification::NONE},
+  };
+
+  EXPECT_EQ(provider.get(), autocomplete_match.provider);
+  EXPECT_EQ(type, autocomplete_match.type);
+  EXPECT_EQ(relevance, autocomplete_match.relevance);
+  EXPECT_EQ(match_url, autocomplete_match.destination_url);
+  EXPECT_EQ(match_url_string, autocomplete_match.contents);
+  EXPECT_TRUE(std::equal(expected_contents_class.begin(),
+                         expected_contents_class.end(),
+                         autocomplete_match.contents_class.begin()));
+  EXPECT_EQ(match_title, autocomplete_match.description);
+  EXPECT_TRUE(std::equal(expected_description_class.begin(),
+                         expected_description_class.end(),
+                         autocomplete_match.description_class.begin()));
+  EXPECT_FALSE(autocomplete_match.allowed_to_be_default_match);
+  EXPECT_EQ(match_url_string, autocomplete_match.fill_into_edit);

[Mark P, 2016/12/23 22:38:49]

Please reverse the order of lines 173 and 174; allowed_to_be_default_match
naturally goes with inline_autocompletion.
ditto above

[mattreynolds, 2017/01/04 00:58:02]

Done.

+  EXPECT_TRUE(autocomplete_match.inline_autocompletion.empty());
+}