Modify Entity Suggestion support and Add Profile Suggest support

chrome/browser/autocomplete/search_provider.cc

 // Copyright 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 "chrome/browser/autocomplete/search_provider.h"
 
 #include <algorithm>
 #include <cmath>
 
 #include "base/callback.h"
@@ skipping 83 matching lines @@
       }
     }
   }
   return false;
 }
 
 // Builds the match contents and classification for the contents, and updates
 // the given |AutocompleteMatch|.
 void SetAndClassifyMatchContents(const string16& query_string,
                                  const string16& input_text,
-                                 const string16& match_contents,
-                                 const string16& annotation,
+                                 const string16& title,
+                                 const ACMatchClassifications& title_class,
                                  AutocompleteMatch* match) {
-  size_t match_contents_start = 0;
-  size_t annotation_start = match_contents.size();
-  // Append annotation if present.
-  if (annotation.empty()) {
-    match->contents = match_contents;
+  if (title.empty()) {
+    match->contents = query_string;
   } else {
-    std::vector<size_t> positions;
-    match->contents = l10n_util::GetStringFUTF16(
-        IDS_ANNOTATED_SUGGESTION, match_contents, annotation, &positions);
-    match_contents_start = positions[0];
-    annotation_start = positions[1];
+    match->contents = title;
+    if (!title_class.empty()) {
+      match->contents_class = title_class;
+      return;
+    }
   }
-  size_t match_contents_end = match_contents_start + match_contents.size();
-
-  if (!annotation.empty() && (annotation_start < match_contents_start))
-    match->contents_class.push_back(ACMatchClassification(
-        annotation_start, ACMatchClassification::DIM));
-
   // We do intra-string highlighting for suggestions - the suggested segment
   // will be highlighted, e.g. for input_text = "you" the suggestion may be
   // "youtube", so we'll bold the "tube" section: you*tube*.
-  if (input_text != match_contents) {
-    size_t input_position = match->contents.substr(
-        match_contents_start, match_contents.length()).find(input_text);
+  if (input_text != title) {
+    size_t input_position = match->contents.find(input_text);
     if (input_position == string16::npos) {
       // The input text is not a substring of the query string, e.g. input
       // text is "slasdot" and the query string is "slashdot", so we bold the
       // whole thing.
       match->contents_class.push_back(ACMatchClassification(
-          match_contents_start, ACMatchClassification::MATCH));
+          0, ACMatchClassification::MATCH));
     } else {
-      input_position += match_contents_start;
-
       // TODO(beng): ACMatchClassification::MATCH now seems to just mean
       //             "bold" this. Consider modifying the terminology.
       // We don't iterate over the string here annotating all matches because
       // it looks odd to have every occurrence of a substring that may be as
       // short as a single character highlighted in a query suggestion result,
       // e.g. for input text "s" and query string "southwest airlines", it
       // looks odd if both the first and last s are highlighted.
-      if (input_position != match_contents_start) {
+      if (input_position != 0) {
         match->contents_class.push_back(ACMatchClassification(
-            match_contents_start, ACMatchClassification::MATCH));
+            0, ACMatchClassification::MATCH));
       }
       match->contents_class.push_back(
           ACMatchClassification(input_position, ACMatchClassification::NONE));
       size_t next_fragment_position = input_position + input_text.length();
       if (next_fragment_position < query_string.length()) {
         match->contents_class.push_back(ACMatchClassification(
             next_fragment_position, ACMatchClassification::MATCH));
       }
     }
   } else {
     // Otherwise, |match| is a verbatim (what-you-typed) match, either for the
     // default provider or a keyword search provider.
     match->contents_class.push_back(ACMatchClassification(
-        match_contents_start, ACMatchClassification::NONE));
+        0, ACMatchClassification::NONE));
   }
+}
 
-  if (!annotation.empty() && (annotation_start >= match_contents_start))
-    match->contents_class.push_back(ACMatchClassification(
-        match_contents_end, ACMatchClassification::DIM));
+AutocompleteMatchType::Type GetAutocompleteMatchType(const std::string& type) {
+  if (type == "ENTITY")
+    return AutocompleteMatchType::SEARCH_SUGGEST_ENTITY;
+  if (type == "INFINITE")
+    return AutocompleteMatchType::SEARCH_SUGGEST_INFINITE;
+  return AutocompleteMatchType::SEARCH_SUGGEST;
+}
+
+// Whether the given suggestion types supports suggestion detail.

[Peter Kasting, 2013/11/05 23:36:58]

Nit: types -> type

[Anuj, 2013/12/09 08:24:56]

Method removed.

