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_service.h"
 #include "components/bookmarks/browser/bookmark_utils.h"
 #include "content/public/browser/browser_thread.h"
 
+namespace {
+
+// These magic numbers are duplicated from HistoryURLProvider
+const int kScoreForBestInlineableResult = 1413;
+const int kScoreForUnvisitedIntranetResult = 1403;
+const int kScoreForWhatYouTypedResult = 1203;
+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;
@@ skipping 121 matching lines @@
   raw_score_ = GetFinalRelevancyScore(topicality_score, frecency_score);
   raw_score_ =
       (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) ||

[blundell, 2014/05/26 06:58:35]

This whole function feels like it should be abstracted through the client.
Sylvain, had you thought about that?

[sdefresne, 2014/05/30 15:22:25]

No, I had not thought of that, but it makes sense to abstract through the client
those few lines (not the whole function since it is the constructor for
ScoredHistoryMatch, but the line computing the hup_like_score).

Once https://codereview.chromium.org/305443004/ lands, then you should be able
to add a HUPLikeScore method to HistoryClient, implement it in
ChromeHistoryClient and call it from here (note that the HistoryClient may be
NULL during testing).

[blundell, 2014/06/02 11:11:49]

Note: I think that we should come up with a more abstract name than
HUPLikeScore. Is it about giving the client a chance to adjust the score?

On 2014/05/30 15:22:25, sdefresne wrote:

         (IsHostOnly() && (row.typed_count() == 1));
     int hup_like_score = promote_to_inline ?
-        HistoryURLProvider::kScoreForBestInlineableResult :
-        HistoryURLProvider::kBaseScoreForNonInlineableResult;
+        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
@@ skipping 398 matching lines @@
   also_do_hup_like_scoring_ = false;
   // 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_) {

[blundell, 2014/05/26 06:58:35]

Am I missing something, or is this dead code? Line 597 sets this variable to
false.

     max_assigned_score_for_non_inlineable_matches_ =
-        HistoryURLProvider::kScoreForBestInlineableResult - 1;
+        kScoreForBestInlineableResult - 1;
   }
   bookmark_value_ = OmniboxFieldTrial::HQPBookmarkValue();
   allow_tld_matches_ = OmniboxFieldTrial::HQPAllowMatchInTLDValue();
   allow_scheme_matches_ = OmniboxFieldTrial::HQPAllowMatchInSchemeValue();
   initialized_ = true;
 }
 
 }  // namespace history