Commit 8e7c43b9 authored by tby's avatar tby Committed by Commit Bot

[Suggested files] Fixes for chip results.

This CL contains a handful of fixes that prevent chip results appearing.

 - On login, query ItemSuggest once, so we have results ready to display
   the first time the launcher is opened. This is mostly copied from
   drive_quick_access_provider.

 - Change some display and result type enums for chip results, needed to
   rank and display them correctly.

 - Use a separate schema for chip results, which prevents them being
   deduplicated with list results.

Bug: 1034842
Change-Id: I4da6cc219e19ad05383acdd2f98f64c6b4451a71
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2434111
Commit-Queue: Tony Yeoman <tby@chromium.org>
Reviewed-by: default avatarThanh Nguyen <thanhdng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811626}
parent 937b9ad7
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#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"
#include "base/strings/utf_string_conversions.h"
#include "base/task/post_task.h" #include "base/task/post_task.h"
#include "base/task/task_traits.h" #include "base/task/task_traits.h"
#include "base/task/thread_pool.h" #include "base/task/thread_pool.h"
...@@ -27,7 +28,9 @@ ...@@ -27,7 +28,9 @@
namespace app_list { namespace app_list {
namespace { namespace {
constexpr char kSchema[] = "drive_zero_state://"; // Schemas of result IDs for the results list and suggestion chips.
constexpr char kListSchema[] = "drive_zero_state://";
constexpr char kChipSchema[] = "drive_zero_state_chip://";
// Given an absolute path representing a file in the user's Drive, returns a // Given an absolute path representing a file in the user's Drive, returns a
// reparented version of the path within the user's drive fs mount. // reparented version of the path within the user's drive fs mount.
...@@ -56,18 +59,36 @@ DriveZeroStateProvider::DriveZeroStateProvider( ...@@ -56,18 +59,36 @@ DriveZeroStateProvider::DriveZeroStateProvider(
task_runner_ = base::ThreadPool::CreateSequencedTaskRunner( task_runner_ = base::ThreadPool::CreateSequencedTaskRunner(
{base::TaskPriority::BEST_EFFORT, base::MayBlock(), {base::TaskPriority::BEST_EFFORT, base::MayBlock(),
base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN}); base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN});
// Warm the results cache if or when drivefs is mounted by fetching from the
// Drive QuickAccess API. This is necessary only if the suggested files
// experiment is enabled, so that results are ready for display in the
// suggested chips on the first launcher open after login.
if (suggested_files_enabled_ && drive_service_) {
if (drive_service_->IsMounted()) {
// Drivefs is mounted, so we can fetch results immediately.
OnFileSystemMounted();
} else {
// Wait for DriveFS to be mounted, then fetch results. This happens in
// OnFileSystemMounted.
drive_service_->AddObserver(this);
}
}
} }
DriveZeroStateProvider::~DriveZeroStateProvider() = default; DriveZeroStateProvider::~DriveZeroStateProvider() {
if (suggested_files_enabled_ && drive_service_)
drive_service_->RemoveObserver(this);
}
void DriveZeroStateProvider::OnFileSystemMounted() { void DriveZeroStateProvider::OnFileSystemMounted() {
// This method is called on login, and each time the device wakes from sleep.
// We only want to warm the cache once.
if (have_warmed_up_cache_) if (have_warmed_up_cache_)
return; return;
have_warmed_up_cache_ = true; have_warmed_up_cache_ = true;
// TODO(crbug.com/1034842): Query ItemSuggest. We may need to call item_suggest_cache_.UpdateCache();
// SearchController::Start afterwards, or preferably could just publish the
// results for this search provider.
} }
void DriveZeroStateProvider::AppListShown() { void DriveZeroStateProvider::AppListShown() {
...@@ -144,10 +165,10 @@ void DriveZeroStateProvider::OnFilePathsLocated( ...@@ -144,10 +165,10 @@ void DriveZeroStateProvider::OnFilePathsLocated(
// the result. // the result.
provider_results.emplace_back( provider_results.emplace_back(
MakeResult(path_or_error->get_path(), score, /*is_chip=*/false)); MakeListResult(path_or_error->get_path(), score));
if (suggested_files_enabled_) { if (suggested_files_enabled_) {
provider_results.emplace_back( provider_results.emplace_back(
MakeResult(path_or_error->get_path(), score, /*is_chip=*/true)); MakeChipResult(path_or_error->get_path(), score));
} }
} }
...@@ -155,16 +176,22 @@ void DriveZeroStateProvider::OnFilePathsLocated( ...@@ -155,16 +176,22 @@ void DriveZeroStateProvider::OnFilePathsLocated(
SwapResults(&provider_results); SwapResults(&provider_results);
} }
std::unique_ptr<FileResult> DriveZeroStateProvider::MakeResult( std::unique_ptr<FileResult> DriveZeroStateProvider::MakeListResult(
const base::FilePath& filepath,
const float relevance) {
return std::make_unique<FileResult>(
kListSchema, ReparentToDriveMount(filepath, drive_service_),
ash::AppListSearchResultType::kDriveQuickAccess,
ash::SearchResultDisplayType::kList, relevance, profile_);
}
std::unique_ptr<FileResult> DriveZeroStateProvider::MakeChipResult(
const base::FilePath& filepath, const base::FilePath& filepath,
const float relevance, const float relevance) {
const bool is_chip) {
return std::make_unique<FileResult>( return std::make_unique<FileResult>(
kSchema, ReparentToDriveMount(filepath, drive_service_), kChipSchema, ReparentToDriveMount(filepath, drive_service_),
ash::AppListSearchResultType::kDriveQuickAccessChip, ash::AppListSearchResultType::kDriveQuickAccessChip,
is_chip ? ash::SearchResultDisplayType::kChip ash::SearchResultDisplayType::kChip, relevance, profile_);
: ash::SearchResultDisplayType::kList,
relevance, profile_);
} }
} // namespace app_list } // namespace app_list
...@@ -50,9 +50,10 @@ class DriveZeroStateProvider : public SearchProvider, ...@@ -50,9 +50,10 @@ class DriveZeroStateProvider : public SearchProvider,
void OnFilePathsLocated( void OnFilePathsLocated(
base::Optional<std::vector<drivefs::mojom::FilePathOrErrorPtr>> paths); base::Optional<std::vector<drivefs::mojom::FilePathOrErrorPtr>> paths);
std::unique_ptr<FileResult> MakeResult(const base::FilePath& filepath, std::unique_ptr<FileResult> MakeListResult(const base::FilePath& filepath,
const float relevance, const float relevance);
const bool is_chip); std::unique_ptr<FileResult> MakeChipResult(const base::FilePath& filepath,
const float relevance);
Profile* const profile_; Profile* const profile_;
drive::DriveIntegrationService* const drive_service_; drive::DriveIntegrationService* const drive_service_;
......
...@@ -58,8 +58,10 @@ FileResult::FileResult(const std::string& schema, ...@@ -58,8 +58,10 @@ FileResult::FileResult(const std::string& schema,
SetResultType(result_type); SetResultType(result_type);
switch (result_type) { switch (result_type) {
case ResultType::kDriveQuickAccess: case ResultType::kDriveQuickAccess:
case ResultType::kDriveQuickAccessChip:
SetMetricsType(ash::DRIVE_QUICK_ACCESS); SetMetricsType(ash::DRIVE_QUICK_ACCESS);
break; break;
case ResultType::kFileChip:
case ResultType::kZeroStateFile: case ResultType::kZeroStateFile:
SetMetricsType(ash::ZERO_STATE_FILE); SetMetricsType(ash::ZERO_STATE_FILE);
break; break;
......
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