Commit 55f66141 authored by jdonnelly's avatar jdonnelly Committed by Commit bot

[AiS] Remove answer contents from all matches other than the first.

It's possible to get a copy of an answer from previous matches and get the
same answer attached to a similar server-provided answer.  In the future,
we may decide that we want to have different answers attached to multiple
suggestions, but the current assumption is that there should only ever be
one suggestion with an answer.

BUG=

Review URL: https://codereview.chromium.org/603683004

Cr-Commit-Position: refs/heads/master@{#296818}
parent 489d6713
......@@ -3420,3 +3420,32 @@ TEST_F(SearchProviderTest, AnswersCache) {
provider_->prefetch_data_.full_query_text);
EXPECT_EQ(base::ASCIIToUTF16("2334"), provider_->prefetch_data_.query_type);
}
TEST_F(SearchProviderTest, RemoveExtraAnswers) {
ACMatches matches;
AutocompleteMatch match1, match2, match3, match4, match5;
match1.answer_contents = base::ASCIIToUTF16("the answer");
match1.answer_type = base::ASCIIToUTF16("42");
match3.answer_contents = base::ASCIIToUTF16("not to play");
match3.answer_type = base::ASCIIToUTF16("1983");
match5.answer_contents = base::ASCIIToUTF16("a man");
match5.answer_type = base::ASCIIToUTF16("423");
matches.push_back(match1);
matches.push_back(match2);
matches.push_back(match3);
matches.push_back(match4);
matches.push_back(match5);
SearchProvider::RemoveExtraAnswers(&matches);
EXPECT_EQ(base::ASCIIToUTF16("the answer"), matches[0].answer_contents);
EXPECT_EQ(base::ASCIIToUTF16("42"), matches[0].answer_type);
EXPECT_TRUE(matches[1].answer_contents.empty());
EXPECT_TRUE(matches[1].answer_type.empty());
EXPECT_TRUE(matches[2].answer_contents.empty());
EXPECT_TRUE(matches[2].answer_type.empty());
EXPECT_TRUE(matches[3].answer_contents.empty());
EXPECT_TRUE(matches[3].answer_type.empty());
EXPECT_TRUE(matches[4].answer_contents.empty());
EXPECT_TRUE(matches[4].answer_type.empty());
}
......@@ -873,7 +873,7 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() {
// scores, unless we have already accepted AutocompleteResult::kMaxMatches
// higher-scoring matches under the conditions above.
std::sort(matches.begin(), matches.end(), &AutocompleteMatch::MoreRelevant);
matches_.clear();
// Guarantee that if there's a legal default match anywhere in the result
// set that it'll get returned. The rotate() call does this by moving the
// default match to the front of the list.
......@@ -881,6 +881,15 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() {
if (default_match != matches.end())
std::rotate(matches.begin(), default_match, default_match + 1);
// It's possible to get a copy of an answer from previous matches and get the
// same or a different answer to another server-provided suggestion. In the
// future we may decide that we want to have answers attached to multiple
// suggestions, but the current assumption is that there should only ever be
// one suggestion with an answer. To maintain this assumption, remove any
// answers after the first.
RemoveExtraAnswers(&matches);
matches_.clear();
size_t num_suggestions = 0;
for (ACMatches::const_iterator i(matches.begin());
(i != matches.end()) &&
......@@ -908,6 +917,20 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() {
base::TimeTicks::Now() - start_time);
}
void SearchProvider::RemoveExtraAnswers(ACMatches* matches) {
bool answer_seen = false;
for (ACMatches::iterator it = matches->begin(); it != matches->end(); ++it) {
if (!it->answer_contents.empty()) {
if (!answer_seen) {
answer_seen = true;
} else {
it->answer_contents.clear();
it->answer_type.clear();
}
}
}
}
ACMatches::const_iterator SearchProvider::FindTopMatch() const {
ACMatches::const_iterator it = matches_.begin();
while ((it != matches_.end()) && !it->allowed_to_be_default_match)
......
......@@ -11,6 +11,9 @@
#ifndef COMPONENTS_OMNIBOX_SEARCH_PROVIDER_H_
#define COMPONENTS_OMNIBOX_SEARCH_PROVIDER_H_
#include <string>
#include <vector>
#include "base/basictypes.h"
#include "base/compiler_specific.h"
#include "base/memory/scoped_ptr.h"
......@@ -85,6 +88,7 @@ class SearchProvider : public BaseSearchProvider,
FRIEND_TEST_ALL_PREFIXES(SearchProviderTest, SuggestQueryUsesToken);
FRIEND_TEST_ALL_PREFIXES(SearchProviderTest, SessionToken);
FRIEND_TEST_ALL_PREFIXES(SearchProviderTest, AnswersCache);
FRIEND_TEST_ALL_PREFIXES(SearchProviderTest, RemoveExtraAnswers);
FRIEND_TEST_ALL_PREFIXES(AutocompleteProviderTest, GetDestinationURL);
FRIEND_TEST_ALL_PREFIXES(InstantExtendedPrefetchTest, ClearPrefetchedResults);
FRIEND_TEST_ALL_PREFIXES(InstantExtendedPrefetchTest, SetPrefetchQuery);
......@@ -231,6 +235,10 @@ class SearchProvider : public BaseSearchProvider,
// Converts the parsed results to a set of AutocompleteMatches, |matches_|.
void ConvertResultsToAutocompleteMatches();
// Remove answer contents from each match in |matches| other than the first
// that appears.
static void RemoveExtraAnswers(ACMatches* matches);
// Returns an iterator to the first match in |matches_| which might
// be chosen as default.
ACMatches::const_iterator FindTopMatch() const;
......
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