[Launcher settings] Improve ranking of settings results.

This CL sets up the MVP version of settings result ranking. We:

 - Show at most two results
 - Which are glued to the top of the launcher
 - But show no results for single-character queries.

To avoid confusion, I've removed the max results constant from the
search controller factory, and replaced it with a 'generic max results'
version. This generic version is necessary for now because a value for
max results is mandatory, but we're moving away from needing it.

Bug: 1068851
Change-Id: I188324413dd120d6f8abbf1b0a8791d4fcaac0d9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2214583
Commit-Queue: Tony Yeoman <tby@chromium.org>
Reviewed-by: Thanh Nguyen <thanhdng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771501}

chrome/browser/ui/app_list/search/os_settings_provider.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/ui/app_list/search/os_settings_provider.h"
 
+#include <algorithm>
 #include <memory>
 #include <string>
 
@@ -28,7 +29,11 @@
 namespace app_list {
 namespace {
 
+using SearchResultPtr = chromeos::settings::mojom::SearchResultPtr;
+
 constexpr char kOsSettingsResultPrefix[] = "os-settings://";
+constexpr float kScoreEps = 1e-5;
+constexpr int kMaxResults = 2;
 
 // Various error states of the OsSettingsProvider. kOk is currently not emitted,
 // but may be used in future. These values persist to logs. Entries should not
@@ -50,12 +55,13 @@ void LogError(Error error) {
 OsSettingsResult::OsSettingsResult(
     Profile* profile,
     const chromeos::settings::mojom::SearchResultPtr& result,
+    const float relevance_score,
     const gfx::ImageSkia& icon)
     : profile_(profile), url_path_(result->url_path_with_parameters) {
-  // TODO(crbug.com/1068851): Results need a useful relevance score and details
-  // text. Once this is available in the SearchResultPtr, set the metadata here.
+  // TODO(crbug.com/1068851): Results need useful details text. Once this is
+  // available in the SearchResultPtr, set the metadata here.
   set_id(kOsSettingsResultPrefix + url_path_);
-  set_relevance(8.0f);
+  set_relevance(relevance_score);
   SetTitle(result->result_text);
   SetResultType(ResultType::kOsSettings);
   SetDisplayType(DisplayType::kList);
@@ -112,8 +118,9 @@ void OsSettingsProvider::Start(const base::string16& query) {
 
   ClearResultsSilently();
 
-  // This provider does not handle zero-state.
-  if (query.empty())
+  // This provider does not handle zero-state, and shouldn't return any results
+  // for a single-character query because they aren't meaningful enough.
+  if (query.size() <= 1)
     return;
 
   // Invalidate weak pointers to cancel existing searches.
@@ -129,10 +136,22 @@ void OsSettingsProvider::Start(const base::string16& query) {
 
 void OsSettingsProvider::OnSearchReturned(
     std::vector<chromeos::settings::mojom::SearchResultPtr> results) {
+  std::sort(results.begin(), results.end(),
+            [](const SearchResultPtr& a, const SearchResultPtr& b) {
+              return a->relevance_score > b->relevance_score;
+            });
+
+  // TODO(crbug.com/1068851): We are currently not ranking settings results.
+  // Instead, we are gluing at most two to the top of the search box. Consider
+  // ranking these with other results in the next version of the feature.
   SearchProvider::Results search_results;
-  for (const auto& result : results) {
+  const int num_results =
+      std::min(static_cast<int>(results.size()), kMaxResults);
+  for (int i = 0; i < num_results; ++i) {
+    const auto& result = results[i];
+    const float score = 1.0f - i * kScoreEps;
     search_results.emplace_back(
-        std::make_unique<OsSettingsResult>(profile_, result, icon_));
+        std::make_unique<OsSettingsResult>(profile_, result, score, icon_));
   }
 
   if (icon_.isNull())

chrome/browser/ui/app_list/search/os_settings_provider.h

@@ -39,6 +39,7 @@ class OsSettingsResult : public ChromeSearchResult {
  public:
   OsSettingsResult(Profile* profile,
                    const chromeos::settings::mojom::SearchResultPtr& result,
+                   float relevance_score,
                    const gfx::ImageSkia& icon);
   ~OsSettingsResult() override;
 

chrome/browser/ui/app_list/search/search_controller_factory.cc

@@ -42,6 +42,15 @@ namespace {
 
 // Maximum number of results to show in each mixer group.
 
+// A generic value for max results, which is large enough to not interfere with
+// the actual results displayed. This should be used by providers that configure
+// their maximum number of results within the provider itself.
+//
+// TODO(crbug.com/1028447): Use this value for other providers that don't really
+// need a max results limit. Eventually, make this an optional constraint on a
+// Group.
+constexpr size_t kGenericMaxResults = 10;
+
 // Some app results may be blacklisted(e.g. continue reading) for rendering
 // in some UI, so we need to allow returning more results than actual maximum
 // number of results to be displayed in UI.
@@ -52,7 +61,6 @@ constexpr size_t kMaxLauncherSearchResults = 2;
 constexpr size_t kMaxZeroStateFileResults = 20;
 constexpr size_t kMaxDriveQuickAccessResults = 10;
 constexpr size_t kMaxAppReinstallSearchResults = 1;
-constexpr size_t kMaxOsSettingsResults = 5;
 // We show up to 6 Play Store results. However, part of Play Store results may
 // be filtered out because they may correspond to already installed Web apps. So
 // we request twice as many Play Store apps as we can show. Note that this still
@@ -198,7 +206,7 @@ std::unique_ptr<SearchController> CreateSearchController(
 
   if (app_list_features::IsLauncherSettingsSearchEnabled()) {
     size_t os_settings_search_group_id =
-        controller->AddGroup(kMaxOsSettingsResults, 1.0, 0.0);
+        controller->AddGroup(kGenericMaxResults, 1.0, 0.0);
     controller->AddProvider(os_settings_search_group_id,
                             std::make_unique<OsSettingsProvider>(profile));
   }