Omnibox: Make HistoryURL Highlight Titles like HistoryQuick Provider

Previously HistoryURL provider would highlight page titles by
searching for the first occurrence of the input term and highlighting
it.  This was case sensitive.

Now HistoryURL provider highlights page titles in the same way as
HistoryQuick provider: it normalizes the title (i.e., lowercases it)
and highlights all occurrences that appear at word breaks.

TBR=sky

BUG=318329

Review URL: https://codereview.chromium.org/77453007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@237504 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 106 insertions, 84 deletions

diff --git a/chrome/browser/autocomplete/history_provider.cc b/chrome/browser/autocomplete/history_provider.cc
index 64d7063..efd6bb9 100644
--- a/chrome/browser/autocomplete/history_provider.cc
+++ b/chrome/browser/autocomplete/history_provider.cc
@@ -13,6 +13,7 @@
 #include "chrome/browser/autocomplete/autocomplete_provider_listener.h"
 #include "chrome/browser/history/history_service.h"
 #include "chrome/browser/history/history_service_factory.h"
+#include "chrome/browser/history/in_memory_url_index_types.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/net/url_fixer_upper.h"
 #include "chrome/common/url_constants.h"
@@ -157,3 +158,36 @@ bool HistoryProvider::PreventInlineAutocomplete(
       (!input.text().empty() &&
        IsWhitespace(input.text()[input.text().length() - 1]));
 }
+
+// static
+ACMatchClassifications HistoryProvider::SpansFromTermMatch(
+    const history::TermMatches& matches,
+    size_t text_length,
+    bool is_url) {
+  ACMatchClassification::Style url_style =
+      is_url ? ACMatchClassification::URL : ACMatchClassification::NONE;
+  ACMatchClassifications spans;
+  if (matches.empty()) {
+    if (text_length)
+      spans.push_back(ACMatchClassification(0, url_style));
+    return spans;
+  }
+  if (matches[0].offset)
+    spans.push_back(ACMatchClassification(0, url_style));
+  size_t match_count = matches.size();
+  for (size_t i = 0; i < match_count;) {
+    size_t offset = matches[i].offset;
+    spans.push_back(ACMatchClassification(offset,
+        ACMatchClassification::MATCH | url_style));
+    // Skip all adjacent matches.
+    do {
+      offset += matches[i].length;
+      ++i;
+    } while ((i < match_count) && (offset == matches[i].offset));
+    if (offset < text_length)
+      spans.push_back(ACMatchClassification(offset, url_style));
+  }
+
+  return spans;
+}
+
diff --git a/chrome/browser/autocomplete/history_provider.h b/chrome/browser/autocomplete/history_provider.h
index 6be0e2b..9a72b86 100644
--- a/chrome/browser/autocomplete/history_provider.h
+++ b/chrome/browser/autocomplete/history_provider.h
@@ -7,6 +7,7 @@
 
 #include "base/compiler_specific.h"
 #include "chrome/browser/autocomplete/autocomplete_provider.h"
+#include "chrome/browser/history/in_memory_url_index_types.h"
 
 class AutocompleteInput;
 struct AutocompleteMatch;
@@ -53,6 +54,13 @@ class HistoryProvider : public AutocompleteProvider {
   // |input.prevent_inline_autocomplete()| is true or the input text contains
   // trailing whitespace.
   bool PreventInlineAutocomplete(const AutocompleteInput& input);
+
+  // Fill and return an ACMatchClassifications structure given the |matches|
+  // to highlight.
+  static ACMatchClassifications SpansFromTermMatch(
+      const history::TermMatches& matches,
+      size_t text_length,
+      bool is_url);
 };
 
 #endif  // CHROME_BROWSER_AUTOCOMPLETE_HISTORY_PROVIDER_H_
diff --git a/chrome/browser/autocomplete/history_quick_provider.cc b/chrome/browser/autocomplete/history_quick_provider.cc
index 5982a71..0ae6b11 100644
--- a/chrome/browser/autocomplete/history_quick_provider.cc
+++ b/chrome/browser/autocomplete/history_quick_provider.cc
@@ -316,35 +316,3 @@ history::InMemoryURLIndex* HistoryQuickProvider::GetIndex() {
 
   return history_service->InMemoryIndex();
 }
