ambient: close ui when images fail to load

After 3 consecutive failures to load an image, notify observers of
failure.

BUG=b:169591750
TEST=ash_unittests --gtest_filter=AmbientBackendModelTest*

Cq-Include-Trybots: luci.chrome.try:linux-chromeos-chrome
Change-Id: I7a9302e65fc58d3272a0da31f13c6a44efe1cc58
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2449937
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@{#816733}

ash/ambient/ambient_constants.h

@@ -45,6 +45,10 @@ constexpr int kMaxImageSizeInBytes = 5 * 1024 * 1024;
 
 constexpr int kMaxReservedAvailableDiskSpaceByte = 200 * 1024 * 1024;
 
+// The maximum number of consecutive failures in downloading or reading an image
+// from disk.
+constexpr int kMaxConsecutiveReadPhotoFailures = 3;
+
 constexpr char kPhotoFileExt[] = ".img";
 constexpr char kPhotoDetailsFileExt[] = ".txt";
 

ash/ambient/ambient_controller.cc

@@ -297,6 +297,10 @@ void AmbientController::OnLockStateChanged(bool locked) {
     return;
   }
 
+  // Reset image failures to allow retrying ambient mode after lock state
+  // changes.
+  GetAmbientBackendModel()->ResetImageFailures();
+
   // We have 3 options to manage the token for lock screen. Here use option 1.
   // 1. Request only one time after entering lock screen. We will use it once
   //    to request all the image links and no more requests.
@@ -374,6 +378,11 @@ void AmbientController::ScreenBrightnessChanged(
   if (!is_screen_off_)
     return;
   is_screen_off_ = false;
+
+  // Reset image failures to allow retrying ambient mode because screen has
+  // turned back on.
+  GetAmbientBackendModel()->ResetImageFailures();
+
   // If screen is back on, turn on ambient mode for lock screen.
   if (LockScreen::HasInstance())
     ShowHiddenUi();
@@ -394,6 +403,17 @@ void AmbientController::ScreenIdleStateChanged(
   if (!idle_state.dimmed())
     return;
 
+  // Do not show the UI if lockscreen is active. The inactivity monitor should
+  // have activated ambient mode.
+  if (LockScreen::HasInstance())
+    return;
+
+  // Do not show UI if loading images was unsuccessful.
+  if (GetAmbientBackendModel()->ImageLoadingFailed()) {
+    VLOG(1) << "Skipping ambient mode activation due to prior failure";
+    return;
+  }
+
   ShowUi();
 }
 
@@ -537,6 +557,11 @@ void AmbientController::OnImagesReady() {
   CreateAndShowWidget();
 }
 
+void AmbientController::OnImagesFailed() {
+  LOG(ERROR) << "Ambient mode failed to start";
+  ambient_ui_model_.SetUiVisibility(AmbientUiVisibility::kClosed);
+}
+
 std::unique_ptr<AmbientContainerView> AmbientController::CreateContainerView() {
   DCHECK(!container_view_);
 

ash/ambient/ambient_controller.h

@@ -127,6 +127,7 @@ class ASH_EXPORT AmbientController
 
   // AmbientBackendModelObserver overrides:
   void OnImagesReady() override;
+  void OnImagesFailed() override;
 
   // Initializes the |container_view_|. Called in |CreateWidget()| to create the
   // contents view.

ash/ambient/ambient_photo_controller.cc

@@ -62,8 +62,8 @@ constexpr net::BackoffEntry::Policy kFetchTopicRetryBackoffPolicy = {
 };
 
 constexpr net::BackoffEntry::Policy kResumeFetchImageBackoffPolicy = {
-    0,              // Number of initial errors to ignore.
-    500,            // Initial delay in ms.
+    kMaxConsecutiveReadPhotoFailures,  // 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.
@@ -540,7 +540,12 @@ void AmbientPhotoController::TryReadPhotoRawData() {
   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()) {
+      ambient_backend_model_.AddImageFailure();
+      // Do not refresh image if image loading has failed repeatedly, or there
+      // are no more topics to retry.
+      if (ambient_backend_model_.ImageLoadingFailed() ||
+          topic_index_ == ambient_backend_model_.topics().size()) {
+        LOG(WARNING) << "Not attempting image refresh";
         image_refresh_started_ = false;
         return;
       }

ash/ambient/model/ambient_backend_model.cc

@@ -8,6 +8,7 @@
 
 #include "ash/ambient/ambient_constants.h"
 #include "ash/ambient/model/ambient_backend_model_observer.h"
+#include "base/logging.h"
 
 namespace ash {
 
@@ -62,6 +63,7 @@ bool AmbientBackendModel::ImagesReady() const {
 
 void AmbientBackendModel::AddNextImage(
     const PhotoWithDetails& photo_with_details) {
+  ResetImageFailures();
   if (current_image_.IsNull()) {
     current_image_ = photo_with_details;
   } else if (next_image_.IsNull()) {
@@ -79,6 +81,23 @@ bool AmbientBackendModel::HashMatchesNextImage(const std::string& hash) const {
   return GetNextImage().hash == hash;
 }
 
+void AmbientBackendModel::AddImageFailure() {
+  failures_++;
+  if (ImageLoadingFailed()) {
+    DVLOG(3) << "image loading failed";
+    for (auto& observer : observers_)
+      observer.OnImagesFailed();
+  }
+}
+
+void AmbientBackendModel::ResetImageFailures() {
+  failures_ = 0;
+}
+
+bool AmbientBackendModel::ImageLoadingFailed() {
+  return !ImagesReady() && failures_ >= kMaxConsecutiveReadPhotoFailures;
+}
+
 base::TimeDelta AmbientBackendModel::GetPhotoRefreshInterval() {
   if (!ImagesReady())
     return base::TimeDelta();

ash/ambient/model/ambient_backend_model.h

@@ -65,6 +65,13 @@ class ASH_EXPORT AmbientBackendModel {
   // If the hash matches the hash of the next image to be displayed.
   bool HashMatchesNextImage(const std::string& hash) const;
 
+  // Record that fetching an image has failed.
+  void AddImageFailure();
+
+  void ResetImageFailures();
+
+  bool ImageLoadingFailed();
+
   // Get/Set the photo refresh interval.
   base::TimeDelta GetPhotoRefreshInterval();
   void SetPhotoRefreshInterval(base::TimeDelta interval);
@@ -98,6 +105,7 @@ class ASH_EXPORT AmbientBackendModel {
 
  private:
   friend class AmbientBackendModelTest;
+  friend class AmbientAshTestBase;
 
   void NotifyTopicsChanged();
   void NotifyImagesChanged();
@@ -115,6 +123,9 @@ class ASH_EXPORT AmbientBackendModel {
   float temperature_fahrenheit_ = 0.0f;
   bool show_celsius_ = false;
 
+  // The number of consecutive failures to load the next image.
+  int failures_ = 0;
+
   // The interval to refresh photos.
   base::TimeDelta photo_refresh_interval_;
 

ash/ambient/model/ambient_backend_model_observer.h

@@ -24,6 +24,10 @@ class ASH_PUBLIC_EXPORT AmbientBackendModelObserver
   // Invoked when enough images are loaded in memory to start ambient mode.
   virtual void OnImagesReady() {}
 
+  // Invoked when fetching images has failed and not enough images are present
+  // to start ambient mode.
+  virtual void OnImagesFailed() {}
+
   // Invoked when the weather info (condition icon or temperature) stored in the
   // model has been updated.
   virtual void OnWeatherInfoUpdated() {}

ash/ambient/model/ambient_backend_model_unittest.cc

@@ -10,12 +10,25 @@
 #include "ash/ambient/ambient_constants.h"
 #include "ash/ambient/model/ambient_backend_model_observer.h"
 #include "ash/test/ash_test_base.h"
+#include "base/scoped_observer.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/image/image_unittest_util.h"
 #include "ui/views/controls/image_view.h"
 
 namespace ash {
 
+namespace {
+class MockAmbientBackendModelObserver : public AmbientBackendModelObserver {
+ public:
+  MockAmbientBackendModelObserver() = default;
+  ~MockAmbientBackendModelObserver() override = default;
+
+  MOCK_METHOD(void, OnImagesFailed, (), (override));
+};
+
+}  // namespace
+
 class AmbientBackendModelTest : public AshTestBase {
  public:
   AmbientBackendModelTest() = default;
@@ -75,6 +88,8 @@ class AmbientBackendModelTest : public AshTestBase {
     return ambient_backend_model_->GetNextImage();
   }
 
+  int failure_count() { return ambient_backend_model_->failures_; }
+
  private:
   std::unique_ptr<AmbientBackendModel> ambient_backend_model_;
 };
@@ -116,4 +131,39 @@ TEST_F(AmbientBackendModelTest, ShouldReturnExpectedPhotoRefreshInterval) {
   EXPECT_EQ(GetPhotoRefreshInterval(), interval);
 }
 
+TEST_F(AmbientBackendModelTest, ShouldNotifyObserversIfImagesFailed) {
+  ambient_backend_model()->Clear();
+  testing::NiceMock<MockAmbientBackendModelObserver> observer;
+  ScopedObserver<AmbientBackendModel, AmbientBackendModelObserver> scoped_obs{
+      &observer};
+
+  scoped_obs.Add(ambient_backend_model());
+
+  EXPECT_CALL(observer, OnImagesFailed).Times(1);
+
+  for (int i = 0; i < kMaxConsecutiveReadPhotoFailures; i++) {
+    ambient_backend_model()->AddImageFailure();
+  }
+}
+
+TEST_F(AmbientBackendModelTest, ShouldResetFailuresOnAddImage) {
+  testing::NiceMock<MockAmbientBackendModelObserver> observer;
+  ScopedObserver<AmbientBackendModel, AmbientBackendModelObserver> scoped_obs{
+      &observer};
+
+  scoped_obs.Add(ambient_backend_model());
+
+  EXPECT_CALL(observer, OnImagesFailed).Times(0);
+
+  for (int i = 0; i < kMaxConsecutiveReadPhotoFailures - 1; i++) {
+    ambient_backend_model()->AddImageFailure();
+  }
+
+  EXPECT_EQ(failure_count(), kMaxConsecutiveReadPhotoFailures - 1);
+
+  AddNTestImages(1);
+
+  EXPECT_EQ(failure_count(), 0);
+}
+
 }  // namespace ash