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 63 matching lines @@
         WordStartsMapEntry;
 
 // Algorithm Functions ---------------------------------------------------------
 
 // Comparison function for sorting search terms by descending length.
 bool LengthGreater(const base::string16& string_a,
                    const base::string16& string_b) {
   return string_a.length() > string_b.length();
 }
 
-

[Mark P, 2016/09/13 03:58:19]

style comment: if you're trying to change the style by using only one blank line
before section breaks in this file, you should do so consistently.  Instead,
used one here but two in the section (// URLIndexPrivateData) immediately below.

[Lavar Askew, 2016/09/13 18:14:30]

Done.

 // UpdateRecentVisitsFromHistoryDBTask -----------------------------------------
 
 // HistoryDBTask used to update the recent visit data for a particular
 // row from the history database.
 class UpdateRecentVisitsFromHistoryDBTask : public history::HistoryDBTask {
  public:
   explicit UpdateRecentVisitsFromHistoryDBTask(
       URLIndexPrivateData* private_data,
       history::URLID url_id);
 
@@ skipping 56 matching lines @@
       post_filter_item_count_(0),
       post_scoring_item_count_(0) {
 }
 
 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.
-  if ((cursor_position != base::string16::npos) &&
-      (cursor_position < search_string.length()) &&
-      (cursor_position > 0)) {
-    search_string.insert(cursor_position, base::ASCIIToUTF16(" "));
-  }
   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 =
       net::UnescapeURLComponent(lower_raw_string,
           net::UnescapeRule::SPACES | net::UnescapeRule::PATH_SEPARATORS |
           net::UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS);
+

[Mark P, 2016/09/13 03:58:19]

The lines above naturally go with the lines below.  I wouldn't add a blank line
here.

[Lavar Askew, 2016/09/13 18:14:30]

Done.

   // Extract individual 'words' (as opposed to 'terms'; see below) from the
   // search string. When the user types "colspec=ID%20Mstone Release" we get
   // four 'words': "colspec", "id", "mstone" and "release".
   String16Vector lower_words(
       String16VectorFromString16(lower_unescaped_string, false, nullptr));
   ScoredHistoryMatches scored_items;
 
   // Do nothing if we have indexed no words (probably because we've not been
   // initialized yet) or the search string has no words.
   if (word_list_.empty() || lower_words.empty()) {
@@ skipping 42 matching lines @@
   // "sort=pri&colspec=ID%20Mstone%20Release" we want to make sure that that
   // specific substring appears in the URL or page title.
 
   // We call these 'terms' (as opposed to 'words'; see above) as in this case
   // we only want to break up the search string on 'true' whitespace rather than
   // escaped whitespace. When the user types "colspec=ID%20Mstone Release" we
   // get two 'terms': "colspec=id%20mstone" and "release".
   String16Vector lower_raw_terms = base::SplitString(
       lower_raw_string, base::kWhitespaceUTF16, base::KEEP_WHITESPACE,
       base::SPLIT_WANT_NONEMPTY);
+

[Mark P, 2016/09/13 03:58:19]

ditto

[Lavar Askew, 2016/09/13 18:14:30]

Done.

   if (lower_raw_terms.empty()) {
     // Don't score matches when there are no terms to score against.  (It's
     // possible that the word break iterater that extracts words to search
     // for in the database allows some whitespace "words" whereas SplitString
     // excludes a long list of whitespace.)  One could write a scoring
     // function that gives a reasonable order to matches when there
     // are no terms (i.e., all the words are some form of whitespace),
     // but this is such a rare edge case that it's not worth the time.
     return scored_items;
   }
+
   scored_items =
       std::for_each(
           history_id_set.begin(), history_id_set.end(),
           AddHistoryMatch(bookmark_model, template_url_service, *this,
-                          lower_raw_string, lower_raw_terms,
-                          base::Time::Now())).ScoredMatches();
+                          lower_raw_string, lower_raw_terms, base::Time::Now()))
+          .ScoredMatches();
 
+  // 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.

[Mark P, 2016/09/13 03:58:19]

Please make this comment normal paragraph style, i.e., wrap closer to 80
characters instead of its current ragged state.

[Lavar Askew, 2016/09/13 18:14:30]

Done.

+  // This phenomena will be referred to as space ghosting.

[Mark P, 2016/09/13 03:58:19]

Don't introduce a term ("space ghosting") only to use it once.  I think you can
remove this sentence and simply revise the last sentence (the single place you
use the term).

[Lavar Askew, 2016/09/13 18:14:30]

Done.

+  // Going forward we perform a similar process for obtaining
+  // scored items as above.  In fact all of the variable names are the same
+  // except that they are prepended with 'transformed_' reflecting that these
+  // variables are related to space ghosting.
+  if ((cursor_position != base::string16::npos) &&
+      (cursor_position < search_string.length()) && (cursor_position > 0)) {
+    // The original search_string broken at cursor position.
+    base::string16 transformed_search_string(search_string);
+    transformed_search_string.insert(cursor_position, base::ASCIIToUTF16(" "));
+
+    // The same as lower_raw_string and defined for the same reasons.
+    base::string16 transformed_lower_raw_string(
+        base::i18n::ToLower(transformed_search_string));
+    String16Vector transformed_lower_raw_terms =
+        base::SplitString(transformed_lower_raw_string, base::kWhitespaceUTF16,
+                          base::KEEP_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
+    if (!transformed_lower_raw_terms.empty()) {
+      base::string16 transormed_lower_unescaped_string =
+          net::UnescapeURLComponent(
+              transformed_lower_raw_string,
+              net::UnescapeRule::SPACES | net::UnescapeRule::PATH_SEPARATORS |
+                  net::UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS);
+      String16Vector transformed_lower_words(String16VectorFromString16(
+          transormed_lower_unescaped_string, false, nullptr));
+      HistoryIDSet transformed_history_id_set =
+          HistoryIDSetFromWords(transformed_lower_words);
+
+      if (history_id_set != transformed_history_id_set) {
+        pre_filter_item_count_ = history_id_set.size();
+        // Trim the candidate pool if it is large as above.
+        was_trimmed = (pre_filter_item_count_ > kItemsToScoreLimit);
+        if (was_trimmed) {
+          HistoryIDVector transformed_history_ids;
+          std::copy(transformed_history_id_set.begin(),
+                    transformed_history_id_set.end(),
+                    std::back_inserter(transformed_history_ids));
+          // Trim down the set by sorting by typed-count, visit-count, and last
+          // visit.
+          HistoryItemFactorGreater item_factor_functor(history_info_map_);
+          std::partial_sort(
+              transformed_history_ids.begin(),
+              transformed_history_ids.begin() + kItemsToScoreLimit,
+              transformed_history_ids.end(), item_factor_functor);
+          transformed_history_id_set.clear();
+          std::copy(transformed_history_ids.begin(),
+                    transformed_history_ids.begin() + kItemsToScoreLimit,
+                    std::inserter(transformed_history_id_set,
+                                  transformed_history_id_set.end()));
+        }
+        history_id_set.insert(transformed_history_id_set.begin(),
+                              transformed_history_id_set.end());
+        ScoredHistoryMatches transformed_scored_items =
+            std::for_each(
+                history_id_set.begin(), history_id_set.end(),
+                AddHistoryMatch(bookmark_model, template_url_service, *this,
+                                transformed_lower_raw_string,
+                                transformed_lower_raw_terms, base::Time::Now()))
+                .ScoredMatches();
+        scored_items.insert(scored_items.end(),
+                            transformed_scored_items.begin(),
+                            transformed_scored_items.end());
+      }
+    }
+  }
   // Select and sort only the top |max_matches| results.
   if (scored_items.size() > max_matches) {
-    std::partial_sort(scored_items.begin(),
-                      scored_items.begin() +
-                          max_matches,
+    std::partial_sort(scored_items.begin(), scored_items.begin() + max_matches,
                       scored_items.end(),
                       ScoredHistoryMatch::MatchScoreGreater);
-      scored_items.resize(max_matches);
+    scored_items.resize(max_matches);
   } else {
     std::sort(scored_items.begin(), scored_items.end(),
               ScoredHistoryMatch::MatchScoreGreater);
   }
   post_scoring_item_count_ = scored_items.size();
 
   if (was_trimmed) {
     search_term_cache_.clear();  // Invalidate the term cache.
   } else {
     // Remove any stale SearchTermCacheItems.
@@ skipping 966 matching lines @@
   return true;
 }
 
 // static
 bool URLIndexPrivateData::URLSchemeIsWhitelisted(
     const GURL& gurl,
     const std::set<std::string>& whitelist) {
   return whitelist.find(gurl.scheme()) != whitelist.end();
 }
 
-

[Mark P, 2016/09/13 03:58:19]

ditto style comment about single versus double blank lines.  Consistency.

 // SearchTermCacheItem ---------------------------------------------------------
 
 URLIndexPrivateData::SearchTermCacheItem::SearchTermCacheItem(
     const WordIDSet& word_id_set,
     const HistoryIDSet& history_id_set)
     : word_id_set_(word_id_set), history_id_set_(history_id_set), used_(true) {
 }
 
 URLIndexPrivateData::SearchTermCacheItem::SearchTermCacheItem() : used_(true) {
 }
@@ skipping 89 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());
 }