Generating autocomplete results with and without word breaks in the Omnibox.

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 143 matching lines @@
 
 ScoredHistoryMatches URLIndexPrivateData::HistoryItemsForTerms(
     base::string16 search_string,
     size_t cursor_position,
     size_t max_matches,
     bookmarks::BookmarkModel* bookmark_model,
     TemplateURLService* template_url_service) {
   // If cursor position is set and useful (not at either end of the
   // string), allow the search string to be broken at cursor position.
   // We do this by pretending there's a space where the cursor is.
+  // Furthermore, we also keep a copy of the original search string
+  // without the pretend space.  The rationale behind this is to
+  // build the History ID Set with both the search string with and
+  // without the pretend space.
+  base::string16 search_string_without_break;

[Peter Kasting, 2016/07/28 20:32:20]

It would be nice to avoid the need for this temp and keep all the added logic
here together.  One way might be: move the conditional that was here down to
where you added your other block, so we unconditionally search for the original
input string.  Then combine this block and the other new block, so that we check
if we should insert a space, and if so, go ahead and look in the history DB for
its words and merge in the results.

[Lavar Askew, 2016/08/10 15:24:03]

Done.

   if ((cursor_position != base::string16::npos) &&
       (cursor_position < search_string.length()) &&
       (cursor_position > 0)) {
-    search_string.insert(cursor_position, base::ASCIIToUTF16(" "));
+
+    base::string16 blank_space =
+      base::ASCIIToUTF16(" ");
+
+    if ((search_string.compare(cursor_position, 1, blank_space) != 0) ||
+        (search_string.compare(cursor_position - 1, 1, blank_space) != 0) ||

[Peter Kasting, 2016/07/28 20:32:20]

I suspect that rather than comparing against " " specifically, you want to check
if this is somewhere ICU would line-break, to match how
String16VectorFromString16() splits into words.

The dumbest way to do this would be to simply call String16VectorFromString16()
twice, once with the space and once without, and throw away one set if it's the
same as the other.  There may be a better way, though.  I would check with
mpearson and jshin.

[Lavar Askew, 2016/08/10 15:24:03]

Done.

I have removed this code after following your "copy-and-pastes" refactoring
suggestion below.

+        (search_string.compare(cursor_position + 1, 1, blank_space) != 0)) {

[Peter Kasting, 2016/07/28 20:32:20]

I think you don't want to compare cursor_position + 1.  That would be the next
character after whatever is to the right of the cursor.

[Lavar Askew, 2016/08/10 15:24:03]

Done.

I have removed this code.

+
+      search_string_without_break = search_string;
+
+      search_string.insert(cursor_position, blank_space);
+    }
   }
   pre_filter_item_count_ = 0;
   post_filter_item_count_ = 0;
   post_scoring_item_count_ = 0;
   // The search string we receive may contain escaped characters. For reducing
   // the index we need individual, lower-cased words, ignoring escapings. For
   // the final filtering we need whitespace separated substrings possibly
   // containing escaped characters.
   base::string16 lower_raw_string(base::i18n::ToLower(search_string));
   base::string16 lower_unescaped_string =
@@ skipping 13 matching lines @@
     search_term_cache_.clear();  // Invalidate the term cache.
     return scored_items;
   }
 
   // Reset used_ flags for search_term_cache_. We use a basic mark-and-sweep
   // approach.
   ResetSearchTermCache();
 
   HistoryIDSet history_id_set = HistoryIDSetFromWords(lower_words);
 
+  // Add to history_id_set the ids that are related to the original search
+  // string without the break.
+  if (!search_string_without_break.empty() &&
+      search_string.compare(search_string_without_break) != 0) {

[Peter Kasting, 2016/07/28 20:32:20]

It looks to me as if this copy-and-pastes some of the code above, and perhaps
the duplicated parts should be pulled out into a helper function instead.

[Lavar Askew, 2016/08/10 15:24:03]

Done.

+    base::string16 lower_raw_string_without_break(
+      base::i18n::ToLower(search_string_without_break));
+
+    base::string16 lower_unescaped_string_without_break =
+      net::UnescapeURLComponent(lower_raw_string_without_break,
+          net::UnescapeRule::SPACES | net::UnescapeRule::PATH_SEPARATORS |
+          net::UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS);
+
+    String16Vector lower_words_without_break(
+        String16VectorFromString16(
+          lower_unescaped_string_without_break, false, nullptr));
+
+    HistoryIDSet history_id_set_without_break =
+      HistoryIDSetFromWords(lower_words_without_break);
+
+    if (history_id_set_without_break.size() > 0) {
+      history_id_set.insert(history_id_set_without_break.begin(),
+        history_id_set_without_break.end());
+    }
+  }
   // Trim the candidate pool if it is large. Note that we do not filter out
   // items that do not contain the search terms as proper substrings -- doing
   // so is the performance-costly operation we are trying to avoid in order
   // to maintain omnibox responsiveness.
   const size_t kItemsToScoreLimit = 500;
   pre_filter_item_count_ = history_id_set.size();
   // If we trim the results set we do not want to cache the results for next
   // time as the user's ultimately desired result could easily be eliminated
   // in this early rough filter.
   bool was_trimmed = (pre_filter_item_count_ > kItemsToScoreLimit);
@@ skipping 1143 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());
 }