Omnibox - Warm Up PSuggest on Focus

components/omnibox/browser/search_provider.cc

Index: components/omnibox/browser/search_provider.cc
diff --git a/components/omnibox/browser/search_provider.cc b/components/omnibox/browser/search_provider.cc
index e01d3b350d4f4767b31b813692ff49f02eb262c0..830f160e735d13350bf633ac36a4452d4c2bcf93 100644
--- a/components/omnibox/browser/search_provider.cc
+++ b/components/omnibox/browser/search_provider.cc
@@ -12,6 +12,7 @@
 #include "base/base64.h"
 #include "base/bind.h"
 #include "base/callback.h"
+#include "base/feature_list.h"
 #include "base/i18n/break_iterator.h"
 #include "base/i18n/case_conversion.h"
 #include "base/json/json_string_value_serializer.h"
@@ -222,9 +223,11 @@ void SearchProvider::Start(const AutocompleteInput& input,
   matches_.clear();
   set_field_trial_triggered(false);
 
-  // Can't return search/suggest results for bogus input.
-  if (input.from_omnibox_focus() ||
-      input.type() == metrics::OmniboxInputType::INVALID) {
+  // Unless warming up the suggest server on focus, SearchProvider doesn't do
+  // do anything useful for on-focus inputs or empty inputs.  Exit early.
+  if (!base::FeatureList::IsEnabled(omnibox::kSearchProviderWarmUpOnFocus) &&
+      (input.from_omnibox_focus() ||
+       input.type() == metrics::OmniboxInputType::INVALID)) {
     Stop(true, false);
     return;
   }
@@ -267,7 +270,15 @@ void SearchProvider::Start(const AutocompleteInput& input,
 
   providers_.set(default_provider_keyword, keyword_provider_keyword);
 
-  if (input.text().empty()) {
+  if (input.from_omnibox_focus()) {
+    // Don't display any suggestions for on-focus requests.  Stop any pending
+    // requests here (there likely aren't yet, though it doesn't hurt to be safe

[Peter Kasting, 2017/03/02 22:50:42]

Nit: Missing paren

[Mark P, 2017/03/03 22:12:24]

Now moot.

+    // so that we can create a new request later in this flow (to warm-up the
+    // suggest server by alerting it that the user is likely about to start
+    // typing).
+    StopSuggest();
+    ClearAllResults();
+  } else if (input.text().empty()) {
     // User typed "?" alone.  Give them a placeholder result indicating what
     // this syntax does.
     if (default_provider) {
@@ -286,21 +297,23 @@ void SearchProvider::Start(const AutocompleteInput& input,
 
   input_ = input;
 
-  DoHistoryQuery(minimal_changes);
-  // 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.
-  ScoreHistoryResults(raw_default_history_results_,
-                      false,
-                      &transformed_default_history_results_);
-  ScoreHistoryResults(raw_keyword_history_results_,
-                      true,
-                      &transformed_keyword_history_results_);
-  prefetch_data_ = FindAnswersPrefetchData();
-
-  // Raw results are not needed any more.
-  raw_default_history_results_.clear();
-  raw_keyword_history_results_.clear();
+  // Don't search the query history database for on-focus inputs; these inputs
+  // should only be used to warm up the suggest server.
+  if (!input.from_omnibox_focus()) {
+    DoHistoryQuery(minimal_changes);
+    // 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.
+    ScoreHistoryResults(raw_default_history_results_, false,
+                        &transformed_default_history_results_);
+    ScoreHistoryResults(raw_keyword_history_results_, true,
+                        &transformed_keyword_history_results_);
+    prefetch_data_ = FindAnswersPrefetchData();
+
+    // Raw results are not needed any more.
+    raw_default_history_results_.clear();
+    raw_keyword_history_results_.clear();
+  }
 
   StartOrStopSuggestQuery(minimal_changes);
   UpdateMatches();
@@ -394,7 +407,12 @@ void SearchProvider::OnURLFetchComplete(const net::URLFetcher* source) {
   LogFetchComplete(request_succeeded, is_keyword);
 
   bool results_updated = false;
-  if (request_succeeded) {
+  // Ignore (i.e., don't display) any suggestions for on-focus inputs.
+  // SearchProvider is not intended to give suggestions on on-focus inputs;
+  // that's left to ZeroSuggestProvider and friends.  Furthermore, it's not
+  // clear if the suggest server will send back sensible results to the
+  // request we're constructing here for on-focus inputs.
+  if (!input_.from_omnibox_focus() && request_succeeded) {
     std::unique_ptr<base::Value> data(
         SearchSuggestionParser::DeserializeJsonData(
             SearchSuggestionParser::ExtractJsonData(source)));
@@ -495,11 +513,25 @@ void SearchProvider::LogFetchComplete(bool success, bool is_keyword) {
 }
 
 void SearchProvider::UpdateMatches() {
-  PersistTopSuggestions(&default_results_);
-  PersistTopSuggestions(&keyword_results_);
-  ConvertResultsToAutocompleteMatches();
+  // On-focus inputs display no suggestions, so we do not need to persist the
+  // previous top suggestions, add new suggestions, or revise suggestions to
+  // enforce constraints about inlineability in this case.  Indeed, most of
+  // these steps would be bad, as they'd add a suggestion of some form, thus
+  // opening the dropdown (which we do not want to happen).
+  if (!input_.from_omnibox_focus()) {
+    PersistTopSuggestions(&default_results_);
+    PersistTopSuggestions(&keyword_results_);
+    ConvertResultsToAutocompleteMatches();
+    EnforceConstraints();
+    UMA_HISTOGRAM_CUSTOM_COUNTS("Omnibox.SearchProviderMatches",
+                                matches_.size(), 1, 6, 7);
+    RecordTopSuggestion();
+  }
 
-  // Check constraints that may be violated by suggested relevances.
+  UpdateDone();
+}
+
+void SearchProvider::EnforceConstraints() {
   if (!matches_.empty() &&
       (default_results_.HasServerProvidedScores() ||
        keyword_results_.HasServerProvidedScores())) {
@@ -549,10 +581,9 @@ void SearchProvider::UpdateMatches() {
     DCHECK(is_extension_keyword ||
            (AutocompleteResult::FindTopMatch(&matches_) != matches_.end()));
   }
-  UMA_HISTOGRAM_CUSTOM_COUNTS(
-      "Omnibox.SearchProviderMatches", matches_.size(), 1, 6, 7);
+}
 
-  // Record the top suggestion (if any) for future use.
+void SearchProvider::RecordTopSuggestion() {
   top_query_suggestion_match_contents_ = base::string16();
   top_navigation_suggestion_ = GURL();
   ACMatches::const_iterator first_match =
@@ -567,8 +598,6 @@ void SearchProvider::UpdateMatches() {
     else
       top_navigation_suggestion_ = first_match->destination_url;
   }
-
-  UpdateDone();
 }
 
 void SearchProvider::Run(bool query_is_private) {
@@ -735,10 +764,10 @@ bool SearchProvider::IsQuerySuitableForSuggest(bool* query_is_private) const {
 }
 
 bool SearchProvider::IsQueryPotentionallyPrivate() const {
-  // If the input type might be a URL, we take extra care so that private data
-  // isn't sent to the server.
+  if (input_.text().empty())
+    return false;
 
-  // Next we check the scheme.  If this is UNKNOWN/URL with a scheme that isn't
+  // Check the scheme.  If this is UNKNOWN/URL with a scheme that isn't
   // http/https/ftp, we shouldn't send it.  Sending things like file: and data:
   // is both a waste of time and a disclosure of potentially private, local
   // data.  Other "schemes" may actually be usernames, and we don't want to send