[M87 merge] Reland "ambient: cache two backup photos"
This is a reland of a81c26884bde81a386ffb2dcbdb7a12f0ecd1215
Original change's description:
> ambient: cache two backup photos
>
> Handle failure cases better by having backup photos on disk.
> Refresh these images after user logs in.
>
> BUG=b:167332126
>
> Cq-Include-Trybots: luci.chrome.try:linux-chromeos-chrome
> Change-Id: I7046a20ed9606638e851247f8c213d112af6c30b
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2429585
> Commit-Queue: Jeffrey Young <[email protected]>
> Reviewed-by: Xiaohui Chen <[email protected]>
> Reviewed-by: Tao Wu <[email protected]>
> Cr-Commit-Position: refs/heads/master@{#813904}
(cherry picked from commit 2c34160725973ce93507b98fde9dfda735fcb565)
Bug: b:167332126
Bug: b:170312454
Change-Id: If5306f627e1a6dfb5e4eb37842a470d17968ad4e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2458894
Commit-Queue: Jeffrey Young <[email protected]>
Reviewed-by: Tao Wu <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#814968}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2464073
Reviewed-by: Xiaohui Chen <[email protected]>
Cr-Commit-Position: refs/branch-heads/4280@{#262}
Cr-Branched-From: ea420fb963f9658c9969b6513c56b8f47efa1a2a-refs/heads/master@{#812852}
diff --git a/ash/ambient/ambient_constants.h b/ash/ambient/ambient_constants.h
index 426a538..0b82785 100644
--- a/ash/ambient/ambient_constants.h
+++ b/ash/ambient/ambient_constants.h
@@ -24,6 +24,10 @@
constexpr base::TimeDelta kPhotoRefreshInterval =
base::TimeDelta::FromSeconds(60);
+// The default interval to fetch backup cache photos.
+constexpr base::TimeDelta kBackupPhotoRefreshDelay =
+ base::TimeDelta::FromMinutes(5);
+
// The default interval to refresh weather.
constexpr base::TimeDelta kWeatherRefreshInterval =
base::TimeDelta::FromMinutes(5);
@@ -47,6 +51,10 @@
// Directory name of ambient mode.
constexpr char kAmbientModeDirectoryName[] = "ambient-mode";
+constexpr char kAmbientModeCacheDirectoryName[] = "cache";
+
+constexpr char kAmbientModeBackupCacheDirectoryName[] = "backup";
+
// The buffer time to use the access token.
constexpr base::TimeDelta kTokenUsageTimeBuffer =
base::TimeDelta::FromMinutes(10);
diff --git a/ash/ambient/ambient_controller.cc b/ash/ambient/ambient_controller.cc
index f0ea88cf..35d6b1b 100644
--- a/ash/ambient/ambient_controller.cc
+++ b/ash/ambient/ambient_controller.cc
@@ -324,6 +324,11 @@
}
}
+void AmbientController::OnFirstSessionStarted() {
+ if (IsAmbientModeEnabled())
+ ambient_photo_controller_.ScheduleFetchBackupImages();
+}
+
void AmbientController::OnPowerStatusChanged() {
if (ambient_ui_model_.ui_visibility() != AmbientUiVisibility::kShown) {
// No action needed if ambient screen is not shown.
diff --git a/ash/ambient/ambient_controller.h b/ash/ambient/ambient_controller.h
index 740f03f..7926945 100644
--- a/ash/ambient/ambient_controller.h
+++ b/ash/ambient/ambient_controller.h
@@ -63,6 +63,7 @@
// SessionObserver:
void OnLockStateChanged(bool locked) override;
+ void OnFirstSessionStarted() override;
// PowerStatus::Observer:
void OnPowerStatusChanged() override;
diff --git a/ash/ambient/ambient_photo_controller.cc b/ash/ambient/ambient_photo_controller.cc
index b5ea5c9..bc5d11e 100644
--- a/ash/ambient/ambient_photo_controller.cc
+++ b/ash/ambient/ambient_photo_controller.cc
@@ -4,8 +4,10 @@
#include "ash/ambient/ambient_photo_controller.h"
+#include <array>
#include <string>
#include <utility>
+#include <vector>
#include "ash/ambient/ambient_constants.h"
#include "ash/ambient/ambient_controller.h"
@@ -81,17 +83,6 @@
base::BindOnce(std::move(callback)));
}
-// Get the root path for ambient mode.
-base::FilePath GetRootPath() {
- base::FilePath home_dir;
- CHECK(base::PathService::Get(base::DIR_HOME, &home_dir));
- return home_dir.Append(FILE_PATH_LITERAL(kAmbientModeDirectoryName));
-}
-
-void DeletePathRecursively(const base::FilePath& path) {
- base::DeletePathRecursively(path);
-}
-
void ToImageSkia(DownloadCallback callback, const SkBitmap& image) {
if (image.isNull()) {
std::move(callback).Run(gfx::ImageSkia());
@@ -109,9 +100,34 @@
base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN};
}
+// Get the root path for ambient mode.
+base::FilePath GetRootPath() {
+ base::FilePath home_dir;
+ CHECK(base::PathService::Get(base::DIR_HOME, &home_dir));
+ return home_dir.Append(FILE_PATH_LITERAL(kAmbientModeDirectoryName));
+}
+
+base::FilePath GetCachePath() {
+ return GetRootPath().Append(
+ FILE_PATH_LITERAL(kAmbientModeCacheDirectoryName));
+}
+
+base::FilePath GetBackupCachePath() {
+ return GetRootPath().Append(
+ FILE_PATH_LITERAL(kAmbientModeBackupCacheDirectoryName));
+}
+
+base::FilePath GetBackupFilePath(size_t index) {
+ return GetBackupCachePath().Append(base::NumberToString(index) +
+ kPhotoFileExt);
+}
+
+bool CreateDirIfNotExists(const base::FilePath& path) {
+ return base::DirectoryExists(path) || base::CreateDirectory(path);
+}
+
void WriteFile(const base::FilePath& path, const std::string& data) {
- if (!base::PathExists(GetRootPath()) &&
- !base::CreateDirectory(GetRootPath())) {
+ if (!CreateDirIfNotExists(GetCachePath())) {
LOG(ERROR) << "Cannot create ambient mode directory.";
return;
}
@@ -143,6 +159,13 @@
LOG(ERROR) << "Cannot replace the temporary file.";
}
+const std::array<const char*, 2>& GetBackupPhotoUrls() {
+ return Shell::Get()
+ ->ambient_controller()
+ ->ambient_backend_controller()
+ ->GetBackupPhotoUrls();
+}
+
} // namespace
class AmbientURLLoaderImpl : public AmbientURLLoader {
@@ -154,13 +177,7 @@
void Download(
const std::string& url,
network::SimpleURLLoader::BodyAsStringCallback callback) override {
- auto resource_request = std::make_unique<network::ResourceRequest>();
- resource_request->url = GURL(url);
- resource_request->method = "GET";
- resource_request->credentials_mode = network::mojom::CredentialsMode::kOmit;
-
- auto simple_loader = network::SimpleURLLoader::Create(
- std::move(resource_request), NO_TRAFFIC_ANNOTATION_YET);
+ auto simple_loader = CreateSimpleURLLoader(url);
auto* loader_ptr = simple_loader.get();
auto loader_factory = AmbientClient::Get()->GetURLLoaderFactory();
loader_ptr->DownloadToString(
@@ -171,7 +188,45 @@
kMaxImageSizeInBytes);
}
+ void DownloadToFile(
+ const std::string& url,
+ network::SimpleURLLoader::DownloadToFileCompleteCallback callback,
+ const base::FilePath& file_path) override {
+ auto simple_loader = CreateSimpleURLLoader(url);
+ auto loader_factory = AmbientClient::Get()->GetURLLoaderFactory();
+ auto* loader_ptr = simple_loader.get();
+ // Download to temp file first to guarantee entire image is written without
+ // errors before attempting to read it.
+
+ // Create a temp file.
+ base::FilePath temp_file;
+ if (!base::CreateTemporaryFileInDir(file_path.DirName(), &temp_file)) {
+ LOG(ERROR) << "Cannot create a temporary file";
+ std::move(callback).Run(base::FilePath());
+ return;
+ }
+
+ loader_ptr->DownloadToFile(
+ loader_factory.get(),
+ base::BindOnce(&AmbientURLLoaderImpl::OnUrlDownloadedToFile,
+ weak_factory_.GetWeakPtr(), std::move(callback),
+ std::move(simple_loader), std::move(loader_factory),
+ file_path),
+ temp_file);
+ }
+
private:
+ std::unique_ptr<network::SimpleURLLoader> CreateSimpleURLLoader(
+ const std::string& url) {
+ auto resource_request = std::make_unique<network::ResourceRequest>();
+ resource_request->url = GURL(url);
+ resource_request->method = "GET";
+ resource_request->credentials_mode = network::mojom::CredentialsMode::kOmit;
+
+ return network::SimpleURLLoader::Create(std::move(resource_request),
+ NO_TRAFFIC_ANNOTATION_YET);
+ }
+
// Called when the download completes.
void OnUrlDownloaded(
network::SimpleURLLoader::BodyAsStringCallback callback,
@@ -183,18 +238,42 @@
return;
}
- int response_code = -1;
- if (simple_loader->ResponseInfo() &&
- simple_loader->ResponseInfo()->headers) {
- response_code = simple_loader->ResponseInfo()->headers->response_code();
- }
-
- LOG(ERROR) << "Downloading Backdrop proto failed with error code: "
- << response_code << " with network error"
+ LOG(ERROR) << "Downloading to string failed with error code: "
+ << GetResponseCode(simple_loader.get()) << " with network error"
<< simple_loader->NetError();
std::move(callback).Run(std::make_unique<std::string>());
}
+ void OnUrlDownloadedToFile(
+ network::SimpleURLLoader::DownloadToFileCompleteCallback callback,
+ std::unique_ptr<network::SimpleURLLoader> simple_loader,
+ scoped_refptr<network::SharedURLLoaderFactory> loader_factory,
+ const base::FilePath& desired_path,
+ base::FilePath temp_path) {
+ if (simple_loader->NetError() != net::OK || temp_path.empty()) {
+ LOG(ERROR) << "Downloading to file failed with error code: "
+ << GetResponseCode(simple_loader.get())
+ << " with network error" << simple_loader->NetError();
+ std::move(callback).Run(base::FilePath());
+ return;
+ }
+ if (!base::ReplaceFile(temp_path, desired_path, /*error=*/nullptr)) {
+ LOG(ERROR) << "Unable to move downloaded file to ambient directory";
+ std::move(callback).Run(base::FilePath());
+ return;
+ }
+ std::move(callback).Run(std::move(desired_path));
+ }
+
+ int GetResponseCode(network::SimpleURLLoader* simple_loader) {
+ if (simple_loader->ResponseInfo() &&
+ simple_loader->ResponseInfo()->headers) {
+ return simple_loader->ResponseInfo()->headers->response_code();
+ } else {
+ return -1;
+ }
+ }
+
base::WeakPtrFactory<AmbientURLLoaderImpl> weak_factory_{this};
};
@@ -234,6 +313,12 @@
FROM_HERE, kWeatherRefreshInterval,
base::BindRepeating(&AmbientPhotoController::FetchWeather,
weak_factory_.GetWeakPtr()));
+ if (backup_photo_refresh_timer_.IsRunning()) {
+ // Would use |timer_.FireNow()| but this does not execute if screen is
+ // locked. Manually call the expected callback instead.
+ backup_photo_refresh_timer_.Stop();
+ PrepareFetchBackupImages();
+ }
}
void AmbientPhotoController::StopScreenUpdate() {
@@ -242,12 +327,25 @@
topic_index_ = 0;
image_refresh_started_ = false;
retries_to_read_from_cache_ = kMaxNumberOfCachedImages;
+ backup_retries_to_read_from_cache_ = GetBackupPhotoUrls().size();
fetch_topic_retry_backoff_.Reset();
resume_fetch_image_backoff_.Reset();
ambient_backend_model_.Clear();
weak_factory_.InvalidateWeakPtrs();
}
+void AmbientPhotoController::ScheduleFetchBackupImages() {
+ if (backup_photo_refresh_timer_.IsRunning())
+ return;
+
+ backup_photo_refresh_timer_.Start(
+ FROM_HERE,
+ std::max(kBackupPhotoRefreshDelay,
+ resume_fetch_image_backoff_.GetTimeUntilRelease()),
+ base::BindOnce(&AmbientPhotoController::PrepareFetchBackupImages,
+ weak_factory_.GetWeakPtr()));
+}
+
void AmbientPhotoController::OnTopicsChanged() {
if (ambient_backend_model_.topics().size() < kMaxNumberOfCachedImages)
ScheduleFetchTopics(/*backoff=*/false);
@@ -279,19 +377,23 @@
void AmbientPhotoController::ClearCache() {
task_runner_->PostTask(FROM_HERE,
- base::BindOnce(&DeletePathRecursively, GetRootPath()));
+ base::BindOnce(
+ [](const base::FilePath& file_path) {
+ base::DeletePathRecursively(file_path);
+ },
+ GetCachePath()));
}
void AmbientPhotoController::ScheduleFetchTopics(bool backoff) {
// If retry, using the backoff delay, otherwise the default delay.
- const base::TimeDelta kDelay =
+ const base::TimeDelta delay =
backoff ? fetch_topic_retry_backoff_.GetTimeUntilRelease()
: kTopicFetchInterval;
base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE,
base::BindOnce(&AmbientPhotoController::FetchTopics,
weak_factory_.GetWeakPtr()),
- kDelay);
+ delay);
}
void AmbientPhotoController::ScheduleRefreshImage() {
@@ -307,6 +409,37 @@
weak_factory_.GetWeakPtr()));
}
+void AmbientPhotoController::PrepareFetchBackupImages() {
+ task_runner_->PostTaskAndReply(
+ FROM_HERE,
+ base::BindOnce([]() { CreateDirIfNotExists(GetBackupCachePath()); }),
+ base::BindOnce(&AmbientPhotoController::FetchBackupImages,
+ weak_factory_.GetWeakPtr()));
+}
+
+void AmbientPhotoController::FetchBackupImages() {
+ const auto& backup_photo_urls = GetBackupPhotoUrls();
+ backup_retries_to_read_from_cache_ = backup_photo_urls.size();
+ for (size_t i = 0; i < backup_photo_urls.size(); i++) {
+ url_loader_->DownloadToFile(
+ backup_photo_urls.at(i),
+ base::BindOnce(&AmbientPhotoController::OnBackupImageFetched,
+ weak_factory_.GetWeakPtr()),
+ GetBackupFilePath(i));
+ }
+}
+
+void AmbientPhotoController::OnBackupImageFetched(base::FilePath file_path) {
+ if (file_path.empty()) {
+ // TODO(b/169807068) Change to retry individual failed images.
+ resume_fetch_image_backoff_.InformOfRequest(/*succeeded=*/false);
+ LOG(WARNING) << "Downloading backup image failed.";
+ ScheduleFetchBackupImages();
+ return;
+ }
+ resume_fetch_image_backoff_.InformOfRequest(/*succeeded=*/true);
+}
+
const AmbientModeTopic* AmbientPhotoController::GetNextTopic() {
const auto& topics = ambient_backend_model_.topics();
// If no more topics, will read from cache.
@@ -383,21 +516,54 @@
}
void AmbientPhotoController::TryReadPhotoRawData() {
+ auto on_done =
+ base::BindRepeating(&AmbientPhotoController::OnAllPhotoRawDataAvailable,
+ weak_factory_.GetWeakPtr(),
+ /*from_downloading=*/false);
+
// Stop reading from cache after the max number of retries.
if (retries_to_read_from_cache_ == 0) {
- if (topic_index_ == ambient_backend_model_.topics().size()) {
- image_refresh_started_ = false;
+ if (backup_retries_to_read_from_cache_ == 0) {
+ LOG(WARNING) << "Failed to read from cache";
+ if (topic_index_ == ambient_backend_model_.topics().size()) {
+ image_refresh_started_ = false;
+ return;
+ }
+
+ // Try to resume normal workflow with backoff.
+ const base::TimeDelta delay =
+ resume_fetch_image_backoff_.GetTimeUntilRelease();
+ base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
+ FROM_HERE,
+ base::BindOnce(&AmbientPhotoController::ScheduleRefreshImage,
+ weak_factory_.GetWeakPtr()),
+ delay);
return;
}
- // Try to resume normal workflow with backoff.
- const base::TimeDelta kDelay =
- resume_fetch_image_backoff_.GetTimeUntilRelease();
- base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
+ --backup_retries_to_read_from_cache_;
+ // Try to read a backup image.
+ auto photo_data = std::make_unique<std::string>();
+ auto* photo_data_ptr = photo_data.get();
+ task_runner_->PostTaskAndReply(
FROM_HERE,
- base::BindOnce(&AmbientPhotoController::ScheduleRefreshImage,
- weak_factory_.GetWeakPtr()),
- kDelay);
+ base::BindOnce(
+ [](size_t index, std::string* data) {
+ if (!base::ReadFileToString(GetBackupFilePath(index), data)) {
+ LOG(ERROR) << "Unable to read from backup cache.";
+ data->clear();
+ }
+ },
+ backup_cache_index_for_display_, photo_data_ptr),
+ base::BindOnce(&AmbientPhotoController::OnPhotoRawDataAvailable,
+ weak_factory_.GetWeakPtr(), /*from_downloading=*/false,
+ /*is_related_image=*/false, std::move(on_done),
+ /*details=*/std::make_unique<std::string>(),
+ std::move(photo_data)));
+
+ backup_cache_index_for_display_++;
+ if (backup_cache_index_for_display_ == GetBackupPhotoUrls().size())
+ backup_cache_index_for_display_ = 0;
return;
}
@@ -409,22 +575,18 @@
auto photo_data = std::make_unique<std::string>();
auto photo_details = std::make_unique<std::string>();
- auto on_done =
- base::BindRepeating(&AmbientPhotoController::OnAllPhotoRawDataAvailable,
- weak_factory_.GetWeakPtr(),
- /*from_downloading=*/false);
task_runner_->PostTaskAndReply(
FROM_HERE,
base::BindOnce(
[](const std::string& file_name, std::string* photo_data,
std::string* photo_details) {
if (!base::ReadFileToString(
- GetRootPath().Append(file_name + kPhotoFileExt),
+ GetCachePath().Append(file_name + kPhotoFileExt),
photo_data)) {
photo_data->clear();
}
if (!base::ReadFileToString(
- GetRootPath().Append(file_name + kPhotoDetailsFileExt),
+ GetCachePath().Append(file_name + kPhotoDetailsFileExt),
photo_details)) {
photo_details->clear();
}
@@ -432,7 +594,7 @@
file_name, photo_data.get(), photo_details.get()),
base::BindOnce(&AmbientPhotoController::OnPhotoRawDataAvailable,
weak_factory_.GetWeakPtr(), /*from_downloading=*/false,
- /*is_related_image=*/false, on_done,
+ /*is_related_image=*/false, std::move(on_done),
std::move(photo_details), std::move(photo_data)));
}
@@ -485,8 +647,8 @@
[](const std::string& file_name, bool need_to_save,
const std::string& data, const std::string& details) {
if (need_to_save) {
- WriteFile(GetRootPath().Append(file_name + kPhotoFileExt), data);
- WriteFile(GetRootPath().Append(file_name + kPhotoDetailsFileExt),
+ WriteFile(GetCachePath().Append(file_name + kPhotoFileExt), data);
+ WriteFile(GetCachePath().Append(file_name + kPhotoDetailsFileExt),
details);
}
},
@@ -538,6 +700,8 @@
}
retries_to_read_from_cache_ = kMaxNumberOfCachedImages;
+ backup_retries_to_read_from_cache_ = GetBackupPhotoUrls().size();
+
if (from_downloading)
resume_fetch_image_backoff_.InformOfRequest(/*succeeded=*/true);
@@ -603,4 +767,8 @@
FetchPhotoRawData();
}
+void AmbientPhotoController::FetchBackupImagesForTesting() {
+ PrepareFetchBackupImages();
+}
+
} // namespace ash
diff --git a/ash/ambient/ambient_photo_controller.h b/ash/ambient/ambient_photo_controller.h
index d2894dc0..d8ef32a2 100644
--- a/ash/ambient/ambient_photo_controller.h
+++ b/ash/ambient/ambient_photo_controller.h
@@ -44,6 +44,11 @@
virtual void Download(
const std::string& url,
network::SimpleURLLoader::BodyAsStringCallback callback) = 0;
+
+ virtual void DownloadToFile(
+ const std::string& url,
+ network::SimpleURLLoader::DownloadToFileCompleteCallback callback,
+ const base::FilePath& file_path) = 0;
};
// A wrapper class of |data_decoder| to decode the photo raw data. In the test,
@@ -87,6 +92,8 @@
void StartScreenUpdate();
void StopScreenUpdate();
+ void ScheduleFetchBackupImages();
+
AmbientBackendModel* ambient_backend_model() {
return &ambient_backend_model_;
}
@@ -95,6 +102,10 @@
return photo_refresh_timer_;
}
+ const base::OneShotTimer& backup_photo_refresh_timer_for_testing() const {
+ return backup_photo_refresh_timer_;
+ }
+
// AmbientBackendModelObserver:
void OnTopicsChanged() override;
@@ -112,6 +123,14 @@
void ScheduleRefreshImage();
+ // Create the backup cache directory and start downloading images.
+ void PrepareFetchBackupImages();
+
+ // Download backup cache images.
+ void FetchBackupImages();
+
+ void OnBackupImageFetched(base::FilePath file_path);
+
void GetScreenUpdateInfo();
// Return a topic to download the image.
@@ -178,11 +197,16 @@
void FetchImageForTesting();
+ void FetchBackupImagesForTesting();
+
AmbientBackendModel ambient_backend_model_;
// The timer to refresh photos.
base::OneShotTimer photo_refresh_timer_;
+ // The timer to refresh backup cache photos.
+ base::OneShotTimer backup_photo_refresh_timer_;
+
// The timer to refresh weather information.
base::RepeatingTimer weather_refresh_timer_;
@@ -194,6 +218,10 @@
// to read from the next cached file by increasing this index by 1.
int cache_index_for_display_ = 0;
+ // Current index of backup cached image to display when no other cached images
+ // are available.
+ size_t backup_cache_index_for_display_ = 0;
+
// Current index of cached image to save for the latest downloaded photo.
// The write command could fail. This index will increase 1 no matter writing
// successfully or not. But theoretically we could not to change this index if
@@ -207,6 +235,8 @@
// read cached images.
int retries_to_read_from_cache_ = kMaxNumberOfCachedImages;
+ int backup_retries_to_read_from_cache_ = 0;
+
// Backoff for fetch topics retries.
net::BackoffEntry fetch_topic_retry_backoff_;
diff --git a/ash/ambient/ambient_photo_controller_unittest.cc b/ash/ambient/ambient_photo_controller_unittest.cc
index 70aa5f5..c56d81c 100644
--- a/ash/ambient/ambient_photo_controller_unittest.cc
+++ b/ash/ambient/ambient_photo_controller_unittest.cc
@@ -31,7 +31,48 @@
namespace ash {
-using AmbientPhotoControllerTest = AmbientAshTestBase;
+class AmbientPhotoControllerTest : public AmbientAshTestBase {
+ public:
+ // AmbientAshTestBase:
+ void SetUp() override {
+ AmbientAshTestBase::SetUp();
+ CleanupAmbientDir();
+ }
+ void TearDown() override {
+ AmbientAshTestBase::TearDown();
+ CleanupAmbientDir();
+ }
+
+ void CleanupAmbientDir() { base::DeletePathRecursively(GetRootDir()); }
+
+ std::vector<base::FilePath> GetFilePathsInDir(const base::FilePath& dir) {
+ std::vector<base::FilePath> result;
+ base::FileEnumerator files(
+ dir, /*recursive=*/false,
+ base::FileEnumerator::FILES | base::FileEnumerator::DIRECTORIES);
+ for (base::FilePath current = files.Next(); !current.empty();
+ current = files.Next()) {
+ result.emplace_back(current);
+ }
+ return result;
+ }
+
+ base::FilePath GetRootDir() {
+ base::FilePath home_dir;
+ base::PathService::Get(base::DIR_HOME, &home_dir);
+ return home_dir.Append(FILE_PATH_LITERAL(kAmbientModeDirectoryName));
+ }
+
+ base::FilePath GetCacheDir() {
+ return GetRootDir().Append(
+ FILE_PATH_LITERAL(kAmbientModeCacheDirectoryName));
+ }
+
+ base::FilePath GetBackupCacheDir() {
+ return GetRootDir().Append(
+ FILE_PATH_LITERAL(kAmbientModeBackupCacheDirectoryName));
+ }
+};
// Test that topics are downloaded when starting screen update.
TEST_F(AmbientPhotoControllerTest, ShouldStartToDownloadTopics) {
@@ -101,53 +142,29 @@
// Test that image is saved.
TEST_F(AmbientPhotoControllerTest, ShouldSaveImagesOnDisk) {
- base::FilePath home_dir;
- base::PathService::Get(base::DIR_HOME, &home_dir);
-
- base::FilePath ambient_image_path =
- home_dir.Append(FILE_PATH_LITERAL(kAmbientModeDirectoryName));
-
- // Clean up.
- base::DeletePathRecursively(ambient_image_path);
+ base::FilePath ambient_image_path = GetCacheDir();
// Start to refresh images. It will download a test image and write it in
// |ambient_image_path| in a delayed task.
photo_controller()->StartScreenUpdate();
FastForwardToNextImage();
+ // Count files and directories in ambient_image_path. There should only be
+ // four files that were just created to save image files for this ambient mode
+ // session.
EXPECT_TRUE(base::PathExists(ambient_image_path));
-
- {
- // Count files and directories in root_path. There should only be one file
- // that was just created to save image files for this ambient mode session.
- base::FileEnumerator files(
- ambient_image_path, /*recursive=*/false,
- base::FileEnumerator::FILES | base::FileEnumerator::DIRECTORIES);
- int count = 0;
- for (base::FilePath current = files.Next(); !current.empty();
- current = files.Next()) {
- EXPECT_FALSE(files.GetInfo().IsDirectory());
- count++;
- }
-
- // Two image files and two attribution files.
- EXPECT_EQ(count, 4);
+ auto file_paths = GetFilePathsInDir(ambient_image_path);
+ // Two image files and two attribution files.
+ EXPECT_EQ(file_paths.size(), 4u);
+ for (auto& path : file_paths) {
+ // No sub directories.
+ EXPECT_FALSE(base::DirectoryExists(path));
}
-
- // Clean up.
- base::DeletePathRecursively(ambient_image_path);
}
// Test that image is save and will be deleted when stopping ambient mode.
TEST_F(AmbientPhotoControllerTest, ShouldNotDeleteImagesOnDisk) {
- base::FilePath home_dir;
- base::PathService::Get(base::DIR_HOME, &home_dir);
-
- base::FilePath ambient_image_path =
- home_dir.Append(FILE_PATH_LITERAL(kAmbientModeDirectoryName));
-
- // Clean up.
- base::DeletePathRecursively(ambient_image_path);
+ base::FilePath ambient_image_path = GetCacheDir();
// Start to refresh images. It will download a test image and write it in
// |ambient_image_path| in a delayed task.
@@ -169,37 +186,22 @@
image = photo_controller()->ambient_backend_model()->GetNextImage();
EXPECT_TRUE(image.IsNull());
- {
- // Count files and directories in root_path. There should only be one file
- // that was just created to save image files for this ambient mode session.
- base::FileEnumerator files(
- ambient_image_path, /*recursive=*/false,
- base::FileEnumerator::FILES | base::FileEnumerator::DIRECTORIES);
- int count = 0;
- for (base::FilePath current = files.Next(); !current.empty();
- current = files.Next()) {
- EXPECT_FALSE(files.GetInfo().IsDirectory());
- count++;
- }
-
- // Two image files and two attribution files.
- EXPECT_EQ(count, 4);
+ // Count files and directories in ambient_image_path. There should only be
+ // four files that were just created to save image files for the prior ambient
+ // mode session.
+ EXPECT_TRUE(base::PathExists(ambient_image_path));
+ auto file_paths = GetFilePathsInDir(ambient_image_path);
+ // Two image files and two attribution files.
+ EXPECT_EQ(file_paths.size(), 4u);
+ for (auto& path : file_paths) {
+ // No sub directories.
+ EXPECT_FALSE(base::DirectoryExists(path));
}
-
- // Clean up.
- base::DeletePathRecursively(ambient_image_path);
}
// Test that image is read from disk when no more topics.
TEST_F(AmbientPhotoControllerTest, ShouldReadCacheWhenNoMoreTopics) {
- base::FilePath home_dir;
- base::PathService::Get(base::DIR_HOME, &home_dir);
-
- base::FilePath ambient_image_path =
- home_dir.Append(FILE_PATH_LITERAL(kAmbientModeDirectoryName));
-
- // Clean up.
- base::DeletePathRecursively(ambient_image_path);
+ base::FilePath ambient_image_path = GetCacheDir();
FetchImage();
FastForwardToNextImage();
@@ -226,14 +228,7 @@
// Test that will try 100 times to read image from disk when no more topics.
TEST_F(AmbientPhotoControllerTest,
ShouldTry100TimesToReadCacheWhenNoMoreTopics) {
- base::FilePath home_dir;
- base::PathService::Get(base::DIR_HOME, &home_dir);
-
- base::FilePath ambient_image_path =
- home_dir.Append(FILE_PATH_LITERAL(kAmbientModeDirectoryName));
-
- // Clean up.
- base::DeletePathRecursively(ambient_image_path);
+ base::FilePath ambient_image_path = GetCacheDir();
FetchImage();
FastForwardToNextImage();
@@ -253,21 +248,11 @@
FastForwardToNextImage();
image = photo_controller()->ambient_backend_model()->GetNextImage();
EXPECT_FALSE(image.IsNull());
-
- // Clean up.
- base::DeletePathRecursively(ambient_image_path);
}
// Test that image is read from disk when image downloading failed.
TEST_F(AmbientPhotoControllerTest, ShouldReadCacheWhenImageDownloadingFailed) {
- base::FilePath home_dir;
- base::PathService::Get(base::DIR_HOME, &home_dir);
-
- base::FilePath ambient_image_path =
- home_dir.Append(FILE_PATH_LITERAL(kAmbientModeDirectoryName));
-
- // Clean up.
- base::DeletePathRecursively(ambient_image_path);
+ base::FilePath ambient_image_path = GetCacheDir();
SetUrlLoaderData(std::make_unique<std::string>());
FetchTopics();
@@ -290,23 +275,13 @@
task_environment()->FastForwardBy(0.2 * kTopicFetchInterval);
image = photo_controller()->ambient_backend_model()->GetNextImage();
EXPECT_FALSE(image.IsNull());
-
- // Clean up.
- base::DeletePathRecursively(ambient_image_path);
}
// Test that image is read from disk when image decoding failed.
TEST_F(AmbientPhotoControllerTest, ShouldReadCacheWhenImageDecodingFailed) {
- base::FilePath home_dir;
- base::PathService::Get(base::DIR_HOME, &home_dir);
+ base::FilePath ambient_image_path = GetCacheDir();
- base::FilePath ambient_image_path =
- home_dir.Append(FILE_PATH_LITERAL(kAmbientModeDirectoryName));
-
- // Clean up.
- base::DeletePathRecursively(ambient_image_path);
-
- SeteImageDecoderImage(gfx::ImageSkia());
+ SetImageDecoderImage(gfx::ImageSkia());
FetchTopics();
// Forward a little bit time. FetchTopics() will succeed.
// Downloading succeed and save the data to disk.
@@ -314,21 +289,11 @@
task_environment()->FastForwardBy(0.2 * kTopicFetchInterval);
auto image = photo_controller()->ambient_backend_model()->GetNextImage();
EXPECT_FALSE(image.IsNull());
-
- // Clean up.
- base::DeletePathRecursively(ambient_image_path);
}
// Test that image will refresh when have more topics.
TEST_F(AmbientPhotoControllerTest, ShouldResumWhenHaveMoreTopics) {
- base::FilePath home_dir;
- base::PathService::Get(base::DIR_HOME, &home_dir);
-
- base::FilePath ambient_image_path =
- home_dir.Append(FILE_PATH_LITERAL(kAmbientModeDirectoryName));
-
- // Clean up.
- base::DeletePathRecursively(ambient_image_path);
+ base::FilePath ambient_image_path = GetCacheDir();
FetchImage();
FastForwardToNextImage();
@@ -341,9 +306,95 @@
task_environment()->FastForwardBy(0.2 * kTopicFetchInterval);
image = photo_controller()->ambient_backend_model()->GetNextImage();
EXPECT_FALSE(image.IsNull());
+}
- // Clean up.
- base::DeletePathRecursively(ambient_image_path);
+TEST_F(AmbientPhotoControllerTest, ShouldDownloadBackupImagesWhenScheduled) {
+ base::FilePath backup_image_path = GetBackupCacheDir();
+
+ std::string expected_data = "backup data";
+ SetUrlLoaderData(std::make_unique<std::string>(expected_data));
+
+ photo_controller()->ScheduleFetchBackupImages();
+
+ EXPECT_TRUE(
+ photo_controller()->backup_photo_refresh_timer_for_testing().IsRunning());
+
+ // TImer is running but download has not started yet.
+ EXPECT_FALSE(base::DirectoryExists(GetBackupCacheDir()));
+ task_environment()->FastForwardBy(kBackupPhotoRefreshDelay);
+
+ // Timer should have stopped.
+ EXPECT_FALSE(
+ photo_controller()->backup_photo_refresh_timer_for_testing().IsRunning());
+
+ // Download has triggered and backup cache directory is created.
+ EXPECT_TRUE(base::DirectoryExists(backup_image_path));
+
+ // Should be two files in backup cache directory.
+ auto paths = GetFilePathsInDir(backup_image_path);
+ std::sort(paths.begin(), paths.end());
+ EXPECT_EQ(paths.size(), 2u);
+ EXPECT_EQ(paths[0].BaseName().value(), "0.img");
+ EXPECT_EQ(paths[1].BaseName().value(), "1.img");
+ for (const auto& path : paths) {
+ std::string data;
+ base::ReadFileToString(path, &data);
+ EXPECT_EQ(data, expected_data);
+ }
+}
+
+TEST_F(AmbientPhotoControllerTest, ShouldResetTimerWhenBackupImagesFail) {
+ photo_controller()->ScheduleFetchBackupImages();
+
+ EXPECT_TRUE(
+ photo_controller()->backup_photo_refresh_timer_for_testing().IsRunning());
+
+ // Simulate an error in DownloadToFile.
+ SetUrlLoaderData(nullptr);
+ task_environment()->FastForwardBy(kBackupPhotoRefreshDelay);
+
+ // Directory should have been created, but with no files in it.
+ EXPECT_TRUE(base::DirectoryExists(GetBackupCacheDir()));
+
+ auto paths = GetFilePathsInDir(GetBackupCacheDir());
+ EXPECT_EQ(paths.size(), 0u);
+
+ // Timer should have restarted.
+ EXPECT_TRUE(
+ photo_controller()->backup_photo_refresh_timer_for_testing().IsRunning());
+}
+
+TEST_F(AmbientPhotoControllerTest,
+ ShouldStartDownloadBackupImagesOnAmbientModeStart) {
+ photo_controller()->ScheduleFetchBackupImages();
+
+ EXPECT_TRUE(
+ photo_controller()->backup_photo_refresh_timer_for_testing().IsRunning());
+
+ SetUrlLoaderData(std::make_unique<std::string>("image data"));
+
+ photo_controller()->StartScreenUpdate();
+
+ // Download should have started immediately.
+ EXPECT_FALSE(
+ photo_controller()->backup_photo_refresh_timer_for_testing().IsRunning());
+
+ task_environment()->RunUntilIdle();
+
+ // Download has triggered and backup cache directory is created.
+ EXPECT_TRUE(base::DirectoryExists(GetBackupCacheDir()));
+
+ // Should be two files in backup cache directory.
+ auto paths = GetFilePathsInDir(GetBackupCacheDir());
+ std::sort(paths.begin(), paths.end());
+ EXPECT_EQ(paths.size(), 2u);
+ EXPECT_EQ(paths[0].BaseName().value(), "0.img");
+ EXPECT_EQ(paths[1].BaseName().value(), "1.img");
+ for (const auto& path : paths) {
+ std::string data;
+ base::ReadFileToString(path, &data);
+ EXPECT_EQ(data, "image data");
+ }
}
TEST_F(AmbientPhotoControllerTest, ShouldStartToRefreshWeather) {
diff --git a/ash/ambient/backdrop/ambient_backend_controller_impl.cc b/ash/ambient/backdrop/ambient_backend_controller_impl.cc
index d7fc74a..b9e343d 100644
--- a/ash/ambient/backdrop/ambient_backend_controller_impl.cc
+++ b/ash/ambient/backdrop/ambient_backend_controller_impl.cc
@@ -4,6 +4,7 @@
#include "ash/ambient/backdrop/ambient_backend_controller_impl.h"
+#include <array>
#include <string>
#include <utility>
#include <vector>
@@ -23,6 +24,7 @@
#include "base/guid.h"
#include "base/optional.h"
#include "base/time/time.h"
+#include "chromeos/assistant/internal/ambient/backdrop_client_config.h"
#include "chromeos/assistant/internal/proto/google3/backdrop/backdrop.pb.h"
#include "chromeos/constants/chromeos_features.h"
#include "components/prefs/pref_service.h"
@@ -455,6 +457,11 @@
std::move(backdrop_url_loader)));
}
+const std::array<const char*, 2>&
+AmbientBackendControllerImpl::GetBackupPhotoUrls() const {
+ return chromeos::ambient::kBackupPhotoUrls;
+}
+
void AmbientBackendControllerImpl::FetchScreenUpdateInfoInternal(
int num_topics,
OnScreenUpdateInfoFetchedCallback callback,
diff --git a/ash/ambient/backdrop/ambient_backend_controller_impl.h b/ash/ambient/backdrop/ambient_backend_controller_impl.h
index be25bd60..8fd9d3b3 100644
--- a/ash/ambient/backdrop/ambient_backend_controller_impl.h
+++ b/ash/ambient/backdrop/ambient_backend_controller_impl.h
@@ -5,6 +5,7 @@
#ifndef ASH_AMBIENT_BACKDROP_AMBIENT_BACKEND_CONTROLLER_IMPL_H_
#define ASH_AMBIENT_BACKDROP_AMBIENT_BACKEND_CONTROLLER_IMPL_H_
+#include <array>
#include <memory>
#include <string>
#include <utility>
@@ -48,6 +49,7 @@
OnSettingsAndAlbumsFetchedCallback callback) override;
void SetPhotoRefreshInterval(base::TimeDelta interval) override;
void FetchWeather(FetchWeatherCallback callback) override;
+ const std::array<const char*, 2>& GetBackupPhotoUrls() const override;
private:
using BackdropClientConfig = chromeos::ambient::BackdropClientConfig;
diff --git a/ash/ambient/test/ambient_ash_test_base.cc b/ash/ambient/test/ambient_ash_test_base.cc
index 90b08f4..9b86a31 100644
--- a/ash/ambient/test/ambient_ash_test_base.cc
+++ b/ash/ambient/test/ambient_ash_test_base.cc
@@ -21,9 +21,11 @@
#include "ash/session/session_controller_impl.h"
#include "ash/shell.h"
#include "base/callback.h"
+#include "base/files/file_util.h"
#include "base/memory/ptr_util.h"
#include "base/run_loop.h"
#include "base/sequenced_task_runner.h"
+#include "base/threading/scoped_blocking_call.h"
#include "base/threading/sequenced_task_runner_handle.h"
#include "base/time/time.h"
#include "chromeos/constants/chromeos_features.h"
@@ -48,18 +50,42 @@
void Download(
const std::string& url,
network::SimpleURLLoader::BodyAsStringCallback callback) override {
- std::string data = data_ ? *data_ : "test";
// Pretend to respond asynchronously.
base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE,
base::BindOnce(std::move(callback),
- std::make_unique<std::string>(data)),
+ std::make_unique<std::string>(data_ ? *data_ : "test")),
base::TimeDelta::FromMilliseconds(1));
}
+ void DownloadToFile(
+ const std::string& url,
+ network::SimpleURLLoader::DownloadToFileCompleteCallback callback,
+ const base::FilePath& file_path) override {
+ if (!data_) {
+ base::SequencedTaskRunnerHandle::Get()->PostTask(
+ FROM_HERE, base::BindOnce(std::move(callback), base::FilePath()));
+ return;
+ }
+
+ if (!WriteFile(file_path, *data_)) {
+ LOG(WARNING) << "error writing file to file_path: " << file_path;
+
+ base::SequencedTaskRunnerHandle::Get()->PostTask(
+ FROM_HERE, base::BindOnce(std::move(callback), base::FilePath()));
+ return;
+ }
+
+ base::SequencedTaskRunnerHandle::Get()->PostTask(
+ FROM_HERE, base::BindOnce(std::move(callback), file_path));
+ }
void SetData(std::unique_ptr<std::string> data) { data_ = std::move(data); }
private:
+ bool WriteFile(const base::FilePath& file_path, const std::string& data) {
+ base::ScopedBlockingCall blocking(FROM_HERE, base::BlockingType::MAY_BLOCK);
+ return base::WriteFile(file_path, data);
+ }
// If not null, will return this data.
std::unique_ptr<std::string> data_;
};
@@ -352,6 +378,10 @@
photo_controller()->FetchImageForTesting();
}
+void AmbientAshTestBase::FetchBackupImages() {
+ photo_controller()->FetchBackupImagesForTesting();
+}
+
void AmbientAshTestBase::SetUrlLoaderData(std::unique_ptr<std::string> data) {
auto* url_loader_ = static_cast<TestAmbientURLLoaderImpl*>(
photo_controller()->get_url_loader_for_testing());
@@ -359,7 +389,7 @@
url_loader_->SetData(std::move(data));
}
-void AmbientAshTestBase::SeteImageDecoderImage(const gfx::ImageSkia& image) {
+void AmbientAshTestBase::SetImageDecoderImage(const gfx::ImageSkia& image) {
auto* image_decoder = static_cast<TestAmbientImageDecoderImpl*>(
photo_controller()->get_image_decoder_for_testing());
diff --git a/ash/ambient/test/ambient_ash_test_base.h b/ash/ambient/test/ambient_ash_test_base.h
index a863d78..35a8e7b 100644
--- a/ash/ambient/test/ambient_ash_test_base.h
+++ b/ash/ambient/test/ambient_ash_test_base.h
@@ -141,9 +141,11 @@
void FetchImage();
+ void FetchBackupImages();
+
void SetUrlLoaderData(std::unique_ptr<std::string> data);
- void SeteImageDecoderImage(const gfx::ImageSkia& image);
+ void SetImageDecoderImage(const gfx::ImageSkia& image);
private:
base::test::ScopedFeatureList scoped_feature_list_;
diff --git a/ash/public/cpp/ambient/ambient_backend_controller.h b/ash/public/cpp/ambient/ambient_backend_controller.h
index b7b7132..77f5931 100644
--- a/ash/public/cpp/ambient/ambient_backend_controller.h
+++ b/ash/public/cpp/ambient/ambient_backend_controller.h
@@ -5,6 +5,7 @@
#ifndef ASH_PUBLIC_CPP_AMBIENT_AMBIENT_BACKEND_CONTROLLER_H_
#define ASH_PUBLIC_CPP_AMBIENT_AMBIENT_BACKEND_CONTROLLER_H_
+#include <array>
#include <string>
#include <vector>
@@ -156,6 +157,10 @@
// Fetch the weather information.
virtual void FetchWeather(FetchWeatherCallback) = 0;
+
+ // Get stock photo urls to cache in advance in case Ambient mode is started
+ // without internet access.
+ virtual const std::array<const char*, 2>& GetBackupPhotoUrls() const = 0;
};
} // namespace ash
diff --git a/ash/public/cpp/ambient/fake_ambient_backend_controller_impl.cc b/ash/public/cpp/ambient/fake_ambient_backend_controller_impl.cc
index 8a9a1eb..f54c394 100644
--- a/ash/public/cpp/ambient/fake_ambient_backend_controller_impl.cc
+++ b/ash/public/cpp/ambient/fake_ambient_backend_controller_impl.cc
@@ -4,6 +4,7 @@
#include "ash/public/cpp/ambient/fake_ambient_backend_controller_impl.h"
+#include <array>
#include <utility>
#include "ash/public/cpp/ambient/ambient_backend_controller.h"
@@ -26,6 +27,9 @@
constexpr char kFakeDetails[] = "fake-photo-attribution";
+constexpr std::array<const char*, 2> kFakeBackupPhotoUrls = {kFakeUrl,
+ kFakeUrl};
+
AmbientSettings CreateFakeSettings() {
AmbientSettings settings;
settings.topic_source = kTopicSource;
@@ -154,6 +158,11 @@
std::move(callback).Run(weather_info_);
}
+const std::array<const char*, 2>&
+FakeAmbientBackendControllerImpl::GetBackupPhotoUrls() const {
+ return kFakeBackupPhotoUrls;
+}
+
void FakeAmbientBackendControllerImpl::ReplyFetchSettingsAndAlbums(
bool success) {
if (!pending_fetch_settings_albums_callback_)
diff --git a/ash/public/cpp/ambient/fake_ambient_backend_controller_impl.h b/ash/public/cpp/ambient/fake_ambient_backend_controller_impl.h
index c217a9f..39ffabf9 100644
--- a/ash/public/cpp/ambient/fake_ambient_backend_controller_impl.h
+++ b/ash/public/cpp/ambient/fake_ambient_backend_controller_impl.h
@@ -5,6 +5,8 @@
#ifndef ASH_PUBLIC_CPP_AMBIENT_FAKE_AMBIENT_BACKEND_CONTROLLER_IMPL_H_
#define ASH_PUBLIC_CPP_AMBIENT_FAKE_AMBIENT_BACKEND_CONTROLLER_IMPL_H_
+#include <array>
+
#include "ash/public/cpp/ambient/ambient_backend_controller.h"
#include "ash/public/cpp/ash_public_export.h"
#include "base/callback.h"
@@ -40,6 +42,7 @@
OnSettingsAndAlbumsFetchedCallback callback) override;
void SetPhotoRefreshInterval(base::TimeDelta interval) override;
void FetchWeather(FetchWeatherCallback callback) override;
+ const std::array<const char*, 2>& GetBackupPhotoUrls() const override;
// Simulate to reply the request of FetchSettingsAndAlbums().
// If |success| is true, will return fake data.
diff --git a/chrome/browser/ui/ash/ambient/ambient_client_impl_unittest.cc b/chrome/browser/ui/ash/ambient/ambient_client_impl_unittest.cc
index 348ba85..7cf4213 100644
--- a/chrome/browser/ui/ash/ambient/ambient_client_impl_unittest.cc
+++ b/chrome/browser/ui/ash/ambient/ambient_client_impl_unittest.cc
@@ -32,7 +32,6 @@
chromeos::features::kAmbientModeFeature);
// Needed by ash.
ambient_client_ = std::make_unique<AmbientClientImpl>();
- AshTestBase::SetUp();
ASSERT_TRUE(data_dir_.CreateUniqueTempDir());
profile_manager_ = std::make_unique<TestingProfileManager>(
@@ -48,6 +47,8 @@
std::make_unique<IdentityTestEnvironmentProfileAdaptor>(profile_);
user_manager_enabler_ = std::make_unique<user_manager::ScopedUserManager>(
std::make_unique<chromeos::FakeChromeUserManager>());
+
+ AshTestBase::SetUp();
}
void TearDown() override {
