Commit 8fc6db24 authored by Thanh Nguyen's avatar Thanh Nguyen Committed by Commit Bot

[Cros SR] Add training for Arc App Shorcuts

See here for context:

  https://docs.google.com/document/d/1GTQ3vvrszK4I1pDzmTFb6krmUzAXox0V9ZIqIi_GH1E/edit?usp=sharing

We are expanding Roselle to re-rank the results returned when the user enters
a query. However, these results include Arc app shortcuts as well as apps
themselves, so they need to be ranked. This CL adds the training support for
Arc App Shortcuts and use Roselle score to update current relevance score.

Bug: 931149
Change-Id: I979ebd500504681c9b0c0e59aa0412b4cbd47315
Reviewed-on: https://chromium-review.googlesource.com/c/1481176
Commit-Queue: Thanh Nguyen <thanhdng@chromium.org>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Reviewed-by: default avatarJia Meng <jiameng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#634881}
parent 60b9fd39
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#include "chrome/browser/ui/app_list/search/arc/arc_app_shortcuts_search_provider.h" #include "chrome/browser/ui/app_list/search/arc/arc_app_shortcuts_search_provider.h"
#include <memory> #include <memory>
#include <string>
#include <utility> #include <utility>
#include "ash/public/cpp/app_list/app_list_features.h" #include "ash/public/cpp/app_list/app_list_features.h"
...@@ -14,6 +15,7 @@ ...@@ -14,6 +15,7 @@
#include "chrome/browser/ui/app_list/arc/arc_app_utils.h" #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/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/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_bridge_service.h"
#include "components/arc/arc_service_manager.h" #include "components/arc/arc_service_manager.h"
...@@ -54,10 +56,19 @@ void ArcAppShortcutsSearchProvider::Start(const base::string16& query) { ...@@ -54,10 +56,19 @@ void ArcAppShortcutsSearchProvider::Start(const base::string16& query) {
weak_ptr_factory_.GetWeakPtr())); 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( void ArcAppShortcutsSearchProvider::OnGetAppShortcutGlobalQueryItems(
std::vector<arc::mojom::AppShortcutItemPtr> shortcut_items) { std::vector<arc::mojom::AppShortcutItemPtr> shortcut_items) {
const ArcAppListPrefs* arc_prefs = ArcAppListPrefs::Get(profile_); const ArcAppListPrefs* arc_prefs = ArcAppListPrefs::Get(profile_);
DCHECK(arc_prefs); 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; SearchProvider::Results search_results;
for (auto& item : shortcut_items) { for (auto& item : shortcut_items) {
...@@ -68,16 +79,16 @@ void ArcAppShortcutsSearchProvider::OnGetAppShortcutGlobalQueryItems( ...@@ -68,16 +79,16 @@ void ArcAppShortcutsSearchProvider::OnGetAppShortcutGlobalQueryItems(
// Ignore shortcuts for apps that are not present in the launcher. // Ignore shortcuts for apps that are not present in the launcher.
if (!app_info || !app_info->show_in_launcher) if (!app_info || !app_info->show_in_launcher)
continue; continue;
search_results.emplace_back(std::make_unique<ArcAppShortcutSearchResult>( auto result = std::make_unique<ArcAppShortcutSearchResult>(
std::move(item), profile_, list_controller_)); std::move(item), profile_, list_controller_);
// TODO(crbug.com/931149): update the formula for relevance scores.
if (app_list_features::IsAppSearchResultRankerEnabled() && // This formula should be updated in the same way as query-based
ranker_ != nullptr) { // app search results
// TODO(crbug.com/931149): tweak the scores of each search result item const auto find_in_ranker = ranker_scores.find(result->id());
// using the ranker. 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); SwapResults(&search_results);
} }
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#ifndef CHROME_BROWSER_UI_APP_LIST_SEARCH_ARC_ARC_APP_SHORTCUTS_SEARCH_PROVIDER_H_ #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_ #define CHROME_BROWSER_UI_APP_LIST_SEARCH_ARC_ARC_APP_SHORTCUTS_SEARCH_PROVIDER_H_
#include <string>
#include <vector> #include <vector>
#include "base/macros.h" #include "base/macros.h"
...@@ -30,6 +31,7 @@ class ArcAppShortcutsSearchProvider : public SearchProvider { ...@@ -30,6 +31,7 @@ class ArcAppShortcutsSearchProvider : public SearchProvider {
// SearchProvider: // SearchProvider:
void Start(const base::string16& query) override; void Start(const base::string16& query) override;
void Train(const std::string& id, RankingItemType type) override;
private: private:
void OnGetAppShortcutGlobalQueryItems( void OnGetAppShortcutGlobalQueryItems(
...@@ -38,8 +40,6 @@ class ArcAppShortcutsSearchProvider : public SearchProvider { ...@@ -38,8 +40,6 @@ class ArcAppShortcutsSearchProvider : public SearchProvider {
const int max_results_; const int max_results_;
Profile* const profile_; // Owned by ProfileInfo. Profile* const profile_; // Owned by ProfileInfo.
AppListControllerDelegate* const list_controller_; // Owned by AppListClient. 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_; AppSearchResultRanker* ranker_;
base::WeakPtrFactory<ArcAppShortcutsSearchProvider> weak_ptr_factory_; base::WeakPtrFactory<ArcAppShortcutsSearchProvider> weak_ptr_factory_;
......
...@@ -17,6 +17,7 @@ ...@@ -17,6 +17,7 @@
#include "chrome/browser/ui/app_list/arc/arc_app_test.h" #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/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/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 "chrome/browser/ui/app_list/test/test_app_list_controller_delegate.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -109,6 +110,31 @@ TEST_P(ArcAppShortcutsSearchProviderTest, Basic) { ...@@ -109,6 +110,31 @@ TEST_P(ArcAppShortcutsSearchProviderTest, Basic) {
base::UTF16ToUTF8(results[i]->title())); base::UTF16ToUTF8(results[i]->title()));
EXPECT_EQ(ash::SearchResultDisplayType::kTile, results[i]->display_type()); 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(
AddArcAppAndShortcut(
CreateAppInfo("FakeName", "FakeActivity", kFakeAppPackageName),
launchable),
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()); 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