Commit a71c7f43 authored by Nico Weber's avatar Nico Weber Committed by Commit Bot

Revert "[Cros SR] Add training for Arc App Shorcuts"

This reverts commit 8fc6db24.

Reason for revert: https://crbug.com/935122

Original change's description:
> [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: Xiyuan Xia <xiyuan@chromium.org>
> Reviewed-by: Jia Meng <jiameng@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#634881}

TBR=xiyuan@chromium.org,jiameng@chromium.org,thanhdng@chromium.org

Change-Id: I9846f900136f23578ee0bb67cc4761c22cff5b3b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 931149
Reviewed-on: https://chromium-review.googlesource.com/c/1485057Reviewed-by: default avatarNico Weber <thakis@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#634976}
parent 914f6b7e
...@@ -5,7 +5,6 @@ ...@@ -5,7 +5,6 @@
#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"
...@@ -15,7 +14,6 @@ ...@@ -15,7 +14,6 @@
#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"
...@@ -56,19 +54,10 @@ void ArcAppShortcutsSearchProvider::Start(const base::string16& query) { ...@@ -56,19 +54,10 @@ 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) {
...@@ -79,16 +68,16 @@ void ArcAppShortcutsSearchProvider::OnGetAppShortcutGlobalQueryItems( ...@@ -79,16 +68,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;
auto result = std::make_unique<ArcAppShortcutSearchResult>( search_results.emplace_back(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.
// This formula should be updated in the same way as query-based if (app_list_features::IsAppSearchResultRankerEnabled() &&
// app search results ranker_ != nullptr) {
const auto find_in_ranker = ranker_scores.find(result->id()); // TODO(crbug.com/931149): tweak the scores of each search result item
if (find_in_ranker != ranker_scores.end()) // using the ranker.
result->set_relevance(result->relevance() + find_in_ranker->second / 10); }
search_results.emplace_back(std::move(result));
} }
SwapResults(&search_results); SwapResults(&search_results);
} }
......
...@@ -5,7 +5,6 @@ ...@@ -5,7 +5,6 @@
#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"
...@@ -31,7 +30,6 @@ class ArcAppShortcutsSearchProvider : public SearchProvider { ...@@ -31,7 +30,6 @@ 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(
...@@ -40,6 +38,8 @@ class ArcAppShortcutsSearchProvider : public SearchProvider { ...@@ -40,6 +38,8 @@ 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,7 +17,6 @@ ...@@ -17,7 +17,6 @@
#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"
...@@ -110,31 +109,6 @@ TEST_P(ArcAppShortcutsSearchProviderTest, Basic) { ...@@ -110,31 +109,6 @@ 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