Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(218)

Unified Diff: ui/app_list/search/mixer.cc

Issue 1136363003: Remove AppListMixer field trial. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@applist-mixer-priority-finch-test
Patch Set: Remove old comment. Created 4 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « ui/app_list/search/mixer.h ('k') | ui/app_list/search/mixer_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: ui/app_list/search/mixer.cc
diff --git a/ui/app_list/search/mixer.cc b/ui/app_list/search/mixer.cc
index e49f3ba4dd06aa283e9aee9d95b3259bad406619..f5b7e998d1a5d4adee4245ced1101207a3db8df6 100644
--- a/ui/app_list/search/mixer.cc
+++ b/ui/app_list/search/mixer.cc
@@ -10,10 +10,7 @@
#include <string>
#include <vector>
-#include "base/command_line.h"
#include "base/macros.h"
-#include "base/metrics/field_trial.h"
-#include "ui/app_list/app_list_switches.h"
#include "ui/app_list/search_provider.h"
#include "ui/app_list/search_result.h"
@@ -21,19 +18,8 @@ namespace app_list {
namespace {
-// Maximum number of results to show. Ignored if the AppListMixer field trial is
-// "Blended".
-const size_t kMaxResults = 6;
-
-// The minimum number of results to show, if the AppListMixer field trial is
-// "Blended". If this quota is not reached, the per-group limitations are
-// removed and we try again. (We may still not reach the minumum, but at least
-// we tried.) Ignored if the field trial is off.
-const size_t kMinBlendedResults = 6;
-
-const char kAppListMixerFieldTrialName[] = "AppListMixer";
-const char kAppListMixerFieldTrialEnabled[] = "Blended";
-const char kAppListMixerFieldTrialDisabled[] = "Control";
+// Maximum number of results to show.
+const size_t kMinResults = 6;
void UpdateResult(const SearchResult& source, SearchResult* target) {
target->set_display_type(source.display_type());
@@ -43,37 +29,6 @@ void UpdateResult(const SearchResult& source, SearchResult* target) {
target->set_details_tags(source.details_tags());
}
-// Returns true if the "AppListMixer" trial is set to "Blended". This is an
-// experiment on the new Mixer logic that allows results from different groups
-// to be blended together, rather than stratified.
-bool IsBlendedMixerTrialEnabled() {
- // Note: It's important to query the field trial state first, to ensure that
- // UMA reports the correct group.
- const std::string group_name =
- base::FieldTrialList::FindFullName(kAppListMixerFieldTrialName);
-
- // Respect command-line flags first.
- if (base::CommandLine::ForCurrentProcess()->HasSwitch(
- switches::kDisableNewAppListMixer)) {
- return false;
- }
-
- if (base::CommandLine::ForCurrentProcess()->HasSwitch(
- switches::kEnableNewAppListMixer)) {
- return true;
- }
-
- // Next, respect field-trial groups.
- if (group_name == kAppListMixerFieldTrialEnabled)
- return true;
-
- if (group_name == kAppListMixerFieldTrialDisabled)
- return false;
-
- // By default, enable the new logic if the experimental app list is enabled.
- return app_list::switches::IsExperimentalAppListEnabled();
-}
-
} // namespace
Mixer::SortData::SortData() : result(NULL), score(0.0) {
@@ -91,8 +46,8 @@ bool Mixer::SortData::operator<(const SortData& other) const {
// Used to group relevant providers together for mixing their results.
class Mixer::Group {
public:
- Group(size_t max_results, double boost, double multiplier)
- : max_results_(max_results), boost_(boost), multiplier_(multiplier) {}
+ Group(size_t max_results, double multiplier)
+ : max_results_(max_results), multiplier_(multiplier) {}
~Group() {}
void AddProvider(SearchProvider* provider) { providers_.push_back(provider); }
@@ -110,7 +65,7 @@ class Mixer::Group {
double relevance = std::min(std::max(result->relevance(), 0.0), 1.0);
double multiplier = multiplier_;
- double boost = boost_;
+ double boost = 0.0;
// Recommendations should not be affected by query-to-launch correlation
// from KnownResults as it causes recommendations to become dominated by
@@ -159,7 +114,6 @@ class Mixer::Group {
private:
typedef std::vector<SearchProvider*> Providers;
const size_t max_results_;
- const double boost_;
const double multiplier_;
Providers providers_; // Not owned.
@@ -174,28 +128,11 @@ Mixer::Mixer(AppListModel::SearchResults* ui_results)
Mixer::~Mixer() {
}
-size_t Mixer::AddGroup(size_t max_results, double boost, double multiplier) {
- // Only consider |boost| if the AppListMixer field trial is default.
- // Only consider |multiplier| if the AppListMixer field trial is "Blended".
- if (IsBlendedMixerTrialEnabled())
- boost = 0.0;
- else
- multiplier = 1.0;
- groups_.push_back(new Group(max_results, boost, multiplier));
+size_t Mixer::AddGroup(size_t max_results, double multiplier) {
+ groups_.push_back(new Group(max_results, multiplier));
return groups_.size() - 1;
}
-size_t Mixer::AddOmniboxGroup(size_t max_results,
- double boost,
- double multiplier) {
- // There should not already be an omnibox group.
- DCHECK(!has_omnibox_group_);
- size_t id = AddGroup(max_results, boost, multiplier);
- omnibox_group_ = id;
- has_omnibox_group_ = true;
- return id;
-}
-
void Mixer::AddProviderToGroup(size_t group_id, SearchProvider* provider) {
groups_[group_id]->AddProvider(provider);
}
@@ -205,78 +142,39 @@ void Mixer::MixAndPublish(bool is_voice_query,
FetchResults(is_voice_query, known_results);
SortedResults results;
-
- if (IsBlendedMixerTrialEnabled()) {
- results.reserve(kMinBlendedResults);
-
- // Add results from each group. Limit to the maximum number of results in
- // each group.
+ results.reserve(kMinResults);
+
+ // Add results from each group. Limit to the maximum number of results in each
+ // group.
+ for (const Group* group : groups_) {
+ size_t num_results =
+ std::min(group->results().size(), group->max_results());
+ 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);
+ std::sort(results.begin(), results.end());
+
+ if (results.size() < kMinResults) {
+ size_t original_size = results.size();
+ // 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.)
for (const Group* group : groups_) {
- size_t num_results =
- std::min(group->results().size(), group->max_results());
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);
- std::sort(results.begin(), results.end());
-
- if (results.size() < kMinBlendedResults) {
- size_t original_size = results.size();
- // 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.)
- for (const Group* 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
- // group has a soft maximum of 4 results. (Otherwise, the People result
- // would not be seen at all once the result list is truncated.)
- std::sort(results.begin() + original_size, results.end());
- }
- } else {
- results.reserve(kMaxResults);
-
- // 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) {
- if (!has_omnibox_group_ || i != omnibox_group_) {
- const Group& group = *groups_[i];
- size_t num_results =
- std::min(group.results().size(), group.max_results());
- results.insert(results.end(), group.results().begin(),
- group.results().begin() + num_results);
- }
- }
-
- // Collapse duplicate apps from local and web store.
- RemoveDuplicates(&results);
-
- // 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
- // 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_) {
- CHECK_LT(omnibox_group_, groups_.size());
- const Group& omnibox_group = *groups_[omnibox_group_];
- const size_t omnibox_results = std::min(
- omnibox_group.results().size(),
- results.size() < kMaxResults ? kMaxResults - results.size() : 1);
- results.insert(results.end(), omnibox_group.results().begin(),
- omnibox_group.results().begin() + omnibox_results);
+ group->results().end());
}
-
- std::sort(results.begin(), results.end());
RemoveDuplicates(&results);
- if (results.size() > kMaxResults)
- results.resize(kMaxResults);
+ // 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
+ // group has a soft maximum of 4 results. (Otherwise, the People result
+ // would not be seen at all once the result list is truncated.)
+ std::sort(results.begin() + original_size, results.end());
}
Publish(results, ui_results_);
« no previous file with comments | « ui/app_list/search/mixer.h ('k') | ui/app_list/search/mixer_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698