Omnibox - Boost Frequency Scores Based on Number of Matching Pages

components/omnibox/browser/scored_history_match_unittest.cc

Index: components/omnibox/browser/scored_history_match_unittest.cc
diff --git a/components/omnibox/browser/scored_history_match_unittest.cc b/components/omnibox/browser/scored_history_match_unittest.cc
index 1f416e4918dafd210da09a572a8e86dc13953625..0aa0fabdd75210001181f84fad8ed08dc241a039 100644
--- a/components/omnibox/browser/scored_history_match_unittest.cc
+++ b/components/omnibox/browser/scored_history_match_unittest.cc
@@ -122,7 +122,7 @@ float ScoredHistoryMatchTest::GetTopicalityScoreOfTermAgainstURLAndTitle(
   String16SetFromString16(title, &row_word_starts.title_word_starts_);
   ScoredHistoryMatch scored_match(history::URLRow(GURL(url)), VisitInfoVector(),
                                   term, term_vector, term_word_starts,
-                                  row_word_starts, false, base::Time::Max());
+                                  row_word_starts, false, 1, base::Time::Max());
   scored_match.url_matches = MatchTermInString(term, url, 0);
   scored_match.title_matches = MatchTermInString(term, title, 0);
   scored_match.topicality_threshold_ = -1;
@@ -144,7 +144,7 @@ TEST_F(ScoredHistoryMatchTest, Scoring) {
   visits_a[0].second = ui::PAGE_TRANSITION_TYPED;
   ScoredHistoryMatch scored_a(row_a, visits_a, ASCIIToUTF16("abc"),
                               Make1Term("abc"), one_word_no_offset,
-                              word_starts_a, false, now);
+                              word_starts_a, false, 1, now);
 
   // Test scores based on visit_count.
   history::URLRow row_b(MakeURLRow("http://abcdef", "abcd bcd", 10, 30, 1));
@@ -154,7 +154,7 @@ TEST_F(ScoredHistoryMatchTest, Scoring) {
   visits_b[0].second = ui::PAGE_TRANSITION_TYPED;
   ScoredHistoryMatch scored_b(row_b, visits_b, ASCIIToUTF16("abc"),
                               Make1Term("abc"), one_word_no_offset,
-                              word_starts_b, false, now);
+                              word_starts_b, false, 1, now);
   EXPECT_GT(scored_b.raw_score, scored_a.raw_score);
 
   // Test scores based on last_visit.
@@ -165,7 +165,7 @@ TEST_F(ScoredHistoryMatchTest, Scoring) {
   visits_c[0].second = ui::PAGE_TRANSITION_TYPED;
   ScoredHistoryMatch scored_c(row_c, visits_c, ASCIIToUTF16("abc"),
                               Make1Term("abc"), one_word_no_offset,
-                              word_starts_c, false, now);
+                              word_starts_c, false, 1, now);
   EXPECT_GT(scored_c.raw_score, scored_a.raw_score);
 
   // Test scores based on typed_count.
@@ -178,7 +178,7 @@ TEST_F(ScoredHistoryMatchTest, Scoring) {
   visits_d[2].second = ui::PAGE_TRANSITION_TYPED;
   ScoredHistoryMatch scored_d(row_d, visits_d, ASCIIToUTF16("abc"),
                               Make1Term("abc"), one_word_no_offset,
-                              word_starts_d, false, now);
+                              word_starts_d, false, 1, now);
   EXPECT_GT(scored_d.raw_score, scored_a.raw_score);
 
   // Test scores based on a terms appearing multiple times.
@@ -190,14 +190,14 @@ TEST_F(ScoredHistoryMatchTest, Scoring) {
   const VisitInfoVector visits_e = visits_d;
   ScoredHistoryMatch scored_e(row_e, visits_e, ASCIIToUTF16("csi"),
                               Make1Term("csi"), one_word_no_offset,
-                              word_starts_e, false, now);
+                              word_starts_e, false, 1, now);
   EXPECT_LT(scored_e.raw_score, 1400);
 
   // Test that a result with only a mid-term match (i.e., not at a word
   // boundary) scores 0.
   ScoredHistoryMatch scored_f(row_a, visits_a, ASCIIToUTF16("cd"),
                               Make1Term("cd"), one_word_no_offset,
-                              word_starts_a, false, now);
+                              word_starts_a, false, 1, now);
   EXPECT_EQ(scored_f.raw_score, 0);
 }
 
