[AiS] Use top local result for prefetch.

components/omnibox/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 "components/omnibox/search_provider.h"
 
 #include <algorithm>
 #include <cmath>
 
 #include "base/base64.h"
@@ skipping 247 matching lines @@
       match.allowed_to_be_default_match = true;
       matches_.push_back(match);
     }
     Stop(true);
     return;
   }
 
   input_ = input;
 
   DoHistoryQuery(minimal_changes);
-  DoAnswersQuery(input);
+  // Answers needs scored history results before any suggest query has been
+  // started, since the query for answer-bearing results needs additional
+  // prefetch information based on the highest-scored local history result.
+  if (OmniboxFieldTrial::EnableAnswersInSuggest()) {
+    ScoreHistoryResultsMultiWord(raw_default_history_results_,
+                                 false,
+                                 &transformed_default_history_results_);
+    ScoreHistoryResultsMultiWord(raw_keyword_history_results_,
+                                 true,
+                                 &transformed_keyword_history_results_);
+
+    prefetch_data_ = FindAnswersPrefetchData(input.text());
+  } else {
+    transformed_default_history_results_.clear();
+    transformed_keyword_history_results_.clear();
+  }
+
   StartOrStopSuggestQuery(minimal_changes);
   UpdateMatches();
 }
 
 void SearchProvider::Stop(bool clear_cached_results) {
   StopSuggest();
   done_ = true;
 
   if (clear_cached_results)
     ClearAllResults();
@@ skipping 233 matching lines @@
       listener_->OnProviderUpdate(false);
   }
 }
 
 void SearchProvider::DoHistoryQuery(bool minimal_changes) {
   // The history query results are synchronous, so if minimal_changes is true,
   // we still have the last results and don't need to do anything.
   if (minimal_changes)
     return;
 
-  keyword_history_results_.clear();
-  default_history_results_.clear();
+  raw_keyword_history_results_.clear();
+  raw_default_history_results_.clear();
 
   if (OmniboxFieldTrial::SearchHistoryDisable(
       input_.current_page_classification()))
     return;
 
   history::URLDatabase* url_db = client_->InMemoryDatabase();
   if (!url_db)
     return;
 
   // Request history for both the keyword and default provider.  We grab many
   // more matches than we'll ultimately clamp to so that if there are several
   // recent multi-word matches who scores are lowered (see
-  // AddHistoryResultsToMap()), they won't crowd out older, higher-scoring
+  // ScoreHistoryResultsMultiWord()), they won't crowd out older, higher-scoring
   // matches.  Note that this doesn't fix the problem entirely, but merely
   // limits it to cases with a very large number of such multi-word matches; for
   // now, this seems OK compared with the complexity of a real fix, which would
   // require multiple searches and tracking of "single- vs. multi-word" in the
   // database.
   int num_matches = kMaxMatches * 5;
   const TemplateURL* default_url = providers_.GetDefaultProviderURL();
   if (default_url) {
     const base::TimeTicks start_time = base::TimeTicks::Now();
-    url_db->GetMostRecentKeywordSearchTerms(default_url->id(), input_.text(),
-        num_matches, &default_history_results_);
+    url_db->GetMostRecentKeywordSearchTerms(default_url->id(),
+                                            input_.text(),
+                                            num_matches,
+                                            &raw_default_history_results_);
     UMA_HISTOGRAM_TIMES(
         "Omnibox.SearchProvider.GetMostRecentKeywordTermsDefaultProviderTime",
         base::TimeTicks::Now() - start_time);
   }
   const TemplateURL* keyword_url = providers_.GetKeywordProviderURL();
   if (keyword_url) {
     url_db->GetMostRecentKeywordSearchTerms(keyword_url->id(),
-        keyword_input_.text(), num_matches, &keyword_history_results_);
+                                            keyword_input_.text(),
+                                            num_matches,
+                                            &raw_keyword_history_results_);
   }
 }
 
 void SearchProvider::StartOrStopSuggestQuery(bool minimal_changes) {
   if (!IsQuerySuitableForSuggest()) {
     StopSuggest();
     ClearAllResults();
     return;
   }
 
@@ skipping 271 matching lines @@
             trimmed_verbatim, AutocompleteMatchType::SEARCH_OTHER_ENGINE,
             trimmed_verbatim, base::string16(), base::string16(),
             base::string16(), base::string16(), std::string(), std::string(),
             true, keyword_verbatim_relevance, keyword_relevance_from_server,
             false, trimmed_verbatim);
         AddMatchToMap(verbatim, std::string(),
                       did_not_accept_keyword_suggestion, false, true, &map);
       }
     }
   }