+bool TypeSupportsSuggestionDetail(const std::string& type) {
+  return (type == "ENTITY") || (type == "INFINITE") || (type == "PROFILE") ||
+      (type == "PERSONALIZED_QUERY");
 }
 
 }  // namespace
 
 
 // SearchProvider::Providers --------------------------------------------------
 
 SearchProvider::Providers::Providers(TemplateURLService* template_url_service)
     : template_url_service_(template_url_service) {
 }
@@ skipping 20 matching lines @@
 }
 
 SearchProvider::Result::~Result() {
 }
 
 
 // SearchProvider::SuggestResult ----------------------------------------------
 
 SearchProvider::SuggestResult::SuggestResult(
     const string16& suggestion,
-    const string16& match_contents,
+    AutocompleteMatchType::Type type,
+    const string16& title,
+    ACMatchClassifications title_class,
     const string16& annotation,
+    ACMatchClassifications annotation_class,
     const std::string& suggest_query_params,
     bool from_keyword_provider,
     int relevance,
     bool relevance_from_server,
     bool should_prefetch)
     : Result(from_keyword_provider, relevance, relevance_from_server),
       suggestion_(suggestion),
-      match_contents_(match_contents),
+      type_(type),
+      title_(title),
+      title_class_(title_class),
       annotation_(annotation),
+      annotation_class_(annotation_class),
       suggest_query_params_(suggest_query_params),
       should_prefetch_(should_prefetch) {
 }
 
 SearchProvider::SuggestResult::~SuggestResult() {
 }
 
 bool SearchProvider::SuggestResult::IsInlineable(const string16& input) const {
   return StartsWith(suggestion_, input, false);
 }
@@ skipping 110 matching lines @@
 }
 
 // static
 AutocompleteMatch SearchProvider::CreateSearchSuggestion(
     AutocompleteProvider* autocomplete_provider,
     const AutocompleteInput& input,
     const string16& input_text,
     int relevance,
     AutocompleteMatch::Type type,
     bool is_keyword,
-    const string16& match_contents,
+    const string16& title,
+    const ACMatchClassifications& title_class,

[Peter Kasting, 2013/11/05 23:36:58]

I'm not terribly keen on changing the param names here to be |title| and
|title_class|, because this function doesn't know or care that |title| is
actually a Title, and for many calls (that are standard search suggestions) it
won't be.  It seems like the old |match_contents| name was better here and in
SetAndClassifyMatchContents(), and maybe in AddMatchToMap() also.

[Anuj, 2013/11/06 00:18:28]

This change was in accordance with our in person discussion last week. I will
revert this pending other questions.

     const string16& annotation,
+    const ACMatchClassifications& annotation_class,

[Peter Kasting, 2013/11/05 23:36:58]

Do we have to pass |annotation| and |annotation_class| in at all?  Why can't the
one caller who needs something special w.r.t. them set the match description +
classifications directly?

[Anuj, 2013/11/06 00:18:28]

Please elaborate.
What do you mean by "one caller"?

Currently I plan to use it for Entity and Profile suggestions.

[Peter Kasting, 2013/11/06 00:34:26]

Only AddMatchToMap() needs to do anything with this field.  So the relevant code
below can just be moved there.

[Anuj, 2013/12/09 08:24:56]

Classification parameters removed

     const TemplateURL* template_url,
     const string16& query_string,
     const std::string& suggest_query_params,
     int accepted_suggestion,
     int omnibox_start_margin,
     bool append_extra_query_params) {
   AutocompleteMatch match(autocomplete_provider, relevance, false, type);
 
   if (!template_url)
     return match;
   match.keyword = template_url->keyword();
 
   SetAndClassifyMatchContents(
-      query_string, input_text, match_contents, annotation, &match);
+      query_string, input_text, title, title_class, &match);
 