@@ -214,12 +214,12 @@ TEST_F(ScoredHistoryMatchTest, ScoringBookmarks) {
   WordStarts one_word_no_offset(1, 0u);
   VisitInfoVector visits = CreateVisitInfoVector(8, 3, now);
   ScoredHistoryMatch scored(row, visits, ASCIIToUTF16("abc"), Make1Term("abc"),
-                            one_word_no_offset, word_starts, false, now);
+                            one_word_no_offset, word_starts, false, 1, now);
   // Now check that if URL is bookmarked then its score increases.
   base::AutoReset<float> reset(&ScoredHistoryMatch::bookmark_value_, 5);
   ScoredHistoryMatch scored_with_bookmark(row, visits, ASCIIToUTF16("abc"),
                                           Make1Term("abc"), one_word_no_offset,
-                                          word_starts, true, now);
+                                          word_starts, true, 1, now);
   EXPECT_GT(scored_with_bookmark.raw_score, scored.raw_score);
 }
 
@@ -238,14 +238,14 @@ TEST_F(ScoredHistoryMatchTest, ScoringTLD) {
   VisitInfoVector visits = CreateVisitInfoVector(8, 3, now);
   ScoredHistoryMatch scored(row, visits, ASCIIToUTF16("fed com"),
                             Make2Terms("fed", "com"), two_words_no_offsets,
-                            word_starts, false, now);
+                            word_starts, false, 1, now);
   EXPECT_EQ(0, scored.raw_score);
 
   // Now allow credit for the match in the TLD.
   base::AutoReset<bool> reset(&ScoredHistoryMatch::allow_tld_matches_, true);
   ScoredHistoryMatch scored_with_tld(
       row, visits, ASCIIToUTF16("fed com"), Make2Terms("fed", "com"),
-      two_words_no_offsets, word_starts, false, now);
+      two_words_no_offsets, word_starts, false, 1, now);
   EXPECT_GT(scored_with_tld.raw_score, 0);
 }
 
@@ -264,14 +264,14 @@ TEST_F(ScoredHistoryMatchTest, ScoringScheme) {
   VisitInfoVector visits = CreateVisitInfoVector(8, 3, now);
   ScoredHistoryMatch scored(row, visits, ASCIIToUTF16("fed http"),
                             Make2Terms("fed", "http"), two_words_no_offsets,
-                            word_starts, false, now);
+                            word_starts, false, 1, now);
   EXPECT_EQ(0, scored.raw_score);
 
   // Now allow credit for the match in the scheme.
   base::AutoReset<bool> reset(&ScoredHistoryMatch::allow_scheme_matches_, true);
   ScoredHistoryMatch scored_with_scheme(
       row, visits, ASCIIToUTF16("fed http"), Make2Terms("fed", "http"),
-      two_words_no_offsets, word_starts, false, now);
+      two_words_no_offsets, word_starts, false, 1, now);
   EXPECT_GT(scored_with_scheme.raw_score, 0);
 }
 
@@ -288,16 +288,16 @@ TEST_F(ScoredHistoryMatchTest, Inlining) {
         MakeURLRow("http://www.google.com", "abcdef", 3, 30, 1));
     PopulateWordStarts(row, &word_starts);
     ScoredHistoryMatch scored_a(row, visits, ASCIIToUTF16("g"), Make1Term("g"),
-                                one_word_no_offset, word_starts, false, now);
+                                one_word_no_offset, word_starts, false, 1, now);
     EXPECT_FALSE(scored_a.match_in_scheme);
     ScoredHistoryMatch scored_b(row, visits, ASCIIToUTF16("w"), Make1Term("w"),
-                                one_word_no_offset, word_starts, false, now);
+                                one_word_no_offset, word_starts, false, 1, now);
     EXPECT_FALSE(scored_b.match_in_scheme);
     ScoredHistoryMatch scored_c(row, visits, ASCIIToUTF16("h"), Make1Term("h"),
-                                one_word_no_offset, word_starts, false, now);
+                                one_word_no_offset, word_starts, false, 1, now);
     EXPECT_TRUE(scored_c.match_in_scheme);
     ScoredHistoryMatch scored_d(row, visits, ASCIIToUTF16("o"), Make1Term("o"),
-                                one_word_no_offset, word_starts, false, now);
+                                one_word_no_offset, word_starts, false, 1, now);
     EXPECT_FALSE(scored_d.match_in_scheme);
   }
 
