Factor out AutocompleteMatch creation from BookmarkProvider

components/omnibox/browser/autocomplete_provider_utils.cc

+// Copyright (c) 2016 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/autocomplete_provider_utils.h"
+
+#include "base/strings/utf_string_conversions.h"
+#include "components/bookmarks/browser/titled_url_node.h"
+#include "components/omnibox/browser/autocomplete_match.h"
+#include "components/omnibox/browser/history_provider.h"
+#include "components/omnibox/browser/url_prefix.h"
+#include "components/url_formatter/url_formatter.h"
+
+namespace {
+
+// Converts |positions| into ACMatchClassifications and returns the
+// classifications. |text_length| is used to determine the need to add an
+// 'unhighlighted' classification span so the tail of the source string
+// properly highlighted.
+ACMatchClassifications ClassificationsFromMatchPositions(

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

I assume you didn't change this code in any meaningful way.

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

I renamed ClassificationsFromMatch -> ClassificationsFromMatchPositions (to
match the actual type name)
Also switched from using query_parser::Snippet::MatchPositions::const_iterator
to bookmarks::TitledUrlMatch::MatchPositions::const_iterator as these are
typedefs for the same type.

Otherwise unchanged.

[Mark P, 2017/01/05 20:56:02]

Acknowledged.

+    const bookmarks::TitledUrlMatch::MatchPositions& positions,
+    size_t text_length,
+    bool is_url) {
+  ACMatchClassification::Style url_style =
+      is_url ? ACMatchClassification::URL : ACMatchClassification::NONE;
+  ACMatchClassifications classifications;
+  if (positions.empty()) {
+    if (text_length > 0) {
+      classifications.push_back(ACMatchClassification(0, url_style));
+    }
+    return classifications;
+  }
+
+  for (bookmarks::TitledUrlMatch::MatchPositions::const_iterator i =
+           positions.begin();
+       i != positions.end();
+       ++i) {
+    AutocompleteMatch::ACMatchClassifications new_class;
+    AutocompleteMatch::ClassifyLocationInString(i->first, i->second - i->first,
+        text_length, url_style, &new_class);
+    classifications = AutocompleteMatch::MergeClassifications(
+        classifications, new_class);
+  }
+  return classifications;
+}
+}  // namespace

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

nit: blank line above 46

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

Done.

+
+AutocompleteMatch TitledUrlMatchToAutocompleteMatch(
+    AutocompleteProvider* provider,
+    const AutocompleteSchemeClassifier& scheme_classifier,
+    const AutocompleteInput& input,
+    const base::string16& fixed_up_input_text,
+    const bookmarks::TitledUrlMatch& titled_url_match,
+    AutocompleteMatchType::Type type,
+    int relevance) {

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

I assume you didn't change this implementation in any meaningful way.

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

Renamed from TitledUrlMatchToACMatch -> TitledUrlMatchToAutocompleteMatch to be
more explicit about the different match types we're converting between
The original had logic for computing the bookmark match relevance, which was
factored out into BookmarkProvider::CalculateBookmarkMatchRelevance and a
relevance param was added.

[Mark P, 2017/01/05 20:56:02]

Acknowledged.

+  const GURL& url = titled_url_match.node->GetTitledUrlNodeUrl();
+  base::string16 title = titled_url_match.node->GetTitledUrlNodeTitle();
+
+  AutocompleteMatch match(provider, relevance, false, type);

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

Please either keep the comment about non-deleteable matches here (or, better
yet, revise it appropriately), or make this function take a deletable parameter
and use that.

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

Revised the comment.

+  bookmarks::TitledUrlMatch::MatchPositions new_title_match_positions =
+      titled_url_match.title_match_positions;
+  bookmarks::TitledUrlMatch::CorrectTitleAndMatchPositions(
+      &title, &new_title_match_positions);
+  const base::string16& url_utf16 = base::UTF8ToUTF16(url.spec());
+  size_t inline_autocomplete_offset = URLPrefix::GetInlineAutocompleteOffset(
+      input.text(), fixed_up_input_text, false, url_utf16);
+  match.destination_url = url;
+  const size_t match_start = titled_url_match.url_match_positions.empty() ?
+      0 : titled_url_match.url_match_positions[0].first;
+  const bool trim_http = !AutocompleteInput::HasHTTPScheme(input.text()) &&
+      ((match_start == base::string16::npos) || (match_start != 0));
+  std::vector<size_t> offsets =
+      bookmarks::TitledUrlMatch::OffsetsFromMatchPositions(
+          titled_url_match.url_match_positions);
+  // In addition to knowing how |offsets| is transformed, we need to know how
+  // |inline_autocomplete_offset| is transformed.  We add it to the end of
+  // |offsets|, compute how everything is transformed, then remove it from the
+  // end.
+  offsets.push_back(inline_autocomplete_offset);
+  match.contents = url_formatter::FormatUrlWithOffsets(
+      url, url_formatter::kFormatUrlOmitAll &
+      ~(trim_http ? 0 : url_formatter::kFormatUrlOmitHTTP),
+      net::UnescapeRule::SPACES, nullptr, nullptr, &offsets);
+  inline_autocomplete_offset = offsets.back();
+  offsets.pop_back();
+  bookmarks::TitledUrlMatch::MatchPositions new_url_match_positions =
+      bookmarks::TitledUrlMatch::ReplaceOffsetsInMatchPositions(
+          titled_url_match.url_match_positions, offsets);
+  match.contents_class =
+      ClassificationsFromMatchPositions(new_url_match_positions,
+                                        match.contents.size(),
+                                        true);
+  match.fill_into_edit =
+      AutocompleteInput::FormattedStringWithEquivalentMeaning(
+          url, match.contents, scheme_classifier);
+  if (inline_autocomplete_offset != base::string16::npos) {
+    // |inline_autocomplete_offset| may be beyond the end of the
+    // |fill_into_edit| if the user has typed an URL with a scheme and the
+    // last character typed is a slash.  That slash is removed by the
+    // FormatURLWithOffsets call above.
+    if (inline_autocomplete_offset < match.fill_into_edit.length()) {
+      match.inline_autocompletion =
+      match.fill_into_edit.substr(inline_autocomplete_offset);

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

nit: spacing / git cl format

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

Done.

+    }
+    match.allowed_to_be_default_match = match.inline_autocompletion.empty() ||
+        !HistoryProvider::PreventInlineAutocomplete(input);
+  }
+  match.description = title;
+  match.description_class =
+      ClassificationsFromMatchPositions(titled_url_match.title_match_positions,
+                                        match.description.size(),
+                                        false);
+
+  return match;
+}