-  match.allowed_to_be_default_match = (input_text == match_contents);
+  if (!annotation.empty()) {
+    match.description = annotation;
+    if (!annotation_class.empty())
+      match.description_class = annotation_class;
+  }
+
+  match.allowed_to_be_default_match = (input_text == title);
 
   // When the user forced a query, we need to make sure all the fill_into_edit
   // values preserve that property.  Otherwise, if the user starts editing a
   // suggestion, non-Search results will suddenly appear.
   if (input.type() == AutocompleteInput::FORCED_QUERY)
     match.fill_into_edit.assign(ASCIIToUTF16("?"));
   if (is_keyword)
     match.fill_into_edit.append(match.keyword + char16(' '));
   if (!input.prevent_inline_autocomplete() &&
       StartsWith(query_string, input_text, false)) {
@@ skipping 651 matching lines @@
       // Do not blindly trust the URL coming from the server to be valid.
       GURL url(URLFixerUpper::FixupURL(UTF16ToUTF8(suggestion), std::string()));
       if (url.is_valid()) {
         string16 title;
         if (descriptions != NULL)
           descriptions->GetString(index, &title);
         results->navigation_results.push_back(NavigationResult(
             *this, url, title, is_keyword, relevance, true));
       }
     } else {
+      AutocompleteMatchType::Type match_type = GetAutocompleteMatchType(type);
       bool should_prefetch = static_cast<int>(index) == prefetch_index;
       DictionaryValue* suggestion_detail = NULL;
-      string16 match_contents = suggestion;
-      string16 disambiguating_query;
+      string16 title;
+      ACMatchClassifications title_class;
       string16 annotation;
+      ACMatchClassifications annotation_class;
+      std::string classifications;
       std::string suggest_query_params;
-      if (suggestion_details && (type == "ENTITY") &&
+      if (suggestion_details && TypeSupportsSuggestionDetail(type) &&
           suggestion_details->GetDictionary(index, &suggestion_detail) &&
           suggestion_detail) {
-        suggestion_detail->GetString("a", &annotation);
-        if (suggestion_detail->GetString("dq", &disambiguating_query) &&
-            !disambiguating_query.empty())
-          suggestion = disambiguating_query;
-        suggestion_detail->GetString("q", &suggest_query_params);
+        suggestion_detail->GetString("title", &title) ||
+            suggestion_detail->GetString("t", &title);
+        suggestion_detail->GetString("title_class", &classifications) ||
+            suggestion_detail->GetString("tc", &classifications);
+        title_class =
+            AutocompleteMatch::ClassificationsFromString(classifications);

[Peter Kasting, 2013/11/05 23:36:58]

The more I think about this the less OK I am with the idea that the server is
going to pass us data that we're going to feed directly to
ClassificationsFromString(), for several reasons.

We should pass down data that Chrome decides how to mark up, as we were doing
before.  Even if we wanted to pass explicit markup control from the server we'd
need to do it as a separate XML/JSON set of directives that we then parsed, but
I think even that is a bad idea.

[Anuj, 2013/11/06 00:18:28]

I failed to identify "several reasons" in your comment.

We discussed and agreed that server can provide rendering hints using the
AutocompleteMatchType. I fail to understand why is this a bad idea now. This
scheme allows server to exercise variations within the bounds specified by the
Chrome UX.

If the serialization/deserialization is concerned, I think that is a trivial
matter. ClassificationsFromString is as good as any other option as long as
server is going to be aware of the general structure of ACMatchClassifications.

[Peter Kasting, 2013/11/06 00:34:26]

That's because I didn't give them.  I'd really rather not write out things and
debate.  But the short summary is that I don't want the server to control the
layout at this low level in principle; I don't want to expose the low-level
details of the classification values and the stringified forms to anyone outside
the Chrome codebase; and I don't want to pass server data directly to conversion
functions without making them robust against attacks (e.g. passing in very long
strings or invalid offsets or something).

These reasons haven't changed from why I had you pass down the annotation and
disambiguating query, and add IDS_ANNOTATED_SUGGESTION and code to mark things
up using it, in the first place.  I'm not OK with reversing that course.
The server can tell you what kind of a suggestion you're getting, sure.  That's
far from telling you precisely how to format some string of text.

[Anuj, 2013/12/09 08:24:56]

Classifications removed.

+        classifications.clear();
+
+        suggestion_detail->GetString("annotation", &annotation) ||
+            suggestion_detail->GetString("a", &annotation);
+        suggestion_detail->GetString("annotation_class", &classifications) ||
+            suggestion_detail->GetString("ac", &classifications);
+        annotation_class =
+            AutocompleteMatch::ClassificationsFromString(classifications);
+        classifications.clear();
+
+        suggestion_detail->GetString("query_params", &suggest_query_params) ||
+            suggestion_detail->GetString("q", &suggest_query_params);
       }
       // TODO(kochi): Improve calculator suggestion presentation.
       results->suggest_results.push_back(SuggestResult(
-          suggestion, match_contents, annotation, suggest_query_params,
-          is_keyword, relevance, true, should_prefetch));
+          suggestion, match_type, title,  title_class, annotation,

[Peter Kasting, 2013/11/05 23:36:58]

Nit: Two spaces

[Anuj, 2013/12/09 08:24:56]

Done.

+          annotation_class, suggest_query_params, is_keyword, relevance, true,
+          should_prefetch));
     }
   }
 
   // Apply calculated relevance scores if a valid list was not provided.
   if (relevances == NULL) {
     ApplyCalculatedSuggestRelevance(&results->suggest_results);
     ApplyCalculatedNavigationRelevance(&results->navigation_results);
   }
   // Keep the result lists sorted.
   const CompareScoredResults comparator = CompareScoredResults();
