ambient: construct photo_controller on enabled

AmbientPhotoController and AmbientPhotoCache are only necessary
when a user is logged in and ambient mode is enabled. Construct them
at that time, and destruct them if ambient mode is disabled.

Schedule fetching backup images when AmbientPhotoController is
constructed, rather than in OnFirstSessionStarted.

BUG=b:176094707
TEST=ash_unittests --gtest_filter="AmbientControllerTest.*"

Cq-Include-Trybots: luci.chrome.try:linux-chromeos-chrome
Change-Id: Id87bb16e6d4cc14074dd9bdb352fb5795d0f8915
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2605744
Commit-Queue: Jeffrey Young <cowmoo@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Reviewed-by: Jeroen Dhollander <jeroendh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842133}

ash/ambient/ambient_controller.cc

@@ -253,6 +253,7 @@ void AmbientController::OnAmbientUiVisibilityChanged(
         }
       } else {
         DCHECK(visibility == AmbientUiVisibility::kClosed);
+        GetAmbientBackendModel()->ResetImageFailures();
         inactivity_timer_.Stop();
         user_activity_observer_.Reset();
         power_status_observer_.Reset();
@@ -313,11 +314,6 @@ void AmbientController::OnLockStateChanged(bool locked) {
   }
 }
 
-void AmbientController::OnFirstSessionStarted() {
-  if (IsAmbientModeEnabled())
-    ambient_photo_controller_.ScheduleFetchBackupImages();
-}
-
 void AmbientController::OnActiveUserPrefServiceChanged(
     PrefService* pref_service) {
   if (!AmbientClient::Get()->IsAmbientModeAllowed() ||
@@ -449,7 +445,6 @@ void AmbientController::CloseUi() {
   DVLOG(1) << __func__;
 
   ambient_ui_model_.SetUiVisibility(AmbientUiVisibility::kClosed);
-  GetAmbientBackendModel()->ResetImageFailures();
 }
 
 void AmbientController::ToggleInSessionUi() {
@@ -509,10 +504,6 @@ void AmbientController::CloseAllWidgets(bool immediately) {
 }
 
 void AmbientController::OnEnabledPrefChanged() {
-  // TODO(b/176094707) conditionally create/destroy photo_controller and cache
-  // if Ambient is enabled
-  ambient_photo_controller_.InitCache();
-
   if (IsAmbientModeEnabled()) {
     DVLOG(1) << "Ambient mode enabled";
 
@@ -539,10 +530,11 @@ void AmbientController::OnEnabledPrefChanged() {
     OnLockScreenBackgroundTimeoutPrefChanged();
     OnPhotoRefreshIntervalPrefChanged();
 
+    ambient_photo_controller_ = std::make_unique<AmbientPhotoController>();
+
     ambient_ui_model_observer_.Observe(&ambient_ui_model_);
 
-    ambient_backend_model_observer_.Observe(
-        ambient_photo_controller_.ambient_backend_model());
+    ambient_backend_model_observer_.Observe(GetAmbientBackendModel());
 
     auto* power_manager_client = chromeos::PowerManagerClient::Get();
     DCHECK(power_manager_client);
@@ -564,6 +556,8 @@ void AmbientController::OnEnabledPrefChanged() {
         pref_change_registrar_->Remove(pref_name);
     }
 
+    ambient_photo_controller_.reset();
+
     ambient_ui_model_observer_.Reset();
     ambient_backend_model_observer_.Reset();
     power_manager_client_observer_.Reset();
@@ -630,7 +624,8 @@ void AmbientController::DismissUI() {
 }
 
 AmbientBackendModel* AmbientController::GetAmbientBackendModel() {
-  return ambient_photo_controller_.ambient_backend_model();
+  DCHECK(ambient_photo_controller_);
+  return ambient_photo_controller_->ambient_backend_model();
 }
 
 void AmbientController::OnImagesReady() {
@@ -685,11 +680,13 @@ void AmbientController::CreateAndShowWidgets() {
 }
 
 void AmbientController::StartRefreshingImages() {
-  ambient_photo_controller_.StartScreenUpdate();
+  DCHECK(ambient_photo_controller_);
+  ambient_photo_controller_->StartScreenUpdate();
 }
 
 void AmbientController::StopRefreshingImages() {
-  ambient_photo_controller_.StopScreenUpdate();
+  DCHECK(ambient_photo_controller_);
+  ambient_photo_controller_->StopScreenUpdate();
 }
 
 void AmbientController::set_backend_controller_for_testing(

ash/ambient/ambient_controller.h

@@ -65,7 +65,6 @@ class ASH_EXPORT AmbientController
 
   // SessionObserver:
   void OnLockStateChanged(bool locked) override;
-  void OnFirstSessionStarted() override;
   void OnActiveUserPrefServiceChanged(PrefService* pref_service) override;
 
   // PowerStatus::Observer:
@@ -119,7 +118,7 @@ class ASH_EXPORT AmbientController
   }
 
   AmbientPhotoController* ambient_photo_controller() {
-    return &ambient_photo_controller_;
+    return ambient_photo_controller_.get();
   }
 
   AmbientUiModel* ambient_ui_model() { return &ambient_ui_model_; }
@@ -176,7 +175,7 @@ class ASH_EXPORT AmbientController
 
   AmbientAccessTokenController access_token_controller_;
   std::unique_ptr<AmbientBackendController> ambient_backend_controller_;
-  AmbientPhotoController ambient_photo_controller_;
+  std::unique_ptr<AmbientPhotoController> ambient_photo_controller_;
 
   // Monitors the device inactivity and controls the auto-show of ambient.
   base::OneShotTimer inactivity_timer_;

ash/ambient/ambient_photo_controller.cc

@@ -104,9 +104,14 @@ base::FilePath GetCacheRootPath() {
 AmbientPhotoController::AmbientPhotoController()
     : fetch_topic_retry_backoff_(&kFetchTopicRetryBackoffPolicy),
       resume_fetch_image_backoff_(&kResumeFetchImageBackoffPolicy),
+      photo_cache_(AmbientPhotoCache::Create(GetCacheRootPath().Append(
+          FILE_PATH_LITERAL(kAmbientModeCacheDirectoryName)))),
+      backup_photo_cache_(AmbientPhotoCache::Create(GetCacheRootPath().Append(
+          FILE_PATH_LITERAL(kAmbientModeBackupCacheDirectoryName)))),
       task_runner_(
           base::ThreadPool::CreateSequencedTaskRunner(GetTaskTraits())) {
   ambient_backend_model_observer_.Add(&ambient_backend_model_);
+  ScheduleFetchBackupImages();
 }
 
 AmbientPhotoController::~AmbientPhotoController() = default;
@@ -139,18 +144,6 @@ void AmbientPhotoController::StopScreenUpdate() {
   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::FetchBackupImages,
-                     weak_factory_.GetWeakPtr()));
-}
-
 void AmbientPhotoController::OnTopicsChanged() {
   if (ambient_backend_model_.topics().size() < kMaxNumberOfCachedImages)
     ScheduleFetchTopics(/*backoff=*/false);
@@ -187,17 +180,6 @@ void AmbientPhotoController::ClearCache() {
   backup_photo_cache_->Clear();
 }
 
-void AmbientPhotoController::InitCache() {
-  if (!photo_cache_) {
-    photo_cache_ = AmbientPhotoCache::Create(GetCacheRootPath().Append(
-        FILE_PATH_LITERAL(kAmbientModeCacheDirectoryName)));
-  }
-  if (!backup_photo_cache_) {
-    backup_photo_cache_ = AmbientPhotoCache::Create(GetCacheRootPath().Append(
-        FILE_PATH_LITERAL(kAmbientModeBackupCacheDirectoryName)));
-  }
-}
-
 void AmbientPhotoController::ScheduleFetchTopics(bool backoff) {
   // If retry, using the backoff delay, otherwise the default delay.
   const base::TimeDelta delay =
@@ -217,6 +199,19 @@ void AmbientPhotoController::ScheduleRefreshImage() {
                      weak_factory_.GetWeakPtr()));
 }
 
+void AmbientPhotoController::ScheduleFetchBackupImages() {
+  DVLOG(3) << __func__;
+  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::FetchBackupImages,
+                     weak_factory_.GetWeakPtr()));
+}
+
 void AmbientPhotoController::FetchBackupImages() {
   const auto& backup_photo_urls = GetBackupPhotoUrls();
   backup_retries_to_read_from_cache_ = backup_photo_urls.size();

ash/ambient/ambient_photo_controller.h

@@ -58,8 +58,6 @@ class ASH_EXPORT AmbientPhotoController : public AmbientBackendModelObserver {
   void StartScreenUpdate();
   void StopScreenUpdate();
 
-  void ScheduleFetchBackupImages();
-
   AmbientBackendModel* ambient_backend_model() {
     return &ambient_backend_model_;
   }
@@ -68,7 +66,7 @@ class ASH_EXPORT AmbientPhotoController : public AmbientBackendModelObserver {
     return photo_refresh_timer_;
   }
 
-  const base::OneShotTimer& backup_photo_refresh_timer_for_testing() const {
+  base::OneShotTimer& backup_photo_refresh_timer_for_testing() {
     return backup_photo_refresh_timer_;
   }
 
@@ -78,10 +76,9 @@ class ASH_EXPORT AmbientPhotoController : public AmbientBackendModelObserver {
   // Clear cache when Settings changes.
   void ClearCache();
 
-  void InitCache();
-
  private:
   friend class AmbientAshTestBase;
+  friend class AmbientPhotoControllerTest;
 
   void FetchTopics();
 
@@ -91,6 +88,8 @@ class ASH_EXPORT AmbientPhotoController : public AmbientBackendModelObserver {
 
   void ScheduleRefreshImage();
 
+  void ScheduleFetchBackupImages();
+
   // Download backup cache images.
   void FetchBackupImages();
 

ash/ambient/ambient_photo_controller_unittest.cc

@@ -80,6 +80,10 @@ class AmbientPhotoControllerTest : public AmbientAshTestBase {
                               loop.QuitClosure());
     loop.Run();
   }
+
+  void ScheduleFetchBackupImages() {
+    photo_controller()->ScheduleFetchBackupImages();
+  }
 };
 
 // Test that topics are downloaded when starting screen update.
@@ -282,7 +286,7 @@ TEST_F(AmbientPhotoControllerTest, ShouldDownloadBackupImagesWhenScheduled) {
   std::string expected_data = "backup data";
   SetBackupDownloadPhotoData(expected_data);
 
-  photo_controller()->ScheduleFetchBackupImages();
+  ScheduleFetchBackupImages();
 
   EXPECT_TRUE(
       photo_controller()->backup_photo_refresh_timer_for_testing().IsRunning());
@@ -308,7 +312,7 @@ TEST_F(AmbientPhotoControllerTest, ShouldDownloadBackupImagesWhenScheduled) {
 }
 
 TEST_F(AmbientPhotoControllerTest, ShouldResetTimerWhenBackupImagesFail) {
-  photo_controller()->ScheduleFetchBackupImages();
+  ScheduleFetchBackupImages();
 
   EXPECT_TRUE(
       photo_controller()->backup_photo_refresh_timer_for_testing().IsRunning());
@@ -326,7 +330,7 @@ TEST_F(AmbientPhotoControllerTest, ShouldResetTimerWhenBackupImagesFail) {
 
 TEST_F(AmbientPhotoControllerTest,
        ShouldStartDownloadBackupImagesOnAmbientModeStart) {
-  photo_controller()->ScheduleFetchBackupImages();
+  ScheduleFetchBackupImages();
 
   EXPECT_TRUE(
       photo_controller()->backup_photo_refresh_timer_for_testing().IsRunning());

ash/ambient/test/ambient_ash_test_base.cc

@@ -185,10 +185,6 @@ void AmbientAshTestBase::SetUp() {
   ambient_controller()->set_backend_controller_for_testing(nullptr);
   ambient_controller()->set_backend_controller_for_testing(
       std::make_unique<FakeAmbientBackendControllerImpl>());
-  photo_controller()->set_photo_cache_for_testing(
-      std::make_unique<TestAmbientPhotoCacheImpl>());
-  photo_controller()->set_backup_photo_cache_for_testing(
-      std::make_unique<TestAmbientPhotoCacheImpl>());
   token_controller()->SetTokenUsageBufferForTesting(
       base::TimeDelta::FromSeconds(30));
   SetAmbientModeEnabled(true);
@@ -202,6 +198,14 @@ void AmbientAshTestBase::TearDown() {
 void AmbientAshTestBase::SetAmbientModeEnabled(bool enabled) {
   Shell::Get()->session_controller()->GetActivePrefService()->SetBoolean(
       ambient::prefs::kAmbientModeEnabled, enabled);
+
+  if (enabled) {
+    photo_controller()->set_photo_cache_for_testing(
+        std::make_unique<TestAmbientPhotoCacheImpl>());
+    photo_controller()->set_backup_photo_cache_for_testing(
+        std::make_unique<TestAmbientPhotoCacheImpl>());
+    photo_controller()->backup_photo_refresh_timer_for_testing().Stop();
+  }
 }
 
 void AmbientAshTestBase::ShowAmbientScreen() {