ambient: Move photo controller to ash

This cl is a preparation to extract the token management logic for
ambient mode.
After moving photo controller to ash, ambient controller can manage the
token in a single place.

Major changes:
1. Adds a public interface of AmbientBackendController so that Settings
   can call it to get/update ambient mode Settings.
2. Adds a public interface of AmbientClient to provide profile related
   info, e.g. access token.
3. Adds new unittests. Will refactor ambient mode tests in a follow up
   cl to consolidate common logic by creating AmbientAshTestBase.

Bug: b/148463064
Test: unittests
Change-Id: I78a0273b1019fdf86162da73b8122e578e8b3779
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2136853
Commit-Queue: Tao Wu <wutao@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#763163}

ash/BUILD.gn

@@ -4,6 +4,7 @@
 
 import("//build/config/features.gni")
 import("//build/config/ui.gni")
+import("//chromeos/assistant/ambient.gni")
 import("//mojo/public/tools/bindings/mojom.gni")
 import("//testing/test.gni")
 import("//tools/grit/repack.gni")
@@ -164,8 +165,12 @@ component("ash") {
     "ambient/ambient_constants.h",
     "ambient/ambient_controller.cc",
     "ambient/ambient_controller.h",
+    "ambient/ambient_photo_controller.cc",
+    "ambient/ambient_photo_controller.h",
     "ambient/ambient_view_delegate_impl.cc",
     "ambient/ambient_view_delegate_impl.h",
+    "ambient/fake_ambient_backend_controller_impl.cc",
+    "ambient/fake_ambient_backend_controller_impl.h",
     "ambient/model/ambient_backend_model.cc",
     "ambient/model/ambient_backend_model.h",
     "ambient/model/ambient_backend_model_observer.h",
@@ -1567,6 +1572,7 @@ component("ash") {
     "//services/content/public/cpp",
     "//services/data_decoder/public/cpp",
     "//services/metrics/public/cpp:ukm_builders",
+    "//services/network/public/cpp:cpp",
     "//services/preferences/public/cpp",
 
     # TODO(msw): Remove this; ash should not depend on blink/webkit.
@@ -1632,6 +1638,18 @@ component("ash") {
     "//components/exo",
     "//components/exo/wayland",
   ]
+
+  if (enable_cros_ambient_mode_backend) {
+    sources += [
+      "ambient/backdrop/ambient_backend_controller_impl.cc",
+      "ambient/backdrop/ambient_backend_controller_impl.h",
+    ]
+
+    deps += [
+      "//chromeos/assistant/internal/ambient",
+      "//chromeos/assistant/internal/proto/google3",
+    ]
+  }
 }
 
 static_library("ash_shell_lib") {
@@ -1784,6 +1802,7 @@ test("ash_unittests") {
     "accessibility/touch_exploration_controller_unittest.cc",
     "accessibility/touch_exploration_manager_unittest.cc",
     "ambient/ambient_controller_unittest.cc",
+    "ambient/ambient_photo_controller_unittest.cc",
     "ambient/model/ambient_backend_model_unittest.cc",
     "ambient/ui/ambient_container_view_unittest.cc",
     "app_list/app_list_controller_impl_unittest.cc",

ash/DEPS

@@ -30,6 +30,7 @@ include_rules = [
   "+services/device/public",
   "+services/data_decoder/public",
   "+services/media_session/public",
+  "+services/network/public",
   "+services/preferences/public",
   "+services/viz/public",
   "+skia/ext",

ash/ambient/DEPS

@@ -0,0 +1,9 @@
+specific_include_rules = {
+  "ambient_backend_controller_impl\.*": [
+    "+chromeos/assistant/internal/ambient/backdrop_client_config.h",
+    "+chromeos/assistant/internal/proto/google3/backdrop/backdrop.pb.h",
+  ],
+  "ambient_controller.cc": [
+    "+chromeos/assistant/buildflags.h",
+  ],
+}

ash/ambient/ambient_controller.cc

@@ -7,28 +7,36 @@
 #include <string>
 
 #include "ash/ambient/ambient_constants.h"
+#include "ash/ambient/fake_ambient_backend_controller_impl.h"
 #include "ash/ambient/model/ambient_backend_model_observer.h"
 #include "ash/ambient/ui/ambient_container_view.h"
 #include "ash/ambient/util/ambient_util.h"
 #include "ash/login/ui/lock_screen.h"
+#include "ash/public/cpp/ambient/ambient_backend_controller.h"
+#include "ash/public/cpp/ambient/ambient_client.h"
 #include "ash/public/cpp/ambient/ambient_mode_state.h"
 #include "ash/public/cpp/ambient/ambient_prefs.h"
-#include "ash/public/cpp/ambient/photo_controller.h"
 #include "ash/public/cpp/assistant/controller/assistant_ui_controller.h"
 #include "ash/session/session_controller_impl.h"
 #include "ash/shell.h"
+#include "build/buildflag.h"
+#include "chromeos/assistant/buildflags.h"
 #include "chromeos/constants/chromeos_features.h"
 #include "chromeos/services/assistant/public/mojom/assistant.mojom.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "ui/views/widget/widget.h"
 
+#if BUILDFLAG(ENABLE_CROS_AMBIENT_MODE_BACKEND)
+#include "ash/ambient/backdrop/ambient_backend_controller_impl.h"
+#endif  // BUILDFLAG(ENABLE_CROS_AMBIENT_MODE_BACKEND)
+
 namespace ash {
 
 namespace {
 
 bool CanStartAmbientMode() {
-  return chromeos::features::IsAmbientModeEnabled() && PhotoController::Get() &&
-         !ambient::util::IsShowing(LockScreen::ScreenType::kLogin);
+  return chromeos::features::IsAmbientModeEnabled() &&
+         AmbientClient::Get()->IsAmbientModeAllowedForActiveUser();
 }
 
 void CloseAssistantUi() {
@@ -37,6 +45,14 @@ void CloseAssistantUi() {
       chromeos::assistant::mojom::AssistantExitPoint::kUnspecified);
 }
 
+std::unique_ptr<AmbientBackendController> CreateAmbientBackendController() {
+#if BUILDFLAG(ENABLE_CROS_AMBIENT_MODE_BACKEND)
+  return std::make_unique<AmbientBackendControllerImpl>();
+#else
+  return std::make_unique<FakeAmbientBackendControllerImpl>();
+#endif  // BUILDFLAG(ENABLE_CROS_AMBIENT_MODE_BACKEND)
+}
+
 }  // namespace
 
 // static
@@ -53,6 +69,7 @@ void AmbientController::RegisterProfilePrefs(PrefRegistrySimple* registry) {
 }
 
 AmbientController::AmbientController() {
+  ambient_backend_controller_ = CreateAmbientBackendController();
   ambient_state_.AddObserver(this);
   // |SessionController| is initialized before |this| in Shell.
   Shell::Get()->session_controller()->AddObserver(this);
@@ -130,8 +147,8 @@ void AmbientController::Toggle() {
 void AmbientController::OnBackgroundPhotoEvents() {
   refresh_timer_.Stop();
 
-  // Move the |AmbientModeContainer| beneath the |LockScreenWidget| to show the
-  // lock screen contents on top before the fade-out animation.
+  // Move the |AmbientModeContainer| beneath the |LockScreenWidget| to show
+  // the lock screen contents on top before the fade-out animation.
   auto* ambient_window = container_view_->GetWidget()->GetNativeWindow();
   ambient_window->parent()->StackChildAtBottom(ambient_window);
 
@@ -188,7 +205,7 @@ void AmbientController::ScheduleRefreshImage() {
 
 void AmbientController::GetNextImage() {
   // Start requesting photo and weather information from the backdrop server.
-  PhotoController::Get()->GetNextScreenUpdateInfo(
+  ambient_photo_controller_.GetNextScreenUpdateInfo(
       base::BindOnce(&AmbientController::OnPhotoDownloaded,
                      weak_factory_.GetWeakPtr()),
       base::BindOnce(&AmbientController::OnWeatherConditionIconDownloaded,
@@ -200,7 +217,6 @@ void AmbientController::OnPhotoDownloaded(const gfx::ImageSkia& image) {
   if (image.isNull())
     return;
 
-  DCHECK(!image.isNull());
   ambient_backend_model_.AddNextImage(image);
   ScheduleRefreshImage();
 }
@@ -216,4 +232,9 @@ void AmbientController::OnWeatherConditionIconDownloaded(
   ambient_backend_model_.UpdateWeatherInfo(icon, temp_f.value());
 }
 
+void AmbientController::set_backend_controller_for_testing(
+    std::unique_ptr<AmbientBackendController> backend_controller) {
+  ambient_backend_controller_ = std::move(backend_controller);
+}
+
 }  // namespace ash

ash/ambient/ambient_controller.h

@@ -5,6 +5,7 @@
 #ifndef ASH_AMBIENT_AMBIENT_CONTROLLER_H_
 #define ASH_AMBIENT_AMBIENT_CONTROLLER_H_
 
+#include "ash/ambient/ambient_photo_controller.h"
 #include "ash/ambient/ambient_view_delegate_impl.h"
 #include "ash/ambient/model/ambient_backend_model.h"
 #include "ash/ash_export.h"
@@ -24,7 +25,9 @@ class ImageSkia;
 
 namespace ash {
 
+class AmbientBackendController;
 class AmbientContainerView;
+class AmbientPhotoController;
 
 // Class to handle all ambient mode functionalities.
 class ASH_EXPORT AmbientController : public views::WidgetObserver,
@@ -72,7 +75,14 @@ class ASH_EXPORT AmbientController : public views::WidgetObserver,
     return refresh_timer_;
   }
 
+  AmbientBackendController* ambient_backend_controller() {
+    return ambient_backend_controller_.get();
+  }
+
  private:
+  friend class AmbientContainerViewTest;
+  friend class AmbientPhotoControllerTest;
+
   void CreateContainerView();
   void DestroyContainerView();
   void RefreshImage();
@@ -87,10 +97,19 @@ class ASH_EXPORT AmbientController : public views::WidgetObserver,
 
   void StartFadeOutAnimation();
 
+  AmbientPhotoController* get_ambient_photo_controller_for_testing() {
+    return &ambient_photo_controller_;
+  }
+
+  void set_backend_controller_for_testing(
+      std::unique_ptr<AmbientBackendController> photo_client);
+
   AmbientViewDelegateImpl delegate_{this};
   AmbientContainerView* container_view_ = nullptr;   // Owned by view hierarchy.
   AmbientBackendModel ambient_backend_model_;
   AmbientModeState ambient_state_;
+  std::unique_ptr<AmbientBackendController> ambient_backend_controller_;
+  AmbientPhotoController ambient_photo_controller_;
   base::OneShotTimer refresh_timer_;
   base::WeakPtrFactory<AmbientController> weak_factory_{this};
 

chrome/browser/ui/ash/ambient/photo_controller_impl.cc → ash/ambient/ambient_photo_controller.cc

@@ -2,61 +2,63 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/ui/ash/ambient/photo_controller_impl.h"
+#include "ash/ambient/ambient_photo_controller.h"
 
 #include <utility>
 
+#include "ash/ambient/ambient_controller.h"
 #include "ash/public/cpp/assistant/assistant_image_downloader.h"
+#include "ash/public/cpp/session/session_types.h"
+#include "ash/session/session_controller_impl.h"
+#include "ash/shell.h"
 #include "base/bind.h"
 #include "base/optional.h"
-#include "chrome/browser/chromeos/profiles/profile_helper.h"
-#include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/profiles/profile_manager.h"
 #include "components/account_id/account_id.h"
 #include "ui/gfx/image/image_skia.h"
 
+namespace ash {
+
 namespace {
+
 using DownloadCallback = base::OnceCallback<void(const gfx::ImageSkia&)>;
 
 void DownloadImageFromUrl(const std::string& url, DownloadCallback callback) {
   DCHECK(!url.empty());
-  AccountId account_id =
-      chromeos::ProfileHelper::Get()
-          ->GetUserByProfile(ProfileManager::GetActiveUserProfile())
-          ->GetAccountId();
+  const UserSession* user_session =
+      Shell::Get()->session_controller()->GetUserSession(0);
+  if (!user_session) {
+    LOG(WARNING) << "Unable to retrieve active user session.";
+    std::move(callback).Run(gfx::ImageSkia());
+    return;
+  }
+
+  AccountId account_id = user_session->user_info.account_id;
   ash::AssistantImageDownloader::GetInstance()->Download(account_id, GURL(url),
                                                          std::move(callback));
 }
 
 }  // namespace
 
-PhotoControllerImpl::PhotoControllerImpl()
-    : photo_client_(PhotoClient::Create()) {}
+AmbientPhotoController::AmbientPhotoController() = default;
 
-PhotoControllerImpl::~PhotoControllerImpl() = default;
+AmbientPhotoController::~AmbientPhotoController() = default;
 
-void PhotoControllerImpl::GetNextScreenUpdateInfo(
+void AmbientPhotoController::GetNextScreenUpdateInfo(
     PhotoDownloadCallback photo_callback,
     WeatherIconDownloadCallback icon_callback) {
-  photo_client_->FetchScreenUpdateInfo(
-      base::BindOnce(&PhotoControllerImpl::OnNextScreenUpdateInfoFetched,
-                     weak_factory_.GetWeakPtr(), std::move(photo_callback),
-                     std::move(icon_callback)));
+  Shell::Get()
+      ->ambient_controller()
+      ->ambient_backend_controller()
+      ->FetchScreenUpdateInfo(
+          base::BindOnce(&AmbientPhotoController::OnNextScreenUpdateInfoFetched,
+                         weak_factory_.GetWeakPtr(), std::move(photo_callback),
+                         std::move(icon_callback)));
 }
 
-void PhotoControllerImpl::GetSettings(GetSettingsCallback callback) {
-  photo_client_->GetSettings(std::move(callback));
-}
-
-void PhotoControllerImpl::UpdateSettings(int topic_source,
-                                         UpdateSettingsCallback callback) {
-  photo_client_->UpdateSettings(topic_source, std::move(callback));
-}
-
-void PhotoControllerImpl::OnNextScreenUpdateInfoFetched(
+void AmbientPhotoController::OnNextScreenUpdateInfoFetched(
     PhotoDownloadCallback photo_callback,
     WeatherIconDownloadCallback icon_callback,
-    const ash::PhotoController::ScreenUpdate& screen_update) {
+    const ash::ScreenUpdate& screen_update) {
   // It is possible that |screen_update| is an empty instance if fatal errors
   // happened during the fetch.
   if (screen_update.next_topics.size() == 0 &&
@@ -71,8 +73,8 @@ void PhotoControllerImpl::OnNextScreenUpdateInfoFetched(
   StartDownloadingWeatherConditionIcon(screen_update, std::move(icon_callback));
 }
 
-void PhotoControllerImpl::StartDownloadingPhotoImage(
-    const ash::PhotoController::ScreenUpdate& screen_update,
+void AmbientPhotoController::StartDownloadingPhotoImage(
+    const ash::ScreenUpdate& screen_update,
     PhotoDownloadCallback photo_callback) {
   // We specified the size of |next_topics| in the request. So if nothing goes
   // wrong, we should get that amount of |Topic| in the response.
@@ -83,13 +85,13 @@ void PhotoControllerImpl::StartDownloadingPhotoImage(
   }
 
   // TODO(b/148462257): Handle a batch of topics.
-  ash::PhotoController::Topic topic = screen_update.next_topics[0];
+  ash::AmbientModeTopic topic = screen_update.next_topics[0];
   const std::string& image_url = topic.portrait_image_url.value_or(topic.url);
   DownloadImageFromUrl(image_url, base::BindOnce(std::move(photo_callback)));
 }
 
-void PhotoControllerImpl::StartDownloadingWeatherConditionIcon(
-    const ash::PhotoController::ScreenUpdate& screen_update,
+void AmbientPhotoController::StartDownloadingWeatherConditionIcon(
+    const ash::ScreenUpdate& screen_update,
     WeatherIconDownloadCallback icon_callback) {
   if (!screen_update.weather_info.has_value()) {
     LOG(WARNING) << "No weather info included in the response.";
@@ -114,3 +116,5 @@ void PhotoControllerImpl::StartDownloadingWeatherConditionIcon(
                        base::BindOnce(std::move(icon_callback),
                                       screen_update.weather_info->temp_f));
 }
+
+}  // namespace ash