@@ skipping 25 matching lines @@
       TemplateURLRef::NO_SUGGESTION_CHOSEN;
   if (verbatim_relevance > 0) {
     AddMatchToMap(input_.text(),
                   verbatim_relevance,
                   relevance_from_server,
                   false,
                   std::string(),
                   AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED,
                   false,
                   input_.text(),
+                  ACMatchClassifications(),
                   string16(),
+                  ACMatchClassifications(),
                   input_.text(),
                   did_not_accept_default_suggestion,
                   std::string(),
                   &map);
   }
   if (!keyword_input_.text().empty()) {
     const TemplateURL* keyword_url = providers_.GetKeywordProviderURL();
     // We only create the verbatim search query match for a keyword
     // if it's not an extension keyword.  Extension keywords are handled
     // in KeywordProvider::Start().  (Extensions are complicated...)
     // Note: in this provider, SEARCH_OTHER_ENGINE must correspond
     // to the keyword verbatim search query.  Do not create other matches
     // of type SEARCH_OTHER_ENGINE.
     if (keyword_url &&
         (keyword_url->GetType() != TemplateURL::OMNIBOX_API_EXTENSION)) {
       bool keyword_relevance_from_server;
       const int keyword_verbatim_relevance =
           GetKeywordVerbatimRelevance(&keyword_relevance_from_server);
       if (keyword_verbatim_relevance > 0) {
         AddMatchToMap(keyword_input_.text(),
                       keyword_verbatim_relevance,
                       keyword_relevance_from_server,
                       false,
                       std::string(),
                       AutocompleteMatchType::SEARCH_OTHER_ENGINE,
                       true,
                       keyword_input_.text(),
+                      ACMatchClassifications(),
                       string16(),
+                      ACMatchClassifications(),
                       keyword_input_.text(),
                       did_not_accept_keyword_suggestion,
                       std::string(),
                       &map);
       }
     }
   }
   AddHistoryResultsToMap(keyword_history_results_, true,
                          did_not_accept_keyword_suggestion, &map);
   AddHistoryResultsToMap(default_history_results_, false,
@@ skipping 227 matching lines @@
   for (SuggestResults::const_iterator i(scored_results.begin());
        i != scored_results.end(); ++i) {
     AddMatchToMap(input_text,
                   i->relevance(),
                   false,
                   false,
                   std::string(),
                   AutocompleteMatchType::SEARCH_HISTORY,
                   is_keyword,
                   i->suggestion(),
+                  ACMatchClassifications(),
                   string16(),
+                  ACMatchClassifications(),
                   i->suggestion(),
                   did_not_accept_suggestion,
                   std::string(),
                   map);
   }
   UMA_HISTOGRAM_TIMES("Omnibox.SearchProvider.AddHistoryResultsTime",
                       base::TimeTicks::Now() - start_time);
 }
 
 SearchProvider::SuggestResults SearchProvider::ScoreHistoryResults(
@@ skipping 31 matching lines @@
     if (!prevent_inline_autocomplete && classifier && (i->term != input_text)) {
       AutocompleteMatch match;
       classifier->Classify(i->term, false, false, &match, NULL);
       prevent_inline_autocomplete =
           !AutocompleteMatch::IsSearchType(match.type);
     }
 
     int relevance = CalculateRelevanceForHistory(
         i->time, is_keyword, !prevent_inline_autocomplete,
         prevent_search_history_inlining);
-    scored_results.push_back(
-        SuggestResult(i->term, string16(), string16(), std::string(),
-                      is_keyword, relevance, false, false));
+    scored_results.push_back(SuggestResult(
+        i->term, AutocompleteMatchType::SEARCH_HISTORY, string16(),
+        ACMatchClassifications(), string16(), ACMatchClassifications(),
+        std::string(), is_keyword, relevance, false, false));
   }
 
   // History returns results sorted for us.  However, we may have docked some
   // results' scores, so things are no longer in order.  Do a stable sort to get
   // things back in order without otherwise disturbing results with equal
   // scores, then force the scores to be unique, so that the order in which
   // they're shown is deterministic.
   std::stable_sort(scored_results.begin(), scored_results.end(),
                    CompareScoredResults());
   int last_relevance = 0;
