Commit 2579eba5 authored by mgiuca's avatar mgiuca Committed by Commit bot

Minor refactor to support experiments in AppListMixer.

Removed the concept of max_results == 0 meaning "unlimited" in mixer
groups. Now, the omnibox group is specifically considered unlimited (and
all other groups have a defined maximum).

BUG=487494

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

Cr-Commit-Position: refs/heads/master@{#329590}
parent 3e80624d
...@@ -31,7 +31,7 @@ namespace { ...@@ -31,7 +31,7 @@ namespace {
// Maximum number of results to show in each mixer group. // Maximum number of results to show in each mixer group.
const size_t kMaxAppsGroupResults = 4; const size_t kMaxAppsGroupResults = 4;
const size_t kMaxOmniboxResults = 0; // Unlimited. const size_t kMaxOmniboxResults = 0; // Ignored.
const size_t kMaxWebstoreResults = 2; const size_t kMaxWebstoreResults = 2;
const size_t kMaxPeopleResults = 2; const size_t kMaxPeopleResults = 2;
const size_t kMaxSuggestionsResults = 6; const size_t kMaxSuggestionsResults = 6;
......
...@@ -20,9 +20,6 @@ namespace { ...@@ -20,9 +20,6 @@ namespace {
// Maximum number of results to show. // Maximum number of results to show.
const size_t kMaxResults = 6; const size_t kMaxResults = 6;
// A value to indicate no max number of results limit.
const size_t kNoMaxResultsLimit = 0;
void UpdateResult(const SearchResult& source, SearchResult* target) { void UpdateResult(const SearchResult& source, SearchResult* target) {
target->set_display_type(source.display_type()); target->set_display_type(source.display_type());
target->set_title(source.title()); target->set_title(source.title());
...@@ -45,7 +42,7 @@ bool Mixer::SortData::operator<(const SortData& other) const { ...@@ -45,7 +42,7 @@ bool Mixer::SortData::operator<(const SortData& other) const {
return score > other.score; return score > other.score;
} }
// Used to group relevant providers together fox mixing their results. // Used to group relevant providers together for mixing their results.
class Mixer::Group { class Mixer::Group {
public: public:
Group(size_t max_results, double boost) Group(size_t max_results, double boost)
...@@ -98,12 +95,12 @@ class Mixer::Group { ...@@ -98,12 +95,12 @@ class Mixer::Group {
} }
std::sort(results_.begin(), results_.end()); std::sort(results_.begin(), results_.end());
if (max_results_ != kNoMaxResultsLimit && results_.size() > max_results_)
results_.resize(max_results_);
} }
const SortedResults& results() const { return results_; } const SortedResults& results() const { return results_; }
size_t max_results() const { return max_results_; }
private: private:
typedef std::vector<SearchProvider*> Providers; typedef std::vector<SearchProvider*> Providers;
const size_t max_results_; const size_t max_results_;
...@@ -146,12 +143,15 @@ void Mixer::MixAndPublish(bool is_voice_query, ...@@ -146,12 +143,15 @@ void Mixer::MixAndPublish(bool is_voice_query,
SortedResults results; SortedResults results;
results.reserve(kMaxResults); results.reserve(kMaxResults);
// Add results from non-omnibox groups first. // Add results from non-omnibox groups first. Limit to the maximum number of
// results in each group.
for (size_t i = 0; i < groups_.size(); ++i) { for (size_t i = 0; i < groups_.size(); ++i) {
if (!has_omnibox_group_ || i != omnibox_group_) { if (!has_omnibox_group_ || i != omnibox_group_) {
const Group& group = *groups_[i]; const Group& group = *groups_[i];
size_t num_results =
std::min(group.results().size(), group.max_results());
results.insert(results.end(), group.results().begin(), results.insert(results.end(), group.results().begin(),
group.results().end()); group.results().begin() + num_results);
} }
} }
...@@ -161,6 +161,7 @@ void Mixer::MixAndPublish(bool is_voice_query, ...@@ -161,6 +161,7 @@ void Mixer::MixAndPublish(bool is_voice_query,
// Fill the remaining slots with omnibox results. Always add at least one // Fill the remaining slots with omnibox results. Always add at least one
// omnibox result (even if there are no more slots; if we over-fill the // omnibox result (even if there are no more slots; if we over-fill the
// vector, the web store and people results will be removed in a later step). // vector, the web store and people results will be removed in a later step).
// Note: max_results() is ignored for the omnibox group.
if (has_omnibox_group_) { if (has_omnibox_group_) {
CHECK_LT(omnibox_group_, groups_.size()); CHECK_LT(omnibox_group_, groups_.size());
const Group& omnibox_group = *groups_[omnibox_group_]; const Group& omnibox_group = *groups_[omnibox_group_];
......
...@@ -75,7 +75,10 @@ class APP_LIST_EXPORT Mixer { ...@@ -75,7 +75,10 @@ class APP_LIST_EXPORT Mixer {
static void Publish(const SortedResults& results, static void Publish(const SortedResults& results,
AppListModel::SearchResults* ui_results); AppListModel::SearchResults* ui_results);
// Removes duplicates from |results|. // 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); static void RemoveDuplicates(SortedResults* results);
void FetchResults(bool is_voice_query, const KnownResults& known_results); void FetchResults(bool is_voice_query, const KnownResults& known_results);
......
...@@ -21,7 +21,7 @@ namespace test { ...@@ -21,7 +21,7 @@ namespace test {
// Maximum number of results to show in each mixer group. // Maximum number of results to show in each mixer group.
const size_t kMaxAppsGroupResults = 4; const size_t kMaxAppsGroupResults = 4;
const size_t kMaxOmniboxResults = 0; // Unlimited. const size_t kMaxOmniboxResults = 0; // Ignored.
const size_t kMaxWebstoreResults = 2; const size_t kMaxWebstoreResults = 2;
const size_t kMaxPeopleResults = 2; const size_t kMaxPeopleResults = 2;
......
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