Omnibox: Hook up match_in_subdomain, etc. to URL formatting.

This CL prevents the match part of the URL from being elided by
the experimental URL formatting flags.

Bug: 732582
Change-Id: Ic08a8cad845f4d4581d8f6310e59423191d4bd28
Reviewed-on: https://chromium-review.googlesource.com/594722
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491150}

components/omnibox/browser/autocomplete_match.cc

@@ -489,25 +489,29 @@ GURL AutocompleteMatch::GURLToStrippedGURL(
 
 // static
 url_formatter::FormatUrlTypes AutocompleteMatch::GetFormatTypes(
-    bool trim_scheme) {
+    bool preserve_scheme,
+    bool preserve_subdomain,
+    bool preserve_after_host) {
   auto format_types = url_formatter::kFormatUrlOmitAll;
-  if (!trim_scheme) {
+  if (preserve_scheme) {
     format_types &= ~url_formatter::kFormatUrlOmitHTTP;
   } else if (base::FeatureList::IsEnabled(
                  omnibox::kUIExperimentHideSuggestionUrlScheme)) {
     format_types |= url_formatter::kFormatUrlExperimentalOmitHTTPS;
   }
 
-  if (base::FeatureList::IsEnabled(
-          omnibox::kUIExperimentElideSuggestionUrlAfterHost)) {
-    format_types |= url_formatter::kFormatUrlExperimentalElideAfterHost;
-  }
-
-  if (base::FeatureList::IsEnabled(
+  if (!preserve_subdomain &&
+      base::FeatureList::IsEnabled(
           omnibox::kUIExperimentHideSuggestionUrlTrivialSubdomains)) {
     format_types |= url_formatter::kFormatUrlExperimentalOmitTrivialSubdomains;
   }
 
+  if (!preserve_after_host &&
+      base::FeatureList::IsEnabled(
+          omnibox::kUIExperimentElideSuggestionUrlAfterHost)) {
+    format_types |= url_formatter::kFormatUrlExperimentalElideAfterHost;
+  }
+
   return format_types;
 }
 

components/omnibox/browser/autocomplete_match.h

@@ -215,7 +215,13 @@ struct AutocompleteMatch {
   //
   // This function returns flags that may destructively format the URL, and
   // therefore should never be used for the |fill_into_edit| field.
-  static url_formatter::FormatUrlTypes GetFormatTypes(bool trim_scheme);
+  //
+  // |preserve_scheme|, |preserve_subdomain|, and |preserve_after_host| indicate
+  // that these URL components are important (part of the match), and should
+  // not be trimmed or elided.
+  static url_formatter::FormatUrlTypes GetFormatTypes(bool preserve_scheme,
+                                                      bool preserve_subdomain,
+                                                      bool preserve_after_host);
 
   // Computes the stripped destination URL (via GURLToStrippedGURL()) and
   // stores the result in |stripped_destination_url|.  |input| is used for the

components/omnibox/browser/autocomplete_match_unittest.cc

@@ -118,14 +118,19 @@ TEST(AutocompleteMatchTest, FormatUrlForSuggestionDisplay) {
   // correct behavior within AutocompleteMatch::GetFormatTypes.
   struct FormatUrlTestData {
     const std::string url;
-    bool trim_scheme;
+    bool preserve_scheme;
+    bool preserve_subdomain;
+    bool preserve_after_host;
     const wchar_t* expected_result;
 
     void Validate() {
       SCOPED_TRACE(testing::Message()
-                   << " url= " << url << " trim_scheme=" << trim_scheme
+                   << " url=" << url << " preserve_scheme=" << preserve_scheme
+                   << " preserve_subdomain=" << preserve_subdomain
+                   << " preserve_after_host=" << preserve_after_host
                    << " expected_result=" << expected_result);
-      auto format_types = AutocompleteMatch::GetFormatTypes(trim_scheme);
+      auto format_types = AutocompleteMatch::GetFormatTypes(
+          preserve_scheme, preserve_subdomain, preserve_after_host);
       EXPECT_EQ(base::WideToUTF16(expected_result),
                 url_formatter::FormatUrl(GURL(url), format_types,
                                          net::UnescapeRule::SPACES, nullptr,
@@ -135,16 +140,17 @@ TEST(AutocompleteMatchTest, FormatUrlForSuggestionDisplay) {
 
   FormatUrlTestData normal_cases[] = {
       // Test trim_scheme parameter without any feature flags.
-      {"http://google.com", true, L"google.com"},
-      {"https://google.com", true, L"https://google.com"},
-      {"http://google.com", false, L"http://google.com"},
-      {"https://google.com", false, L"https://google.com"},
-
-      // Test that paths are preserved in the default case.
-      {"http://google.com/foobar", true, L"google.com/foobar"},
+      {"http://google.com", false, true, true, L"google.com"},
+      {"https://google.com", false, true, true, L"https://google.com"},
+      {"http://google.com", true, true, true, L"http://google.com"},
+      {"https://google.com", true, true, true, L"https://google.com"},
 
       // Verify that trivial subdomains are preserved in the normal case.
-      {"http://www.google.com", false, L"http://www.google.com"}};
+      {"http://www.google.com", false, false, false, L"www.google.com"},
+
+      // Test that paths are preserved in the default case.
+      {"http://google.com/foobar", false, false, false, L"google.com/foobar"},
+  };
   for (FormatUrlTestData& test_case : normal_cases)
     test_case.Validate();
 
@@ -155,33 +161,39 @@ TEST(AutocompleteMatchTest, FormatUrlForSuggestionDisplay) {
       omnibox::kUIExperimentHideSuggestionUrlScheme);
 
   FormatUrlTestData omit_scheme_cases[] = {
-      {"http://google.com", true, L"google.com"},
-      {"https://google.com", true, L"google.com"},
-      {"http://google.com", false, L"http://google.com"},
-      {"https://google.com", false, L"https://google.com"},
+      {"http://google.com", false, false, false, L"google.com"},
+      {"https://google.com", false, false, false, L"google.com"},
+      {"http://google.com", true, false, false, L"http://google.com"},
+      {"https://google.com", true, false, false, L"https://google.com"},
   };
   for (FormatUrlTestData& test_case : omit_scheme_cases)
     test_case.Validate();
 
-  // Test the elide-after-host feature flag.
+  // Test the trim trivial subdomains feature flag.
   feature_list.reset(new base::test::ScopedFeatureList);
   feature_list->InitAndEnableFeature(
-      omnibox::kUIExperimentElideSuggestionUrlAfterHost);
-  FormatUrlTestData hide_path_cases[] = {
-      {"http://google.com/foobar", true, L"google.com/\x2026\x0000"},
-      {"http://google.com/foobar", false, L"http://google.com/\x2026\x0000"},
+      omnibox::kUIExperimentHideSuggestionUrlTrivialSubdomains);
+
+  FormatUrlTestData trim_trivial_subdomains_cases[] = {
+      {"http://www.m.google.com", false, false, false, L"google.com"},
+      {"http://www.m.google.com", false, true, false, L"www.m.google.com"},
   };
-  for (FormatUrlTestData& test_case : hide_path_cases)
+
+  for (FormatUrlTestData& test_case : trim_trivial_subdomains_cases)
     test_case.Validate();
 
-  // Test the trim trivial subdomains feature flag.
+  // Test the elide-after-host feature flag.
   feature_list.reset(new base::test::ScopedFeatureList);
   feature_list->InitAndEnableFeature(
-      omnibox::kUIExperimentHideSuggestionUrlTrivialSubdomains);
+      omnibox::kUIExperimentElideSuggestionUrlAfterHost);
 
-  FormatUrlTestData trim_trivial_subdomains_case = {
-      "http://www.m.google.com", false, L"http://google.com"};
-  trim_trivial_subdomains_case.Validate();
+  FormatUrlTestData hide_path_cases[] = {
+      {"http://google.com/foobar", false, false, false,
+       L"google.com/\x2026\x0000"},
+      {"http://google.com/foobar", false, false, true, L"google.com/foobar"},
+  };
+  for (FormatUrlTestData& test_case : hide_path_cases)
+    test_case.Validate();
 }
 
 TEST(AutocompleteMatchTest, SupportsDeletion) {

components/omnibox/browser/clipboard_url_provider.cc

@@ -86,7 +86,7 @@ void ClipboardURLProvider::Start(const AutocompleteInput& input,
   // Add the clipboard match. The relevance is 800 to beat ZeroSuggest results.
   AutocompleteMatch match(this, 800, false, AutocompleteMatchType::CLIPBOARD);
   match.destination_url = url;
-  auto format_types = AutocompleteMatch::GetFormatTypes(true);
+  auto format_types = AutocompleteMatch::GetFormatTypes(false, false, false);
   match.contents.assign(url_formatter::FormatUrl(
       url, format_types, net::UnescapeRule::SPACES, nullptr, nullptr, nullptr));
   AutocompleteMatch::ClassifyLocationInString(

components/omnibox/browser/history_quick_provider.cc

@@ -235,7 +235,9 @@ AutocompleteMatch HistoryQuickProvider::QuickMatchToACMatch(
       OffsetsFromTermMatches(history_match.url_matches);
   match.contents = url_formatter::FormatUrlWithOffsets(
       info.url(),
-      AutocompleteMatch::GetFormatTypes(!history_match.match_in_scheme),
+      AutocompleteMatch::GetFormatTypes(history_match.match_in_scheme,
+                                        history_match.match_in_subdomain,
+                                        history_match.match_after_host),
       net::UnescapeRule::SPACES, nullptr, nullptr, &offsets);
 
   TermMatches new_matches =

components/omnibox/browser/history_url_provider.cc

@@ -1196,7 +1196,8 @@ AutocompleteMatch HistoryURLProvider::HistoryMatchToACMatch(
 
   size_t match_start = history_match.input_location;
   const auto format_types = AutocompleteMatch::GetFormatTypes(
-      params.trim_http && !history_match.match_in_scheme);
+      !params.trim_http || history_match.match_in_scheme,
+      history_match.match_in_subdomain, history_match.match_after_host);
   match.contents = url_formatter::FormatUrl(info.url(), format_types,
                                             net::UnescapeRule::SPACES, nullptr,
                                             nullptr, &match_start);

components/omnibox/browser/physical_web_provider.cc

@@ -234,8 +234,8 @@ void PhysicalWebProvider::ConstructZeroSuggestMatches(
     match.destination_url = url;
 
     match.contents = url_formatter::FormatUrl(
-        url, AutocompleteMatch::GetFormatTypes(true), net::UnescapeRule::SPACES,
-        nullptr, nullptr, nullptr);
+        url, AutocompleteMatch::GetFormatTypes(false, false, false),
+        net::UnescapeRule::SPACES, nullptr, nullptr, nullptr);
     match.contents_class.push_back(
         ACMatchClassification(0, ACMatchClassification::URL));
 

components/omnibox/browser/search_suggestion_parser.cc

@@ -280,7 +280,7 @@ SearchSuggestionParser::NavigationResult::CalculateAndClassifyMatchContents(
   bool trim_http = !AutocompleteInput::HasHTTPScheme(input_text) &&
                    (!prefix || (match_start != 0));
   base::string16 match_contents = url_formatter::FormatUrl(
-      url_, AutocompleteMatch::GetFormatTypes(trim_http),
+      url_, AutocompleteMatch::GetFormatTypes(!trim_http, false, false),
       net::UnescapeRule::SPACES, nullptr, nullptr, &match_start);
   // If the first match in the untrimmed string was inside a scheme that we
   // trimmed, look for a subsequent match.

components/omnibox/browser/titled_url_match_utils.cc

@@ -85,7 +85,8 @@ AutocompleteMatch TitledUrlMatchToAutocompleteMatch(
   // |offsets|, compute how everything is transformed, then remove it from the
   // end.
   offsets.push_back(inline_autocomplete_offset);
-  auto format_types = AutocompleteMatch::GetFormatTypes(trim_http);
+  auto format_types =
+      AutocompleteMatch::GetFormatTypes(!trim_http, false, false);
   match.contents = url_formatter::FormatUrlWithOffsets(
       url, format_types, net::UnescapeRule::SPACES, nullptr, nullptr, &offsets);
   inline_autocomplete_offset = offsets.back();

components/omnibox/browser/zero_suggest_provider.cc

@@ -375,7 +375,7 @@ AutocompleteMatch ZeroSuggestProvider::NavigationToMatch(
   match.destination_url = navigation.url();
 
   // Zero suggest results should always omit protocols and never appear bold.
-  auto format_types = AutocompleteMatch::GetFormatTypes(true);
+  auto format_types = AutocompleteMatch::GetFormatTypes(false, false, false);
   match.contents = url_formatter::FormatUrl(navigation.url(), format_types,
                                             net::UnescapeRule::SPACES, nullptr,
                                             nullptr, nullptr);