@@ skipping 11 matching lines @@
                                             const std::string& metadata,
                                             MatchMap* map) {
   for (size_t i = 0; i < results.size(); ++i) {
     const bool is_keyword = results[i].from_keyword_provider();
     const string16& input = is_keyword ? keyword_input_.text() : input_.text();
     AddMatchToMap(input,
                   results[i].relevance(),
                   results[i].relevance_from_server(),
                   results[i].should_prefetch(),
                   metadata,
-                  AutocompleteMatchType::SEARCH_SUGGEST,
+                  results[i].type(),
                   is_keyword,
-                  results[i].match_contents(),
+                  results[i].title(),
+                  results[i].title_class(),
                   results[i].annotation(),
+                  results[i].annotation_class(),
                   results[i].suggestion(),
                   i,
                   results[i].suggest_query_params(),
                   map);
   }
 }
 
 int SearchProvider::GetVerbatimRelevance(bool* relevance_from_server) const {
   // Use the suggested verbatim relevance score if it is non-negative (valid),
   // if inline autocomplete isn't prevented (always show verbatim on backspace),
@@ skipping 95 matching lines @@
   // Don't let scores go below 0.  Negative relevance scores are meaningful in
   // a different way.
   int base_score;
   if (is_primary_provider)
     base_score = (input_.type() == AutocompleteInput::URL) ? 750 : 1050;
   else
     base_score = 200;
   return std::max(0, base_score - score_discount);
 }
 
-void SearchProvider::AddMatchToMap(const string16& input_text,
-                                   int relevance,
-                                   bool relevance_from_server,
-                                   bool should_prefetch,
-                                   const std::string& metadata,
-                                   AutocompleteMatch::Type type,
-                                   bool is_keyword,
-                                   const string16& match_contents,
-                                   const string16& annotation,
-                                   const string16& query_string,
-                                   int accepted_suggestion,
-                                   const std::string& suggest_query_params,
-                                   MatchMap* map) {
+void SearchProvider::AddMatchToMap(
+    const string16& input_text,
+    int relevance,
+    bool relevance_from_server,
+    bool should_prefetch,
+    const std::string& metadata,
+    AutocompleteMatch::Type type,
+    bool is_keyword,
+    const string16& title,
+    const ACMatchClassifications& title_class,
+    const string16& annotation,
+    const ACMatchClassifications& annotation_class,
+    const string16& query_string,
+    int accepted_suggestion,
+    const std::string& suggest_query_params,
+    MatchMap* map) {
   // On non-mobile, ask the instant controller for the appropriate start margin.
   // On mobile the start margin is unused, so leave the value as default there.
   int omnibox_start_margin = chrome::kDisableStartMargin;
 #if !defined(OS_ANDROID) && !defined(IOS)
   if (chrome::IsInstantExtendedAPIEnabled()) {
     Browser* browser =
         chrome::FindBrowserWithProfile(profile_, chrome::GetActiveDesktop());
     if (browser && browser->instant_controller() &&
         browser->instant_controller()->instant()) {
       omnibox_start_margin =
           browser->instant_controller()->instant()->omnibox_bounds().x();
     }
   }
 #endif  // !defined(OS_ANDROID) && !defined(IOS)
 
   const TemplateURL* template_url = is_keyword ?
       providers_.GetKeywordProviderURL() : providers_.GetDefaultProviderURL();
   AutocompleteMatch match = CreateSearchSuggestion(
-      this, input_, input_text, relevance, type, is_keyword, match_contents,
-      annotation, template_url, query_string, suggest_query_params,
-      accepted_suggestion, omnibox_start_margin,
+      this, input_, input_text, relevance, type, is_keyword, title, title_class,
+      annotation, annotation_class, template_url, query_string,
+      suggest_query_params, accepted_suggestion, omnibox_start_margin,
       !is_keyword || providers_.default_provider().empty());
   if (!match.destination_url.is_valid())
     return;
   match.search_terms_args->bookmark_bar_pinned =
       profile_->GetPrefs()->GetBoolean(prefs::kShowBookmarkBar);
   match.RecordAdditionalInfo(kRelevanceFromServerKey,
                              relevance_from_server ? kTrue : kFalse);
   match.RecordAdditionalInfo(kShouldPrefetchKey,
                              should_prefetch ? kTrue : kFalse);
 
@@ skipping 152 matching lines @@
     it->set_relevance(max_query_relevance);
     it->set_relevance_from_server(relevance_from_server);
   }
 }
 
 void SearchProvider::UpdateDone() {
   // We're done when the timer isn't running, there are no suggest queries
   // pending, and we're not waiting on Instant.
   done_ = !timer_.IsRunning() && (suggest_results_pending_ == 0);
 }