Omnibox - HistoryQuick Provider - Don't Run Duplicate Searches

components/omnibox/browser/url_index_private_data.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 "components/omnibox/browser/url_index_private_data.h"
 
 #include <stdint.h>
 
 #include <functional>
 #include <iterator>
@@ skipping 135 matching lines @@
     size_t cursor_position,
     size_t max_matches,
     bookmarks::BookmarkModel* bookmark_model,
     TemplateURLService* template_url_service) {
   // This list will contain the original search string and any other string
   // transformations.
   String16Vector search_strings;
   search_strings.push_back(original_search_string);
   if ((cursor_position != base::string16::npos) &&
       (cursor_position < original_search_string.length()) &&
-      (cursor_position > 0)) {
+      (cursor_position > 0) &&
+      (original_search_string[cursor_position - 1] != L' ') &&
+      (original_search_string[cursor_position] != L' ')) {

[Peter Kasting, 2017/04/26 01:43:02]

What about other kinds of whitespace/word breaks?  Will the tokenizer only
tokenize the search string on spaces, or will it use ICU to word break?  It
seems like we should match that, ideally by calling the same tokenizer and
checking if there's a break point here (maybe we can avoid duplicate effort by
then passing the tokenized search string).

Nit: I think L' ' is basically correct on Windows but wrong on other platforms,
since L means "wide character literal" and string16 is not a wide string on all
platforms.  I would just use ' ' (the narrow value should always upconvert
correctly).  I don't think this can make any correctness difference anywhere.

[Mark P, 2017/04/26 22:24:44]

It uses ICU via the call to String16VectorFromString16() below.
We could check whether the vector returned is identical to a previous vector
returned.  That seemed like more code and complexity than is worth it.  (It
would mean keep a map of vectors of previous search words.)  I can do that if
you prefer.

     // The original search_string broken at cursor position. This is one type of
-    // transformation.
+    // transformation.  There is no need to break the cursor position at a space
+    // if there is already a space adjacent to the cursor.
     base::string16 transformed_search_string(original_search_string);
     transformed_search_string.insert(cursor_position, base::ASCIIToUTF16(" "));
     search_strings.push_back(transformed_search_string);
   }
 
   ScoredHistoryMatches scored_items;
   // Invalidate the term cache and return if we have indexed no words (probably
   // because we've not been initialized yet).
   if (word_list_.empty()) {
     search_term_cache_.clear();
@@ skipping 1097 matching lines @@
   // First cut: typed count, visit count, recency.
   // TODO(mrossetti): This is too simplistic. Consider an approach which ranks
   // recently visited (within the last 12/24 hours) as highly important. Get
   // input from mpearson.
   if (r1.typed_count() != r2.typed_count())
     return (r1.typed_count() > r2.typed_count());
   if (r1.visit_count() != r2.visit_count())
     return (r1.visit_count() > r2.visit_count());
   return (r1.last_visit() > r2.last_visit());
 }