@@ -305,13 +305,13 @@ TEST_F(ScoredHistoryMatchTest, Inlining) {
     history::URLRow row(MakeURLRow("http://teams.foo.com", "abcdef", 3, 30, 1));
     PopulateWordStarts(row, &word_starts);
     ScoredHistoryMatch scored_a(row, visits, ASCIIToUTF16("t"), Make1Term("t"),
-                                one_word_no_offset, word_starts, false, now);
+                                one_word_no_offset, word_starts, false, 1, now);
     EXPECT_FALSE(scored_a.match_in_scheme);
     ScoredHistoryMatch scored_b(row, visits, ASCIIToUTF16("f"), Make1Term("f"),
-                                one_word_no_offset, word_starts, false, now);
+                                one_word_no_offset, word_starts, false, 1, now);
     EXPECT_FALSE(scored_b.match_in_scheme);
     ScoredHistoryMatch scored_c(row, visits, ASCIIToUTF16("o"), Make1Term("o"),
-                                one_word_no_offset, word_starts, false, now);
+                                one_word_no_offset, word_starts, false, 1, now);
     EXPECT_FALSE(scored_c.match_in_scheme);
   }
 
@@ -320,13 +320,13 @@ TEST_F(ScoredHistoryMatchTest, Inlining) {
         MakeURLRow("https://www.testing.com", "abcdef", 3, 30, 1));
     PopulateWordStarts(row, &word_starts);
     ScoredHistoryMatch scored_a(row, visits, ASCIIToUTF16("t"), Make1Term("t"),
-                                one_word_no_offset, word_starts, false, now);
+                                one_word_no_offset, word_starts, false, 1, now);
     EXPECT_FALSE(scored_a.match_in_scheme);
     ScoredHistoryMatch scored_b(row, visits, ASCIIToUTF16("h"), Make1Term("h"),
-                                one_word_no_offset, word_starts, false, now);
+                                one_word_no_offset, word_starts, false, 1, now);
     EXPECT_TRUE(scored_b.match_in_scheme);
     ScoredHistoryMatch scored_c(row, visits, ASCIIToUTF16("w"), Make1Term("w"),
-                                one_word_no_offset, word_starts, false, now);
+                                one_word_no_offset, word_starts, false, 1, now);
     EXPECT_FALSE(scored_c.match_in_scheme);
   }
 
@@ -335,14 +335,14 @@ TEST_F(ScoredHistoryMatchTest, Inlining) {
         MakeURLRow("http://www.xn--1lq90ic7f1rc.cn/xnblah", "abcd", 3, 30, 1));
     PopulateWordStarts(row, &word_starts);
     ScoredHistoryMatch scored_a(row, visits, ASCIIToUTF16("x"), Make1Term("x"),
-                                one_word_no_offset, word_starts, false, now);
+                                one_word_no_offset, word_starts, false, 1, now);
     EXPECT_FALSE(scored_a.match_in_scheme);
     ScoredHistoryMatch scored_b(row, visits, ASCIIToUTF16("xn"),
                                 Make1Term("xn"), one_word_no_offset,
-                                word_starts, false, now);
+                                word_starts, false, 1, now);
     EXPECT_FALSE(scored_b.match_in_scheme);
     ScoredHistoryMatch scored_c(row, visits, ASCIIToUTF16("w"), Make1Term("w"),
-                                one_word_no_offset, word_starts, false, now);
+                                one_word_no_offset, word_starts, false, 1, now);
     EXPECT_FALSE(scored_c.match_in_scheme);
   }
 }
@@ -481,27 +481,41 @@ TEST_F(ScoredHistoryMatchTest, GetFrequency) {
   base::Time now(base::Time::Max());
   VisitInfoVector visits;
   ScoredHistoryMatch match(row, visits, ASCIIToUTF16("foo"), Make1Term("foo"),
-                           WordStarts{0}, row_word_starts, false, now);
+                           WordStarts{0}, row_word_starts, false, 1, now);
 
   // Record the score for one untyped visit.
   visits = {{now, ui::PAGE_TRANSITION_LINK}};