-
-// static
-ACMatchClassifications HistoryQuickProvider::SpansFromTermMatch(
-    const history::TermMatches& matches,
-    size_t text_length,
-    bool is_url) {
-  ACMatchClassification::Style url_style =
-      is_url ? ACMatchClassification::URL : ACMatchClassification::NONE;
-  ACMatchClassifications spans;
-  if (matches.empty()) {
-    if (text_length)
-      spans.push_back(ACMatchClassification(0, url_style));
-    return spans;
-  }
-  if (matches[0].offset)
-    spans.push_back(ACMatchClassification(0, url_style));
-  size_t match_count = matches.size();
-  for (size_t i = 0; i < match_count;) {
-    size_t offset = matches[i].offset;
-    spans.push_back(ACMatchClassification(offset,
-        ACMatchClassification::MATCH | url_style));
-    // Skip all adjacent matches.
-    do {
-      offset += matches[i].length;
-      ++i;
-    } while ((i < match_count) && (offset == matches[i].offset));
-    if (offset < text_length)
-      spans.push_back(ACMatchClassification(offset, url_style));
-  }
-
-  return spans;
-}
diff --git a/chrome/browser/autocomplete/history_quick_provider.h b/chrome/browser/autocomplete/history_quick_provider.h
index 470e7b9..7d2c5cd 100644
--- a/chrome/browser/autocomplete/history_quick_provider.h
+++ b/chrome/browser/autocomplete/history_quick_provider.h
@@ -16,7 +16,6 @@
 #include "chrome/browser/history/in_memory_url_index.h"
 
 class Profile;