-  AddHistoryResultsToMap(keyword_history_results_, true,
-                         did_not_accept_keyword_suggestion, &map);
-  AddHistoryResultsToMap(default_history_results_, false,
-                         did_not_accept_default_suggestion, &map);
+  AddRawHistoryResultsToMap(raw_keyword_history_results_,
+                            true,
+                            did_not_accept_keyword_suggestion,
+                            &map);
+  AddRawHistoryResultsToMap(raw_default_history_results_,
+                            false,
+                            did_not_accept_default_suggestion,
+                            &map);
 
   AddSuggestResultsToMap(keyword_results_.suggest_results,
                          keyword_results_.metadata, &map);
   AddSuggestResultsToMap(default_results_.suggest_results,
                          default_results_.metadata, &map);
 
   ACMatches matches;
   for (MatchMap::const_iterator i(map.begin()); i != map.end(); ++i)
     matches.push_back(i->second);
 
@@ skipping 69 matching lines @@
   for (SearchSuggestionParser::NavigationResults::const_iterator it =
            navigation_results.begin(); it != navigation_results.end(); ++it) {
     matches->push_back(NavigationToMatch(*it));
     // In the absence of suggested relevance scores, use only the single
     // highest-scoring result.  (The results are already sorted by relevance.)
     if (!it->relevance_from_server())
       return;
   }
 }
 
-void SearchProvider::AddHistoryResultsToMap(const HistoryResults& results,
-                                            bool is_keyword,
-                                            int did_not_accept_suggestion,
-                                            MatchMap* map) {
-  if (results.empty())
+void SearchProvider::AddRawHistoryResultsToMap(
+    const HistoryResults& raw_results,
+    bool is_keyword,
+    int did_not_accept_suggestion,
+    MatchMap* map) {
+  if (raw_results.empty())
     return;
 
   base::TimeTicks start_time(base::TimeTicks::Now());
-  bool prevent_inline_autocomplete = input_.prevent_inline_autocomplete() ||
-      (input_.type() == metrics::OmniboxInputType::URL);
-  const base::string16& input_text =
-      is_keyword ? keyword_input_.text() : input_.text();
-  bool input_multiple_words = HasMultipleWords(input_text);
 
-  SearchSuggestionParser::SuggestResults scored_results;
-  if (!prevent_inline_autocomplete && input_multiple_words) {
-    // ScoreHistoryResults() allows autocompletion of multi-word, 1-visit
-    // queries if the input also has multiple words.  But if we were already
-    // scoring a multi-word, multi-visit query aggressively, and the current
-    // input is still a prefix of it, then changing the suggestion suddenly
-    // feels wrong.  To detect this case, first score as if only one word has
-    // been typed, then check if the best result came from aggressive search
-    // history scoring.  If it did, then just keep that score set.  This
-    // 1200 the lowest possible score in CalculateRelevanceForHistory()'s
-    // aggressive-scoring curve.
-    scored_results = ScoreHistoryResults(results, prevent_inline_autocomplete,
-                                         false, input_text, is_keyword);
-    if ((scored_results.front().relevance() < 1200) ||
-        !HasMultipleWords(scored_results.front().suggestion()))
-      scored_results.clear();  // Didn't detect the case above, score normally.
+  // Until Answers becomes default, scoring of history results will still happen
+  // here for non-Answers Chrome, to prevent scoring regressions from moving the

[Mark P, 2014/09/16 22:24:55]

nit: ^scoring^performance
(I think)

If you are worried about scoring regressions, please tell me why.  (I don't
think there could be any.)

[groby-ooo-7-16, 2014/09/18 23:27:24]

performance regressions, indeed. 

+  // scoring code before the suggest request is sent.
+  // For users with Answers enabled, the history results have already been
+  // scored earlier, right after calling DoHistoryQuery().
+  SearchSuggestionParser::SuggestResults local_transformed_results;
+  const SearchSuggestionParser::SuggestResults* transformed_results = NULL;
+  if (!OmniboxFieldTrial::EnableAnswersInSuggest()) {
+    ScoreHistoryResultsMultiWord(
+        raw_results, is_keyword, &local_transformed_results);
+    transformed_results = &local_transformed_results;
+  } else {
+    // TODO(groby): Once answers is on by default, just pass in scored results.
+    transformed_results = is_keyword ? &transformed_keyword_history_results_

[Mark P, 2014/09/16 22:24:55]

I'd prefer if you could add somewhere (possibly here) that, before using
transformed_history_results, munges them to revise whether suggestions are
available at this instant.  Otherwise when answers is enabled all our history
results will be annotated with "no suggestions available", even if suggestions
are available.

[groby-ooo-7-16, 2014/09/18 23:27:24]

I'm not sure I understand why results would be annotated improperly. Unless I
misunderstand, the annotation should ultimately be stored stored in the
AutoCompleteMatch's |search_term_args.accepted_suggestion|, correct?

Here is what happens when Answers is enabled:

1) Right after DoHistoryQuery, transformed_..._results is populated. As far as I
can tell, "suggestion available" doesn't matter when populating SuggestResults

2) This function - AddRawHistoryResultsToMap() - is called from
ConvertResultsToAutocompleteMatches. |did_not_accept_suggestions| is passed in
from there, and is computed there:
 int did_not_accept_default_suggestion =
      default_results_.suggest_results.empty() ?
      TemplateURLRef::NO_SUGGESTIONS_AVAILABLE :
      TemplateURLRef::NO_SUGGESTION_CHOSEN;

3) AddTransformedHistoryResultsToMap() calls AddMatchToMap(), passing through
|did_not_accept_suggestion|.

4) AddMatchToMap converts the SuggestResult into an AutocompleteMatch, once more
passing through |did_not_accept_suggestion|, using it to set
|match.search_terms_args->accepted_suggestion|

