Commit 09290852 authored by Thanh Nguyen's avatar Thanh Nguyen Committed by Commit Bot

[Cros SR] Reland app shortcut training CL

This CL relands commit 8fc6db24.

Bug: 931149
Change-Id: I666531690b807b3b2a71a95fdfd2fb6ad10784eb
Reviewed-on: https://chromium-review.googlesource.com/c/1484877Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Reviewed-by: default avatarJia Meng <jiameng@chromium.org>
Commit-Queue: Thanh Nguyen <thanhdng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635285}
parent 9406979e
......@@ -5,6 +5,7 @@
#include "chrome/browser/ui/app_list/search/arc/arc_app_shortcuts_search_provider.h"
#include <memory>
#include <string>
#include <utility>
#include "ash/public/cpp/app_list/app_list_features.h"
......@@ -14,6 +15,7 @@
#include "chrome/browser/ui/app_list/arc/arc_app_utils.h"
#include "chrome/browser/ui/app_list/search/arc/arc_app_shortcut_search_result.h"
#include "chrome/browser/ui/app_list/search/search_result_ranker/app_search_result_ranker.h"
#include "chrome/browser/ui/app_list/search/search_result_ranker/ranking_item_util.h"
#include "components/arc/arc_bridge_service.h"
#include "components/arc/arc_service_manager.h"
......@@ -54,10 +56,19 @@ void ArcAppShortcutsSearchProvider::Start(const base::string16& query) {
weak_ptr_factory_.GetWeakPtr()));
}
void ArcAppShortcutsSearchProvider::Train(const std::string& id,
RankingItemType type) {
if (type == RankingItemType::kArcAppShortcut && ranker_ != nullptr)
ranker_->Train(id);
}
void ArcAppShortcutsSearchProvider::OnGetAppShortcutGlobalQueryItems(
std::vector<arc::mojom::AppShortcutItemPtr> shortcut_items) {
const ArcAppListPrefs* arc_prefs = ArcAppListPrefs::Get(profile_);
DCHECK(arc_prefs);
base::flat_map<std::string, float> ranker_scores;
if (app_list_features::IsAppSearchResultRankerEnabled() && ranker_ != nullptr)
ranker_scores = ranker_->Rank();
SearchProvider::Results search_results;
for (auto& item : shortcut_items) {
......@@ -68,16 +79,16 @@ void ArcAppShortcutsSearchProvider::OnGetAppShortcutGlobalQueryItems(
// Ignore shortcuts for apps that are not present in the launcher.
if (!app_info || !app_info->show_in_launcher)
continue;
search_results.emplace_back(std::make_unique<ArcAppShortcutSearchResult>(
std::move(item), profile_, list_controller_));
if (app_list_features::IsAppSearchResultRankerEnabled() &&
ranker_ != nullptr) {
// TODO(crbug.com/931149): tweak the scores of each search result item
// using the ranker.
}
auto result = std::make_unique<ArcAppShortcutSearchResult>(
std::move(item), profile_, list_controller_);
// TODO(crbug.com/931149): update the formula for relevance scores.
// This formula should be updated in the same way as query-based
// app search results
const auto find_in_ranker = ranker_scores.find(result->id());
if (find_in_ranker != ranker_scores.end())
result->set_relevance(result->relevance() + find_in_ranker->second / 10);
search_results.emplace_back(std::move(result));
}
SwapResults(&search_results);
}
......
......@@ -5,6 +5,7 @@
#ifndef CHROME_BROWSER_UI_APP_LIST_SEARCH_ARC_ARC_APP_SHORTCUTS_SEARCH_PROVIDER_H_
#define CHROME_BROWSER_UI_APP_LIST_SEARCH_ARC_ARC_APP_SHORTCUTS_SEARCH_PROVIDER_H_
#include <string>
#include <vector>
#include "base/macros.h"
......@@ -30,6 +31,7 @@ class ArcAppShortcutsSearchProvider : public SearchProvider {
// SearchProvider:
void Start(const base::string16& query) override;
void Train(const std::string& id, RankingItemType type) override;
private:
void OnGetAppShortcutGlobalQueryItems(
......@@ -38,8 +40,6 @@ class ArcAppShortcutsSearchProvider : public SearchProvider {
const int max_results_;
Profile* const profile_; // Owned by ProfileInfo.
AppListControllerDelegate* const list_controller_; // Owned by AppListClient.
// TODO(crbug.com/931149): train this ranker on app shortcut clicks, and use
// it to tweak their relevance scores.
AppSearchResultRanker* ranker_;
base::WeakPtrFactory<ArcAppShortcutsSearchProvider> weak_ptr_factory_;
......
......@@ -17,6 +17,7 @@
#include "chrome/browser/ui/app_list/arc/arc_app_test.h"
#include "chrome/browser/ui/app_list/search/chrome_search_result.h"
#include "chrome/browser/ui/app_list/search/search_result_ranker/app_search_result_ranker.h"
#include "chrome/browser/ui/app_list/search/search_result_ranker/ranking_item_util.h"
#include "chrome/browser/ui/app_list/test/test_app_list_controller_delegate.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -88,7 +89,7 @@ class ArcAppShortcutsSearchProviderTest
TEST_P(ArcAppShortcutsSearchProviderTest, Basic) {
const bool launchable = GetParam();
AddArcAppAndShortcut(
const std::string app_id = AddArcAppAndShortcut(
CreateAppInfo("FakeName", "FakeActivity", kFakeAppPackageName),
launchable);
......@@ -109,6 +110,27 @@ TEST_P(ArcAppShortcutsSearchProviderTest, Basic) {
base::UTF16ToUTF8(results[i]->title()));
EXPECT_EQ(ash::SearchResultDisplayType::kTile, results[i]->display_type());
}
// If ranker_ is nullptr, the program won't break
// TODO(crbug.com/931149): Add more tests to check ranker_ does have some
// effects
auto provider_null_ranker = std::make_unique<ArcAppShortcutsSearchProvider>(
kMaxResults, profile(), controller_.get(), nullptr);
EXPECT_TRUE(provider_null_ranker->results().empty());
arc::IconDecodeRequest::DisableSafeDecodingForTesting();
provider_null_ranker->Start(base::UTF8ToUTF16(kQuery));
provider_null_ranker->Train(app_id, RankingItemType::kArcAppShortcut);
const auto& results_null_ranker = provider_null_ranker->results();
EXPECT_EQ(kMaxResults, results_null_ranker.size());
// Verify search results.
for (size_t i = 0; i < results_null_ranker.size(); ++i) {
EXPECT_EQ(base::StringPrintf("ShortLabel %zu", i),
base::UTF16ToUTF8(results_null_ranker[i]->title()));
EXPECT_EQ(ash::SearchResultDisplayType::kTile,
results_null_ranker[i]->display_type());
}
}
INSTANTIATE_TEST_SUITE_P(, ArcAppShortcutsSearchProviderTest, testing::Bool());
......
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