ambient: Use cached photos for display

There are several situations we can use cached photos for display:
1. No internet.
2. No access token.
3. Failed to fetch topics.
4. Failed to download photos.

When failure happens, we use cache as a ring buffer to get next
available photo on disk. The cached photos are saved during normal
workflow when fetching topics and photos succeeds.

When reading from cache, for simplicity, we can try to read max times,
e.g. loop from index 0 to 99, for all possible file names. If failed,
means no disk cache available, reading from disk will pause until next
time the photo is downloaded successfully.

Cache will be cleared when Settings changes.

Bug: b/162945419
Test: Added new AmbientPhotoControllerTest.
Change-Id: I7d4d04ee1f6e9f40c64a23f6e04c35d0ce46c01f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2348689
Commit-Queue: Tao Wu <wutao@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799540}

ash/ambient/ambient_constants.h

@@ -14,11 +14,33 @@ namespace ash {
 constexpr base::TimeDelta kAnimationDuration =
     base::TimeDelta::FromMilliseconds(500);
 
+// Topic related numbers.
+
+// The default interval to fetch Topics.
+constexpr base::TimeDelta kTopicFetchInterval =
+    base::TimeDelta::FromSeconds(30);
+
 // The default interval to refresh photos.
-// TODO(b/139953713): Change to a correct time interval.
 constexpr base::TimeDelta kPhotoRefreshInterval =
     base::TimeDelta::FromSeconds(60);
 
+// The number of requests to fetch topics.
+constexpr int kNumberOfRequests = 50;
+
+// The batch size of topics to fetch in one request.
+// Magic number 2 is based on experiments that no curation on Google Photos.
+constexpr int kTopicsBatchSize = 2;
+
+// Max cached images.
+constexpr int kMaxNumberOfCachedImages = 100;
+
+constexpr int kMaxImageSizeInBytes = 5 * 1024 * 1024;
+
+constexpr int kMaxReservedAvailableDiskSpaceByte = 200 * 1024 * 1024;
+
+constexpr char kPhotoFileExt[] = ".img";
+constexpr char kPhotoDetailsFileExt[] = ".txt";
+
 // Directory name of ambient mode.
 constexpr char kAmbientModeDirectoryName[] = "ambient-mode";
 

ash/ambient/ambient_controller.h

@@ -94,6 +94,10 @@ class ASH_EXPORT AmbientController
     return ambient_backend_controller_.get();
   }
 
+  AmbientPhotoController* ambient_photo_controller() {
+    return &ambient_photo_controller_;
+  }
+
   AmbientUiModel* ambient_ui_model() { return &ambient_ui_model_; }
 
  private:
@@ -131,10 +135,6 @@ class ASH_EXPORT AmbientController
 
   void CloseWidget(bool immediately);
 
-  AmbientPhotoController* get_ambient_photo_controller_for_testing() {
-    return &ambient_photo_controller_;
-  }
-
   AmbientContainerView* get_container_view_for_testing() {
     return container_view_;
   }

ash/ambient/ambient_photo_controller.cc

@@ -10,6 +10,7 @@
 #include "ash/ambient/ambient_constants.h"
 #include "ash/ambient/ambient_controller.h"
 #include "ash/ambient/model/ambient_backend_model.h"
+#include "ash/public/cpp/ambient/ambient_backend_controller.h"
 #include "ash/public/cpp/ambient/ambient_client.h"
 #include "ash/public/cpp/image_downloader.h"
 #include "ash/shell.h"
@@ -24,6 +25,7 @@
 #include "base/optional.h"
 #include "base/path_service.h"
 #include "base/rand_util.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/system/sys_info.h"
 #include "base/task/post_task.h"
@@ -45,25 +47,25 @@ namespace {
 
 // TODO(b/161357364): refactor utility functions and constants
 
-// Topic related numbers.
-
-// The number of requests to fetch topics.
-constexpr int kNumberOfRequests = 50;
-
-// The batch size of topics to fetch in one request.
-// Magic number 2 is based on experiments that no curation on Google Photos.
-constexpr int kTopicsBatchSize = 2;
-
-// The upper bound of delay to the fetch topics. An random value will be
-// generated in the range of |kTopicFetchDelayMax|/2 to |kTopicFetchDelayMax|.
-constexpr base::TimeDelta kTopicFetchDelayMax =
-    base::TimeDelta::FromSeconds(36);
-
-constexpr int kMaxImageSizeInBytes = 5 * 1024 * 1024;
-
-constexpr int kMaxReservedAvailableDiskSpaceByte = 200 * 1024 * 1024;
+constexpr net::BackoffEntry::Policy kFetchTopicRetryBackoffPolicy = {
+    0,              // Number of initial errors to ignore.
+    500,            // Initial delay in ms.
+    2.0,            // Factor by which the waiting time will be multiplied.
+    0.2,            // Fuzzing percentage.
+    2 * 60 * 1000,  // Maximum delay in ms.
+    -1,             // Never discard the entry.
+    true,           // Use initial delay.
+};
 
-constexpr char kPhotoFileExt[] = ".img";
+constexpr net::BackoffEntry::Policy kResumeFetchImageBackoffPolicy = {
+    0,              // Number of initial errors to ignore.
+    500,            // Initial delay in ms.
+    2.0,            // Factor by which the waiting time will be multiplied.
+    0.2,            // Fuzzing percentage.
+    8 * 60 * 1000,  // Maximum delay in ms.
+    -1,             // Never discard the entry.
+    true,           // Use initial delay.
+};
 
 using DownloadCallback = base::OnceCallback<void(const gfx::ImageSkia&)>;
 
@@ -89,14 +91,6 @@ void DeletePathRecursively(const base::FilePath& path) {
   base::DeletePathRecursively(path);
 }
 
-std::string ToPhotoFileName(const std::string& url) {
-  std::string hash_tag;
-  base::Base64Encode(base::SHA1HashString(url), &hash_tag);
-  // Replace path divider.
-  base::ReplaceSubstringsAfterOffset(&hash_tag, 0, "/", "_");
-  return hash_tag + std::string(kPhotoFileExt);
-}
-
 void ToImageSkia(DownloadCallback callback, const SkBitmap& image) {
   if (image.isNull()) {
     std::move(callback).Run(gfx::ImageSkia());
@@ -127,12 +121,6 @@ void WriteFile(const base::FilePath& path, const std::string& data) {
     return;
   }
 
-  if (!base::PathExists(path.DirName()) &&
-      !base::CreateDirectory(path.DirName())) {
-    LOG(ERROR) << "Cannot create a new session directory.";
-    return;
-  }
-
   // Create a temp file.
   base::FilePath temp_file;
   if (!base::CreateTemporaryFileInDir(path.DirName(), &temp_file)) {
@@ -219,7 +207,7 @@ class AmbientImageDecoderImpl : public AmbientImageDecoder {
       const std::vector<uint8_t>& encoded_bytes,
       base::OnceCallback<void(const gfx::ImageSkia&)> callback) override {
     data_decoder::DecodeImageIsolated(
-        std::move(encoded_bytes), data_decoder::mojom::ImageCodec::DEFAULT,
+        encoded_bytes, data_decoder::mojom::ImageCodec::DEFAULT,
         /*shrink_to_fit=*/true, data_decoder::kDefaultMaxSizeInBytes,
         /*desired_image_frame_size=*/gfx::Size(),
         base::BindOnce(&ToImageSkia, std::move(callback)));
@@ -227,7 +215,9 @@ class AmbientImageDecoderImpl : public AmbientImageDecoder {
 };
 
 AmbientPhotoController::AmbientPhotoController()
-    : url_loader_(std::make_unique<AmbientURLLoaderImpl>()),
+    : fetch_topic_retry_backoff_(&kFetchTopicRetryBackoffPolicy),
+      resume_fetch_image_backoff_(&kResumeFetchImageBackoffPolicy),
+      url_loader_(std::make_unique<AmbientURLLoaderImpl>()),
       image_decoder_(std::make_unique<AmbientImageDecoderImpl>()),
       task_runner_(
           base::ThreadPool::CreateSequencedTaskRunner(GetTaskTraits())) {
@@ -237,33 +227,30 @@ AmbientPhotoController::AmbientPhotoController()
 AmbientPhotoController::~AmbientPhotoController() = default;
 
 void AmbientPhotoController::StartScreenUpdate() {
-  photo_path_ = GetRootPath().Append(FILE_PATH_LITERAL(base::GenerateGUID()));
-  task_runner_->PostTaskAndReply(
-      FROM_HERE, base::BindOnce(&DeletePathRecursively, GetRootPath()),
-      base::BindOnce(&AmbientPhotoController::FetchTopics,
-                     weak_factory_.GetWeakPtr()));
+  FetchTopics();
 }
 
 void AmbientPhotoController::StopScreenUpdate() {
   photo_refresh_timer_.Stop();
   topic_index_ = 0;
   topics_batch_fetched_ = 0;
+  image_refresh_started_ = false;
+  retries_to_read_from_cache_ = kMaxNumberOfCachedImages;
+  fetch_topic_retry_backoff_.Reset();
+  resume_fetch_image_backoff_.Reset();
   ambient_backend_model_.Clear();
   weak_factory_.InvalidateWeakPtrs();
-
-  task_runner_->PostTask(FROM_HERE,
-                         base::BindOnce(&DeletePathRecursively, photo_path_));
-  photo_path_.clear();
 }
 
 void AmbientPhotoController::OnTopicsChanged() {
   ++topics_batch_fetched_;
   if (topics_batch_fetched_ < kNumberOfRequests)
-    ScheduleFetchTopics();
+    ScheduleFetchTopics(/*backoff=*/false);
 
-  // The first OnTopicsChanged event triggers the photo refresh.
-  if (topics_batch_fetched_ == 1)
+  if (!image_refresh_started_) {
+    image_refresh_started_ = true;
     ScheduleRefreshImage();
+  }
 }
 
 void AmbientPhotoController::FetchTopics() {
@@ -276,9 +263,16 @@ void AmbientPhotoController::FetchTopics() {
                          weak_factory_.GetWeakPtr()));
 }
 
-void AmbientPhotoController::ScheduleFetchTopics() {
+void AmbientPhotoController::ClearCache() {
+  task_runner_->PostTask(FROM_HERE,
+                         base::BindOnce(&DeletePathRecursively, GetRootPath()));
+}
+
+void AmbientPhotoController::ScheduleFetchTopics(bool backoff) {
+  // If retry, using the backoff delay, otherwise the default delay.
   const base::TimeDelta kDelay =
-      (base::RandDouble() * kTopicFetchDelayMax) / 2 + kTopicFetchDelayMax / 2;
+      backoff ? fetch_topic_retry_backoff_.GetTimeUntilRelease()
+              : kTopicFetchInterval;
   base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
       FROM_HERE,
       base::BindOnce(&AmbientPhotoController::FetchTopics,
@@ -295,80 +289,68 @@ void AmbientPhotoController::ScheduleRefreshImage() {
   // is true.
   photo_refresh_timer_.Start(
       FROM_HERE, refresh_interval,
-      base::BindOnce(&AmbientPhotoController::TryReadPhotoRawData,
+      base::BindOnce(&AmbientPhotoController::FetchPhotoRawData,
                      weak_factory_.GetWeakPtr()));
 }
 
-const AmbientModeTopic& AmbientPhotoController::GetNextTopic() {
+const AmbientModeTopic* AmbientPhotoController::GetNextTopic() {
   const auto& topics = ambient_backend_model_.topics();
-  DCHECK(!topics.empty());
-
-  // We prefetch the first two photos, which will increase the |topic_index_| to
-  // 2 in the first batch with size of 2. Then it will reset to 0 if we put this
-  // block after the increment of |topic_index_|.
+  // If no more topics, will read from cache.
   if (topic_index_ == topics.size())
-    topic_index_ = 0;
+    return nullptr;
 
-  return topics[topic_index_++];
+  return &topics[topic_index_++];
 }
 
 void AmbientPhotoController::OnScreenUpdateInfoFetched(
     const ash::ScreenUpdate& screen_update) {
   // It is possible that |screen_update| is an empty instance if fatal errors
   // happened during the fetch.
-  // TODO(b/148485116): Implement retry logic.
   if (screen_update.next_topics.empty() &&
       !screen_update.weather_info.has_value()) {
     LOG(ERROR) << "The screen update info fetch has failed.";
+
+    fetch_topic_retry_backoff_.InformOfRequest(/*succeeded=*/false);
+    ScheduleFetchTopics(/*backoff=*/true);
+    if (!image_refresh_started_) {
+      image_refresh_started_ = true;
+      ScheduleRefreshImage();
+    }
     return;
   }
 
+  fetch_topic_retry_backoff_.InformOfRequest(/*succeeded=*/true);
   ambient_backend_model_.AppendTopics(screen_update.next_topics);
   StartDownloadingWeatherConditionIcon(screen_update.weather_info);
 }
 
-void AmbientPhotoController::TryReadPhotoRawData() {
-  const AmbientModeTopic& topic = GetNextTopic();
-
-  base::FilePath path = photo_path_.Append(ToPhotoFileName(topic.GetUrl()));
-  task_runner_->PostTaskAndReplyWithResult(
-      FROM_HERE,
-      base::BindOnce(
-          [](const base::FilePath& path) {
-            auto data = std::make_unique<std::string>();
-            if (!base::ReadFileToString(path, data.get()))
-              data = nullptr;
-            return data;
-          },
-          path),
-      base::BindOnce(&AmbientPhotoController::OnPhotoRawDataRead,
-                     weak_factory_.GetWeakPtr(), topic));
-}
-
-void AmbientPhotoController::OnPhotoRawDataRead(
-    const AmbientModeTopic& topic,
-    std::unique_ptr<std::string> data) {
-  if (!data || data->empty()) {
+void AmbientPhotoController::FetchPhotoRawData() {
+  const AmbientModeTopic* topic = GetNextTopic();
+  if (topic) {
     url_loader_->Download(
-        topic.GetUrl(),
+        topic->GetUrl(),
         base::BindOnce(&AmbientPhotoController::OnPhotoRawDataAvailable,
-                       weak_factory_.GetWeakPtr(), topic,
-                       /*need_to_save=*/true));
-  } else {
-    OnPhotoRawDataAvailable(topic, /*need_to_save=*/false, std::move(data));
+                       weak_factory_.GetWeakPtr(),
+                       /*from_downloading=*/true,
+                       std::make_unique<std::string>(topic->details)));
+    return;
   }
+
+  // If |topic| is nullptr, will try to read from disk cache.
+  TryReadPhotoRawData();
 }
 
-void AmbientPhotoController::OnPhotoRawDataAvailable(
-    const AmbientModeTopic& topic,
-    bool need_to_save,
-    std::unique_ptr<std::string> response_body) {
-  if (!response_body) {
-    LOG(ERROR) << "Failed to download image";
-
-    // Continue to get next photo on error.
-    // TODO(b/148485116): Add exponential backoff retry logic.
-    const base::TimeDelta kDelay = base::TimeDelta::FromMilliseconds(100);
+void AmbientPhotoController::TryReadPhotoRawData() {
+  // 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;
+      return;
+    }
+
+    // Try to resume normal workflow with backoff.
+    const base::TimeDelta kDelay =
+        resume_fetch_image_backoff_.GetTimeUntilRelease();
     base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
         FROM_HERE,
         base::BindOnce(&AmbientPhotoController::ScheduleRefreshImage,
@@ -377,49 +359,113 @@ void AmbientPhotoController::OnPhotoRawDataAvailable(
     return;
   }
 
-  const base::FilePath path =
-      photo_path_.Append(ToPhotoFileName(topic.GetUrl()));
+  --retries_to_read_from_cache_;
+  std::string file_name = base::NumberToString(cache_index_for_display_);
+  ++cache_index_for_display_;
+  if (cache_index_for_display_ == kMaxNumberOfCachedImages)
+    cache_index_for_display_ = 0;
+
+  auto photo_data = std::make_unique<std::string>();
+  auto photo_details = std::make_unique<std::string>();
+  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),
+                    photo_data)) {
+              photo_data->clear();
+            }
+            if (!base::ReadFileToString(
+                    GetRootPath().Append(file_name + kPhotoDetailsFileExt),
+                    photo_details)) {
+              photo_details->clear();
+            }
+          },
+          file_name, photo_data.get(), photo_details.get()),
+      base::BindOnce(&AmbientPhotoController::OnPhotoRawDataAvailable,
+                     weak_factory_.GetWeakPtr(), /*from_downloading=*/false,
+                     std::move(photo_details), std::move(photo_data)));
+}
+
+void AmbientPhotoController::OnPhotoRawDataAvailable(
+    bool from_downloading,
+    std::unique_ptr<std::string> details,
+    std::unique_ptr<std::string> data) {
+  if (!data || data->empty()) {
+    if (from_downloading) {
+      LOG(ERROR) << "Failed to download image";
+      resume_fetch_image_backoff_.InformOfRequest(/*succeeded=*/false);
+    } else {
+      LOG(WARNING) << "Failed to read image";
+    }
+
+    // Try to read from cache when failure happens.
+    TryReadPhotoRawData();
+    return;
+  }
+
+  const std::string file_name = base::NumberToString(cache_index_for_store_);
+  // If the data is fetched from downloading, write to disk.
+  // Note: WriteFile() could fail. The saved file name may not be continuous.
+  if (from_downloading)
+    ++cache_index_for_store_;
+  if (cache_index_for_store_ == kMaxNumberOfCachedImages)
+    cache_index_for_store_ = 0;
+
   task_runner_->PostTaskAndReply(
       FROM_HERE,
       base::BindOnce(
-          [](const base::FilePath& path, bool need_to_save,
-             const std::string& data) {
-            if (need_to_save)
-              WriteFile(path, data);
+          [](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),
+                        details);
+            }
           },
-          path, need_to_save, *response_body),
+          file_name, from_downloading, *data, *details),
       base::BindOnce(&AmbientPhotoController::DecodePhotoRawData,
-                     weak_factory_.GetWeakPtr(), topic,
-                     std::move(response_body)));
+                     weak_factory_.GetWeakPtr(), from_downloading,
+                     std::move(details), std::move(data)));
 }
 
 void AmbientPhotoController::DecodePhotoRawData(
-    const AmbientModeTopic& topic,
+    bool from_downloading,
+    std::unique_ptr<std::string> details,
     std::unique_ptr<std::string> data) {
   std::vector<uint8_t> image_bytes(data->begin(), data->end());
-  image_decoder_->Decode(image_bytes,
-                         base::BindOnce(&AmbientPhotoController::OnPhotoDecoded,
-                                        weak_factory_.GetWeakPtr(), topic));
+  image_decoder_->Decode(
+      image_bytes, base::BindOnce(&AmbientPhotoController::OnPhotoDecoded,
+                                  weak_factory_.GetWeakPtr(), from_downloading,
+                                  std::move(details)));
 }
 
-void AmbientPhotoController::OnPhotoDecoded(const AmbientModeTopic& topic,
-                                            const gfx::ImageSkia& image) {
-  base::TimeDelta kDelay;
+void AmbientPhotoController::OnPhotoDecoded(
+    bool from_downloading,
+    std::unique_ptr<std::string> details,
+    const gfx::ImageSkia& image) {
   if (image.isNull()) {
     LOG(WARNING) << "Image is null";
-    kDelay = base::TimeDelta::FromMilliseconds(100);
-  } else {
-    PhotoWithDetails detailed_photo;
-    detailed_photo.photo = image;
-    detailed_photo.details = topic.details;
-    ambient_backend_model_.AddNextImage(std::move(detailed_photo));
+    if (from_downloading)
+      resume_fetch_image_backoff_.InformOfRequest(/*succeeded=*/false);
+
+    // Try to read from cache when failure happens.
+    TryReadPhotoRawData();
+    return;
   }
 
-  base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE,
-      base::BindOnce(&AmbientPhotoController::ScheduleRefreshImage,
-                     weak_factory_.GetWeakPtr()),
-      kDelay);
+  retries_to_read_from_cache_ = kMaxNumberOfCachedImages;
+  if (from_downloading)
+    resume_fetch_image_backoff_.InformOfRequest(/*succeeded=*/true);
+
+  PhotoWithDetails detailed_photo;
+  detailed_photo.photo = image;
+  detailed_photo.details = *details;
+  ambient_backend_model_.AddNextImage(std::move(detailed_photo));
+
+  ScheduleRefreshImage();
 }
 
 void AmbientPhotoController::StartDownloadingWeatherConditionIcon(
@@ -464,4 +510,12 @@ void AmbientPhotoController::OnWeatherConditionIconDownloaded(
   ambient_backend_model_.UpdateWeatherInfo(icon, temp_f, show_celsius);
 }
 
+void AmbientPhotoController::FetchTopicsForTesting() {
+  FetchTopics();
+}
+
+void AmbientPhotoController::FetchImageForTesting() {
+  FetchPhotoRawData();
+}
+
 }  // namespace ash

ash/ambient/ambient_photo_controller.h

@@ -10,6 +10,7 @@
 #include <utility>
 #include <vector>
 
+#include "ash/ambient/ambient_constants.h"
 #include "ash/ambient/model/ambient_backend_model.h"
 #include "ash/ambient/model/ambient_backend_model_observer.h"
 #include "ash/ash_export.h"
@@ -21,6 +22,7 @@
 #include "base/optional.h"
 #include "base/scoped_observer.h"
 #include "base/timer/timer.h"
+#include "net/base/backoff_entry.h"
 #include "services/network/public/cpp/simple_url_loader.h"
 
 namespace gfx {
@@ -96,38 +98,42 @@ class ASH_EXPORT AmbientPhotoController : public AmbientBackendModelObserver {
   // AmbientBackendModelObserver:
   void OnTopicsChanged() override;
 
+  // Clear cache when Settings changes.
+  void ClearCache();
+
  private:
   friend class AmbientAshTestBase;
 
   void FetchTopics();
 
-  void ScheduleFetchTopics();
+  void ScheduleFetchTopics(bool backoff);
 
   void ScheduleRefreshImage();
 
   void GetScreenUpdateInfo();
 
   // Return a topic to download the image.
-  const AmbientModeTopic& GetNextTopic();
+  // Return nullptr when need to read from disk cache.
+  const AmbientModeTopic* GetNextTopic();
 
   void OnScreenUpdateInfoFetched(const ash::ScreenUpdate& screen_update);
 
-  // Try to read photo raw data from disk.
-  void TryReadPhotoRawData();
+  // Fetch photo raw data by downloading or reading from cache.
+  void FetchPhotoRawData();
 
-  // If photo raw data is read successfully, call OnPhotoRawDataAvailable() to
-  // decode data. Otherwise, download the raw data and save to disk.
-  void OnPhotoRawDataRead(const AmbientModeTopic& topic,
-                          std::unique_ptr<std::string> data);
+  // Try to read photo raw data from cache.
+  void TryReadPhotoRawData();
 
-  void OnPhotoRawDataAvailable(const AmbientModeTopic& topic,
-                               bool need_to_save,
-                               std::unique_ptr<std::string> response_body);
+  void OnPhotoRawDataAvailable(bool from_downloading,
+                               std::unique_ptr<std::string> details,
+                               std::unique_ptr<std::string> data);
 
-  void DecodePhotoRawData(const AmbientModeTopic& topic,
+  void DecodePhotoRawData(bool from_downloading,
+                          std::unique_ptr<std::string> details,
                           std::unique_ptr<std::string> data);
 
-  void OnPhotoDecoded(const AmbientModeTopic& topic,
+  void OnPhotoDecoded(bool from_downloading,
+                      std::unique_ptr<std::string> details,
                       const gfx::ImageSkia& image);
 
   void StartDownloadingWeatherConditionIcon(
@@ -144,6 +150,8 @@ class ASH_EXPORT AmbientPhotoController : public AmbientBackendModelObserver {
     url_loader_ = std::move(url_loader);
   }
 
+  AmbientURLLoader* get_url_loader_for_testing() { return url_loader_.get(); }
+
   void set_image_decoder_for_testing(
       std::unique_ptr<AmbientImageDecoder> image_decoder) {
     image_decoder_ = std::move(image_decoder);
@@ -153,6 +161,10 @@ class ASH_EXPORT AmbientPhotoController : public AmbientBackendModelObserver {
     return image_decoder_.get();
   }
 
+  void FetchTopicsForTesting();
+
+  void FetchImageForTesting();
+
   AmbientBackendModel ambient_backend_model_;
 
   // The timer to refresh photos.
@@ -164,11 +176,33 @@ class ASH_EXPORT AmbientPhotoController : public AmbientBackendModelObserver {
   // Tracking how many batches of topics have been fetched.
   int topics_batch_fetched_ = 0;
 
+  // Current index of cached image to read and display when failure happens.
+  // The image file of this index may not exist or may not be valid. It will try
+  // to read from the next cached file by increasing this index by 1.
+  int 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
+  // failures happen.
+  int cache_index_for_store_ = 0;
+
+  // Whether the image refresh started or not.
+  bool image_refresh_started_ = false;
+
+  // Cached image may not exist or valid. This is the max times of attempts to
+  // read cached images.
+  int retries_to_read_from_cache_ = kMaxNumberOfCachedImages;
+
+  // Backoff for fetch topics retries.
+  net::BackoffEntry fetch_topic_retry_backoff_;
+
+  // Backoff to resume fetch images.
+  net::BackoffEntry resume_fetch_image_backoff_;
+
   ScopedObserver<AmbientBackendModel, AmbientBackendModelObserver>
       ambient_backend_model_observer_{this};
 
-  base::FilePath photo_path_;
-
   std::unique_ptr<AmbientURLLoader> url_loader_;
 
   std::unique_ptr<AmbientImageDecoder> image_decoder_;

ash/ambient/ambient_photo_controller_unittest.cc

@@ -25,7 +25,6 @@
 #include "base/run_loop.h"
 #include "base/system/sys_info.h"
 #include "base/test/bind_test_util.h"
-#include "base/test/task_environment.h"
 #include "base/timer/timer.h"
 #include "ui/gfx/image/image_skia.h"
 
@@ -43,7 +42,7 @@ TEST_F(AmbientPhotoControllerTest, ShouldStartToDownloadTopics) {
   topics = photo_controller()->ambient_backend_model()->topics();
   EXPECT_TRUE(topics.empty());
 
-  task_environment()->FastForwardBy(kPhotoRefreshInterval);
+  FastForwardToNextImage();
   topics = photo_controller()->ambient_backend_model()->topics();
   EXPECT_FALSE(topics.empty());
 
@@ -60,7 +59,7 @@ TEST_F(AmbientPhotoControllerTest, ShouldStartToDownloadImages) {
 
   // Start to refresh images.
   photo_controller()->StartScreenUpdate();
-  task_environment()->FastForwardBy(1.2 * kPhotoRefreshInterval);
+  FastForwardToNextImage();
   image = photo_controller()->ambient_backend_model()->GetNextImage();
   EXPECT_FALSE(image.IsNull());
 
@@ -78,20 +77,18 @@ TEST_F(AmbientPhotoControllerTest, ShouldUpdatePhotoPeriodically) {
 
   // Start to refresh images.
   photo_controller()->StartScreenUpdate();
-  task_environment()->FastForwardBy(1.2 * kPhotoRefreshInterval);
+  FastForwardToNextImage();
   image1 = photo_controller()->ambient_backend_model()->GetNextImage();
   EXPECT_FALSE(image1.IsNull());
   EXPECT_TRUE(image2.IsNull());
 
-  // Fastforward enough time to update the photo.
-  task_environment()->FastForwardBy(1.2 * kPhotoRefreshInterval);
+  FastForwardToNextImage();
   image2 = photo_controller()->ambient_backend_model()->GetNextImage();
   EXPECT_FALSE(image2.IsNull());
   EXPECT_FALSE(image1.photo.BackedBySameObjectAs(image2.photo));
   EXPECT_TRUE(image3.IsNull());
 
-  // Fastforward enough time to update another photo.
-  task_environment()->FastForwardBy(1.2 * kPhotoRefreshInterval);
+  FastForwardToNextImage();
   image3 = photo_controller()->ambient_backend_model()->GetNextImage();
   EXPECT_FALSE(image3.IsNull());
   EXPECT_FALSE(image1.photo.BackedBySameObjectAs(image3.photo));
@@ -101,56 +98,251 @@ TEST_F(AmbientPhotoControllerTest, ShouldUpdatePhotoPeriodically) {
   photo_controller()->StopScreenUpdate();
 }
 
-// Test that image is saved and deleted when starting/stopping screen update.
-TEST_F(AmbientPhotoControllerTest, ShouldSaveAndDeleteImagesOnDisk) {
+// 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));
 
-  // Save a file to check if it gets deleted by StartScreenUpdate.
-  auto file_to_delete = ambient_image_path.Append("file_to_delete");
-  base::WriteFile(file_to_delete, "delete_me");
+  // Clean up.
+  base::DeletePathRecursively(ambient_image_path);
 
-  // Start to refresh images. Kicks off tasks that cleans |ambient_image_path|,
-  // then downloads a test image and writes it to a subdirectory of
+  // Start to refresh images. It will download a test image and write it in
   // |ambient_image_path| in a delayed task.
   photo_controller()->StartScreenUpdate();
-  task_environment()->FastForwardBy(1.2 * kPhotoRefreshInterval);
+  FastForwardToNextImage();
 
   EXPECT_TRUE(base::PathExists(ambient_image_path));
-  EXPECT_FALSE(base::PathExists(file_to_delete));
 
   {
-    // Count files and directories in root_path. There should only be one
-    // subdirectory that was just created to save image files for this ambient
-    // mode session.
+    // 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_TRUE(files.GetInfo().IsDirectory());
+      EXPECT_FALSE(files.GetInfo().IsDirectory());
       count++;
     }
 
-    EXPECT_EQ(count, 1);
+    // 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 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();
+
+  EXPECT_TRUE(base::PathExists(ambient_image_path));
+
   auto image = photo_controller()->ambient_backend_model()->GetNextImage();
   EXPECT_FALSE(image.IsNull());
 
   // Stop to refresh images.
   photo_controller()->StopScreenUpdate();
-  task_environment()->FastForwardBy(1.2 * kPhotoRefreshInterval);
+  FastForwardToNextImage();
 
   EXPECT_TRUE(base::PathExists(ambient_image_path));
-  EXPECT_TRUE(base::IsDirectoryEmpty(ambient_image_path));
+  EXPECT_FALSE(base::IsDirectoryEmpty(ambient_image_path));
 
   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);
+  }
+
+  // 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);
+
+  FetchImage();
+  FastForwardToNextImage();
+  // Topics is empty. Will read from cache, which is empty.
+  auto image = photo_controller()->ambient_backend_model()->GetNextImage();
+  EXPECT_TRUE(image.IsNull());
+
+  // Save a file to check if it gets read for display.
+  auto cached_image = ambient_image_path.Append("0.img");
+  base::CreateDirectory(ambient_image_path);
+  base::WriteFile(cached_image, "cached image");
+
+  // Reset variables in photo controller.
+  photo_controller()->StopScreenUpdate();
+  FetchImage();
+  FastForwardToNextImage();
+  image = photo_controller()->ambient_backend_model()->GetNextImage();
+  EXPECT_FALSE(image.IsNull());
+
+  // Clean up.
+  base::DeletePathRecursively(ambient_image_path);
+}
+
+// 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);
+
+  FetchImage();
+  FastForwardToNextImage();
+  // Topics is empty. Will read from cache, which is empty.
+  auto image = photo_controller()->ambient_backend_model()->GetNextImage();
+  EXPECT_TRUE(image.IsNull());
+
+  // The initial file name to be read is 0. Save a file with 99.img to check if
+  // it gets read for display.
+  auto cached_image = ambient_image_path.Append("99.img");
+  base::CreateDirectory(ambient_image_path);
+  base::WriteFile(cached_image, "cached image");
+
+  // Reset variables in photo controller.
+  photo_controller()->StopScreenUpdate();
+  FetchImage();
+  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);
+
+  SetUrlLoaderData(std::make_unique<std::string>());
+  FetchTopics();
+  // Forward a little bit time. FetchTopics() will succeed. Downloading should
+  // fail. Will read from cache, which is empty.
+  task_environment()->FastForwardBy(0.2 * kTopicFetchInterval);
+  auto image = photo_controller()->ambient_backend_model()->GetNextImage();
+  EXPECT_TRUE(image.IsNull());
+
+  // Save a file to check if it gets read for display.
+  auto cached_image = ambient_image_path.Append("0.img");
+  base::CreateDirectory(ambient_image_path);
+  base::WriteFile(cached_image, "cached image");
+
+  // Reset variables in photo controller.
+  photo_controller()->StopScreenUpdate();
+  FetchTopics();
+  // Forward a little bit time. FetchTopics() will succeed. Downloading should
+  // fail. Will read from cache.
+  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 =
+      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.
+  // First decoding should fail. Will read from cache, and then succeed.
+  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);
+
+  FetchImage();
+  FastForwardToNextImage();
+  // Topics is empty. Will read from cache, which is empty.
+  auto image = photo_controller()->ambient_backend_model()->GetNextImage();
+  EXPECT_TRUE(image.IsNull());
+
+  FetchTopics();
+  // Forward a little bit time. FetchTopics() will succeed and refresh image.
+  task_environment()->FastForwardBy(0.2 * kTopicFetchInterval);
+  image = photo_controller()->ambient_backend_model()->GetNextImage();
+  EXPECT_FALSE(image.IsNull());
+
+  // Clean up.
+  base::DeletePathRecursively(ambient_image_path);
 }
 
 }  // namespace ash

ash/ambient/backdrop/ambient_backend_controller_impl.cc

@@ -390,6 +390,17 @@ void AmbientBackendControllerImpl::OnUpdateSettings(
   const bool success =
       BackdropClientConfig::ParseUpdateSettingsResponse(*response);
   std::move(callback).Run(success);
+
+  // Clear disk cache when Settings changes.
+  // TODO(wutao): Use observer pattern. Need to future narrow down
+  // the clear up only on albums changes, not on temperature unit
+  // changes.
+  if (success) {
+    Shell::Get()
+        ->ambient_controller()
+        ->ambient_photo_controller()
+        ->ClearCache();
+  }
 }
 
 void AmbientBackendControllerImpl::FetchSettingPreviewInternal(

ash/ambient/test/ambient_ash_test_base.cc

@@ -41,13 +41,20 @@ class TestAmbientURLLoaderImpl : public AmbientURLLoader {
   void Download(
       const std::string& url,
       network::SimpleURLLoader::BodyAsStringCallback callback) override {
-    auto data = std::make_unique<std::string>();
-    *data = "test";
+    std::string data = data_ ? *data_ : "test";
     // Pretend to respond asynchronously.
     base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
-        FROM_HERE, base::BindOnce(std::move(callback), std::move(data)),
+        FROM_HERE,
+        base::BindOnce(std::move(callback),
+                       std::make_unique<std::string>(data)),
         base::TimeDelta::FromMilliseconds(1));
   }
+
+  void SetData(std::unique_ptr<std::string> data) { data_ = std::move(data); }
+
+ private:
+  // If not null, will return this data.
+  std::unique_ptr<std::string> data_;
 };
 
 class TestAmbientImageDecoderImpl : public AmbientImageDecoder {
@@ -59,10 +66,14 @@ class TestAmbientImageDecoderImpl : public AmbientImageDecoder {
   void Decode(
       const std::vector<uint8_t>& encoded_bytes,
       base::OnceCallback<void(const gfx::ImageSkia&)> callback) override {
+    gfx::ImageSkia image =
+        image_ ? *image_ : gfx::test::CreateImageSkia(width_, height_);
+    // Only use once.
+    image_.reset();
+
     // Pretend to respond asynchronously.
     base::SequencedTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::BindOnce(std::move(callback),
-                                  gfx::test::CreateImageSkia(width_, height_)));
+        FROM_HERE, base::BindOnce(std::move(callback), image));
   }
 
   void SetImageSize(int width, int height) {
@@ -70,10 +81,15 @@ class TestAmbientImageDecoderImpl : public AmbientImageDecoder {
     height_ = height;
   }
 
+  void SetImage(const gfx::ImageSkia& image) { image_ = image; }
+
  private:
   // Width and height of test images.
   int width_ = 10;
   int height_ = 20;
+
+  // If set, will replay this image.
+  base::Optional<gfx::ImageSkia> image_;
 };
 
 AmbientAshTestBase::AmbientAshTestBase()
@@ -186,9 +202,7 @@ void AmbientAshTestBase::SimulateMediaPlaybackStateChanged(
 
 void AmbientAshTestBase::SetPhotoViewImageSize(int width, int height) {
   auto* image_decoder = static_cast<TestAmbientImageDecoderImpl*>(
-      ambient_controller()
-          ->get_ambient_photo_controller_for_testing()
-          ->get_image_decoder_for_testing());
+      photo_controller()->get_image_decoder_for_testing());
 
   image_decoder->SetImageSize(width, height);
 }
@@ -242,11 +256,33 @@ AmbientController* AmbientAshTestBase::ambient_controller() {
 }
 
 AmbientPhotoController* AmbientAshTestBase::photo_controller() {
-  return ambient_controller()->get_ambient_photo_controller_for_testing();
+  return ambient_controller()->ambient_photo_controller();
 }
 
 AmbientContainerView* AmbientAshTestBase::container_view() {
   return ambient_controller()->get_container_view_for_testing();
 }
 
+void AmbientAshTestBase::FetchTopics() {
+  photo_controller()->FetchTopicsForTesting();
+}
+
+void AmbientAshTestBase::FetchImage() {
+  photo_controller()->FetchImageForTesting();
+}
+
+void AmbientAshTestBase::SetUrlLoaderData(std::unique_ptr<std::string> data) {
+  auto* url_loader_ = static_cast<TestAmbientURLLoaderImpl*>(
+      photo_controller()->get_url_loader_for_testing());
+
+  url_loader_->SetData(std::move(data));
+}
+
+void AmbientAshTestBase::SeteImageDecoderImage(const gfx::ImageSkia& image) {
+  auto* image_decoder = static_cast<TestAmbientImageDecoderImpl*>(
+      photo_controller()->get_image_decoder_for_testing());
+
+  image_decoder->SetImage(image);
+}
+
 }  // namespace ash

ash/ambient/test/ambient_ash_test_base.h

@@ -103,6 +103,14 @@ class AmbientAshTestBase : public AshTestBase {
   // Returns the top-level view which contains all the ambient components.
   AmbientContainerView* container_view();
 
+  void FetchTopics();
+
+  void FetchImage();
+
+  void SetUrlLoaderData(std::unique_ptr<std::string> data);
+
+  void SeteImageDecoderImage(const gfx::ImageSkia& image);
+
  private:
   base::test::ScopedFeatureList scoped_feature_list_;
   std::unique_ptr<TestImageDownloader> image_downloader_;