Skip to content

Commit

Permalink
[Launcher streamlining] Remove boost on search provider groups
Browse files Browse the repository at this point in the history
This is part of a chain of CLs to simplify how we rank search results.
This removes an unused parameter of search provider groups - the boost -
that increases result scores by a fixed amount.

This used to be used to order the UI containers for apps and non-apps,
but now just makes the scores hard to compare.

Bug: 1028447
Change-Id: I407ca422b645069f4460db15d6bac42c44e4a745
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2374936
Reviewed-by: Thanh Nguyen <thanhdng@chromium.org>
Commit-Queue: Tony Yeoman <tby@chromium.org>
Cr-Commit-Position: refs/heads/master@{#801682}
  • Loading branch information
tby authored and Commit Bot committed Aug 26, 2020
1 parent dbd3e53 commit 90fa893
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 40 deletions.
13 changes: 5 additions & 8 deletions chrome/browser/ui/app_list/search/mixer.cc
Expand Up @@ -45,8 +45,7 @@ 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)
: max_results_(max_results), boost_(boost) {}
explicit Group(size_t max_results) : max_results_(max_results) {}
~Group() {}

void AddProvider(SearchProvider* provider) {
Expand All @@ -62,9 +61,8 @@ class Mixer::Group {

// We cannot rely on providers to give relevance scores in the range
// [0.0, 1.0]. Clamp to that range.
const double relevance =
base::ClampToRange(result->relevance(), 0.0, 1.0);
results_.emplace_back(result.get(), relevance + boost_);
results_.emplace_back(
result.get(), base::ClampToRange(result->relevance(), 0.0, 1.0));
}
}

Expand All @@ -80,7 +78,6 @@ class Mixer::Group {
private:
typedef std::vector<SearchProvider*> Providers;
const size_t max_results_;
const double boost_;

Providers providers_; // Not owned.
SortedResults results_;
Expand All @@ -102,8 +99,8 @@ void Mixer::InitializeRankers(Profile* profile,
}
}

