Omnibox: Improve HQP Scoring for Terms that Start with Punctuation

components/omnibox/browser/scored_history_match_unittest.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/scored_history_match.h"
 
 #include <algorithm>
 #include <memory>
 
 #include "base/auto_reset.h"
 #include "base/bind.h"
+#include "base/i18n/break_iterator.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/search_engines/search_terms_data.h"
 #include "components/search_engines/template_url.h"
 #include "components/search_engines/template_url_service.h"
 #include "components/search_engines/template_url_service_client.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ skipping 90 matching lines @@
     const base::string16& title) {
   // Make an empty match and simply populate the fields we need in order
   // to call GetTopicalityScore().
   ScoredHistoryMatch scored_match;
   scored_match.url_matches = MatchTermInString(term, url, 0);
   scored_match.title_matches = MatchTermInString(term, title, 0);
   scored_match.topicality_threshold_ = -1;
   RowWordStarts word_starts;
   String16SetFromString16(url, &word_starts.url_word_starts_);
   String16SetFromString16(title, &word_starts.title_word_starts_);
-  WordStarts one_word_no_offset(1, 0u);
-  return scored_match.GetTopicalityScore(1, url, one_word_no_offset,
+  WordStarts term_word_starts(1, 0u);
+  base::i18n::BreakIterator iter(term, base::i18n::BreakIterator::BREAK_WORD);
+  if (iter.Init()) {
+    // Find the first word start.
+    while (iter.Advance() && !iter.IsWord()) {}
+    term_word_starts[0] = iter.prev();
+  }
+  return scored_match.GetTopicalityScore(1, url, term_word_starts,
                                          word_starts);
 }
 
 TEST_F(ScoredHistoryMatchTest, Scoring) {
   // We use NowFromSystemTime() because MakeURLRow uses the same function
   // to calculate last visit time when building a row.
   base::Time now = base::Time::NowFromSystemTime();
 
   history::URLRow row_a(MakeURLRow("http://fedcba", "abcd bcd", 3, 30, 1));
   RowWordStarts word_starts_a;
@@ skipping 409 matching lines @@
 }
 
 // This function only tests scoring of single terms that match exactly
 // once somewhere in the URL or title.
 TEST_F(ScoredHistoryMatchTest, GetTopicalityScore) {
   base::string16 url = ASCIIToUTF16(
       "http://abc.def.com/path1/path2?"
       "arg1=val1&arg2=val2#hash_component");
   base::string16 title = ASCIIToUTF16("here is a title");
   const float hostname_score = GetTopicalityScoreOfTermAgainstURLAndTitle(
       ASCIIToUTF16("abc"), url, title);

[Peter Kasting, 2016/10/17 23:34:37]

Nit: Define a lambda here (not sure how git cl format would do this):

  auto Score = [&](char* term) {
    return GetTopicalityScoreOfTermAgainstURLAndTitle(
        ASCIIToUTF16(term), url, title);
  }

Now all these lines can just do

  const float hostname_score = Score("abc");

Better still, this syntax is now so brief you can inline some of these temps
below:

  EXPECT_EQ(hostname_score, Score("://abc"));

Or, if you don't mind duplicating some of the work:

  EXPECT_EQ(Score("abc"), Score("://abc"));

[Mark P, 2016/10/18 22:57:29]

Good idea.  I find my aging brain doesn't think of new C++ features to use when
I should.  Done.

I decided not to inline the scores in the comparisons below because I think the
names as useful descriptions.  Otherwise a casual reader of the code would have
to continue referring back to the URL as the top.  It also makes clear the
distinction between certain things, e.g., "://abc" should be a hostname match,
not a scheme match.

   const float hostname_mid_word_score =
       GetTopicalityScoreOfTermAgainstURLAndTitle(ASCIIToUTF16("bc"), url,
                                                  title);
+  const float hostname_score_preceeding_punctuation =
+      GetTopicalityScoreOfTermAgainstURLAndTitle(ASCIIToUTF16("://abc"), url,
+                                                 title);
   const float domain_name_score = GetTopicalityScoreOfTermAgainstURLAndTitle(
       ASCIIToUTF16("def"), url, title);
   const float domain_name_mid_word_score =
       GetTopicalityScoreOfTermAgainstURLAndTitle(ASCIIToUTF16("ef"), url,
                                                  title);
+  const float domain_name_score_preceeding_dot =
+      GetTopicalityScoreOfTermAgainstURLAndTitle(ASCIIToUTF16(".def"), url,
+                                                 title);
   const float tld_score = GetTopicalityScoreOfTermAgainstURLAndTitle(
       ASCIIToUTF16("com"), url, title);
   const float tld_mid_word_score = GetTopicalityScoreOfTermAgainstURLAndTitle(
       ASCIIToUTF16("om"), url, title);
+  const float tld_score_preceeding_dot =
+      GetTopicalityScoreOfTermAgainstURLAndTitle(ASCIIToUTF16(".com"), url,
+                                                 title);
   const float path_score = GetTopicalityScoreOfTermAgainstURLAndTitle(
       ASCIIToUTF16("path1"), url, title);
   const float path_mid_word_score = GetTopicalityScoreOfTermAgainstURLAndTitle(
       ASCIIToUTF16("ath1"), url, title);
+  const float path_score_preceeding_slash =
+      GetTopicalityScoreOfTermAgainstURLAndTitle(ASCIIToUTF16("/path1"), url,
+                                                 title);
   const float arg_score = GetTopicalityScoreOfTermAgainstURLAndTitle(
-      ASCIIToUTF16("arg2"), url, title);
+      ASCIIToUTF16("arg1"), url, title);
   const float arg_mid_word_score = GetTopicalityScoreOfTermAgainstURLAndTitle(
-      ASCIIToUTF16("rg2"), url, title);
+      ASCIIToUTF16("rg1"), url, title);
+  const float arg_score_preceeding_question_mark =
+      GetTopicalityScoreOfTermAgainstURLAndTitle(ASCIIToUTF16("?arg1"), url,
+                                                 title);
   const float protocol_score = GetTopicalityScoreOfTermAgainstURLAndTitle(
       ASCIIToUTF16("htt"), url, title);
   const float protocol_mid_word_score =
       GetTopicalityScoreOfTermAgainstURLAndTitle(ASCIIToUTF16("tt"), url,
                                                  title);
   const float title_score = GetTopicalityScoreOfTermAgainstURLAndTitle(
       ASCIIToUTF16("her"), url, title);
   const float title_mid_word_score = GetTopicalityScoreOfTermAgainstURLAndTitle(
       ASCIIToUTF16("er"), url, title);
   // Verify hostname and domain name > path > arg.
   EXPECT_GT(hostname_score, path_score);
   EXPECT_GT(domain_name_score, path_score);
   EXPECT_GT(path_score, arg_score);
