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

components/omnibox/browser/url_index_private_data.h

 // 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.
 
 #ifndef COMPONENTS_OMNIBOX_BROWSER_URL_INDEX_PRIVATE_DATA_H_
 #define COMPONENTS_OMNIBOX_BROWSER_URL_INDEX_PRIVATE_DATA_H_
 
 #include <stddef.h>
 
 #include <set>
@@ skipping 32 matching lines @@
 // this class is for exclusive use by the InMemoryURLIndex class there should
 // be no calls from any other class.
 //
 // All public member functions are called on the main thread unless otherwise
 // annotated.
 class URLIndexPrivateData
     : public base::RefCountedThreadSafe<URLIndexPrivateData> {
  public:
   URLIndexPrivateData();
 
+  bool TrimCandidatePool (

[Mark P, 2016/08/10 17:46:20]

All of these function should be private.  They're helper function that no one
external to this class needs to know about.

Also, when you move their declarations / make them private, remember to correct
the .cc.  Our style guide requires that the functions in the .cc are implemented
in the same order as they are declared in the header file.

You may to do this when the review is almost over; otherwise the "diff" between
you change and the original code will look large.

[Mark P, 2016/08/10 17:46:20]

This function and the one below should be commented.

[Mark P, 2016/08/10 17:46:20]

Here and throughout the change, you introduce a number of style violations. 
(Indeed, at times the only thing you did on some lines is to change the spacing
to a way that's not consistent with chromium style.)  Can you run "git cl
format" and use the changes it suggests for the parts of the code that you touch
or at least look over your code and change the spacing back?

[Lavar Askew, 2016/08/18 03:07:52]

Done.

[Lavar Askew, 2016/08/18 03:07:52]

"git cl format" did not make any suggestions for me. 

+      HistoryIDSet history_id_set);
+
+  ScoredHistoryMatches GetScoredItemsForSearchString(

[Mark P, 2016/08/10 17:46:19]

Most of this class calls this parameter a "term_string"; please be consistent
with naming the parameter and teh function.

[Lavar Askew, 2016/08/18 03:07:52]

search_string was the name of the variable before I started working on it.  It
made things clear to me in this method.  If you insist that I do that then I
will.

+      base::string16 search_string,
+      HistoryIDSet history_id_set,
+      size_t max_matches,
+      bookmarks::BookmarkModel* bookmark_model,
+      TemplateURLService* template_url_service);
+
+  // Given a base::string16 in |term_string|, scans the history index and
+  // returns the set of history item IDs.
+  HistoryIDSet HistoryItemsForWords(

[Mark P, 2016/08/10 17:46:19]

This function is confusingly named.  It sounds a like like
"HistoryItemsForTerms" but
(i) function HistoryItemsForTerms returns ScoredHistoryMatches; this returns
HistoryIDSets.  Having them both called HistoryItems... is confusing.
(ii) this function takes a term string; not words.  (Indeed, in the middle of
the function a comment explains the difference in terminology.  This function
should not be claiming to take "words".)  Having this function called "ForWords"
is misleading.

[Lavar Askew, 2016/08/18 03:07:52]

Done.

+      base::string16 search_string);
+
   // Given a base::string16 in |term_string|, scans the history index and
   // returns a vector with all scored, matching history items. The
   // |term_string| is broken down into individual terms (words), each of which
   // must occur in the candidate history item's URL or page title for the item
   // to qualify; however, the terms do not necessarily have to be adjacent. We
   // also allow breaking |term_string| at |cursor_position| (if
   // set). Once we have a set of candidates, they are filtered to ensure
   // that all |term_string| terms, as separated by whitespace and the
   // cursor (if set), occur within the candidate's URL or page title.
   // Scores are then calculated on no more than |kItemsToScoreLimit|
@@ skipping 324 matching lines @@
   int saved_cache_version_;
 
   // Used for unit testing only. Records the number of candidate history items
   // at three stages in the index searching process.
   size_t pre_filter_item_count_;    // After word index is queried.
   size_t post_filter_item_count_;   // After trimming large result set.
   size_t post_scoring_item_count_;  // After performing final filter/scoring.
 };
 
 #endif  // COMPONENTS_OMNIBOX_BROWSER_URL_INDEX_PRIVATE_DATA_H_