size_t Mixer::AddGroup(size_t max_results, double boost) {
groups_.push_back(std::make_unique<Group>(max_results, boost));
size_t Mixer::AddGroup(size_t max_results) {
groups_.push_back(std::make_unique<Group>(max_results));
return groups_.size() - 1;
}

Expand Down
5 changes: 2 additions & 3 deletions chrome/browser/ui/app_list/search/mixer.h
Expand Up @@ -43,9 +43,8 @@ class Mixer {
// Adds a new mixer group. A "soft" maximum of |max_results| results will be
// chosen from this group (if 0, will allow unlimited results from this
// group). If there aren't enough results from all groups, more than
// |max_results| may be chosen from this group. Each result in the group will
// have its score increased by |boost|. Returns the group's group_id.
size_t AddGroup(size_t max_results, double boost);
// |max_results| may be chosen from this group. Returns the group's group_id.
size_t AddGroup(size_t max_results);

// Associates a provider with a mixer group.
void AddProviderToGroup(size_t group_id, SearchProvider* provider);
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/ui/app_list/search/search_controller.cc
Expand Up @@ -143,8 +143,8 @@ void SearchController::InvokeResultAction(ChromeSearchResult* result,
result->InvokeAction(action_index, event_flags);
}

size_t SearchController::AddGroup(size_t max_results, double boost) {
return mixer_->AddGroup(max_results, boost);
size_t SearchController::AddGroup(size_t max_results) {
return mixer_->AddGroup(max_results);
}

void SearchController::AddProvider(size_t group_id,
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/app_list/search/search_controller.h
Expand Up @@ -59,7 +59,7 @@ class SearchController {
int event_flags);

// Adds a new mixer group. See Mixer::AddGroup.
size_t AddGroup(size_t max_results, double boost = 0.0f);
size_t AddGroup(size_t max_results);

// Takes ownership of |provider| and associates it with given mixer group.
void AddProvider(size_t group_id, std::unique_ptr<SearchProvider> provider);
Expand Down
28 changes: 7 additions & 21 deletions chrome/browser/ui/app_list/search/search_controller_factory.cc
Expand Up @@ -80,8 +80,6 @@ constexpr size_t kMaxAssistantResults = 1;
// TODO(wutao): Need UX spec.
constexpr size_t kMaxSettingsShortcutResults = 6;

constexpr double kAppBoost = 8.0;

} // namespace

std::unique_ptr<SearchController> CreateSearchController(
Expand All @@ -96,16 +94,8 @@ std::unique_ptr<SearchController> CreateSearchController(
// Set up rankers for search results.
controller->InitializeRankers();

// Add mixer groups. There are four main groups: answer card, apps
// and omnibox. Each group has a "soft" maximum number of results. However, if
// a query turns up very few results, the mixer may take more than this
// maximum from a particular group.

// For the fullscreen app list, apps appear above the answer card, which
// appears above other results. Set boosts to |kAppBoost| for apps of all
// kinds, 5.0 for answer card, and otherwise 0.0.
size_t apps_group_id = controller->AddGroup(kMaxAppsGroupResults, kAppBoost);
size_t answer_card_group_id = controller->AddGroup(1, 5.0);
size_t apps_group_id = controller->AddGroup(kMaxAppsGroupResults);
size_t answer_card_group_id = controller->AddGroup(1);

size_t omnibox_group_id = controller->AddGroup(
ash::AppListConfig::instance().max_search_result_list_items());
Expand All @@ -127,8 +117,7 @@ std::unique_ptr<SearchController> CreateSearchController(
// The Assistant search provider currently only contributes search results
// when launcher chip integration is enabled.
if (chromeos::assistant::features::IsLauncherChipIntegrationEnabled()) {
size_t assistant_group_id =
controller->AddGroup(kMaxAssistantResults, kAppBoost);
size_t assistant_group_id = controller->AddGroup(kMaxAssistantResults);
controller->AddProvider(assistant_group_id,
std::make_unique<AssistantSearchProvider>());
}
Expand All @@ -146,7 +135,7 @@ std::unique_ptr<SearchController> CreateSearchController(
if (app_list_features::IsAppReinstallZeroStateEnabled() &&
arc::IsArcAllowedForProfile(profile)) {
size_t recommended_app_group_id =
controller->AddGroup(kMaxAppReinstallSearchResults, kAppBoost);
controller->AddGroup(kMaxAppReinstallSearchResults);
controller->AddProvider(recommended_app_group_id,
std::make_unique<ArcAppReinstallSearchProvider>(
profile, kMaxAppReinstallSearchResults));
Expand All @@ -155,17 +144,15 @@ std::unique_ptr<SearchController> CreateSearchController(
if (app_list_features::IsPlayStoreAppSearchEnabled()) {
// Set same boost as apps group since Play store results are placed
// with apps.
size_t playstore_api_group_id =
controller->AddGroup(kMaxPlayStoreResults, kAppBoost);
size_t playstore_api_group_id = controller->AddGroup(kMaxPlayStoreResults);
controller->AddProvider(
playstore_api_group_id,
std::make_unique<ArcPlayStoreSearchProvider>(kMaxPlayStoreResults,
profile, list_controller));
}

if (app_list_features::IsAppDataSearchEnabled()) {
size_t app_data_api_group_id =
controller->AddGroup(kMaxAppDataResults, kAppBoost);
size_t app_data_api_group_id = controller->AddGroup(kMaxAppDataResults);
controller->AddProvider(app_data_api_group_id,
std::make_unique<ArcAppDataSearchProvider>(
kMaxAppDataResults, list_controller));
Expand All @@ -182,8 +169,7 @@ std::unique_ptr<SearchController> CreateSearchController(
}

if (arc::IsArcAllowedForProfile(profile)) {
size_t app_shortcut_group_id =
controller->AddGroup(kMaxAppShortcutResults, kAppBoost);
size_t app_shortcut_group_id = controller->AddGroup(kMaxAppShortcutResults);
controller->AddProvider(
app_shortcut_group_id,
std::make_unique<ArcAppShortcutsSearchProvider>(
Expand Down
8 changes: 3 additions & 5 deletions chrome/browser/ui/app_list/search/tests/mixer_unittest.cc
Expand Up @@ -152,11 +152,9 @@ class MixerTest : public testing::Test {
void CreateMixer() {
mixer_ = std::make_unique<Mixer>(model_updater_.get());

// TODO(warx): when fullscreen app list is default enabled, modify this test
// to test answer card/apps group having relevance boost.
size_t apps_group_id = mixer_->AddGroup(kMaxAppsGroupResults, 0.0);
size_t omnibox_group_id = mixer_->AddGroup(kMaxOmniboxResults, 0.0);
size_t playstore_group_id = mixer_->AddGroup(kMaxPlaystoreResults, 0.0);
size_t apps_group_id = mixer_->AddGroup(kMaxAppsGroupResults);
size_t omnibox_group_id = mixer_->AddGroup(kMaxOmniboxResults);
size_t playstore_group_id = mixer_->AddGroup(kMaxPlaystoreResults);

mixer_->AddProviderToGroup(apps_group_id, providers_[0].get());
mixer_->AddProviderToGroup(omnibox_group_id, providers_[1].get());
Expand Down

0 comments on commit 90fa893

Please sign in to comment.