Commit 01bffc5a authored by tby's avatar tby Committed by Commit Bot

[Suggested files] Create search results from ItemSuggestCache.

The steps involved are:
 1. Get some results from the cache
 2. Run them through LocateFilesByItemIds to convert item IDs into
    file paths
 3. Assign relevance scores and create FileResult objects.

Bug: 1034842
Change-Id: If217d5eb5645e9a5eefa8a57609d9ea91014db4b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2434108Reviewed-by: default avatarRachel Wong <wrong@chromium.org>
Commit-Queue: Tony Yeoman <tby@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811122}
parent 734d8db6
...@@ -2574,6 +2574,7 @@ static_library("ui") { ...@@ -2574,6 +2574,7 @@ static_library("ui") {
"//chromeos/components/bloom/public/cpp:bloom_controller_factory", "//chromeos/components/bloom/public/cpp:bloom_controller_factory",
"//chromeos/components/camera_app_ui", "//chromeos/components/camera_app_ui",
"//chromeos/components/diagnostics_ui", "//chromeos/components/diagnostics_ui",
"//chromeos/components/drivefs/mojom:mojom",
"//chromeos/components/help_app_ui", "//chromeos/components/help_app_ui",
"//chromeos/components/local_search_service:local_search_service", "//chromeos/components/local_search_service:local_search_service",
"//chromeos/components/media_app_ui", "//chromeos/components/media_app_ui",
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include <utility> #include <utility>
#include "ash/public/cpp/app_list/app_list_features.h" #include "ash/public/cpp/app_list/app_list_features.h"
#include "ash/public/cpp/app_list/app_list_types.h"
#include "base/bind.h" #include "base/bind.h"
#include "base/files/file_util.h" #include "base/files/file_util.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
...@@ -17,14 +18,18 @@ ...@@ -17,14 +18,18 @@
#include "base/task/thread_pool.h" #include "base/task/thread_pool.h"
#include "base/task_runner_util.h" #include "base/task_runner_util.h"
#include "chrome/browser/chromeos/drive/drive_integration_service.h" #include "chrome/browser/chromeos/drive/drive_integration_service.h"
#include "chrome/browser/ui/app_list/search/drive_quick_access_chip_result.h"
#include "chrome/browser/ui/app_list/search/drive_quick_access_result.h"
#include "chrome/browser/ui/app_list/search/search_controller.h" #include "chrome/browser/ui/app_list/search/search_controller.h"
#include "components/drive/file_errors.h"
#include "content/public/browser/browser_context.h" #include "content/public/browser/browser_context.h"
#include "content/public/browser/browser_task_traits.h" #include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
namespace app_list { namespace app_list {
namespace {
constexpr char kSchema[] = "drive_zero_state://";
}
DriveZeroStateProvider::DriveZeroStateProvider( DriveZeroStateProvider::DriveZeroStateProvider(
Profile* profile, Profile* profile,
...@@ -33,6 +38,8 @@ DriveZeroStateProvider::DriveZeroStateProvider( ...@@ -33,6 +38,8 @@ DriveZeroStateProvider::DriveZeroStateProvider(
: profile_(profile), : profile_(profile),
drive_service_( drive_service_(
drive::DriveIntegrationServiceFactory::GetForProfile(profile)), drive::DriveIntegrationServiceFactory::GetForProfile(profile)),
file_tasks_notifier_(
file_manager::file_tasks::FileTasksNotifier::GetForProfile(profile)),
item_suggest_cache_(profile, std::move(url_loader_factory)), item_suggest_cache_(profile, std::move(url_loader_factory)),
suggested_files_enabled_(app_list_features::IsSuggestedFilesEnabled()) { suggested_files_enabled_(app_list_features::IsSuggestedFilesEnabled()) {
DCHECK(profile_); DCHECK(profile_);
...@@ -54,6 +61,12 @@ void DriveZeroStateProvider::OnFileSystemMounted() { ...@@ -54,6 +61,12 @@ void DriveZeroStateProvider::OnFileSystemMounted() {
// results for this search provider. // results for this search provider.
} }
void DriveZeroStateProvider::AppListShown() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
item_suggest_cache_.UpdateCache();
// TODO(crbug.com/1034842): Query ItemSuggest, consider rate-limiting.
}
ash::AppListSearchResultType DriveZeroStateProvider::ResultType() { ash::AppListSearchResultType DriveZeroStateProvider::ResultType() {
return ash::AppListSearchResultType::kDriveQuickAccess; return ash::AppListSearchResultType::kDriveQuickAccess;
} }
...@@ -61,16 +74,75 @@ ash::AppListSearchResultType DriveZeroStateProvider::ResultType() { ...@@ -61,16 +74,75 @@ ash::AppListSearchResultType DriveZeroStateProvider::ResultType() {
void DriveZeroStateProvider::Start(const base::string16& query) { void DriveZeroStateProvider::Start(const base::string16& query) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
ClearResultsSilently(); ClearResultsSilently();
if (!query.empty())
// TODO(crbug.com/1034842): Add query latency metrics.
// Exit in three cases:
// - this search has a non-empty query, we only handle zero-state.
// - drive fs isn't mounted, as we launch results via drive fs.
// - the |file_tasks_notifier_| is unavailable, as we stat files using it.
const bool drive_fs_mounted = drive_service_ && drive_service_->IsMounted();
if (!query.empty() || !drive_fs_mounted || !file_tasks_notifier_) {
return;
}
// Cancel any in-flight queries for this provider.
weak_factory_.InvalidateWeakPtrs();
// Get the most recent results from the cache.
cache_results_ = item_suggest_cache_.GetResults();
if (!cache_results_)
return; return;
// TODO(crbug.com/1034842): Convert results cache into search results. std::vector<std::string> item_ids;
for (const auto& result : cache_results_->results) {
item_ids.push_back(result.id);
}
drive_service_->LocateFilesByItemIds(
item_ids, base::BindOnce(&DriveZeroStateProvider::OnFilePathsLocated,
weak_factory_.GetWeakPtr()));
} }
void DriveZeroStateProvider::AppListShown() { void DriveZeroStateProvider::OnFilePathsLocated(
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); base::Optional<std::vector<drivefs::mojom::FilePathOrErrorPtr>> paths) {
item_suggest_cache_.UpdateCache(); if (!paths)
// TODO(crbug.com/1034842): Query ItemSuggest, consider rate-limiting. return;
DCHECK(cache_results_);
DCHECK_EQ(cache_results_->results.size(), paths->size());
// Assign scores to results by simply using their position in the results
// list. The order of results from the ItemSuggest API is significant:
// the first is better than the second, etc. Resulting scores are in [0, 1].
const double total_items = static_cast<double>(paths->size());
int item_index = 0;
SearchProvider::Results provider_results;
for (int i = 0; i < static_cast<int>(paths->size()); ++i) {
if ((*paths)[i]->get_error() != drive::FILE_ERROR_OK)
continue;
const double score = 1.0 - (item_index / total_items);
++item_index;
// TODO(crbug.com/1034842): Use |cache_results_| to attach the session id to
// the result.
provider_results.emplace_back(
MakeResult((*paths)[i]->get_path(), score, /*is_chip=*/false));
}
cache_results_.reset();
SwapResults(&provider_results);
}
std::unique_ptr<FileResult> DriveZeroStateProvider::MakeResult(
const base::FilePath& filepath,
const float relevance,
const bool is_chip) {
return std::make_unique<FileResult>(
kSchema, filepath, ash::AppListSearchResultType::kDriveQuickAccessChip,
is_chip ? ash::SearchResultDisplayType::kChip
: ash::SearchResultDisplayType::kList,
relevance, profile_);
} }
} // namespace app_list } // namespace app_list
...@@ -15,8 +15,10 @@ ...@@ -15,8 +15,10 @@
#include "base/time/time.h" #include "base/time/time.h"
#include "chrome/browser/chromeos/drive/drive_integration_service.h" #include "chrome/browser/chromeos/drive/drive_integration_service.h"
#include "chrome/browser/chromeos/file_manager/file_tasks_notifier.h" #include "chrome/browser/chromeos/file_manager/file_tasks_notifier.h"
#include "chrome/browser/ui/app_list/search/files/file_result.h"
#include "chrome/browser/ui/app_list/search/files/item_suggest_cache.h" #include "chrome/browser/ui/app_list/search/files/item_suggest_cache.h"
#include "chrome/browser/ui/app_list/search/search_provider.h" #include "chrome/browser/ui/app_list/search/search_provider.h"
#include "chromeos/components/drivefs/mojom/drivefs.mojom.h"
class Profile; class Profile;
...@@ -36,20 +38,33 @@ class DriveZeroStateProvider : public SearchProvider, ...@@ -36,20 +38,33 @@ class DriveZeroStateProvider : public SearchProvider,
DriveZeroStateProvider(const DriveZeroStateProvider&) = delete; DriveZeroStateProvider(const DriveZeroStateProvider&) = delete;
DriveZeroStateProvider& operator=(const DriveZeroStateProvider&) = delete; DriveZeroStateProvider& operator=(const DriveZeroStateProvider&) = delete;
// drive::DriveIntegrationServiceObserver:
void OnFileSystemMounted() override;
// SearchProvider: // SearchProvider:
void Start(const base::string16& query) override;
void AppListShown() override; void AppListShown() override;
ash::AppListSearchResultType ResultType() override; ash::AppListSearchResultType ResultType() override;
void Start(const base::string16& query) override;
// drive::DriveIntegrationServiceObserver:
void OnFileSystemMounted() override;
private: private:
void OnFilePathsLocated(
base::Optional<std::vector<drivefs::mojom::FilePathOrErrorPtr>> paths);
std::unique_ptr<FileResult> MakeResult(const base::FilePath& filepath,
const float relevance,
const bool is_chip);
Profile* const profile_; Profile* const profile_;
drive::DriveIntegrationService* const drive_service_; drive::DriveIntegrationService* const drive_service_;
file_manager::file_tasks::FileTasksNotifier* const file_tasks_notifier_;
ItemSuggestCache item_suggest_cache_; ItemSuggestCache item_suggest_cache_;
// The most recent results retrieved from |item_suggested_cache_|. This is
// updated on a call to Start and is used only to store the results until
// OnFilePathsLocated has finished.
base::Optional<ItemSuggestCache::Results> cache_results_;
// Whether the suggested files experiment is enabled. // Whether the suggested files experiment is enabled.
const bool suggested_files_enabled_; const bool suggested_files_enabled_;
......
...@@ -208,6 +208,12 @@ ItemSuggestCache::ItemSuggestCache( ...@@ -208,6 +208,12 @@ ItemSuggestCache::ItemSuggestCache(
ItemSuggestCache::~ItemSuggestCache() = default; ItemSuggestCache::~ItemSuggestCache() = default;
base::Optional<ItemSuggestCache::Results> ItemSuggestCache::GetResults() {
// Return a copy because a pointer to |results_| will become invalid whenever
// the cache is updated.
return results_;
}
void ItemSuggestCache::UpdateCache() { void ItemSuggestCache::UpdateCache() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// TODO(crbug.com/1034842): Add rate-limiting for cache updates. // TODO(crbug.com/1034842): Add rate-limiting for cache updates.
......
...@@ -59,6 +59,10 @@ class ItemSuggestCache { ...@@ -59,6 +59,10 @@ class ItemSuggestCache {
ItemSuggestCache(const ItemSuggestCache&) = delete; ItemSuggestCache(const ItemSuggestCache&) = delete;
ItemSuggestCache& operator=(const ItemSuggestCache&) = delete; ItemSuggestCache& operator=(const ItemSuggestCache&) = delete;
// Returns the results currently in the cache.
base::Optional<ItemSuggestCache::Results> GetResults();
// Updates the cache by calling ItemSuggest.
void UpdateCache(); void UpdateCache();
static base::Optional<ItemSuggestCache::Results> ConvertJsonForTest( static base::Optional<ItemSuggestCache::Results> ConvertJsonForTest(
......
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