Reland "ambient: Add Get/Update Settings APIs"

This is a reland of e77cef6b

No changes, submit after the internal code submitted.

TBR=xiaohuic@chromium.org,xiyuan@chromium.org

Original change's description:
> ambient: Add Get/Update Settings APIs
>
> This patch adds the Get/Update Settings APIs so that we can show and
> update it in the Chrome OS Settings ambient mode page, which will be
> implemented in a follow up cl.
>
> Bug: b/148483925
> Test: manual
> Change-Id: Ia74d875813860a6abbfdc01479a67fed0c029fb8
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2125685
> Commit-Queue: Tao Wu <wutao@chromium.org>
> Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#755531}

Bug: b/148483925
Change-Id: I40f57d6e2cfe267a2ff27513e3702273e87d9434
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2135417Reviewed-by: Tao Wu <wutao@chromium.org>
Commit-Queue: Tao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756159}

ash/ambient/ambient_controller.cc

@@ -74,6 +74,8 @@ AmbientController::~AmbientController() {
 
 void AmbientController::OnWidgetDestroying(views::Widget* widget) {
   refresh_timer_.Stop();
+  photo_model_.Clear();
+  weak_factory_.InvalidateWeakPtrs();
   container_view_->GetWidget()->RemoveObserver(this);
   container_view_ = nullptr;
 
@@ -170,9 +172,9 @@ void AmbientController::ScheduleRefreshImage() {
     refresh_interval = base::TimeDelta::FromSeconds(5);
   }
 
-  refresh_timer_.Start(
-      FROM_HERE, refresh_interval,
-      base::BindOnce(&AmbientController::RefreshImage, base::Unretained(this)));
+  refresh_timer_.Start(FROM_HERE, refresh_interval,
+                       base::BindOnce(&AmbientController::RefreshImage,
+                                      weak_factory_.GetWeakPtr()));
 }
 
 void AmbientController::GetNextImage() {

ash/ambient/model/photo_model.cc

@@ -54,6 +54,11 @@ void PhotoModel::AddNextImage(const gfx::ImageSkia& image) {
     NotifyImagesChanged();
 }
 
+void PhotoModel::Clear() {
+  images_.clear();
+  current_image_index_ = 0;
+}
+
 gfx::ImageSkia PhotoModel::GetPrevImage() const {
   if (current_image_index_ == 0)
     return gfx::ImageSkia();

ash/ambient/model/photo_model.h

@@ -34,6 +34,9 @@ class ASH_EXPORT PhotoModel {
   // Add image to local storage.
   void AddNextImage(const gfx::ImageSkia& image);
 
+  // Clear local storage.
+  void Clear();
+
   // Get images from local storage. Could be null image.
   gfx::ImageSkia GetPrevImage() const;
   gfx::ImageSkia GetCurrImage() const;

ash/public/cpp/ambient/photo_controller.cc

@@ -16,13 +16,13 @@ PhotoController* g_photo_controller = nullptr;
 
 PhotoController::Topic::Topic() = default;
 
-PhotoController::Topic::~Topic() = default;
-
 PhotoController::Topic::Topic(const Topic&) = default;
 
 PhotoController::Topic& PhotoController::Topic::operator=(const Topic&) =
     default;
 
+PhotoController::Topic::~Topic() = default;
+
 // static
 PhotoController* PhotoController::Get() {
   return g_photo_controller;

ash/public/cpp/ambient/photo_controller.h

@@ -43,10 +43,20 @@ class ASH_PUBLIC_EXPORT PhotoController {
 
   using PhotoDownloadCallback =
       base::OnceCallback<void(bool success, const gfx::ImageSkia&)>;
+  using GetSettingsCallback =
+      base::OnceCallback<void(const base::Optional<int>& topic_source)>;
+  using UpdateSettingsCallback = base::OnceCallback<void(bool success)>;
 
   // Get next image.
   virtual void GetNextImage(PhotoDownloadCallback callback) = 0;
 
+  // Get settings.
+  virtual void GetSettings(GetSettingsCallback callback) = 0;
+
+  // Update settings.
+  virtual void UpdateSettings(int topic_source,
+                              UpdateSettingsCallback callback) = 0;
+
  protected:
   PhotoController();
   virtual ~PhotoController();

ash/public/cpp/test/test_photo_controller.cc

@@ -22,4 +22,14 @@ void TestPhotoController::GetNextImage(
   std::move(callback).Run(/*success=*/true, image);
 }
 
+void TestPhotoController::GetSettings(GetSettingsCallback callback) {
+  // 0 is the enum number for Google Photos.
+  std::move(callback).Run(/*topic_source=*/0);
+}
+
+void TestPhotoController::UpdateSettings(int topic_source,
+                                         UpdateSettingsCallback callback) {
+  std::move(callback).Run(/*success=*/true);
+}
+
 }  // namespace ash

ash/public/cpp/test/test_photo_controller.h

@@ -19,6 +19,9 @@ class ASH_PUBLIC_EXPORT TestPhotoController : public PhotoController {
 
   // PhotoController:
   void GetNextImage(PhotoController::PhotoDownloadCallback callback) override;
+  void GetSettings(GetSettingsCallback callback) override;
+  void UpdateSettings(int topic_source,
+                      UpdateSettingsCallback callback) override;
 };
 
 }  // namespace ash

chrome/browser/ui/ash/ambient/backdrop/photo_client_impl.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/ui/ash/ambient/backdrop/photo_client_impl.h"
 
 #include <utility>
+#include <vector>
 
 #include "ash/public/cpp/ambient/ambient_prefs.h"
 #include "base/base64.h"
@@ -157,8 +158,23 @@ PhotoClientImpl::~PhotoClientImpl() = default;
 void PhotoClientImpl::FetchTopicInfo(OnTopicInfoFetchedCallback callback) {
   // TODO(b/148463064): Access token will be requested and cached before
   // entering lock screen.
+  // Consolidate the functions of StartToFetchTopicInfo, StartToGetSettings, and
+  // StartToUpdateSettings after this is done.
   RequestAccessToken(base::BindOnce(&PhotoClientImpl::StartToFetchTopicInfo,
-                                    base::Unretained(this),
+                                    weak_factory_.GetWeakPtr(),
+                                    std::move(callback)));
+}
+
+void PhotoClientImpl::GetSettings(GetSettingsCallback callback) {
+  RequestAccessToken(base::BindOnce(&PhotoClientImpl::StartToGetSettings,
+                                    weak_factory_.GetWeakPtr(),
+                                    std::move(callback)));
+}
+
+void PhotoClientImpl::UpdateSettings(int topic_source,
+                                     UpdateSettingsCallback callback) {
+  RequestAccessToken(base::BindOnce(&PhotoClientImpl::StartToUpdateSettings,
+                                    weak_factory_.GetWeakPtr(), topic_source,
                                     std::move(callback)));
 }
 
@@ -172,6 +188,8 @@ void PhotoClientImpl::RequestAccessToken(GetAccessTokenCallback callback) {
   signin::ScopeSet scopes;
   scopes.insert(kPhotosOAuthScope);
   // TODO(b/148463064): Handle retry refresh token and multiple requests.
+  // Currently only one request is allowed.
+  DCHECK(!access_token_fetcher_);
   access_token_fetcher_ = identity_manager->CreateAccessTokenFetcherForAccount(
       account_info.account_id, /*oauth_consumer_name=*/"ChromeOS_AmbientMode",
 
@@ -186,45 +204,22 @@ void PhotoClientImpl::StartToFetchTopicInfo(
     signin::AccessTokenInfo access_token_info) {
   access_token_fetcher_.reset();
   if (gaia_id.empty() || access_token_info.token.empty()) {
-    std::move(callback).Run(/*success=*/false, base::nullopt);
+    std::move(callback).Run(base::nullopt);
     return;
   }
 
   std::string client_id = GetClientId();
   chromeos::ambient::BackdropClientConfig::Request request =
-      backdrop_client_config_.GetFetchTopicInfoRequest(
+      backdrop_client_config_.CreateFetchTopicInfoRequest(
           gaia_id, access_token_info.token, client_id);
   auto resource_request = CreateResourceRequest(request);
 
-  net::NetworkTrafficAnnotationTag traffic_annotation =
-      net::DefineNetworkTrafficAnnotation("backdrop_fetch_topics",
-                                          R"(
-            semantics {
-              sender: "ChromeOS Ambient Mode"
-              description:
-                "The ChromeOS Ambient Mode displays a rich set of photos for "
-                "users to choose from."
-              trigger:
-                "When ChromeOS Ambient Mode starts"
-              data:
-                "The Backdrop protocol buffer messages. Containing user auth"
-                "token and a unique random client id."
-              destination: GOOGLE_OWNED_SERVICE
-            }
-            policy {
-              cookies_allowed: NO
-              setting:
-                "NA"
-              policy_exception_justification:
-                "Not implemented, considered not necessary."
-            })");
-
   // |base::Unretained| is safe because this instance outlives
   // |backdrop_url_loader_|.
   DCHECK(!backdrop_url_loader_);
   backdrop_url_loader_ = std::make_unique<BackdropURLLoader>();
   backdrop_url_loader_->Start(
-      std::move(resource_request), request.body, traffic_annotation,
+      std::move(resource_request), request.body, NO_TRAFFIC_ANNOTATION_YET,
       base::BindOnce(&PhotoClientImpl::OnTopicInfoFetched,
                      base::Unretained(this), std::move(callback)));
 }
@@ -239,5 +234,84 @@ void PhotoClientImpl::OnTopicInfoFetched(
   backdrop::ScreenUpdate::Topic backdrop_topic =
       BackdropClientConfig::ParseFetchTopicInfoResponse(*response);
   ash::PhotoController::Topic topic = CreateTopicFrom(backdrop_topic);
-  std::move(callback).Run(/*success=*/true, topic);
+  std::move(callback).Run(topic);
+}
+
+void PhotoClientImpl::StartToGetSettings(
+    GetSettingsCallback callback,
+    const std::string& gaia_id,
+    GoogleServiceAuthError error,
+    signin::AccessTokenInfo access_token_info) {
+  access_token_fetcher_.reset();
+
+  if (gaia_id.empty() || access_token_info.token.empty()) {
+    std::move(callback).Run(/*topic_source=*/base::nullopt);
+    return;
+  }
+
+  std::string client_id = GetClientId();
+  BackdropClientConfig::Request request =
+      backdrop_client_config_.CreateGetSettingsRequest(
+          gaia_id, access_token_info.token, client_id);
+  auto resource_request = CreateResourceRequest(request);
+
+  // |base::Unretained| is safe because this instance outlives
+  // |backdrop_url_loader_|.
+  DCHECK(!backdrop_url_loader_);
+  backdrop_url_loader_ = std::make_unique<BackdropURLLoader>();
+  backdrop_url_loader_->Start(
+      std::move(resource_request), request.body, NO_TRAFFIC_ANNOTATION_YET,
+      base::BindOnce(&PhotoClientImpl::OnGetSettings, base::Unretained(this),
+                     std::move(callback)));
+}
+
+void PhotoClientImpl::OnGetSettings(GetSettingsCallback callback,
+                                    std::unique_ptr<std::string> response) {
+  DCHECK(backdrop_url_loader_);
+  backdrop_url_loader_.reset();
+
+  int topic_source = BackdropClientConfig::ParseGetSettingsResponse(*response);
+  if (topic_source == -1)
+    std::move(callback).Run(base::nullopt);
+  else
+    std::move(callback).Run(topic_source);
+}
+
+void PhotoClientImpl::StartToUpdateSettings(
+    int topic_source,
+    UpdateSettingsCallback callback,
+    const std::string& gaia_id,
+    GoogleServiceAuthError error,
+    signin::AccessTokenInfo access_token_info) {
+  access_token_fetcher_.reset();
+
+  if (gaia_id.empty() || access_token_info.token.empty()) {
+    std::move(callback).Run(/*success=*/false);
+    return;
+  }
+
+  std::string client_id = GetClientId();
+  BackdropClientConfig::Request request =
+      backdrop_client_config_.CreateUpdateSettingsRequest(
+          gaia_id, access_token_info.token, client_id, topic_source);
+  auto resource_request = CreateResourceRequest(request);
+
+  // |base::Unretained| is safe because this instance outlives
+  // |backdrop_url_loader_|.
+  DCHECK(!backdrop_url_loader_);
+  backdrop_url_loader_ = std::make_unique<BackdropURLLoader>();
+  backdrop_url_loader_->Start(
+      std::move(resource_request), request.body, NO_TRAFFIC_ANNOTATION_YET,
+      base::BindOnce(&PhotoClientImpl::OnUpdateSettings, base::Unretained(this),
+                     std::move(callback)));
+}
+
+void PhotoClientImpl::OnUpdateSettings(UpdateSettingsCallback callback,
+                                       std::unique_ptr<std::string> response) {
+  DCHECK(backdrop_url_loader_);
+  backdrop_url_loader_.reset();
+
+  const bool success =
+      BackdropClientConfig::ParseUpdateSettingsResponse(*response);
+  std::move(callback).Run(success);
 }

chrome/browser/ui/ash/ambient/backdrop/photo_client_impl.h

@@ -7,7 +7,7 @@
 
 #include <memory>
 
-#include "base/callback.h"
+#include "ash/public/cpp/ambient/photo_controller.h"
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/ui/ash/ambient/photo_client.h"
 #include "chromeos/assistant/internal/ambient/backdrop_client_config.h"
@@ -23,15 +23,22 @@ struct AccessTokenInfo;
 // The photo client impl talks to Backdrop service.
 class PhotoClientImpl : public PhotoClient {
  public:
+  using GetSettingsCallback = ash::PhotoController::GetSettingsCallback;
+  using UpdateSettingsCallback = ash::PhotoController::UpdateSettingsCallback;
+
   PhotoClientImpl();
-  ~PhotoClientImpl() override;
   PhotoClientImpl(const PhotoClientImpl&) = delete;
   PhotoClientImpl& operator=(const PhotoClientImpl&) = delete;
+  ~PhotoClientImpl() override;
 
   // PhotoClient:
   void FetchTopicInfo(OnTopicInfoFetchedCallback callback) override;
+  void GetSettings(GetSettingsCallback callback) override;
+  void UpdateSettings(int topic_source,
+                      UpdateSettingsCallback callback) override;
 
  private:
+  using BackdropClientConfig = chromeos::ambient::BackdropClientConfig;
   using GetAccessTokenCallback =
       base::OnceCallback<void(const std::string& gaia_id,
                               GoogleServiceAuthError error,
@@ -47,12 +54,29 @@ class PhotoClientImpl : public PhotoClient {
   void OnTopicInfoFetched(OnTopicInfoFetchedCallback callback,
                           std::unique_ptr<std::string> response);
 
+  void StartToGetSettings(GetSettingsCallback callback,
+                          const std::string& gaia_id,
+                          GoogleServiceAuthError error,
+                          signin::AccessTokenInfo access_token_info);
+
+  void OnGetSettings(GetSettingsCallback callback,
+                     std::unique_ptr<std::string> response);
+
+  void StartToUpdateSettings(int topic_sources,
+                             UpdateSettingsCallback callback,
+                             const std::string& gaia_id,
+                             GoogleServiceAuthError error,
+                             signin::AccessTokenInfo access_token_info);
+
+  void OnUpdateSettings(UpdateSettingsCallback callback,
+                        std::unique_ptr<std::string> response);
+
   // The url loader for the Backdrop service request.
   std::unique_ptr<BackdropURLLoader> backdrop_url_loader_;
 
   std::unique_ptr<signin::AccessTokenFetcher> access_token_fetcher_;
 
-  chromeos::ambient::BackdropClientConfig backdrop_client_config_;
+  BackdropClientConfig backdrop_client_config_;
 
   base::WeakPtrFactory<PhotoClientImpl> weak_factory_{this};
 };

chrome/browser/ui/ash/ambient/photo_client.cc

@@ -4,6 +4,9 @@
 
 #include "chrome/browser/ui/ash/ambient/photo_client.h"
 
+#include <vector>
+
+#include "base/callback.h"
 #include "build/buildflag.h"
 #include "chromeos/assistant/buildflags.h"
 
@@ -21,5 +24,16 @@ std::unique_ptr<PhotoClient> PhotoClient::Create() {
 }
 
 void PhotoClient::FetchTopicInfo(OnTopicInfoFetchedCallback callback) {
-  std::move(callback).Run(/*success=*/false, base::nullopt);
+  std::move(callback).Run(/*topic=*/base::nullopt);
+}
+
+void PhotoClient::GetSettings(
+    ash::PhotoController::GetSettingsCallback callback) {
+  std::move(callback).Run(/*topic_source=*/base::nullopt);
+}
+
+void PhotoClient::UpdateSettings(
+    int topic_source,
+    ash::PhotoController::UpdateSettingsCallback callback) {
+  std::move(callback).Run(/*success=*/false);
 }

chrome/browser/ui/ash/ambient/photo_client.h

@@ -8,14 +8,13 @@
 #include <memory>
 
 #include "ash/public/cpp/ambient/photo_controller.h"
-#include "base/callback.h"
+#include "base/callback_forward.h"
 #include "base/optional.h"
 
 // The interface of a client to retrieve photos.
 class PhotoClient {
  public:
   using OnTopicInfoFetchedCallback = base::OnceCallback<void(
-      bool success,
       const base::Optional<ash::PhotoController::Topic>& topic)>;
 
   // Creates PhotoClient based on the build flag ENABLE_CROS_LIBASSISTANT.
@@ -27,6 +26,12 @@ class PhotoClient {
   virtual ~PhotoClient() = default;
 
   virtual void FetchTopicInfo(OnTopicInfoFetchedCallback callback);
+
+  virtual void GetSettings(ash::PhotoController::GetSettingsCallback callback);
+
+  virtual void UpdateSettings(
+      int topic_source,
+      ash::PhotoController::UpdateSettingsCallback callback);
 };
 
 #endif  // CHROME_BROWSER_UI_ASH_AMBIENT_PHOTO_CLIENT_H_

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

@@ -12,9 +12,8 @@
 #include "components/account_id/account_id.h"
 #include "ui/gfx/image/image_skia.h"
 
-PhotoControllerImpl::PhotoControllerImpl() : weak_factory_(this) {
-  photo_client_ = PhotoClient::Create();
-}
+PhotoControllerImpl::PhotoControllerImpl()
+    : photo_client_(PhotoClient::Create()) {}
 
 PhotoControllerImpl::~PhotoControllerImpl() = default;
 
@@ -24,11 +23,19 @@ void PhotoControllerImpl::GetNextImage(PhotoDownloadCallback callback) {
                      weak_factory_.GetWeakPtr(), std::move(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::OnNextImageInfoFetched(
     PhotoDownloadCallback callback,
-    bool success,
     const base::Optional<ash::PhotoController::Topic>& topic) {
-  if (!success ||
+  if (!topic.has_value() ||
       (topic->url.empty() && !topic->portrait_image_url.has_value())) {
     std::move(callback).Run(/*success=*/false, gfx::ImageSkia());
     return;
@@ -41,5 +48,5 @@ void PhotoControllerImpl::OnNextImageInfoFetched(
           ->GetAccountId();
   ash::AssistantImageDownloader::GetInstance()->Download(
       account_id, GURL(image_url),
-      base::BindOnce(std::move(callback), success));
+      base::BindOnce(std::move(callback), /*success=*/true));
 }

chrome/browser/ui/ash/ambient/photo_controller_impl.h

@@ -24,16 +24,18 @@ class PhotoControllerImpl : public ash::PhotoController {
 
   // ash::PhotoController:
   void GetNextImage(PhotoDownloadCallback callback) override;
+  void GetSettings(GetSettingsCallback callback) override;
+  void UpdateSettings(int topic_source,
+                      UpdateSettingsCallback callback) override;
 
  private:
   void OnNextImageInfoFetched(
       PhotoDownloadCallback callback,
-      bool success,
       const base::Optional<ash::PhotoController::Topic>& topic);
 
   std::unique_ptr<PhotoClient> photo_client_;
 
-  base::WeakPtrFactory<PhotoControllerImpl> weak_factory_;
+  base::WeakPtrFactory<PhotoControllerImpl> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(PhotoControllerImpl);
 };