-class TermMatches;
 
 namespace history {
 class ScoredHistoryMatch;
@@ -64,13 +63,6 @@ class HistoryQuickProvider : public HistoryProvider {
   // Returns the index that should be used for history lookups.
   history::InMemoryURLIndex* GetIndex();
 
-  // Fill and return an ACMatchClassifications structure given the term
-  // matches (|matches|) to highlight where terms were found.
-  static ACMatchClassifications SpansFromTermMatch(
-      const history::TermMatches& matches,
-      size_t text_length,
-      bool is_url);
-
   // Only for use in unittests.  Takes ownership of |index|.
   void set_index(history::InMemoryURLIndex* index) {
     index_for_testing_.reset(index);
diff --git a/chrome/browser/autocomplete/history_url_provider.cc b/chrome/browser/autocomplete/history_url_provider.cc
index d28038a..45c660b 100644
--- a/chrome/browser/autocomplete/history_url_provider.cc
+++ b/chrome/browser/autocomplete/history_url_provider.cc
@@ -21,6 +21,8 @@
 #include "chrome/browser/history/history_service.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/history/history_types.h"
+#include "chrome/browser/history/in_memory_url_index_types.h"
+#include "chrome/browser/history/scored_history_match.h"
 #include "chrome/browser/omnibox/omnibox_field_trial.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search_engines/template_url_service.h"
@@ -771,10 +773,8 @@ bool HistoryURLProvider::FixupExactSuggestion(
       match->deletable = true;
       match->description = classifier.url_row().title();
       RecordAdditionalInfoFromUrlRow(classifier.url_row(), match);
-      AutocompleteMatch::ClassifyMatchInString(
-          input.text(),
-          classifier.url_row().title(),
-          ACMatchClassification::NONE, &match->description_class);
+      match->description_class =
+          ClassifyDescription(input.text(), match->description);
       if (!classifier.url_row().typed_count()) {
         // If we reach here, we must be in the second pass, and we must not have
         // this row's data available during the first pass.  That means we
@@ -1087,10 +1087,25 @@ AutocompleteMatch HistoryURLProvider::HistoryMatchToACMatch(
         &match.contents_class);
   }
   match.description = info.title();
-  AutocompleteMatch::ClassifyMatchInString(params.input.text(),
-                                           info.title(),
-                                           ACMatchClassification::NONE,
-                                           &match.description_class);
+  match.description_class =
+      ClassifyDescription(params.input.text(), match.description);
   RecordAdditionalInfoFromUrlRow(info, &match);
   return match;
 }
+
+// static
+ACMatchClassifications HistoryURLProvider::ClassifyDescription(
+    const string16& input_text,
+    const string16& description) {
+  string16 clean_description = history::CleanUpTitleForMatching(description);
+  history::TermMatches description_matches(SortAndDeoverlapMatches(
+      history::MatchTermInString(input_text, clean_description, 0)));
+  history::WordStarts description_word_starts;
+  history::String16VectorFromString16(
+      clean_description, false, &description_word_starts);
+  description_matches =
+      history::ScoredHistoryMatch::FilterTermMatchesByWordStarts(
+          description_matches, description_word_starts, 0);
+  return SpansFromTermMatch(
+      description_matches, clean_description.length(), false);
+}
diff --git a/chrome/browser/autocomplete/history_url_provider.h b/chrome/browser/autocomplete/history_url_provider.h
index b42faf3..610d35b 100644
--- a/chrome/browser/autocomplete/history_url_provider.h
+++ b/chrome/browser/autocomplete/history_url_provider.h
@@ -291,6 +291,12 @@ class HistoryURLProvider : public HistoryProvider {
       MatchType match_type,
       int relevance);
 
+  // Returns a set of classifications that highlight all the occurrences
+  // of |input_text| at word breaks in |description|.
+  static ACMatchClassifications ClassifyDescription(
+      const string16& input_text,
+      const string16& description);
+
   // Params for the current query.  The provider should not free this directly;
   // instead, it is passed as a parameter through the history backend, and the
   // parameter itself is freed once it's no longer needed.  The only reason we
diff --git a/chrome/browser/history/scored_history_match.cc b/chrome/browser/history/scored_history_match.cc
index f201414..fa2bfc4 100644
--- a/chrome/browser/history/scored_history_match.cc
+++ b/chrome/browser/history/scored_history_match.cc
@@ -245,6 +245,33 @@ bool ScoredHistoryMatch::MatchScoreGreater(const ScoredHistoryMatch& m1,
   return m1.url_info.last_visit() > m2.url_info.last_visit();
 }
 
+// static
+TermMatches ScoredHistoryMatch::FilterTermMatchesByWordStarts(
+    const TermMatches& term_matches,
+    const WordStarts& word_starts,
+    const size_t start_pos) {
+  if (start_pos == std::string::npos)
+    return term_matches;
+  TermMatches filtered_matches;
+  WordStarts::const_iterator next_word_starts = word_starts.begin();
+  WordStarts::const_iterator end_word_starts = word_starts.end();
+  for (TermMatches::const_iterator iter = term_matches.begin();
+       iter != term_matches.end(); ++iter) {
+    // Advance next_word_starts until it's >= the position of the term
+    // we're considering.
+    while ((next_word_starts != end_word_starts) &&
+           (*next_word_starts < iter->offset))
+      ++next_word_starts;
+    // Add the match if it's before the position we start filtering at or
+    // if it's at a word boundary.
+    if ((iter->offset < start_pos) ||
+        ((next_word_starts != end_word_starts) &&
+         (*next_word_starts == iter->offset)))
+      filtered_matches.push_back(*iter);
+  }
+  return filtered_matches;
+}
+
 float ScoredHistoryMatch::GetTopicalityScore(
     const int num_terms,
     const string16& url,
@@ -374,33 +401,6 @@ float ScoredHistoryMatch::GetTopicalityScore(
 }
 
 // static
-TermMatches ScoredHistoryMatch::FilterTermMatchesByWordStarts(
-    const TermMatches& term_matches,
-    const WordStarts& word_starts,
-    const size_t start_pos) {
-  if (start_pos == std::string::npos)
-    return term_matches;
-  TermMatches filtered_matches;
-  WordStarts::const_iterator next_word_starts = word_starts.begin();
-  WordStarts::const_iterator end_word_starts = word_starts.end();
-  for (TermMatches::const_iterator iter = term_matches.begin();
-       iter != term_matches.end(); ++iter) {
-    // Advance next_word_starts until it's >= the position of the term
-    // we're considering.
-    while ((next_word_starts != end_word_starts) &&
-           (*next_word_starts < iter->offset))
-      ++next_word_starts;
-    // Add the match if it's before the position we start filtering at or
-    // if it's at a word boundary.
-    if ((iter->offset < start_pos) ||
-        ((next_word_starts != end_word_starts) &&
-         (*next_word_starts == iter->offset)))
-      filtered_matches.push_back(*iter);
-  }
-  return filtered_matches;
-}
-
-// static
 void ScoredHistoryMatch::FillInTermScoreToTopicalityScoreArray() {
   for (int term_score = 0; term_score < kMaxRawTermScore; ++term_score) {
     float topicality_score;
diff --git a/chrome/browser/history/scored_history_match.h b/chrome/browser/history/scored_history_match.h
index c6a1f63..b641ea3 100644
--- a/chrome/browser/history/scored_history_match.h
+++ b/chrome/browser/history/scored_history_match.h
@@ -64,6 +64,14 @@ class ScoredHistoryMatch : public history::HistoryMatch {
   const TermMatches& title_matches() const { return title_matches_; }
   bool can_inline() const { return can_inline_; }
 
+  // Returns |term_matches| after removing all matches that are not at a
+  // word break that starts after position |start_pos|.  If |start_pos| is
+  // string::npos, does no filtering and simply returns |term_matches|.
+  static TermMatches FilterTermMatchesByWordStarts(
+      const TermMatches& term_matches,
+      const WordStarts& word_starts,
+      const size_t start_pos);
+
  private:
   friend class ScoredHistoryMatchTest;
 
@@ -83,15 +91,6 @@ class ScoredHistoryMatch : public history::HistoryMatch {
                            const string16& cleaned_up_url,
                            const RowWordStarts& word_starts);
 
-  // Helper function for GetTopicalityScore().
-  // Returns |term_matches| after removing all matches that are not at a
-  // word break that starts after position |start_pos|.  If |start_pos| is
-  // string::npos, does no filtering and simply returns |term_matches|.
-  static TermMatches FilterTermMatchesByWordStarts(
-      const TermMatches& term_matches,
-      const WordStarts& word_starts,
-      const size_t start_pos);
-
   // Precalculates raw_term_score_to_topicality_score_, used in
   // GetTopicalityScore().
   static void FillInTermScoreToTopicalityScoreArray();