Omnibox: Fix HistoryQuick provider can_inline bug

chrome/browser/history/scored_history_match.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 "chrome/browser/history/scored_history_match.h"
 
 #include <algorithm>
 #include <functional>
 #include <iterator>
 #include <numeric>
@@ skipping 79 matching lines @@
   // TODO(mpearson): Investigate whether this has any meaningful
   // effect on scoring.  (It's necessary at some point: removing
   // overlaps and sorting is needed to decide what to highlight in the
   // suggestion string.  But this sort and de-overlap doesn't have to
   // be done before scoring.)
   url_matches = SortAndDeoverlapMatches(url_matches);
   title_matches = SortAndDeoverlapMatches(title_matches);
 
   // We can inline autocomplete a result if:
   //  1) there is only one search term
-  //  2) AND EITHER:
-  //    2a) the first match starts at the beginning of the candidate URL, OR
-  //    2b) the candidate URL starts with one of the standard URL prefixes with
-  //        the URL match immediately following that prefix.
+  //  2) AND a match begins immediately after a implicit / inlineable

[Peter Kasting, 2012/09/11 21:57:48]

Nit: a -> an, but maybe this would be clearer as "AND the match begins
immediately after one of the prefixes in URLPrefix (note that one of these is
the empty prefix, for cases where the user has typed the scheme)"?

[Mark P, 2012/09/11 22:05:00]

I like your suggestion a lot.  I took it entirely and added an example.

+  //     prefix such as http://www or https://
   //  3) AND the search string does not end in whitespace (making it look to
   //     the IMUI as though there is a single search term when actually there
   //     is a second, empty term).
-  can_inline = !url_matches.empty() &&
-      terms.size() == 1 &&
-      (url_matches[0].offset == 0 ||
-       URLPrefix::IsURLPrefix(url.substr(0, url_matches[0].offset))) &&
-      !IsWhitespace(*(lower_string.rbegin()));
-  match_in_scheme = can_inline && url_matches[0].offset == 0;
+  // |best_prefix| stores the inlineable prefix computed in clause (2) or
+  // NULL if no such prefix exists.  (The URL is not inlineable.)
+  const URLPrefix* best_prefix = (!url_matches.empty()) && (terms.size() == 1) ?

[Peter Kasting, 2012/09/11 21:57:48]

Nit: No need for parens around unary expression -- I'd be happy to have them
around the binary expression before '?' though

[Mark P, 2012/09/11 22:05:00]

Rearranged parens.

+      URLPrefix::BestURLPrefix(UTF8ToUTF16(gurl.spec()), terms[0]) : NULL;
+  can_inline = (best_prefix != NULL) && !IsWhitespace(*(lower_string.rbegin()));
+  match_in_scheme = can_inline && best_prefix->prefix.empty();
 
   // Determine if the associated URLs is referenced by any bookmarks.
   float bookmark_boost =
       (bookmark_service && bookmark_service->IsBookmarked(gurl)) ? 10.0 : 0.0;
 
   if (use_new_scoring) {
     const float topicality_score = GetTopicalityScore(
         terms.size(), url, url_matches, title_matches, word_starts);
     const float recency_score = GetRecencyScore(
         (now - row.last_visit()).InDays());
@@ skipping 408 matching lines @@
   // Add a beacon to the logs that'll allow us to identify later what
   // new scoring state a user is in.  Do this by incrementing a bucket in
   // a histogram, where the bucket represents the user's new scoring state.
   UMA_HISTOGRAM_ENUMERATION(
       "Omnibox.HistoryQuickProviderNewScoringFieldTrialBeacon",
       new_scoring_option, NUM_OPTIONS);
 
 }
 
 }  // namespace history