Code clean up for ScoredHistoryMatch

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>
 #include <set>
 
 #include <math.h>
 
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
-#include "chrome/browser/autocomplete/history_url_provider.h"
 #include "chrome/browser/autocomplete/url_prefix.h"
 #include "chrome/browser/omnibox/omnibox_field_trial.h"
 #include "components/bookmarks/browser/bookmark_utils.h"
 #include "components/history/core/browser/history_client.h"
 #include "content/public/browser/browser_thread.h"
 
+namespace {
+
+// These magic numbers are duplicated from HistoryURLProvider
+const int kScoreForBestInlineableResult = 1413;
+const int kBaseScoreForNonInlineableResult = 900;
+
+}  // namespace
+
 namespace history {
 
 // ScoredHistoryMatch ----------------------------------------------------------
 
 // static
 const size_t ScoredHistoryMatch::kMaxVisitsToScore = 10;
 const int ScoredHistoryMatch::kDaysToPrecomputeRecencyScoresFor = 366;
 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;
 bool ScoredHistoryMatch::allow_tld_matches_ = false;
 bool ScoredHistoryMatch::allow_scheme_matches_ = false;
 bool ScoredHistoryMatch::also_do_hup_like_scoring_ = false;
-int ScoredHistoryMatch::max_assigned_score_for_non_inlineable_matches_ = -1;
 
 ScoredHistoryMatch::ScoredHistoryMatch()
     : raw_score_(0),
       can_inline_(false) {
   Init();
 }
 
 ScoredHistoryMatch::ScoredHistoryMatch(
     const URLRow& row,
     const VisitInfoVector& visits,
@@ skipping 107 matching lines @@
       (raw_score_ <= kint32max) ? static_cast<int>(raw_score_) : kint32max;
 
   if (also_do_hup_like_scoring_ && can_inline_) {
     // HistoryURL-provider-like scoring gives any match that is
     // capable of being inlined a certain minimum score.  Some of these
     // are given a higher score that lets them be shown in inline.
     // This test here derives from the test in
     // HistoryURLProvider::PromoteMatchForInlineAutocomplete().
     const bool promote_to_inline = (row.typed_count() > 1) ||
         (IsHostOnly() && (row.typed_count() == 1));
-    int hup_like_score = promote_to_inline ?
-        HistoryURLProvider::kScoreForBestInlineableResult :
-        HistoryURLProvider::kBaseScoreForNonInlineableResult;
+    int hup_like_score = promote_to_inline ? kScoreForBestInlineableResult
+                                           : kBaseScoreForNonInlineableResult;
 
     // Also, if the user types the hostname of a host with a typed
     // visit, then everything from that host get given inlineable scores
     // (because the URL-that-you-typed will go first and everything
     // else will be assigned one minus the previous score, as coded
     // at the end of HistoryURLProvider::DoAutocomplete().
     if (base::UTF8ToUTF16(gurl.host()) == terms[0])
-      hup_like_score = HistoryURLProvider::kScoreForBestInlineableResult;
+      hup_like_score = kScoreForBestInlineableResult;
 
     // HistoryURLProvider has the function PromoteOrCreateShorterSuggestion()
     // that's meant to promote prefixes of the best match (if they've
     // been visited enough related to the best match) or
     // create/promote host-only suggestions (even if they've never
     // been typed).  The code is complicated and we don't try to
     // duplicate the logic here.  Instead, we handle a simple case: in
     // low-typed-count ranges, give host-only matches (i.e.,
     // http://www.foo.com/ vs. http://www.foo.com/bar.html) a boost so
     // that the host-only match outscores all the other matches that
     // would normally have the same base score.  This behavior is not
     // identical to what happens in HistoryURLProvider even in these
     // low typed count ranges--sometimes it will create/promote when
     // this test does not (indeed, we cannot create matches like HUP
     // can) and vice versa--but the underlying philosophy is similar.
     if (!promote_to_inline && IsHostOnly())
       hup_like_score++;
 
     // All the other logic to goes into hup-like-scoring happens in
     // the tie-breaker case of MatchScoreGreater().
 
     // Incorporate hup_like_score into raw_score.
     raw_score_ = std::max(raw_score_, hup_like_score);
   }
 
-  // If this match is not inlineable and there's a cap on the maximum

[blundell, 2014/06/11 18:17:36]

Why did this code go away?

[nshaik, 2014/06/11 20:46:50]

That code is applicable only if max_assigned_score_for_non_inlineable_matches_
!= -1
which is -1 always as also_do_hup_like_scoring_ = false.

On 2014/06/11 18:17:36, blundell wrote:

-  // score that can be given to non-inlineable matches, apply the cap.
-  if (!can_inline_ && (max_assigned_score_for_non_inlineable_matches_ != -1)) {
-    raw_score_ = std::min(max_assigned_score_for_non_inlineable_matches_,
-                          raw_score_);
-  }
-
   // Now that we're done processing this entry, correct the offsets of the
   // matches in |url_matches_| so they point to offsets in the original URL
   // spec, not the cleaned-up URL string that we used for matching.
   std::vector<size_t> offsets = OffsetsFromTermMatches(url_matches_);
   base::OffsetAdjuster::UnadjustOffsets(adjustments, &offsets);
   url_matches_ = ReplaceOffsetsInTermMatches(url_matches_, offsets);
 }
 
 ScoredHistoryMatch::~ScoredHistoryMatch() {}
 
@@ skipping 356 matching lines @@
   // in the URL and/or title and that are visited practically all
   // the time using typed visits.  We don't attempt to distinguish
   // between these very good results.)
   const float slope = (1399 - 1300) / (20.0f - 12.0f);
   return std::min(1399.0, 1300 + slope * (intermediate_score - 12.0));
 }
 
 void ScoredHistoryMatch::Init() {
   if (initialized_)
     return;
-  also_do_hup_like_scoring_ = false;

[blundell, 2014/06/11 18:17:36]

This assignment has to stay I would think?

[nshaik, 2014/06/11 20:46:50]

It is always assigned to false. So I deleted it.
I thought this is some legacy code left for clean up

I will On 2014/06/11 18:17:36, blundell wrote:

-  // When doing HUP-like scoring, don't allow a non-inlineable match
-  // to beat the score of good inlineable matches.  This is a problem
-  // because if a non-inlineable match ends up with the highest score
-  // from HistoryQuick provider, all HistoryQuick matches get demoted
-  // to non-inlineable scores (scores less than 1200).  Without
-  // HUP-like-scoring, these results would actually come from the HUP
-  // and not be demoted, thus outscoring the demoted HQP results.
-  // When the HQP provides these, we need to clamp the non-inlineable
-  // results to preserve this behavior.
-  if (also_do_hup_like_scoring_) {
-    max_assigned_score_for_non_inlineable_matches_ =
-        HistoryURLProvider::kScoreForBestInlineableResult - 1;
-  }
   bookmark_value_ = OmniboxFieldTrial::HQPBookmarkValue();
   allow_tld_matches_ = OmniboxFieldTrial::HQPAllowMatchInTLDValue();
   allow_scheme_matches_ = OmniboxFieldTrial::HQPAllowMatchInSchemeValue();
   initialized_ = true;
 }
 
 }  // namespace history