-  const float one_untyped_score = match.GetFrequency(now, false, visits);
+  const float one_untyped_score = match.GetFrequency(now, false, visits, 1);
 
   // The score for one typed visit should be larger.
   visits = VisitInfoVector{{now, ui::PAGE_TRANSITION_TYPED}};
-  const float one_typed_score = match.GetFrequency(now, false, visits);
+  const float one_typed_score = match.GetFrequency(now, false, visits, 1);
   EXPECT_GT(one_typed_score, one_untyped_score);
 
   // It shouldn't matter if the typed visit has a transition qualifier.
   visits = {
       {now, ui::PageTransitionFromInt(ui::PAGE_TRANSITION_TYPED |
                                       ui::PAGE_TRANSITION_SERVER_REDIRECT)}};
-  EXPECT_EQ(one_typed_score, match.GetFrequency(now, false, visits));
+  EXPECT_EQ(one_typed_score, match.GetFrequency(now, false, visits, 1));
 
   // A score for one untyped visit to a bookmarked page should be larger than
   // the one untyped visit to a non-bookmarked page.
   visits = {{now, ui::PAGE_TRANSITION_LINK}};
-  EXPECT_GE(match.GetFrequency(now, true, visits), one_untyped_score);
+  EXPECT_GE(match.GetFrequency(now, true, visits, 1), one_untyped_score);
+
+  // If we pretend there are five items matching the input, the score should
+  // not change.
+  EXPECT_EQ(match.GetFrequency(now, false, visits, 5), one_untyped_score);
+  {
+    // With different parameters, however, it can change.  In this case, the

[Peter Kasting, 2016/12/01 07:07:53]

Nit: parameters -> match multiplier parameters?  (Or something; "parameters"
seems vague.)

[Mark P, 2016/12/04 01:06:42]

Now moot.

+    // score when there's one matching page should be boosted but if there are
+    // five, the score remains unchanged.
+    base::AutoReset<OmniboxFieldTrial::NumMatchesMultipliers> tmp(
+        ScoredHistoryMatch::num_matches_to_frequency_multiplier_,
+        OmniboxFieldTrial::NumMatchesMultipliers{{1, 3.0}});
+    EXPECT_GT(match.GetFrequency(now, false, visits, 1), one_untyped_score);
+    EXPECT_EQ(match.GetFrequency(now, false, visits, 5), one_untyped_score);
+  }

[Peter Kasting, 2016/12/01 07:07:53]

You test this three times.  Helper function?

