Commit 61437c92 authored by Kevin Bailey's avatar Kevin Bailey Committed by Commit Bot

[omnibox] Factor HistoryQuickProvider::DoAutocomplete()

There is a large piece of code within |HQP::DoAutocomplete()| that can
be on its own trivially. This change moves it to another method, and
should not change behavior. This is to facilitate a downstream change.

Bug: 
Change-Id: Ifdbbac697aee1efd0f42068f2e0825bb176a8076
Reviewed-on: https://chromium-review.googlesource.com/747064
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: default avatarJustin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513013}
parent a3774c16
...@@ -74,6 +74,24 @@ void HistoryQuickProvider::DoAutocomplete() { ...@@ -74,6 +74,24 @@ void HistoryQuickProvider::DoAutocomplete() {
if (matches.empty()) if (matches.empty())
return; return;
// Loop over every result and add it to matches_. In the process,
// guarantee that scores are decreasing. |max_match_score| keeps
// track of the highest score we can assign to any later results we
// see.
int max_match_score = FindMaxMatchScore(matches);
for (ScoredHistoryMatches::const_iterator match_iter = matches.begin();
match_iter != matches.end(); ++match_iter) {
const ScoredHistoryMatch& history_match(*match_iter);
// Set max_match_score to the score we'll assign this result.
max_match_score = std::min(max_match_score, history_match.raw_score);
matches_.push_back(QuickMatchToACMatch(history_match, max_match_score));
// Mark this max_match_score as being used.
max_match_score--;
}
}
int HistoryQuickProvider::FindMaxMatchScore(
const ScoredHistoryMatches& matches) {
// Figure out if HistoryURL provider has a URL-what-you-typed match // Figure out if HistoryURL provider has a URL-what-you-typed match
// that ought to go first and what its score will be. // that ought to go first and what its score will be.
bool will_have_url_what_you_typed_match_first = false; bool will_have_url_what_you_typed_match_first = false;
...@@ -160,30 +178,17 @@ void HistoryQuickProvider::DoAutocomplete() { ...@@ -160,30 +178,17 @@ void HistoryQuickProvider::DoAutocomplete() {
} }
} }
} }
// Return a |max_match_score| that is the raw score for the first match, but
// Loop over every result and add it to matches_. In the process, // reduce it if we think there will be a URL-what-you-typed match. (We want
// guarantee that scores are decreasing. |max_match_score| keeps // URL-what-you-typed matches for visited URLs to beat out any longer URLs, no
// track of the highest score we can assign to any later results we // matter how frequently they're visited.) The strength of this reduction
// see. Also, reduce |max_match_score| if we think there will be // depends on the likely score for the URL-what-you-typed result.
// a URL-what-you-typed match. (We want URL-what-you-typed matches for
// visited URLs to beat out any longer URLs, no matter how frequently
// they're visited.) The strength of this reduction depends on the
// likely score for the URL-what-you-typed result.
int max_match_score = matches.begin()->raw_score; int max_match_score = matches.begin()->raw_score;
if (will_have_url_what_you_typed_match_first) { if (will_have_url_what_you_typed_match_first) {
max_match_score = std::min(max_match_score, max_match_score = std::min(max_match_score,
url_what_you_typed_match_score - 1); url_what_you_typed_match_score - 1);
} }
for (ScoredHistoryMatches::const_iterator match_iter = matches.begin(); return max_match_score;
match_iter != matches.end(); ++match_iter) {
const ScoredHistoryMatch& history_match(*match_iter);
// Set max_match_score to the score we'll assign this result.
max_match_score = std::min(max_match_score, history_match.raw_score);
matches_.push_back(QuickMatchToACMatch(history_match, max_match_score));
// Mark this max_match_score as being used.
max_match_score--;
}
} }
AutocompleteMatch HistoryQuickProvider::QuickMatchToACMatch( AutocompleteMatch HistoryQuickProvider::QuickMatchToACMatch(
......
...@@ -47,6 +47,10 @@ class HistoryQuickProvider : public HistoryProvider { ...@@ -47,6 +47,10 @@ class HistoryQuickProvider : public HistoryProvider {
// Performs the autocomplete matching and scoring. // Performs the autocomplete matching and scoring.
void DoAutocomplete(); void DoAutocomplete();
// Calculates the initial max match score for applying to matches, lowering
// it if we believe that there will be a URL-what-you-typed match.
int FindMaxMatchScore(const ScoredHistoryMatches& matches);
// Creates an AutocompleteMatch from |history_match|, assigning it // Creates an AutocompleteMatch from |history_match|, assigning it
// the score |score|. // the score |score|.
AutocompleteMatch QuickMatchToACMatch(const ScoredHistoryMatch& history_match, AutocompleteMatch QuickMatchToACMatch(const ScoredHistoryMatch& history_match,
......
...@@ -488,10 +488,10 @@ void HistoryURLProvider::Start(const AutocompleteInput& input, ...@@ -488,10 +488,10 @@ void HistoryURLProvider::Start(const AutocompleteInput& input,
fixed_up_input, fixed_up_input.canonicalized_url(), trim_http)); fixed_up_input, fixed_up_input.canonicalized_url(), trim_http));
what_you_typed_match.relevance = CalculateRelevance(WHAT_YOU_TYPED, 0); what_you_typed_match.relevance = CalculateRelevance(WHAT_YOU_TYPED, 0);
// Add the WYT match as a fallback in case we can't get the history service or // Add the what-you-typed match as a fallback in case we can't get the history
// URL DB; otherwise, we'll replace this match lower down. Don't do this for // service or URL DB; otherwise, we'll replace this match lower down. Don't
// queries, though -- while we can sometimes mark up a match for them, it's // do this for queries, though -- while we can sometimes mark up a match for
// not what the user wants, and just adds noise. // them, it's not what the user wants, and just adds noise.
if (fixed_up_input.type() != metrics::OmniboxInputType::QUERY) if (fixed_up_input.type() != metrics::OmniboxInputType::QUERY)
matches_.push_back(what_you_typed_match); matches_.push_back(what_you_typed_match);
...@@ -799,7 +799,8 @@ void HistoryURLProvider::DoAutocomplete(history::HistoryBackend* backend, ...@@ -799,7 +799,8 @@ void HistoryURLProvider::DoAutocomplete(history::HistoryBackend* backend,
// what-you-typed match to be added in this case. See comments in // what-you-typed match to be added in this case. See comments in
// PromoteMatchesIfNecessary(). // PromoteMatchesIfNecessary().
// * Otherwise, we should have some sort of QUERY or UNKNOWN input that // * Otherwise, we should have some sort of QUERY or UNKNOWN input that
// the SearchProvider will provide a defaultable WYT match for. // the SearchProvider will provide a defaultable what-you-typed match
// for.
params->promote_type = HistoryURLProviderParams::FRONT_HISTORY_MATCH; params->promote_type = HistoryURLProviderParams::FRONT_HISTORY_MATCH;
} else { } else {
// Failed to promote any URLs. Use the What You Typed match, if we have it. // Failed to promote any URLs. Use the What You Typed match, if we have it.
......
...@@ -610,8 +610,8 @@ TEST_F(HistoryURLProviderTest, CullRedirects) { ...@@ -610,8 +610,8 @@ TEST_F(HistoryURLProviderTest, CullRedirects) {
} }
TEST_F(HistoryURLProviderTestNoSearchProvider, WhatYouTypedNoSearchProvider) { TEST_F(HistoryURLProviderTestNoSearchProvider, WhatYouTypedNoSearchProvider) {
// When no search provider is available, make sure we provide WYT matches // When no search provider is available, make sure we provide what-you-typed
// for text that could be a URL. // matches for text that could be a URL.
const UrlAndLegalDefault results_1[] = { const UrlAndLegalDefault results_1[] = {
{ "http://wytmatch/", true } { "http://wytmatch/", true }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment