Commit 2c341607 authored by Jeffrey Young's avatar Jeffrey Young Committed by Commit Bot

Reland "ambient: cache two backup photos"

This is a reland of a81c2688

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 <cowmoo@chromium.org>
> Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
> Reviewed-by: Tao Wu <wutao@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#813904}

Bug: b:167332126
Bug: b:170312454
Change-Id: If5306f627e1a6dfb5e4eb37842a470d17968ad4e
Cq-Include-Trybots: luci.chrome.try:linux-chromeos-chrome
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2458894
Commit-Queue: Jeffrey Young <cowmoo@chromium.org>
Reviewed-by: default avatarTao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814968}
parent b10bf33a
......@@ -24,6 +24,10 @@ constexpr base::TimeDelta kTopicFetchInterval =
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 @@ constexpr char kPhotoDetailsFileExt[] = ".txt";
// 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);
......
......@@ -323,6 +323,11 @@ void AmbientController::OnLockStateChanged(bool locked) {
}
}
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.
......
......@@ -63,6 +63,7 @@ class ASH_EXPORT AmbientController
// SessionObserver:
void OnLockStateChanged(bool locked) override;
void OnFirstSessionStarted() override;
// PowerStatus::Observer:
void OnPowerStatusChanged() override;
......
This diff is collapsed.
......@@ -44,6 +44,11 @@ class ASH_EXPORT AmbientURLLoader {
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 @@ class ASH_EXPORT AmbientPhotoController : public AmbientBackendModelObserver {
void StartScreenUpdate();
void StopScreenUpdate();
void ScheduleFetchBackupImages();
AmbientBackendModel* ambient_backend_model() {
return &ambient_backend_model_;
}
......@@ -95,6 +102,10 @@ class ASH_EXPORT AmbientPhotoController : public AmbientBackendModelObserver {
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 @@ class ASH_EXPORT AmbientPhotoController : public AmbientBackendModelObserver {
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 @@ class ASH_EXPORT AmbientPhotoController : public AmbientBackendModelObserver {
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 @@ class ASH_EXPORT AmbientPhotoController : public AmbientBackendModelObserver {
// 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 @@ class ASH_EXPORT AmbientPhotoController : public AmbientBackendModelObserver {
// 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_;
......
......@@ -4,6 +4,7 @@
#include "ash/ambient/backdrop/ambient_backend_controller_impl.h"
#include <array>
#include <string>
#include <utility>
#include <vector>
......@@ -24,6 +25,7 @@
#include "base/logging.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"
......@@ -459,6 +461,11 @@ void AmbientBackendControllerImpl::FetchWeather(FetchWeatherCallback callback) {
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,
......
......@@ -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 @@ class AmbientBackendControllerImpl : public AmbientBackendController {
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;
......
......@@ -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 @@ class TestAmbientURLLoaderImpl : public AmbientURLLoader {
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 @@ void AmbientAshTestBase::FetchImage() {
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 @@ void AmbientAshTestBase::SetUrlLoaderData(std::unique_ptr<std::string> data) {
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());
......
......@@ -141,9 +141,11 @@ class AmbientAshTestBase : public AshTestBase {
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_;
......
......@@ -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 @@ class ASH_PUBLIC_EXPORT AmbientBackendController {
// 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
......
......@@ -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 kFakeUrl[] = "chrome://ambient";
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 @@ void FakeAmbientBackendControllerImpl::FetchWeather(
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_)
......
......@@ -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 @@ class ASH_PUBLIC_EXPORT FakeAmbientBackendControllerImpl
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.
......
......@@ -32,7 +32,6 @@ class AmbientClientImplTest : public ChromeAshTestBase {
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 @@ class AmbientClientImplTest : public ChromeAshTestBase {
std::make_unique<IdentityTestEnvironmentProfileAdaptor>(profile_);
user_manager_enabler_ = std::make_unique<user_manager::ScopedUserManager>(
std::make_unique<chromeos::FakeChromeUserManager>());
AshTestBase::SetUp();
}
void TearDown() override {
......
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