Omnibox UI Experiments: Refactor HTTPS trimming into UrlFormatter.

components/omnibox/browser/history_url_provider.cc

 // Copyright (c) 2012 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/history_url_provider.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 553 matching lines @@
 
   AutocompleteMatch match(this, 0, false,
                           AutocompleteMatchType::URL_WHAT_YOU_TYPED);
 
   if (destination_url.is_valid()) {
     match.destination_url = destination_url;
 
     // 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(
             destination_url, display_string, client()->GetSchemeClassifier());
     // The what-you-typed match is generally only allowed to be default for
     // URL inputs or when there is no default search provider.  (It's also
     // allowed to be default for UNKNOWN inputs where the destination is a known
     // intranet site.  In this case, |allowed_to_be_default_match| is revised in
     // FixupExactSuggestion().)
     const bool has_default_search_provider =
@@ skipping 547 matching lines @@
 
   const history::HistoryMatch& history_match = params.matches[match_number];
   const history::URLRow& info = history_match.url_info;
   AutocompleteMatch match(this, relevance,
       !!info.visit_count(), AutocompleteMatchType::HISTORY_URL);
   match.typed_count = info.typed_count();
   match.destination_url = info.url();
   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
             : url_formatter::kFormatUrlOmitHTTP);
   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());
   // |inline_autocomplete_offset| was guaranteed not to be npos before the call
   // to FormatUrl().  If it is npos now, that means the represented location no
   // longer exists as such in the formatted string, e.g. if the offset pointed
   // into the middle of a punycode sequence fixed up to Unicode.  In this case,
   // there can be no inline autocompletion, and the match must not be allowed to
   // be default.
   const bool autocomplete_offset_valid =
       inline_autocomplete_offset != base::string16::npos;
   if (!params.prevent_inline_autocomplete && autocomplete_offset_valid) {
     DCHECK(inline_autocomplete_offset <= match.fill_into_edit.length());
     match.inline_autocompletion =
         match.fill_into_edit.substr(inline_autocomplete_offset);
   }
   // The latter part of the test effectively asks "is the inline completion
   // empty?" (i.e., is this match effectively the what-you-typed match?).
   match.allowed_to_be_default_match = autocomplete_offset_valid &&
       (!params.prevent_inline_autocomplete ||
        (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);
     AutocompleteMatch::ClassifyLocationInString(match_start,
         inline_autocomplete_offset - match_start, match.contents.length(),
         ACMatchClassification::URL, &match.contents_class);
   } else {
     AutocompleteMatch::ClassifyLocationInString(base::string16::npos, 0,
         match.contents.length(), ACMatchClassification::URL,
         &match.contents_class);
   }
   match.description = info.title();
   match.description_class =
       ClassifyDescription(params.input.text(), match.description);
   RecordAdditionalInfoFromUrlRow(info, &match);
   return match;
 }