Omnibox UI Experiments: Refactor HTTPS trimming into UrlFormatter.

components/omnibox/browser/history_url_provider.cc

Index: components/omnibox/browser/history_url_provider.cc
diff --git a/components/omnibox/browser/history_url_provider.cc b/components/omnibox/browser/history_url_provider.cc
index d9f6b7e887db0827d39ba5cba82a1fbc7e936936..f442738490e966e52e5dd2e366a94b7c01b25386 100644
--- a/components/omnibox/browser/history_url_provider.cc
+++ b/components/omnibox/browser/history_url_provider.cc
@@ -571,9 +571,11 @@ AutocompleteMatch HistoryURLProvider::SuggestExactInput(
     // Trim off "http://" if the user didn't type it.
     DCHECK(!trim_http ||
            !AutocompleteInput::HasHTTPScheme(input.text()));
-    base::string16 display_string(
-        AutocompleteMatch::FormatUrlForSuggestionDisplay(
-            destination_url, false /* trim_scheme */, nullptr));
+    auto format_types = AutocompleteMatch::GetFormatForSuggestionDisplay(
+        false /* trim_scheme */);
+    base::string16 display_string(url_formatter::FormatUrl(
+        destination_url, format_types, net::UnescapeRule::SPACES, nullptr,
+        nullptr, nullptr));
     const size_t offset = trim_http ? TrimHttpPrefix(&display_string) : 0;
     match.fill_into_edit =
         AutocompleteInput::FormattedStringWithEquivalentMeaning(
@@ -1141,7 +1143,7 @@ AutocompleteMatch HistoryURLProvider::HistoryMatchToACMatch(
   DCHECK(match.destination_url.is_valid());
   size_t inline_autocomplete_offset =
       history_match.input_location + params.input.text().length();
-  const url_formatter::FormatUrlTypes format_types =
+  const url_formatter::FormatUrlTypes fill_into_edit_format_types =

[Peter Kasting, 2017/06/29 01:05:44]

I suspect what we want here might be:

  const auto format_types = AutocompleteMatch::GetFormatForSuggestionDisplay(
      params.trim_http && !history_match.match_in_scheme);
  match.fill_into_edit = ...format_types &
~url_formatter::kFormatUrlExperimentalOmitHTTPS...

i.e. we want to use the same flags here and below, except it's important never
to trim HTTPS in the fill_into_edit lest it be misinterpreted.

[tommycli, 2017/06/29 16:46:46]

Done. Yes, that makes sense. I missed the "formatted string with equivalent
meaning" catch below...

       url_formatter::kFormatUrlOmitAll &
       ~((params.trim_http && !history_match.match_in_scheme)
             ? 0
@@ -1149,7 +1151,7 @@ AutocompleteMatch HistoryURLProvider::HistoryMatchToACMatch(
   match.fill_into_edit =
       AutocompleteInput::FormattedStringWithEquivalentMeaning(
           info.url(),
-          url_formatter::FormatUrl(info.url(), format_types,
+          url_formatter::FormatUrl(info.url(), fill_into_edit_format_types,
                                    net::UnescapeRule::SPACES, nullptr, nullptr,
                                    &inline_autocomplete_offset),
           client()->GetSchemeClassifier());
@@ -1173,9 +1175,11 @@ AutocompleteMatch HistoryURLProvider::HistoryMatchToACMatch(
        (inline_autocomplete_offset >= match.fill_into_edit.length()));
 
   size_t match_start = history_match.input_location;
-  match.contents = AutocompleteMatch::FormatUrlForSuggestionDisplay(
-      info.url(), params.trim_http && !history_match.match_in_scheme,
-      &match_start);
+  auto format_types = AutocompleteMatch::GetFormatForSuggestionDisplay(
+      params.trim_http && !history_match.match_in_scheme);
+  match.contents = url_formatter::FormatUrl(info.url(), format_types,
+                                            net::UnescapeRule::SPACES, nullptr,
+                                            nullptr, &match_start);
   if ((match_start != base::string16::npos) && autocomplete_offset_valid &&
       (inline_autocomplete_offset != match_start)) {
     DCHECK(inline_autocomplete_offset > match_start);