+  // Verify leading punctuation doesn't confuse scoring.
+  EXPECT_EQ(hostname_score, hostname_score_preceeding_punctuation);
+  EXPECT_EQ(domain_name_score, domain_name_score_preceeding_dot);
+  EXPECT_EQ(tld_score, tld_score_preceeding_dot);
+  EXPECT_EQ(path_score, path_score_preceeding_slash);
+  EXPECT_EQ(arg_score, arg_score_preceeding_question_mark);
   // Verify that domain name > path and domain name > arg for non-word
   // boundaries.
   EXPECT_GT(hostname_mid_word_score, path_mid_word_score);
   EXPECT_GT(domain_name_mid_word_score, path_mid_word_score);
   EXPECT_GT(domain_name_mid_word_score, arg_mid_word_score);
   EXPECT_GT(hostname_mid_word_score, arg_mid_word_score);
   // Also verify that the matches at non-word-boundaries all score
   // worse than the matches at word boundaries.  These three sets suffice.
   EXPECT_GT(arg_score, hostname_mid_word_score);
   EXPECT_GT(arg_score, domain_name_mid_word_score);
@@ skipping 57 matching lines @@
 
   EXPECT_TRUE(
       ScoredHistoryMatch::GetHQPBucketsFromString(buckets_str, &hqp_buckets));
   EXPECT_THAT(hqp_buckets, ElementsAre(Pair(0.0, 400), Pair(1.5, 600),
                                        Pair(12.0, 1300), Pair(20.0, 1399)));
   // invalid string.
   buckets_str = "0.0,400,1.5,600";
   EXPECT_FALSE(
       ScoredHistoryMatch::GetHQPBucketsFromString(buckets_str, &hqp_buckets));
 }