Revert "ambient: cache two backup photos"
This reverts commit 57520f4f6c25567d3339cc44852ea41c9861be37.
Reason for revert: causes crash on device b:170312454
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
>
> (cherry picked from commit a81c26884bde81a386ffb2dcbdb7a12f0ecd1215)
>
> 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-Original-Commit-Position: refs/heads/master@{#813904}
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2451370
> Cr-Commit-Position: refs/branch-heads/4280@{#62}
> Cr-Branched-From: ea420fb963f9658c9969b6513c56b8f47efa1a2a-refs/heads/master@{#812852}
[email protected],[email protected],[email protected]
Change-Id: I9c2dd1becd3c8591d4115ab4d7b97bb9e5f5875b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: b:167332126
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2457367
Reviewed-by: Xiaohui Chen <[email protected]>
Reviewed-by: Tao Wu <[email protected]>
Commit-Queue: Jeffrey Young <[email protected]>
Cr-Commit-Position: refs/branch-heads/4280@{#83}
Cr-Branched-From: ea420fb963f9658c9969b6513c56b8f47efa1a2a-refs/heads/master@{#812852}
diff --git a/ash/ambient/ambient_constants.h b/ash/ambient/ambient_constants.h
index 0b82785..426a538 100644
--- a/ash/ambient/ambient_constants.h
+++ b/ash/ambient/ambient_constants.h
@@ -24,10 +24,6 @@
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);
@@ -51,10 +47,6 @@
// 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 02c4e0e..40d4d5e 100644
--- a/ash/ambient/ambient_controller.cc
+++ b/ash/ambient/ambient_controller.cc
@@ -318,11 +318,6 @@
}
}
-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 1cf45fb..47ccb1b 100644
--- a/ash/ambient/ambient_controller.h
+++ b/ash/ambient/ambient_controller.h
@@ -59,7 +59,6 @@
// 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 20f34ad..b5ea5c9 100644
--- a/ash/ambient/ambient_photo_controller.cc
+++ b/ash/ambient/ambient_photo_controller.cc
@@ -4,10 +4,8 @@
#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"
@@ -83,6 +81,17 @@
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());
@@ -100,34 +109,9 @@
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 (!CreateDirIfNotExists(GetCachePath())) {
+ if (!base::PathExists(GetRootPath()) &&
+ !base::CreateDirectory(GetRootPath())) {
LOG(ERROR) << "Cannot create ambient mode directory.";
return;
}
@@ -159,13 +143,6 @@
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 {
@@ -177,7 +154,13 @@
void Download(
const std::string& url,
network::SimpleURLLoader::BodyAsStringCallback callback) override {
- auto simple_loader = CreateSimpleURLLoader(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;
+
+ auto simple_loader = network::SimpleURLLoader::Create(
+ std::move(resource_request), NO_TRAFFIC_ANNOTATION_YET);
auto* loader_ptr = simple_loader.get();
auto loader_factory = AmbientClient::Get()->GetURLLoaderFactory();
loader_ptr->DownloadToString(
@@ -188,35 +171,7 @@
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.
- loader_ptr->DownloadToTempFile(
- loader_factory.get(),
- base::BindOnce(&AmbientURLLoaderImpl::OnUrlDownloadedToFile,
- weak_factory_.GetWeakPtr(), std::move(callback),
- std::move(simple_loader), std::move(loader_factory),
- file_path));
- }
-
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,
@@ -228,38 +183,16 @@
return;
}
- 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());
- }
- 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());
- }
- std::move(callback).Run(std::move(desired_path));
- }
-
- int GetResponseCode(network::SimpleURLLoader* simple_loader) {
+ int response_code = -1;
if (simple_loader->ResponseInfo() &&
simple_loader->ResponseInfo()->headers) {
- return simple_loader->ResponseInfo()->headers->response_code();
- } else {
- return -1;
+ response_code = simple_loader->ResponseInfo()->headers->response_code();
}
+
+ LOG(ERROR) << "Downloading Backdrop proto failed with error code: "
+ << response_code << " with network error"
+ << simple_loader->NetError();
+ std::move(callback).Run(std::make_unique<std::string>());
}
base::WeakPtrFactory<AmbientURLLoaderImpl> weak_factory_{this};
@@ -301,12 +234,6 @@
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() {
@@ -315,25 +242,12 @@
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);
@@ -365,23 +279,19 @@
void AmbientPhotoController::ClearCache() {
task_runner_->PostTask(FROM_HERE,
- base::BindOnce(
- [](const base::FilePath& file_path) {
- base::DeletePathRecursively(file_path);
- },
- GetCachePath()));
+ base::BindOnce(&DeletePathRecursively, GetRootPath()));
}
void AmbientPhotoController::ScheduleFetchTopics(bool backoff) {
// If retry, using the backoff delay, otherwise the default delay.
- const base::TimeDelta delay =
+ const base::TimeDelta kDelay =
backoff ? fetch_topic_retry_backoff_.GetTimeUntilRelease()
: kTopicFetchInterval;
base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE,
base::BindOnce(&AmbientPhotoController::FetchTopics,
weak_factory_.GetWeakPtr()),
- delay);
+ kDelay);
}
void AmbientPhotoController::ScheduleRefreshImage() {
@@ -397,37 +307,6 @@
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.
@@ -504,54 +383,21 @@
}
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 (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);
+ if (topic_index_ == ambient_backend_model_.topics().size()) {
+ image_refresh_started_ = false;
return;
}
- --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(
+ // Try to resume normal workflow with backoff.
+ const base::TimeDelta kDelay =
+ resume_fetch_image_backoff_.GetTimeUntilRelease();
+ base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE,
- 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;
+ base::BindOnce(&AmbientPhotoController::ScheduleRefreshImage,
+ weak_factory_.GetWeakPtr()),
+ kDelay);
return;
}
@@ -563,18 +409,22 @@
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(
- GetCachePath().Append(file_name + kPhotoFileExt),
+ GetRootPath().Append(file_name + kPhotoFileExt),
photo_data)) {
photo_data->clear();
}
if (!base::ReadFileToString(
- GetCachePath().Append(file_name + kPhotoDetailsFileExt),
+ GetRootPath().Append(file_name + kPhotoDetailsFileExt),
photo_details)) {
photo_details->clear();
}
@@ -582,7 +432,7 @@
file_name, photo_data.get(), photo_details.get()),
base::BindOnce(&AmbientPhotoController::OnPhotoRawDataAvailable,
weak_factory_.GetWeakPtr(), /*from_downloading=*/false,
- /*is_related_image=*/false, std::move(on_done),
+ /*is_related_image=*/false, on_done,
std::move(photo_details), std::move(photo_data)));
}
@@ -635,8 +485,8 @@
[](const std::string& file_name, bool need_to_save,
const std::string& data, const std::string& details) {
if (need_to_save) {
- WriteFile(GetCachePath().Append(file_name + kPhotoFileExt), data);
- WriteFile(GetCachePath().Append(file_name + kPhotoDetailsFileExt),
+ WriteFile(GetRootPath().Append(file_name + kPhotoFileExt), data);
+ WriteFile(GetRootPath().Append(file_name + kPhotoDetailsFileExt),
details);
}
},
@@ -688,8 +538,6 @@
}
retries_to_read_from_cache_ = kMaxNumberOfCachedImages;
- backup_retries_to_read_from_cache_ = GetBackupPhotoUrls().size();
-
if (from_downloading)
resume_fetch_image_backoff_.InformOfRequest(/*succeeded=*/true);
@@ -755,8 +603,4 @@
FetchPhotoRawData();
}
-void AmbientPhotoController::FetchBackupImagesForTesting() {
- PrepareFetchBackupImages();
-}
-
} // namespace ash
diff --git a/ash/ambient/ambient_photo_controller.h b/ash/ambient/ambient_photo_controller.h
index d8ef32a2..d2894dc0 100644
--- a/ash/ambient/ambient_photo_controller.h
+++ b/ash/ambient/ambient_photo_controller.h
@@ -44,11 +44,6 @@
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,
@@ -92,8 +87,6 @@
void StartScreenUpdate();
void StopScreenUpdate();
- void ScheduleFetchBackupImages();
-
AmbientBackendModel* ambient_backend_model() {
return &ambient_backend_model_;
}
@@ -102,10 +95,6 @@
return photo_refresh_timer_;
}
- const base::OneShotTimer& backup_photo_refresh_timer_for_testing() const {
- return backup_photo_refresh_timer_;
- }
-
// AmbientBackendModelObserver:
void OnTopicsChanged() override;
@@ -123,14 +112,6 @@
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.
@@ -197,16 +178,11 @@
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_;
@@ -218,10 +194,6 @@
// 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
@@ -235,8 +207,6 @@
// 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 c56d81c..70aa5f5 100644
--- a/ash/ambient/ambient_photo_controller_unittest.cc
+++ b/ash/ambient/ambient_photo_controller_unittest.cc
@@ -31,48 +31,7 @@
namespace ash {
-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));
- }
-};
+using AmbientPhotoControllerTest = AmbientAshTestBase;
// Test that topics are downloaded when starting screen update.
TEST_F(AmbientPhotoControllerTest, ShouldStartToDownloadTopics) {
@@ -142,29 +101,53 @@
// Test that image is saved.
TEST_F(AmbientPhotoControllerTest, ShouldSaveImagesOnDisk) {
- base::FilePath ambient_image_path = GetCacheDir();
+ 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);
// 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));
- 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));
+
+ {
+ // 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);
}
+
+ // 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 ambient_image_path = GetCacheDir();
+ 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);
// Start to refresh images. It will download a test image and write it in
// |ambient_image_path| in a delayed task.
@@ -186,22 +169,37 @@
image = photo_controller()->ambient_backend_model()->GetNextImage();
EXPECT_TRUE(image.IsNull());
- // 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));
+ {
+ // 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);
}
+
+ // Clean up.
+ base::DeletePathRecursively(ambient_image_path);
}
// Test that image is read from disk when no more topics.
TEST_F(AmbientPhotoControllerTest, ShouldReadCacheWhenNoMoreTopics) {
- base::FilePath ambient_image_path = GetCacheDir();
+ 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);
FetchImage();
FastForwardToNextImage();
@@ -228,7 +226,14 @@
// Test that will try 100 times to read image from disk when no more topics.
TEST_F(AmbientPhotoControllerTest,
ShouldTry100TimesToReadCacheWhenNoMoreTopics) {
- base::FilePath ambient_image_path = GetCacheDir();
+ 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);
FetchImage();
FastForwardToNextImage();
@@ -248,11 +253,21 @@
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 ambient_image_path = GetCacheDir();
+ 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);
SetUrlLoaderData(std::make_unique<std::string>());
FetchTopics();
@@ -275,13 +290,23 @@
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 ambient_image_path = GetCacheDir();
+ base::FilePath home_dir;
+ base::PathService::Get(base::DIR_HOME, &home_dir);
- SetImageDecoderImage(gfx::ImageSkia());
+ base::FilePath ambient_image_path =
+ home_dir.Append(FILE_PATH_LITERAL(kAmbientModeDirectoryName));
+
+ // Clean up.
+ base::DeletePathRecursively(ambient_image_path);
+
+ SeteImageDecoderImage(gfx::ImageSkia());
FetchTopics();
// Forward a little bit time. FetchTopics() will succeed.
// Downloading succeed and save the data to disk.
@@ -289,11 +314,21 @@
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 ambient_image_path = GetCacheDir();
+ 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);
FetchImage();
FastForwardToNextImage();
@@ -306,95 +341,9 @@
task_environment()->FastForwardBy(0.2 * kTopicFetchInterval);
image = photo_controller()->ambient_backend_model()->GetNextImage();
EXPECT_FALSE(image.IsNull());
-}
-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");
- }
+ // Clean up.
+ base::DeletePathRecursively(ambient_image_path);
}
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 b9e343d..d7fc74a 100644
--- a/ash/ambient/backdrop/ambient_backend_controller_impl.cc
+++ b/ash/ambient/backdrop/ambient_backend_controller_impl.cc
@@ -4,7 +4,6 @@
#include "ash/ambient/backdrop/ambient_backend_controller_impl.h"
-#include <array>
#include <string>
#include <utility>
#include <vector>
@@ -24,7 +23,6 @@
#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"
@@ -457,11 +455,6 @@
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 8fd9d3b3..be25bd60 100644
--- a/ash/ambient/backdrop/ambient_backend_controller_impl.h
+++ b/ash/ambient/backdrop/ambient_backend_controller_impl.h
@@ -5,7 +5,6 @@
#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>
@@ -49,7 +48,6 @@
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 9b86a31..90b08f4 100644
--- a/ash/ambient/test/ambient_ash_test_base.cc
+++ b/ash/ambient/test/ambient_ash_test_base.cc
@@ -21,11 +21,9 @@
#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"
@@ -50,42 +48,18 @@
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_ ? *data_ : "test")),
+ std::make_unique<std::string>(data)),
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_;
};
@@ -378,10 +352,6 @@
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());
@@ -389,7 +359,7 @@
url_loader_->SetData(std::move(data));
}
-void AmbientAshTestBase::SetImageDecoderImage(const gfx::ImageSkia& image) {
+void AmbientAshTestBase::SeteImageDecoderImage(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 35a8e7b..a863d78 100644
--- a/ash/ambient/test/ambient_ash_test_base.h
+++ b/ash/ambient/test/ambient_ash_test_base.h
@@ -141,11 +141,9 @@
void FetchImage();
- void FetchBackupImages();
-
void SetUrlLoaderData(std::unique_ptr<std::string> data);
- void SetImageDecoderImage(const gfx::ImageSkia& image);
+ void SeteImageDecoderImage(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 77f5931..b7b7132 100644
--- a/ash/public/cpp/ambient/ambient_backend_controller.h
+++ b/ash/public/cpp/ambient/ambient_backend_controller.h
@@ -5,7 +5,6 @@
#ifndef ASH_PUBLIC_CPP_AMBIENT_AMBIENT_BACKEND_CONTROLLER_H_
#define ASH_PUBLIC_CPP_AMBIENT_AMBIENT_BACKEND_CONTROLLER_H_
-#include <array>
#include <string>
#include <vector>
@@ -157,10 +156,6 @@
// 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 f54c394..8a9a1eb 100644
--- a/ash/public/cpp/ambient/fake_ambient_backend_controller_impl.cc
+++ b/ash/public/cpp/ambient/fake_ambient_backend_controller_impl.cc
@@ -4,7 +4,6 @@
#include "ash/public/cpp/ambient/fake_ambient_backend_controller_impl.h"
-#include <array>
#include <utility>
#include "ash/public/cpp/ambient/ambient_backend_controller.h"
@@ -27,9 +26,6 @@
constexpr char kFakeDetails[] = "fake-photo-attribution";
-constexpr std::array<const char*, 2> kFakeBackupPhotoUrls = {kFakeUrl,
- kFakeUrl};
-
AmbientSettings CreateFakeSettings() {
AmbientSettings settings;
settings.topic_source = kTopicSource;
@@ -158,11 +154,6 @@
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 39ffabf9..c217a9f 100644
--- a/ash/public/cpp/ambient/fake_ambient_backend_controller_impl.h
+++ b/ash/public/cpp/ambient/fake_ambient_backend_controller_impl.h
@@ -5,8 +5,6 @@
#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"
@@ -42,7 +40,6 @@
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 7cf4213..348ba85 100644
--- a/chrome/browser/ui/ash/ambient/ambient_client_impl_unittest.cc
+++ b/chrome/browser/ui/ash/ambient/ambient_client_impl_unittest.cc
@@ -32,6 +32,7 @@
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>(
@@ -47,8 +48,6 @@
std::make_unique<IdentityTestEnvironmentProfileAdaptor>(profile_);
user_manager_enabler_ = std::make_unique<user_manager::ScopedUserManager>(
std::make_unique<chromeos::FakeChromeUserManager>());
-
- AshTestBase::SetUp();
}
void TearDown() override {
