[omnibox] Swap Calculator terms in URL to search correctly

The current URL generated from a Calculator suggestion searches for
the result e.g. "4", not the calculation e.g. "2+2". This CL swaps
the terms (actually, it clears the original query) and, further, treats
the suggestions as distinct from non-calculator suggestions, so that
they don't dupe against other WYT suggestions.

Bug: 873666
Change-Id: I006f9754ee7e48d0a21a1f149c4ef76b3ca509fd
Reviewed-on: https://chromium-review.googlesource.com/1197342
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589228}

components/omnibox/browser/autocomplete_result.cc

@@ -29,6 +29,14 @@
 #include "third_party/metrics_proto/omnibox_input_type.pb.h"
 #include "ui/base/l10n/l10n_util.h"
 
+struct MatchGURLHash {
+  // The |bool| is whether the match is a calculator suggestion. We want them
+  // compare differently against other matches with the same URL.
+  size_t operator()(const std::pair<GURL, bool>& p) const {
+    return std::hash<std::string>()(p.first.spec()) + p.second;
+  }
+};
+
 // A match attribute when a default match's score has been boosted
 // with a higher scoring non-default match.
 static const char kScoreBoostedFrom[] = "score_boosted_from";
@@ -356,16 +364,20 @@ GURL AutocompleteResult::ComputeAlternateNavUrl(
 void AutocompleteResult::SortAndDedupMatches(
     metrics::OmniboxEventProto::PageClassification page_classification,
     ACMatches* matches) {
-  // Group matches by stripped URL.
-  std::unordered_map<GURL, std::list<ACMatches::iterator>, MatchGURLHash>
+  // Group matches by stripped URL and whether it's a calculator suggestion.
+  std::unordered_map<std::pair<GURL, bool>, std::list<ACMatches::iterator>,
+                     MatchGURLHash>
       url_to_matches;
-  for (auto i = matches->begin(); i != matches->end(); ++i)
-    url_to_matches[i->stripped_destination_url].push_back(i);
+  for (auto i = matches->begin(); i != matches->end(); ++i) {
+    std::pair<GURL, bool> p = GetMatchComparisonFields(*i);
+    url_to_matches[p].push_back(i);
+  }
   CompareWithDemoteByType<AutocompleteMatch> compare_demote_by_type(
       page_classification);
   // Find best default, and non-default, match in each group.
   for (auto& group : url_to_matches) {
-    const GURL& gurl = group.first;
+    const auto& p = group.first;
+    const GURL& gurl = p.first;
     // The list of matches whose URL are equivalent.
     auto& duplicate_matches = group.second;
     if (gurl.is_empty() || duplicate_matches.size() == 1)
@@ -413,8 +425,9 @@ void AutocompleteResult::SortAndDedupMatches(
       std::remove_if(
           matches->begin(), matches->end(),
           [&url_to_matches](const AutocompleteMatch& m) {
+            std::pair<GURL, bool> p = GetMatchComparisonFields(m);
             return !m.stripped_destination_url.is_empty() &&
-                   &(*url_to_matches[m.stripped_destination_url].front()) != &m;
+                   &(*url_to_matches[p].front()) != &m;
           }),
       matches->end());
 }
@@ -561,3 +574,9 @@ void AutocompleteResult::MergeMatchesByProvider(
     }
   }
 }
+
+std::pair<GURL, bool> AutocompleteResult::GetMatchComparisonFields(
+    const AutocompleteMatch& match) {
+  return std::make_pair(match.stripped_destination_url,
+                        match.type == AutocompleteMatchType::CALCULATOR);
+}

components/omnibox/browser/autocomplete_result.h

@@ -161,6 +161,14 @@ class AutocompleteResult {
       const ACMatches& old_matches,
       const ACMatches& new_matches);
 
+  // This pulls the relevant fields out of a match for comparison with other
+  // matches for the purpose of deduping. It uses the stripped URL, so that we
+  // collapse similar URLs if necessary, and whether the match is a calculator
+  // suggestion, because we don't want to dedupe them against URLs that simply
+  // happen to go to the same destination.
+  static std::pair<GURL, bool> GetMatchComparisonFields(
+      const AutocompleteMatch& match);
+
   ACMatches matches_;
 
   const_iterator default_match_;

components/omnibox/browser/base_search_provider.cc

@@ -292,13 +292,18 @@ AutocompleteMatch BaseSearchProvider::CreateSearchSuggestion(
         suggestion.suggestion().substr(input.text().length());
     match.allowed_to_be_default_match = true;
   }
-  match.fill_into_edit.append(suggestion.suggestion());
 
   const TemplateURLRef& search_url = template_url->url_ref();
   DCHECK(search_url.SupportsReplacement(search_terms_data));
-  match.search_terms_args.reset(
-      new TemplateURLRef::SearchTermsArgs(suggestion.suggestion()));
-  match.search_terms_args->original_query = input.text();
+  base::string16 query(suggestion.suggestion());
+  base::string16 original_query(input.text());
+  if (suggestion.type() == AutocompleteMatchType::CALCULATOR) {
+    query = original_query;
+    original_query.clear();
+  }
+  match.fill_into_edit.append(query);
+  match.search_terms_args.reset(new TemplateURLRef::SearchTermsArgs(query));
+  match.search_terms_args->original_query = original_query;
   match.search_terms_args->accepted_suggestion = accepted_suggestion;
   match.search_terms_args->additional_query_params =
       suggestion.additional_query_params();

components/omnibox/browser/match_compare.h

@@ -64,10 +64,4 @@ class DestinationSort {
   CompareWithDemoteByType<Match> demote_by_type_;
 };
 
-struct MatchGURLHash {
-  size_t operator()(const GURL& gurl) const {
-    return std::hash<std::string>()(gurl.spec());
-  }
-};
-
 #endif  // COMPONENTS_OMNIBOX_BROWSER_MATCH_COMPARE_H_