Omnibox: Don't Call Classify() Repeatedly

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/base64.h"
@@ skipping 873 matching lines @@
   UMA_HISTOGRAM_TIMES("Omnibox.SearchProvider.AddHistoryResultsTime",
                       base::TimeTicks::Now() - start_time);
 }
 
 SearchProvider::SuggestResults SearchProvider::ScoreHistoryResults(
     const HistoryResults& results,
     bool base_prevent_inline_autocomplete,
     bool input_multiple_words,
     const base::string16& input_text,
     bool is_keyword) {
-  AutocompleteClassifier* classifier =
-      AutocompleteClassifierFactory::GetForProfile(profile_);
   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 =
       OmniboxFieldTrial::SearchHistoryPreventInlining(
           input_.current_page_classification());
   const base::string16& trimmed_input =
       base::CollapseWhitespace(input_text, false);
   for (HistoryResults::const_iterator i(results.begin()); i != results.end();
        ++i) {
     const base::string16& trimmed_suggestion =
         base::CollapseWhitespace(i->term, false);
 
     // Don't autocomplete multi-word queries that have only been seen once
     // unless the user has typed more than one word.
     bool prevent_inline_autocomplete = base_prevent_inline_autocomplete ||
         (!input_multiple_words && (i->visits < 2) &&
          HasMultipleWords(trimmed_suggestion));
 
-    // Don't autocomplete search terms that would normally be treated as URLs
-    // when typed. For example, if the user searched for "google.com" and types
-    // "goog", don't autocomplete to the search term "google.com". Otherwise,
-    // the input will look like a URL but act like a search, which is confusing.
-    // NOTE: We don't check this in the following cases:
-    //  * When inline autocomplete is disabled, we won't be inline
-    //    autocompleting this term, so we don't need to worry about confusion as
-    //    much.  This also prevents calling Classify() again from inside the
-    //    classifier (which will corrupt state and likely crash), since the
-    //    classifier always disables inline autocomplete.
-    //  * When the user has typed the whole term, the "what you typed" history
-    //    match will outrank us for URL-like inputs anyway, so we need not do
-    //    anything special.
-    if (!prevent_inline_autocomplete && classifier &&
-        (trimmed_suggestion != trimmed_input)) {
-      AutocompleteMatch match;
-      classifier->Classify(trimmed_suggestion, false, false,
-                           input_.current_page_classification(), &match, NULL);
-      prevent_inline_autocomplete =
-          !AutocompleteMatch::IsSearchType(match.type);
-    }
-
     int relevance = CalculateRelevanceForHistory(
         i->time, is_keyword, !prevent_inline_autocomplete,
         prevent_search_history_inlining);
     // Add the match to |scored_results| by putting the what-you-typed match
     // on the front and appending all other matches.  We want the what-you-
     // typed match to always be first.
     SuggestResults::iterator insertion_position = scored_results.end();
     if (trimmed_suggestion == trimmed_input) {
       found_what_you_typed_match = true;
       insertion_position = scored_results.begin();
     }
     scored_results.insert(insertion_position, SuggestResult(
         trimmed_suggestion, AutocompleteMatchType::SEARCH_HISTORY,
         trimmed_suggestion, base::string16(), base::string16(),
         base::string16(), base::string16(), std::string(), std::string(),
         is_keyword, relevance, false, false, trimmed_input));
   }
 
   // History returns results sorted for us.  However, we may have docked some
   // results' scores, so things are no longer in order.  While keeping the
   // what-you-typed match at the front (if it exists), 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() +
                        (found_what_you_typed_match ? 1 : 0),
                    scored_results.end(),
                    CompareScoredResults());
+
+  // Don't autocomplete to search terms that would normally be treated as URLs
+  // when typed. For example, if the user searched for "google.com" and types
+  // "goog", don't autocomplete to the search term "google.com". Otherwise,
+  // the input will look like a URL but act like a search, which is confusing.
+  // The 1200 relevance score threshold in the test below is the lowest

[Mark P, 2014/07/23 23:52:39]

I added stuff here to the comment.

+  // possible score in CalculateRelevanceForHistory()'s aggressive-scoring
+  // curve.  This is an appropriate threshold to use to decide if we're overly
+  // aggressively inlining because, if we decide the answer is yes, the
+  // way we resolve it it to not use the aggressive-scoring curve.
+  // NOTE: We don't check for autocompleting to URLs in the following cases:
+  //  * When inline autocomplete is disabled, we won't be inline autocompleting
+  //    this term, so we don't need to worry about confusion as much.  This
+  //    also prevents calling Classify() again from inside the classifier
+  //    (which will corrupt state and likely crash), since the classifier
+  //    always disables inline autocomplete.
+  //  * When the user has typed the whole string before as a query, then it's

[Mark P, 2014/07/23 23:52:39]

I rewrote (re-justified) this second bullet point in the comment.

+  //    likely the user has no expectation that term should be interpreted as
+  //    as a URL, so we need not do anything special to preserve user
+  //    expectation.
+  AutocompleteClassifier* classifier =
+      AutocompleteClassifierFactory::GetForProfile(profile_);
   int last_relevance = 0;
+  if (!base_prevent_inline_autocomplete && !found_what_you_typed_match &&
+      classifier && (scored_results.front().relevance() >= 1200)) {
+    AutocompleteMatch match;
+    classifier->Classify(scored_results.front().suggestion(), false, false,
+                         input_.current_page_classification(), &match, NULL);
+    // Demote this match that would normally be interpreted as a URL to have
+    // the highest score a previously-issued search query could have when
+    // scoring with the non-aggressive method.  A consequence of demoting
+    // by revising |last_relevance| is that this match and all following
+    // matches get demoted; the relative order of matches is preserved.
+    // One could imagine demoting only those matches that might cause
+    // confusion (which, by the way, might change the relative order of
+    // matches.  We have decided to go with the simple demote-all approach
+    // because selective demotion requires multiple Classify() calls and
+    // such calls can be expensive (as expensive as running the whole
+    // autocomplete system).
+    if (!AutocompleteMatch::IsSearchType(match.type)) {
+      last_relevance = CalculateRelevanceForHistory(
+          base::Time::Now(), is_keyword, false,
+          prevent_search_history_inlining);
+    }
+  }
+
   for (SuggestResults::iterator i(scored_results.begin());
        i != scored_results.end(); ++i) {
-    if ((i != scored_results.begin()) && (i->relevance() >= last_relevance))
+    if ((last_relevance != 0) && (i->relevance() >= last_relevance))
       i->set_relevance(last_relevance - 1);
     last_relevance = i->relevance();
   }
 
   return scored_results;
 }
 
 void SearchProvider::AddSuggestResultsToMap(const SuggestResults& results,
                                             const std::string& metadata,
                                             MatchMap* map) {
@@ skipping 202 matching lines @@
     // Make the base64 encoded value URL and filename safe(see RFC 3548).
     std::replace(current_token_.begin(), current_token_.end(), '+', '-');
     std::replace(current_token_.begin(), current_token_.end(), '/', '_');
   }
 
   // Extend expiration time another 60 seconds.
   token_expiration_time_ = current_time + base::TimeDelta::FromSeconds(60);
 
   return current_token_;
 }