Skip to content

GitLab

T
tangled
Commit 9b5c43cf authored by Kevin Bailey's avatar Kevin Bailey Committed by Commit Bot
Browse files
Options

[omnibox] Trim SuggestResult constructor arguments 

Most of the time, we only use the most important arguments when
constructing a SuggestResult, those passed to the Result
sub-constructor. This change adds a constructor for this most often
used case, and also adds methods for setting the answer-related
fields separately.

Change-Id: I2ed077b93b142f90fa16d53d146e6081174f7907
Reviewed-on: https://chromium-review.googlesource.com/1178102Reviewed-by: default avatarMark Pearson <mpearson@chromium.org>
Reviewed-by: default avatarJustin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Kevin Bailey <krb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584729}
parent 3e0dea7b
Show whitespace changes
Inline Side-by-side
Showing with 81 additions and 126 deletions
chrome/browser/autocomplete/search_provider_unittest.cc
View file @ 9b5c43cf
...@@ -3566,21 +3566,8 @@ TEST_F(SearchProviderTest, AnswersCache) { ...@@ -3566,21 +3566,8 @@ TEST_F(SearchProviderTest, AnswersCache) {
base::string16 query = base::ASCIIToUTF16("weather los angeles"); base::string16 query = base::ASCIIToUTF16("weather los angeles");
SearchSuggestionParser::SuggestResult suggest_result( SearchSuggestionParser::SuggestResult suggest_result(
query, AutocompleteMatchType::SEARCH_HISTORY, query, AutocompleteMatchType::SEARCH_HISTORY,
/*subtype_identifier=*/0, /*subtype_identifier=*/0, /*from_keyword_provider=*/false,
/*match_contents=*/query, /*relevance=*/1200, /*relevance_from_server=*/false,
/*match_contents_prefix=*/base::string16(),
/*annotation=*/base::string16(),
/*answer_contents=*/base::string16(),
/*answer_type=*/base::string16(),
/*answer=*/nullptr,
/*suggest_query_params=*/std::string(),
/*deletion_url=*/std::string(),
/*image_dominant_color=*/std::string(),
/*image_url=*/std::string(),
/*from_keyword_provider=*/false,
/*relevance=*/1200,
/*relevance_from_server=*/false,
/*should_prefetch=*/false,
/*input_text=*/query); /*input_text=*/query);
QueryForInput(ASCIIToUTF16("weather l"), false, false); QueryForInput(ASCIIToUTF16("weather l"), false, false);
provider_->transformed_default_history_results_.push_back(suggest_result); provider_->transformed_default_history_results_.push_back(suggest_result);
......
components/omnibox/browser/base_search_provider.cc
View file @ 9b5c43cf
...@@ -158,21 +158,8 @@ AutocompleteMatch BaseSearchProvider::CreateSearchSuggestion( ...@@ -158,21 +158,8 @@ AutocompleteMatch BaseSearchProvider::CreateSearchSuggestion(
// mode. They also assume the caller knows what it's doing and we set // mode. They also assume the caller knows what it's doing and we set
// this match to look as if it was received/created synchronously. // this match to look as if it was received/created synchronously.
SearchSuggestionParser::SuggestResult suggest_result( SearchSuggestionParser::SuggestResult suggest_result(
suggestion, type, suggestion, type, /*subtype_identifier=*/0, from_keyword_provider,
/*subtype_identifier=*/0, /*relevance=*/0, /*relevance_from_server=*/false,
/*match_contents=*/suggestion,
/*match_contents_prefix=*/base::string16(),
/*annotation=*/base::string16(),
/*answer_contents=*/base::string16(),
/*answer_type=*/base::string16(),
/*answer=*/nullptr,
/*suggest_query_params=*/std::string(),
/*deletion_url=*/std::string(),
/*image_dominant_color=*/std::string(),
/*image_url=*/std::string(), from_keyword_provider,
/*relevance=*/0,
/*relevance_from_server=*/false,
/*should_prefetch=*/false,
/*input_text=*/base::string16()); /*input_text=*/base::string16());
suggest_result.set_received_after_last_keystroke(false); suggest_result.set_received_after_last_keystroke(false);
return CreateSearchSuggestion(nullptr, AutocompleteInput(), return CreateSearchSuggestion(nullptr, AutocompleteInput(),
......
components/omnibox/browser/base_search_provider_unittest.cc
View file @ 9b5c43cf
...@@ -109,21 +109,8 @@ TEST_F(BaseSearchProviderTest, PreserveAnswersWhenDeduplicating) { ...@@ -109,21 +109,8 @@ TEST_F(BaseSearchProviderTest, PreserveAnswersWhenDeduplicating) {
SearchSuggestionParser::SuggestResult more_relevant( SearchSuggestionParser::SuggestResult more_relevant(
query, AutocompleteMatchType::SEARCH_HISTORY, query, AutocompleteMatchType::SEARCH_HISTORY,
/*subtype_identifier=*/0, /*subtype_identifier=*/0, /*from_keyword_provider=*/false,
/*match_contents=*/query, /*relevance=*/1300, /*relevance_from_server=*/true,
/*match_contents_prefix=*/base::string16(),
/*annotation=*/base::string16(),
/*answer_contents=*/base::string16(),
/*answer_type=*/base::string16(),
/*answer=*/nullptr,
/*suggest_query_params=*/std::string(),
/*deletion_url=*/std::string(),
/*image_dominant_color=*/std::string(),
/*image_url=*/std::string(),
/*from_keyword_provider=*/false,
/*relevance=*/1300,
/*relevance_from_server=*/true,
/*should_prefetch=*/false,
/*input_text=*/query); /*input_text=*/query);
provider_->AddMatchToMap( provider_->AddMatchToMap(
more_relevant, std::string(), TemplateURLRef::NO_SUGGESTION_CHOSEN, more_relevant, std::string(), TemplateURLRef::NO_SUGGESTION_CHOSEN,
...@@ -131,20 +118,11 @@ TEST_F(BaseSearchProviderTest, PreserveAnswersWhenDeduplicating) { ...@@ -131,20 +118,11 @@ TEST_F(BaseSearchProviderTest, PreserveAnswersWhenDeduplicating) {
SearchSuggestionParser::SuggestResult less_relevant( SearchSuggestionParser::SuggestResult less_relevant(
query, AutocompleteMatchType::SEARCH_SUGGEST, query, AutocompleteMatchType::SEARCH_SUGGEST,
/*subtype_identifier=*/0, /*subtype_identifier=*/0, /*from_keyword_provider=*/false,
/*match_contents=*/query, /*relevance=*/850, /*relevance_from_server=*/true,
/*match_contents_prefix=*/base::string16(),
/*annotation=*/base::string16(), answer_contents, answer_type,
SuggestionAnswer::copy(answer.get()),
/*suggest_query_params=*/std::string(),
/*deletion_url=*/std::string(),
/*image_dominant_color=*/std::string(),
/*image_url=*/std::string(),
/*from_keyword_provider=*/false,
/*relevance=*/850,
/*relevance_from_server=*/true,
/*should_prefetch=*/false,
/*input_text=*/query); /*input_text=*/query);
less_relevant.SetAnswer(answer_contents, answer_type,
SuggestionAnswer::copy(answer.get()));
provider_->AddMatchToMap( provider_->AddMatchToMap(
less_relevant, std::string(), TemplateURLRef::NO_SUGGESTION_CHOSEN, less_relevant, std::string(), TemplateURLRef::NO_SUGGESTION_CHOSEN,
false, false, &map); false, false, &map);
...@@ -174,20 +152,11 @@ TEST_F(BaseSearchProviderTest, PreserveAnswersWhenDeduplicating) { ...@@ -174,20 +152,11 @@ TEST_F(BaseSearchProviderTest, PreserveAnswersWhenDeduplicating) {
answer2->set_type(8242); answer2->set_type(8242);
more_relevant = SearchSuggestionParser::SuggestResult( more_relevant = SearchSuggestionParser::SuggestResult(
query, AutocompleteMatchType::SEARCH_HISTORY, query, AutocompleteMatchType::SEARCH_HISTORY,
/*subtype_identifier=*/0, /*subtype_identifier=*/0, /*from_keyword_provider=*/false,
/*match_contents_prefix=*/query, /*relevance=*/1300, /*relevance_from_server=*/true,
/*annotation=*/base::string16(),
/*answer_contents=*/base::string16(), answer_contents2, answer_type2,
SuggestionAnswer::copy(answer2.get()),
/*suggest_query_params=*/std::string(),
/*deletion_url=*/std::string(),
/*image_dominant_color=*/std::string(),
/*image_url=*/std::string(),
/*from_keyword_provider=*/false,
/*relevance=*/1300,
/*relevance_from_server=*/true,
/*should_prefetch=*/false,
/*input_text=*/query); /*input_text=*/query);
more_relevant.SetAnswer(answer_contents2, answer_type2,
SuggestionAnswer::copy(answer2.get()));
provider_->AddMatchToMap( provider_->AddMatchToMap(
more_relevant, std::string(), TemplateURLRef::NO_SUGGESTION_CHOSEN, more_relevant, std::string(), TemplateURLRef::NO_SUGGESTION_CHOSEN,
false, false, &map); false, false, &map);
...@@ -234,9 +203,6 @@ TEST_F(BaseSearchProviderTest, MatchTailSuggestionProperly) { ...@@ -234,9 +203,6 @@ TEST_F(BaseSearchProviderTest, MatchTailSuggestionProperly) {
/*match_contents=*/query, /*match_contents=*/query,
/*match_contents_prefix=*/base::ASCIIToUTF16("..."), /*match_contents_prefix=*/base::ASCIIToUTF16("..."),
/*annotation=*/base::string16(), /*annotation=*/base::string16(),
/*answer_contents=*/base::string16(),
/*answer_type=*/base::string16(),
/*answer=*/nullptr,
/*suggest_query_params=*/std::string(), /*suggest_query_params=*/std::string(),
/*deletion_url=*/std::string(), /*deletion_url=*/std::string(),
/*image_dominant_color=*/std::string(), /*image_dominant_color=*/std::string(),
......
components/omnibox/browser/search_provider.cc
View file @ 9b5c43cf
...@@ -1006,17 +1006,10 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() { ...@@ -1006,17 +1006,10 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() {
SearchSuggestionParser::SuggestResult verbatim( SearchSuggestionParser::SuggestResult verbatim(
/*suggestion=*/trimmed_verbatim, /*suggestion=*/trimmed_verbatim,
AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED,
/*subtype_identifier=*/0, /*subtype_identifier=*/0, /*from_keyword_provider=*/false,
/*match_contents=*/trimmed_verbatim, verbatim_relevance, relevance_from_server,
/*match_contents_prefix=*/base::string16(),
/*annotation=*/base::string16(), answer_contents, answer_type,
std::move(answer), /*suggest_query_params=*/std::string(),
/*deletion_url=*/std::string(),
/*image_dominant_color=*/std::string(),
/*image_url=*/std::string(),
/*from_keyword_provider=*/false, verbatim_relevance,
relevance_from_server, /*should_prefetch=*/false,
/*input_text=*/trimmed_verbatim); /*input_text=*/trimmed_verbatim);
verbatim.SetAnswer(answer_contents, answer_type, std::move(answer));
AddMatchToMap(verbatim, std::string(), did_not_accept_default_suggestion, AddMatchToMap(verbatim, std::string(), did_not_accept_default_suggestion,
false, keyword_url != nullptr, &map); false, keyword_url != nullptr, &map);
} }
...@@ -1038,20 +1031,8 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() { ...@@ -1038,20 +1031,8 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() {
SearchSuggestionParser::SuggestResult verbatim( SearchSuggestionParser::SuggestResult verbatim(
/*suggestion=*/trimmed_verbatim, /*suggestion=*/trimmed_verbatim,
AutocompleteMatchType::SEARCH_OTHER_ENGINE, AutocompleteMatchType::SEARCH_OTHER_ENGINE,
/*subtype_identifier=*/0, /*subtype_identifier=*/0, /*from_keyword_provider=*/true,
/*match_contents=*/trimmed_verbatim, keyword_verbatim_relevance, keyword_relevance_from_server,
/*match_contents_prefix=*/base::string16(),
/*annotation=*/base::string16(),
/*answer_contents=*/base::string16(),
/*answer_type=*/base::string16(),
/*answer=*/nullptr,
/*suggest_query_params=*/std::string(),
/*deletion_url=*/std::string(),
/*image_dominant_color=*/std::string(),
/*image_url=*/std::string(),
/*from_keyword_provider=*/true, keyword_verbatim_relevance,
keyword_relevance_from_server,
/*should_prefetch=*/false,
/*input_text=*/trimmed_verbatim); /*input_text=*/trimmed_verbatim);
AddMatchToMap(verbatim, std::string(), AddMatchToMap(verbatim, std::string(),
did_not_accept_keyword_suggestion, false, true, &map); did_not_accept_keyword_suggestion, false, true, &map);
...@@ -1234,19 +1215,8 @@ SearchProvider::ScoreHistoryResultsHelper(const HistoryResults& results, ...@@ -1234,19 +1215,8 @@ SearchProvider::ScoreHistoryResultsHelper(const HistoryResults& results,
SearchSuggestionParser::SuggestResult history_suggestion( SearchSuggestionParser::SuggestResult history_suggestion(
/*suggestion=*/trimmed_suggestion, /*suggestion=*/trimmed_suggestion,
AutocompleteMatchType::SEARCH_HISTORY, AutocompleteMatchType::SEARCH_HISTORY,
/*subtype_identifier=*/0, /*subtype_identifier=*/0, is_keyword, relevance,
/*match_contents=*/trimmed_suggestion, /*relevance_from_server=*/false, /*input_text=*/trimmed_input);
/*match_contents_prefix=*/base::string16(),
/*annotation=*/base::string16(),
/*answer_contents=*/base::string16(),
/*answer_type=*/base::string16(),
/*answer=*/nullptr,
/*suggest_query_params=*/std::string(),
/*deletion_url=*/std::string(),
/*image_dominant_color=*/std::string(),
/*image_url=*/std::string(), is_keyword, relevance,
/*relevance_from_server=*/false,
/*should_prefetch=*/false, /*input_text=*/trimmed_input);
// History results are synchronous; they are received on the last keystroke. // History results are synchronous; they are received on the last keystroke.
history_suggestion.set_received_after_last_keystroke(false); history_suggestion.set_received_after_last_keystroke(false);
scored_results.insert(insertion_position, history_suggestion); scored_results.insert(insertion_position, history_suggestion);
......
components/omnibox/browser/search_suggestion_parser.cc
View file @ 9b5c43cf
...@@ -78,6 +78,30 @@ SearchSuggestionParser::Result::~Result() {} ...@@ -78,6 +78,30 @@ SearchSuggestionParser::Result::~Result() {}
// SearchSuggestionParser::SuggestResult --------------------------------------- // SearchSuggestionParser::SuggestResult ---------------------------------------
SearchSuggestionParser::SuggestResult::SuggestResult(
const base::string16& suggestion,
AutocompleteMatchType::Type type,
int subtype_identifier,
bool from_keyword_provider,
int relevance,
bool relevance_from_server,
const base::string16& input_text)
: SuggestResult(suggestion,
type,
subtype_identifier,
suggestion,
/*match_contents_prefix=*/base::string16(),
/*annotation=*/base::string16(),
/*suggest_query_params=*/"",
/*deletion_url=*/"",
/*image_dominant_color=*/"",
/*image_url=*/"",
from_keyword_provider,
relevance,
relevance_from_server,
/*should_prefetch=*/false,
input_text) {}
SearchSuggestionParser::SuggestResult::SuggestResult( SearchSuggestionParser::SuggestResult::SuggestResult(
const base::string16& suggestion, const base::string16& suggestion,
AutocompleteMatchType::Type type, AutocompleteMatchType::Type type,
...@@ -85,9 +109,6 @@ SearchSuggestionParser::SuggestResult::SuggestResult( ...@@ -85,9 +109,6 @@ SearchSuggestionParser::SuggestResult::SuggestResult(
const base::string16& match_contents, const base::string16& match_contents,
const base::string16& match_contents_prefix, const base::string16& match_contents_prefix,
const base::string16& annotation, const base::string16& annotation,
const base::string16& answer_contents,
const base::string16& answer_type,
std::unique_ptr<SuggestionAnswer> answer,
const std::string& suggest_query_params, const std::string& suggest_query_params,
const std::string& deletion_url, const std::string& deletion_url,
const std::string& image_dominant_color, const std::string& image_dominant_color,
...@@ -107,9 +128,6 @@ SearchSuggestionParser::SuggestResult::SuggestResult( ...@@ -107,9 +128,6 @@ SearchSuggestionParser::SuggestResult::SuggestResult(
match_contents_prefix_(match_contents_prefix), match_contents_prefix_(match_contents_prefix),
annotation_(annotation), annotation_(annotation),
suggest_query_params_(suggest_query_params), suggest_query_params_(suggest_query_params),
answer_contents_(answer_contents),
answer_type_(answer_type),
answer_(std::move(answer)),
image_dominant_color_(image_dominant_color), image_dominant_color_(image_dominant_color),
image_url_(image_url), image_url_(image_url),
should_prefetch_(should_prefetch) { should_prefetch_(should_prefetch) {
...@@ -228,6 +246,15 @@ void SearchSuggestionParser::SuggestResult::ClassifyMatchContents( ...@@ -228,6 +246,15 @@ void SearchSuggestionParser::SuggestResult::ClassifyMatchContents(
} }
} }
void SearchSuggestionParser::SuggestResult::SetAnswer(
const base::string16& answer_contents,
const base::string16& answer_type,
std::unique_ptr<SuggestionAnswer> answer) {
answer_contents_ = answer_contents;
answer_type_ = answer_type;
answer_ = std::move(answer);
}
int SearchSuggestionParser::SuggestResult::CalculateRelevance( int SearchSuggestionParser::SuggestResult::CalculateRelevance(
const AutocompleteInput& input, const AutocompleteInput& input,
bool keyword_provider_requested) const { bool keyword_provider_requested) const {
...@@ -601,12 +628,23 @@ bool SearchSuggestionParser::ParseSuggestResults( ...@@ -601,12 +628,23 @@ bool SearchSuggestionParser::ParseSuggestResults(
bool should_prefetch = static_cast<int>(index) == prefetch_index; bool should_prefetch = static_cast<int>(index) == prefetch_index;
results->suggest_results.push_back(SuggestResult( results->suggest_results.push_back(SuggestResult(
base::CollapseWhitespace(suggestion, false), match_type, base::CollapseWhitespace(suggestion, false),
subtype_identifier, base::CollapseWhitespace(match_contents, false), match_type,
match_contents_prefix, annotation, answer_contents, answer_type_str, subtype_identifier,
std::move(answer), suggest_query_params, deletion_url, base::CollapseWhitespace(match_contents, false),
image_dominant_color, image_url, is_keyword_result, relevance, match_contents_prefix,
relevances != nullptr, should_prefetch, trimmed_input)); annotation,
suggest_query_params,
deletion_url,
image_dominant_color,
image_url,
is_keyword_result,
relevance,
relevances != nullptr,
should_prefetch,
trimmed_input));
results->suggest_results.back().SetAnswer(answer_contents,
answer_type_str, std::move(answer));
} }
} }
results->relevances_from_server = relevances != nullptr; results->relevances_from_server = relevances != nullptr;
......
components/omnibox/browser/search_suggestion_parser.h
View file @ 9b5c43cf
...@@ -121,15 +121,19 @@ class SearchSuggestionParser { ...@@ -121,15 +121,19 @@ class SearchSuggestionParser {
class SuggestResult : public Result { class SuggestResult : public Result {
public: public:
SuggestResult(const base::string16& suggestion,
AutocompleteMatchType::Type type,
int subtype_identifier,
bool from_keyword_provider,
int relevance,
bool relevance_from_server,
const base::string16& input_text);
SuggestResult(const base::string16& suggestion, SuggestResult(const base::string16& suggestion,
AutocompleteMatchType::Type type, AutocompleteMatchType::Type type,
int subtype_identifier, int subtype_identifier,
const base::string16& match_contents, const base::string16& match_contents,
const base::string16& match_contents_prefix, const base::string16& match_contents_prefix,
const base::string16& annotation, const base::string16& annotation,
const base::string16& answer_contents,
const base::string16& answer_type,
std::unique_ptr<SuggestionAnswer> answer,
const std::string& suggest_query_params, const std::string& suggest_query_params,
const std::string& deletion_url, const std::string& deletion_url,
const std::string& image_dominant_color, const std::string& image_dominant_color,
...@@ -153,6 +157,9 @@ class SearchSuggestionParser { ...@@ -153,6 +157,9 @@ class SearchSuggestionParser {
return suggest_query_params_; return suggest_query_params_;
} }
void SetAnswer(const base::string16& answer_contents,
const base::string16& answer_type,
std::unique_ptr<SuggestionAnswer> answer);
const base::string16& answer_contents() const { return answer_contents_; } const base::string16& answer_contents() const { return answer_contents_; }
const base::string16& answer_type() const { return answer_type_; } const base::string16& answer_type() const { return answer_type_; }
const SuggestionAnswer* answer() const { return answer_.get(); } const SuggestionAnswer* answer() const { return answer_.get(); }
......
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