Omnibox: Truncate HistoryContents Searches at 10k Characters

chrome/browser/autocomplete/history_contents_provider.cc

 // Copyright (c) 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/history_contents_provider.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/metrics/histogram.h"
 #include "base/string_util.h"
@@ skipping 13 matching lines @@
 
 using base::TimeTicks;
 using history::HistoryDatabase;
 
 namespace {
 
 // Number of days to search for full text results. The longer this is, the more
 // time it will take.
 const int kDaysToSearch = 30;
 
+// The maximum number of characters used in a search for full text results.
+// This was chosen arbitrarily because omnibox results that come back too
+// late (after tens of seconds) aren't useful to the user so it's not
+// worth spending CPU time on them.  Furthermore, if a URL matches the
+// first 2k of characters the user typed (more likely pasted) into the
+// omnibox, it's likely it matches the rest; there's strongly diminished
+// returns for the ability to add additional search terms.
+const size_t kMaxCharactersToConsider = 2000u;

[Peter Kasting, 2013/04/17 22:59:45]

Nit: Since this is only used in one function I suggest declaring it in the most
local scope there.  (This could be done with kDaysToSearch too.)

[Mark P, 2013/04/17 23:06:48]

I kind of like having parameters called out somewhere, like this, at the top of
a file.  Besides that function is kind of long already; this would add another
doezen lines to it.

If you insist I'll do it.

[Peter Kasting, 2013/04/17 23:09:01]

I don't care strongly.  I always read the style guide's "declare things in the
most local scope possible" as applying to constants too, so that's the source of
the suggestion.  Up to you whether you agree with that interpretation.

[Mark P, 2013/04/17 23:13:28]

Okay, done.

+
 }  // namespace
 
 HistoryContentsProvider::MatchReference::MatchReference(
     const history::URLResult* result,
     int relevance)
     : result(result),
       relevance(relevance) {
 }
 
 // static
@@ skipping 88 matching lines @@
     HistoryService* history =
         HistoryServiceFactory::GetForProfile(profile_,
                                              Profile::EXPLICIT_ACCESS);
     if (history) {
       done_ = false;
 
       history::QueryOptions options;
       options.body_only = body_only_;
       options.SetRecentDayRange(kDaysToSearch);
       options.max_count = kMaxMatches;
-      history->QueryHistory(input.text(), options,
+      history->QueryHistory(
+          input.text().substr(0, kMaxCharactersToConsider),
+          options,
           &request_consumer_,
           base::Bind(&HistoryContentsProvider::QueryComplete,
                      base::Unretained(this)));
     }
   }
 }
 
 void HistoryContentsProvider::Stop(bool clear_cached_results) {
   done_ = true;
   request_consumer_.CancelAllRequests();
@@ skipping 112 matching lines @@
 
 int HistoryContentsProvider::CalculateRelevance(
     const history::URLResult& result) {
   const bool in_title = MatchInTitle(result);
   BookmarkModel* bm_model = BookmarkModelFactory::GetForProfile(profile_);
   if (!bm_model || !bm_model->IsBookmarked(result.url()))
     return in_title ? (700 + title_count_++) : (500 + contents_count_++);
   return in_title ?
       (1000 + star_title_count_++) : (550 + star_contents_count_++);
 }