[Mark P, 2014/09/19 18:45:36]

Thank you for the explanation.  I see my worry was incorrect.  I didn't notice
that we pass through a newly-computed |did_not_accept_suggestion| when we add
the transformed results to the map.  (I was thinking that we used the same value
we used when we computed the transformed results in the first place.)

+                                     : &transformed_default_history_results_;
   }
-  if (scored_results.empty())
-    scored_results = ScoreHistoryResults(results, prevent_inline_autocomplete,
-                                         input_multiple_words, input_text,
-                                         is_keyword);
-  for (SearchSuggestionParser::SuggestResults::const_iterator i(
-           scored_results.begin()); i != scored_results.end(); ++i) {
-    AddMatchToMap(*i, std::string(), did_not_accept_suggestion, true,
-                  providers_.GetKeywordProviderURL() != NULL, map);
-  }
+  DCHECK(transformed_results);
+  AddTransformedHistoryResultsToMap(
+      *transformed_results, did_not_accept_suggestion, map);
   UMA_HISTOGRAM_TIMES("Omnibox.SearchProvider.AddHistoryResultsTime",
                       base::TimeTicks::Now() - start_time);
 }
 
+void SearchProvider::AddTransformedHistoryResultsToMap(
+    const SearchSuggestionParser::SuggestResults& transformed_results,
+    int did_not_accept_suggestion,
+    MatchMap* map) {
+  for (SearchSuggestionParser::SuggestResults::const_iterator i(
+           transformed_results.begin());
+       i != transformed_results.end();
+       ++i) {
+    AddMatchToMap(*i, std::string(), did_not_accept_suggestion, true,
+                  providers_.GetKeywordProviderURL() != NULL, map);
+  }
+}
+
 SearchSuggestionParser::SuggestResults SearchProvider::ScoreHistoryResults(
     const HistoryResults& results,
     bool base_prevent_inline_autocomplete,
     bool input_multiple_words,
     const base::string16& input_text,
     bool is_keyword) {
   SearchSuggestionParser::SuggestResults scored_results;
   // True if the user has asked this exact query previously.
   bool found_what_you_typed_match = false;
   const bool prevent_search_history_inlining =
@@ skipping 91 matching lines @@
   for (SearchSuggestionParser::SuggestResults::iterator i(
            scored_results.begin()); i != scored_results.end(); ++i) {
     if ((last_relevance != 0) && (i->relevance() >= last_relevance))
       i->set_relevance(last_relevance - 1);
     last_relevance = i->relevance();
   }
 
   return scored_results;
 }
 
