Commit 96a264cf authored by tby's avatar tby Committed by Commit Bot

[Launcher streamlining] Remove RemoveDuplicates

The RemoveDuplicates step of mixing is a noop by design, because every
provider returns different IDs because they use different schemas. This
CL removes it, no user-visible change to results.

Bug: 1028447
Change-Id: I900f371c819cd76ec455c50a44b4e3e395e931c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2473317Reviewed-by: default avatarThanh Nguyen <thanhdng@chromium.org>
Commit-Queue: Tony Yeoman <tby@chromium.org>
Cr-Commit-Position: refs/heads/master@{#817360}
parent 9a089aed
......@@ -122,11 +122,6 @@ void Mixer::MixAndPublish(size_t num_max_results, const base::string16& query) {
results.insert(results.end(), group->results().begin(),
group->results().begin() + num_results);
}
// Remove results with duplicate IDs before sorting. If two providers give a
// result with the same ID, the result from the provider with the *lower group
// number* will be kept (e.g., an app result takes priority over a web store
// result with the same ID).
RemoveDuplicates(&results);
// Zero state search results: if any search provider won't have any results
// displayed, but has a high-scoring result that the user hasn't seen many
......@@ -144,13 +139,11 @@ void Mixer::MixAndPublish(size_t num_max_results, const base::string16& query) {
const size_t original_size = results.size();
if (original_size < num_max_results) {
// We didn't get enough results. Insert all the results again, and this
// time, do not limit the maximum number of results from each group. (This
// will result in duplicates, which will be removed by RemoveDuplicates.)
// time, do not limit the maximum number of results from each group.
for (const auto& group : groups_) {
results.insert(results.end(), group->results().begin(),
group->results().end());
}
RemoveDuplicates(&results);
// Sort just the newly added results. This ensures that, for example, if
// there are 6 Omnibox results (score = 0.8) and 1 People result (score =
// 0.4) that the People result will be 5th, not 7th, because the Omnibox
......@@ -167,21 +160,6 @@ void Mixer::MixAndPublish(size_t num_max_results, const base::string16& query) {
model_updater_->PublishSearchResults(new_results);
}
void Mixer::RemoveDuplicates(SortedResults* results) {
SortedResults final;
final.reserve(results->size());
std::set<std::string> id_set;
for (const SortData& sort_data : *results) {
if (!id_set.insert(sort_data.result->id()).second)
continue;
final.emplace_back(sort_data);
}
results->swap(final);
}
void Mixer::FetchResults(const base::string16& query) {
if (search_result_ranker_)
search_result_ranker_->FetchRankings(query);
......
......@@ -88,12 +88,6 @@ class Mixer {
class Group;
typedef std::vector<std::unique_ptr<Group>> Groups;
// Removes entries from |results| with duplicate IDs. When two or more results
// have the same ID, the earliest one in the |results| list is kept.
// NOTE: This is not necessarily the one with the highest *score*, as
// |results| may not have been sorted yet.
static void RemoveDuplicates(SortedResults* results);
void FetchResults(const base::string16& query);
AppListModelUpdater* const model_updater_; // Not owned.
......
......@@ -232,28 +232,5 @@ TEST_F(MixerTest, ResultsWithDisplayIndex) {
GetResults());
}
TEST_F(MixerTest, RemoveDuplicates) {
CreateMixer();
const std::string dup = "dup";
// This gives "dup0,dup1,dup2".
app_provider()->set_prefix(dup);
app_provider()->set_count(3);
// This gives "dup0,dup1".
omnibox_provider()->set_prefix(dup);
omnibox_provider()->set_count(2);
// This gives "dup0".
playstore_provider()->set_prefix(dup);
playstore_provider()->set_count(1);
RunQuery();
// Only three results with unique id are kept.
EXPECT_EQ("dup0,dup1,dup2", GetResults());
}
} // namespace test
} // namespace app_list
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