Omnibox Field Trial: Make HQP Score Bookmarks More Highly

chrome/browser/history/scored_history_match.cc

Index: chrome/browser/history/scored_history_match.cc
diff --git a/chrome/browser/history/scored_history_match.cc b/chrome/browser/history/scored_history_match.cc
index 592b4652ce851120c596a44330c17d5113e7f097..ec591a143f54fe2d5ae151c1084b3f09fdf4cddd 100644
--- a/chrome/browser/history/scored_history_match.cc
+++ b/chrome/browser/history/scored_history_match.cc
@@ -18,6 +18,7 @@
 #include "chrome/browser/autocomplete/history_url_provider.h"
 #include "chrome/browser/autocomplete/url_prefix.h"
 #include "chrome/browser/bookmarks/bookmark_service.h"
+#include "chrome/browser/omnibox/omnibox_field_trial.h"
 #include "chrome/common/chrome_switches.h"
 #include "content/public/browser/browser_thread.h"
 
@@ -32,6 +33,7 @@ const int ScoredHistoryMatch::kMaxRawTermScore = 30;
 float* ScoredHistoryMatch::raw_term_score_to_topicality_score_ = NULL;
 float* ScoredHistoryMatch::days_ago_to_recency_score_ = NULL;
 bool ScoredHistoryMatch::initialized_ = false;
+int ScoredHistoryMatch::bookmark_value_ = -1;

[Peter Kasting, 2013/11/26 02:33:48]

Thought you were changing this to 1?

[Mark P, 2013/11/26 19:31:57]

I did!  I must've lost the change while rebasing.  Changed again.

 bool ScoredHistoryMatch::also_do_hup_like_scoring_ = false;
 int ScoredHistoryMatch::max_assigned_score_for_non_inlineable_matches_ = -1;
 
@@ -40,6 +42,7 @@ ScoredHistoryMatch::ScoredHistoryMatch()
       can_inline_(false) {
   if (!initialized_) {
     InitializeAlsoDoHUPLikeScoringFieldAndMaxScoreField();
+    InitializeBookmarkValue();

[Peter Kasting, 2013/11/26 02:33:48]

Nit: Is there a lot of value in having multiple short InitializeXXX() functions?
 It seems like maybe we should just have one Init() function (optionally named
InitIfNecessary):

void Init() {
  if (initialized_)
    return;
  ... perform all necessary static initialization
}

This results in less boilerplate in the two constructors.

[Mark P, 2013/11/26 19:31:57]

No, there's practically no value.
Done.

     initialized_ = true;
   }
 }
@@ -57,6 +60,7 @@ ScoredHistoryMatch::ScoredHistoryMatch(const URLRow& row,
       can_inline_(false) {
   if (!initialized_) {
     InitializeAlsoDoHUPLikeScoringFieldAndMaxScoreField();
+    InitializeBookmarkValue();
     initialized_ = true;
   }
 
@@ -150,7 +154,8 @@ ScoredHistoryMatch::ScoredHistoryMatch(const URLRow& row,
 
   const float topicality_score =
       GetTopicalityScore(terms.size(), url, word_starts);
-  const float frecency_score = GetFrecency(now, visits);
+  const float frecency_score = GetFrecency(
+      now, (bookmark_service && bookmark_service->IsBookmarked(gurl)), visits);
   raw_score_ = GetFinalRelevancyScore(topicality_score, frecency_score);
   raw_score_ =
       (raw_score_ <= kint32max) ? static_cast<int>(raw_score_) : kint32max;
@@ -482,6 +487,7 @@ void ScoredHistoryMatch::FillInDaysAgoToRecencyScoreArray() {
 
 // static
 float ScoredHistoryMatch::GetFrecency(const base::Time& now,
+                                      const bool bookmarked,
                                       const VisitInfoVector& visits) {
   // Compute the weighted average |value_of_transition| over the last at
   // most kMaxVisitsToScore visits, where each visit is weighted using
@@ -494,8 +500,10 @@ float ScoredHistoryMatch::GetFrecency(const base::Time& now,
     return 0.0f;
   float summed_visit_points = 0;
   for (int i = 0; i < total_sampled_visits; ++i) {
-    const int value_of_transition =
+    int value_of_transition =
         (visits[i].second == content::PAGE_TRANSITION_TYPED) ? 20 : 1;
+    if (bookmarked && (bookmark_value_ > value_of_transition))

[Peter Kasting, 2013/11/26 02:33:48]

Nit: Simpler?:

    if (bookmarked)
      value_of_transition = std::max(value_of_transition, bookmark_value_);

[Mark P, 2013/11/26 19:31:57]

I don't care either way, so I changed it as you suggested.

+      value_of_transition = bookmark_value_;
     const float bucket_weight =
         GetRecencyScore((now - visits[i].first).InDays());
     summed_visit_points += (value_of_transition * bucket_weight);
@@ -561,4 +569,8 @@ void ScoredHistoryMatch::InitializeAlsoDoHUPLikeScoringFieldAndMaxScoreField() {
   }
 }
 
+void ScoredHistoryMatch::InitializeBookmarkValue() {
+  bookmark_value_ = OmniboxFieldTrial::HQPBookmarkValue();
+}
+
 }  // namespace history