Do the further tests actually test what you want them to (i.e. that the two
behavior fixes don't interact with this modifier list) given that you use vague
tests like EXPECT_GT instead of checking exact values?  Maybe they're not really
valuable and should just go away?

[Mark P, 2016/12/04 01:06:42]

Now moot.

 
   // Now consider pages visited twice, with one visit being typed and one
   // untyped.
@@ -510,7 +524,7 @@ TEST_F(ScoredHistoryMatchTest, GetFrequency) {
   // single typed visit score.
   visits = {{now, ui::PAGE_TRANSITION_TYPED},
             {now - base::TimeDelta::FromDays(1), ui::PAGE_TRANSITION_LINK}};
-  const float two_visits_score = match.GetFrequency(now, false, visits);
+  const float two_visits_score = match.GetFrequency(now, false, visits, 1);
   EXPECT_GT(two_visits_score, one_typed_score);
 
   // Likewise, with |frequency_uses_sum_|, the two-visit score should be higher
@@ -518,20 +532,33 @@ TEST_F(ScoredHistoryMatchTest, GetFrequency) {
   float two_visits_score_uses_sum;
   {
     base::AutoReset<bool> tmp(&ScoredHistoryMatch::frequency_uses_sum_, true);
-    two_visits_score_uses_sum = match.GetFrequency(now, false, visits);
+    two_visits_score_uses_sum = match.GetFrequency(now, false, visits, 1);
     EXPECT_GT(two_visits_score_uses_sum, one_typed_score);
   }
 
   // With |fix_few_visits_bug_|, its two-visit score should be higher than the
   // one-visit score but lower than the regular two-visit score because the
   // fix-few-visits score computes the average visit score and the untyped
-  // visit brings the average down.
+  // visit brings the average down.  The behavior with respect to boosting
+  // the score based on the number of matching pages should remain unchanged.
   float two_visits_score_fix_few_visits;
   {
     base::AutoReset<bool> tmp(&ScoredHistoryMatch::fix_few_visits_bug_, true);
-    two_visits_score_fix_few_visits = match.GetFrequency(now, false, visits);
+    two_visits_score_fix_few_visits = match.GetFrequency(now, false, visits, 1);
     EXPECT_GT(two_visits_score_fix_few_visits, one_typed_score);
     EXPECT_LT(two_visits_score_fix_few_visits, two_visits_score);
+
+    EXPECT_EQ(match.GetFrequency(now, false, visits, 5),
+              two_visits_score_fix_few_visits);
+    {
+      base::AutoReset<OmniboxFieldTrial::NumMatchesMultipliers> tmp2(
+          ScoredHistoryMatch::num_matches_to_frequency_multiplier_,
+          OmniboxFieldTrial::NumMatchesMultipliers{{1, 3.0}});
+      EXPECT_GT(match.GetFrequency(now, false, visits, 1),
+                two_visits_score_fix_few_visits);
+      EXPECT_EQ(match.GetFrequency(now, false, visits, 5),
+                two_visits_score_fix_few_visits);
+    }
   }
 
   // Add an third untyped visit.
@@ -539,15 +566,28 @@ TEST_F(ScoredHistoryMatchTest, GetFrequency) {
       {now - base::TimeDelta::FromDays(2), ui::PAGE_TRANSITION_LINK});
 
   // With default scoring and with |frequency_uses_sum_|, the score should be
-  // higher than the two-visit score.
-  const float three_visits_score = match.GetFrequency(now, false, visits);
+  // higher than the two-visit score.  The behavior with respect to boosting
+  // the score based on the number of matching pages should remain unchanged
+  const float three_visits_score = match.GetFrequency(now, false, visits, 1);
   EXPECT_GT(three_visits_score, two_visits_score);
   {
     base::AutoReset<bool> tmp(&ScoredHistoryMatch::frequency_uses_sum_, true);
     const float three_visits_score_uses_sum =
-        match.GetFrequency(now, false, visits);
+        match.GetFrequency(now, false, visits, 1);
     EXPECT_GT(three_visits_score_uses_sum, two_visits_score);
     EXPECT_GT(three_visits_score_uses_sum, two_visits_score_uses_sum);
+
+    EXPECT_EQ(match.GetFrequency(now, false, visits, 5),
+              three_visits_score_uses_sum);
+    {
+      base::AutoReset<OmniboxFieldTrial::NumMatchesMultipliers> tmp2(
+          ScoredHistoryMatch::num_matches_to_frequency_multiplier_,
+          OmniboxFieldTrial::NumMatchesMultipliers{{1, 3.0}});
+      EXPECT_GT(match.GetFrequency(now, false, visits, 1),
+                three_visits_score_uses_sum);
+      EXPECT_EQ(match.GetFrequency(now, false, visits, 5),
+                three_visits_score_uses_sum);
+    }
   }
 
   // With |fix_few_visits_bug_|, the score should also go up but not as much as
@@ -555,7 +595,7 @@ TEST_F(ScoredHistoryMatchTest, GetFrequency) {
   {
     base::AutoReset<bool> tmp(&ScoredHistoryMatch::fix_few_visits_bug_, true);
     const float three_visits_score_fix_few_visits =
-        match.GetFrequency(now, false, visits);
+        match.GetFrequency(now, false, visits, 1);
     EXPECT_GT(three_visits_score_fix_few_visits,
               two_visits_score_fix_few_visits);
     EXPECT_LT(three_visits_score_fix_few_visits, three_visits_score);
@@ -572,12 +612,12 @@ TEST_F(ScoredHistoryMatchTest, GetFrequency) {
     base::AutoReset<size_t> tmp1(&ScoredHistoryMatch::max_visits_to_score_, 2);
     base::AutoReset<bool> tmp2(&ScoredHistoryMatch::frequency_uses_sum_, true);
     EXPECT_EQ(two_visits_score_uses_sum,
-              match.GetFrequency(now, false, visits));
+              match.GetFrequency(now, false, visits, 1));
 
     // Check again with the third visit being typed.
     visits[2].second = ui::PAGE_TRANSITION_TYPED;
     EXPECT_EQ(two_visits_score_uses_sum,
-              match.GetFrequency(now, false, visits));
+              match.GetFrequency(now, false, visits, 1));
   }
 }