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

components/omnibox/browser/url_index_private_data.cc

Index: components/omnibox/browser/url_index_private_data.cc
diff --git a/components/omnibox/browser/url_index_private_data.cc b/components/omnibox/browser/url_index_private_data.cc
index 54123a335ca874ae527c5babe079cf242c08fc5e..e91bd928f580e15a638848c1d5e78df860706c58 100644
--- a/components/omnibox/browser/url_index_private_data.cc
+++ b/components/omnibox/browser/url_index_private_data.cc
@@ -161,10 +161,26 @@ ScoredHistoryMatches URLIndexPrivateData::HistoryItemsForTerms(
   // 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;
@@ -198,6 +214,30 @@ ScoredHistoryMatches URLIndexPrivateData::HistoryItemsForTerms(
 
   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