Code clean up for ScoredHistoryMatch

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 e713b2575df6caf2d206ec399eb901b3bd96db01..9014d1599f02f2fdd75384a37e7a04d207b24e1d 100644
--- a/chrome/browser/history/scored_history_match.cc
+++ b/chrome/browser/history/scored_history_match.cc
@@ -16,13 +16,22 @@
 #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 ----------------------------------------------------------
@@ -167,8 +176,8 @@ ScoredHistoryMatch::ScoredHistoryMatch(
     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
@@ -176,7 +185,7 @@ ScoredHistoryMatch::ScoredHistoryMatch(
     // 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
@@ -597,7 +606,7 @@ void ScoredHistoryMatch::Init() {
   // 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();