+void SearchProvider::ScoreHistoryResultsMultiWord(
+    const HistoryResults& results,
+    bool is_keyword,
+    SearchSuggestionParser::SuggestResults* scored_results) {
+  DCHECK(scored_results);
+  if (results.empty()) {
+    scored_results->clear();
+    return;
+  }
+
+  bool prevent_inline_autocomplete = input_.prevent_inline_autocomplete() ||
+      (input_.type() == metrics::OmniboxInputType::URL);
+  const base::string16& input_text = GetInput(is_keyword).text();
+  bool input_multiple_words = HasMultipleWords(input_text);
+
+  if (!prevent_inline_autocomplete && input_multiple_words) {
+    // ScoreHistoryResults() allows autocompletion of multi-word, 1-visit
+    // queries if the input also has multiple words.  But if we were already
+    // scoring a multi-word, multi-visit query aggressively, and the current
+    // input is still a prefix of it, then changing the suggestion suddenly
+    // feels wrong.  To detect this case, first score as if only one word has
+    // been typed, then check if the best result came from aggressive search
+    // history scoring.  If it did, then just keep that score set.  This
+    // 1200 the lowest possible score in CalculateRelevanceForHistory()'s
+    // aggressive-scoring curve.
+    *scored_results = ScoreHistoryResults(
+        results, prevent_inline_autocomplete, false, input_text, is_keyword);
+    if ((scored_results->front().relevance() < 1200) ||
+        !HasMultipleWords(scored_results->front().suggestion()))
+      scored_results->clear();  // Didn't detect the case above, score normally.
+  }
+  if (scored_results->empty()) {
+    *scored_results = ScoreHistoryResults(results,
+                                          prevent_inline_autocomplete,
+                                          input_multiple_words,
+                                          input_text,
+                                          is_keyword);
+  }
+}
+
 void SearchProvider::AddSuggestResultsToMap(
     const SearchSuggestionParser::SuggestResults& results,
     const std::string& metadata,
     MatchMap* map) {
   for (size_t i = 0; i < results.size(); ++i) {
     AddMatchToMap(results[i], metadata, i, false,
                   providers_.GetKeywordProviderURL() != NULL, map);
   }
 }
 
@@ skipping 221 matching lines @@
   if (match->answer_contents.empty() && result.size() > 1)
     ++match;
   if (match->answer_contents.empty() || match->answer_type.empty() ||
       match->fill_into_edit.empty())
     return;
 
   // Valid answer encountered, cache it for further queries.
   answers_cache_.UpdateRecentAnswers(match->fill_into_edit, match->answer_type);
 }
 
-void SearchProvider::DoAnswersQuery(const AutocompleteInput& input) {
-  prefetch_data_ = answers_cache_.GetTopAnswerEntry(input.text());
+AnswersQueryData SearchProvider::FindAnswersPrefetchData(
+    const base::string16& input) {
+  // Retrieve the top entry from scored history results.
+
+  // TODO(groby): This seems a bit of a waste. Figure out if we can either
+  // re-use a partially populated map, or if we should just scan through the
+  // results, duplicates be damned.

[Mark P, 2014/09/16 22:24:55]

Given that you've discarded this idea (fine by me), perhaps you want to remove
the TODO?

[groby-ooo-7-16, 2014/09/18 23:27:24]

Done.

+  MatchMap map;
+  // TODO(groby): Can I just default to marking suggestions as "not accepted"?
+  // This seems to only matter for AQS....

[Mark P, 2014/09/16 22:24:55]

I'm comfortable with this here.  As I understand it, we only use the results of
this AddTransformedHistoryResultsToMap() call to calculate prefetch_data, and we
never look at the suggestions_available field in prefetch_data.  Please correct
me if I'm wrong.

[groby-ooo-7-16, 2014/09/18 23:27:24]

Correct, so removed comment.

+  AddTransformedHistoryResultsToMap(transformed_keyword_history_results_,
+                                    TemplateURLRef::NO_SUGGESTIONS_AVAILABLE,
+                                    &map);
+  AddTransformedHistoryResultsToMap(transformed_default_history_results_,
+                                    TemplateURLRef::NO_SUGGESTIONS_AVAILABLE,
+                                    &map);
+
+  ACMatches matches;
+  for (MatchMap::const_iterator i(map.begin()); i != map.end(); ++i)
+    matches.push_back(i->second);
+  std::sort(matches.begin(), matches.end(), &AutocompleteMatch::MoreRelevant);
+
+  LOG(ERROR) << "SIZE:" << matches.size();

[Mark P, 2014/09/16 22:24:55]

nit: remove unneeded LOG lines

[groby-ooo-7-16, 2014/09/18 23:27:24]

Done.

+
+  // If there is a top scoring entry, find the corresponding answer.
+  if (!matches.empty() &&
+      StartsWith(matches[0].contents, input, false /* case-insensitive */)) {

[Mark P, 2014/09/16 22:24:55]

Minor question: why this is StartsWith necessary? What's the harm if it's
missing?

[groby-ooo-7-16, 2014/09/18 23:27:24]

By missing you mean leaving the test out completely? As long as history results
are always prefix-matching on input, nothing. Probably just paranoia on my part,
so taking it out.

[Mark P, 2014/09/19 18:45:36]

Yes, search history matches should always be prefix matches.

+    return answers_cache_.GetTopAnswerEntry(matches[0].contents);
+  }
+  return AnswersQueryData();
 }