ambient: filter returned topics via finch params

Only allow approved topics to be displayed. All topics are defaulted to
false and must be enabled by finch params.

BUG=b:167458920
TEST=turn on ambient mode with "GeoPhotosEnabled" param set to false.

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

ash/ambient/ambient_constants.h

@@ -24,9 +24,6 @@ constexpr base::TimeDelta kTopicFetchInterval =
 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;

ash/ambient/ambient_photo_controller.cc

@@ -49,7 +49,7 @@ namespace {
 // TODO(b/161357364): refactor utility functions and constants
 
 constexpr net::BackoffEntry::Policy kFetchTopicRetryBackoffPolicy = {
-    0,              // Number of initial errors to ignore.
+    10,             // 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.
@@ -234,7 +234,6 @@ void AmbientPhotoController::StartScreenUpdate() {
 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();
@@ -244,8 +243,7 @@ void AmbientPhotoController::StopScreenUpdate() {
 }
 
 void AmbientPhotoController::OnTopicsChanged() {
-  ++topics_batch_fetched_;
-  if (topics_batch_fetched_ < kNumberOfRequests)
+  if (ambient_backend_model_.topics().size() < kMaxNumberOfCachedImages)
     ScheduleFetchTopics(/*backoff=*/false);
 
   if (!image_refresh_started_) {
@@ -307,9 +305,8 @@ 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.
-  if (screen_update.next_topics.empty() &&
-      !screen_update.weather_info.has_value()) {
-    LOG(ERROR) << "The screen update info fetch has failed.";
+  if (screen_update.next_topics.empty()) {
+    LOG(WARNING) << "The screen update has no topics.";
 
     fetch_topic_retry_backoff_.InformOfRequest(/*succeeded=*/false);
     ScheduleFetchTopics(/*backoff=*/true);

ash/ambient/ambient_photo_controller.h

@@ -184,9 +184,6 @@ class ASH_EXPORT AmbientPhotoController : public AmbientBackendModelObserver {
   // The index of a topic to download.
   size_t topic_index_ = 0;
 
-  // 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.

ash/ambient/backdrop/ambient_backend_controller_impl.cc

@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "ash/ambient/ambient_controller.h"
+#include "ash/ambient/util/ambient_util.h"
 #include "ash/public/cpp/ambient/ambient_backend_controller.h"
 #include "ash/public/cpp/ambient/ambient_client.h"
 #include "ash/public/cpp/ambient/ambient_metrics.h"
@@ -116,6 +117,31 @@ void BuildBackdropTopicDetails(
   }
 }
 
+AmbientModeTopicType ToAmbientModeTopicType(
+    const backdrop::ScreenUpdate_Topic& topic) {
+  if (!topic.has_topic_type())
+    return AmbientModeTopicType::kOther;
+
+  switch (topic.topic_type()) {
+    case backdrop::CURATED:
+      return AmbientModeTopicType::kCurated;
+    case backdrop::PERSONAL_PHOTO:
+      return AmbientModeTopicType::kPersonal;
+    case backdrop::FEATURED_PHOTO:
+      return AmbientModeTopicType::kFeatured;
+    case backdrop::GEO_PHOTO:
+      return AmbientModeTopicType::kGeo;
+    case backdrop::CULTURAL_INSTITUTE:
+      return AmbientModeTopicType::kCulturalInstitute;
+    case backdrop::RSS_TOPIC:
+      return AmbientModeTopicType::kRss;
+    case backdrop::CAPTURED_ON_PIXEL:
+      return AmbientModeTopicType::kCapturedOnPixel;
+    default:
+      return AmbientModeTopicType::kOther;
+  }
+}
+
 // Helper function to save the information we got from the backdrop server to a
 // public struct so that they can be accessed by public codes.
 ScreenUpdate ToScreenUpdate(
@@ -125,9 +151,14 @@ ScreenUpdate ToScreenUpdate(
   int topics_size = backdrop_screen_update.next_topics_size();
   if (topics_size > 0) {
     for (auto& backdrop_topic : backdrop_screen_update.next_topics()) {
-      AmbientModeTopic ambient_topic;
       DCHECK(backdrop_topic.has_url());
 
+      auto topic_type = ToAmbientModeTopicType(backdrop_topic);
+      if (!ambient::util::IsAmbientModeTopicTypeAllowed(topic_type))
+        continue;
+
+      AmbientModeTopic ambient_topic;
+      ambient_topic.topic_type = topic_type;
       if (backdrop_topic.has_portrait_image_url())
         ambient_topic.url = backdrop_topic.portrait_image_url();
       else

ash/ambient/test/ambient_ash_test_base.cc

@@ -101,8 +101,10 @@ AmbientAshTestBase::AmbientAshTestBase()
 AmbientAshTestBase::~AmbientAshTestBase() = default;
 
 void AmbientAshTestBase::SetUp() {
-  scoped_feature_list_.InitAndEnableFeature(
-      chromeos::features::kAmbientModeFeature);
+  scoped_feature_list_.InitAndEnableFeatureWithParameters(
+      chromeos::features::kAmbientModeFeature,
+      {{"GeoPhotosEnabled", "true"},
+       {"CapturedOnPixelPhotosEnabled", "false"}});
   image_downloader_ = std::make_unique<TestImageDownloader>();
   ambient_client_ = std::make_unique<TestAmbientClient>(&wake_lock_provider_);
   chromeos::PowerManagerClient::InitializeFake();

ash/ambient/util/ambient_util.cc

@@ -4,8 +4,10 @@
 
 #include "ash/ambient/util/ambient_util.h"
 
+#include "ash/public/cpp/ambient/ambient_backend_controller.h"
 #include "ash/public/cpp/ambient/ambient_client.h"
 #include "base/no_destructor.h"
+#include "chromeos/constants/chromeos_features.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/shadow_value.h"
@@ -32,6 +34,28 @@ gfx::ShadowValues GetTextShadowValues() {
                                                    kTextShadowColor);
 }
 
+bool IsAmbientModeTopicTypeAllowed(AmbientModeTopicType topic_type) {
+  switch (topic_type) {
+    case ash::AmbientModeTopicType::kCurated:
+      return chromeos::features::kAmbientModeDefaultFeedEnabled.Get();
+    case ash::AmbientModeTopicType::kCapturedOnPixel:
+      return chromeos::features::kAmbientModeCapturedOnPixelPhotosEnabled.Get();
+    case ash::AmbientModeTopicType::kCulturalInstitute:
+      return chromeos::features::kAmbientModeCulturalInstitutePhotosEnabled
+          .Get();
+    case ash::AmbientModeTopicType::kFeatured:
+      return chromeos::features::kAmbientModeFeaturedPhotosEnabled.Get();
+    case ash::AmbientModeTopicType::kGeo:
+      return chromeos::features::kAmbientModeGeoPhotosEnabled.Get();
+    case ash::AmbientModeTopicType::kPersonal:
+      return chromeos::features::kAmbientModePersonalPhotosEnabled.Get();
+    case ash::AmbientModeTopicType::kRss:
+      return chromeos::features::kAmbientModeRssPhotosEnabled.Get();
+    case ash::AmbientModeTopicType::kOther:
+      return false;
+  }
+}
+
 }  // namespace util
 }  // namespace ambient
 }  // namespace ash

ash/ambient/util/ambient_util.h

@@ -7,6 +7,7 @@
 
 #include "ash/ash_export.h"
 #include "ash/login/ui/lock_screen.h"
+#include "ash/public/cpp/ambient/ambient_backend_controller.h"
 #include "ui/gfx/font_list.h"
 #include "ui/gfx/shadow_value.h"
 
@@ -24,6 +25,8 @@ ASH_EXPORT const gfx::FontList& GetDefaultFontlist();
 // Returns the default static text shadow for Ambient Mode.
 ASH_EXPORT gfx::ShadowValues GetTextShadowValues();
 
+ASH_EXPORT bool IsAmbientModeTopicTypeAllowed(AmbientModeTopicType topic);
+
 }  // namespace util
 }  // namespace ambient
 }  // namespace ash

ash/public/cpp/ambient/ambient_backend_controller.h

@@ -19,6 +19,17 @@ class TimeDelta;
 
 namespace ash {
 
+enum class AmbientModeTopicType {
+  kCurated,
+  kPersonal,
+  kFeatured,
+  kGeo,
+  kCulturalInstitute,
+  kRss,
+  kCapturedOnPixel,
+  kOther,
+};
+
 // AmbientModeTopic contains the information we need for rendering photo frame
 // for Ambient Mode. Corresponding to the |backdrop::ScreenUpdate::Topic| proto.
 struct ASH_PUBLIC_EXPORT AmbientModeTopic {
@@ -36,6 +47,8 @@ struct ASH_PUBLIC_EXPORT AmbientModeTopic {
 
   // Only support portrait image tiling in landscape orientation.
   base::Optional<std::string> related_image_url;
+
+  AmbientModeTopicType topic_type = AmbientModeTopicType::kOther;
 };
 
 // WeatherInfo contains the weather information we need for rendering a

ash/public/cpp/ambient/fake_ambient_backend_controller_impl.cc

@@ -6,6 +6,7 @@
 
 #include <utility>
 
+#include "ash/public/cpp/ambient/ambient_backend_controller.h"
 #include "ash/public/cpp/ambient/common/ambient_settings.h"
 #include "base/callback.h"
 #include "base/optional.h"
@@ -82,6 +83,7 @@ void FakeAmbientBackendControllerImpl::FetchScreenUpdateInfo(
   topic.url = kFakeUrl;
   topic.details = kFakeDetails;
   topic.related_image_url = kFakeUrl;
+  topic.topic_type = AmbientModeTopicType::kCulturalInstitute;
 
   ash::WeatherInfo weather_info;
   weather_info.temp_f = .0f;

chromeos/constants/chromeos_features.cc

@@ -42,6 +42,27 @@ constexpr base::FeatureParam<bool> kAmbientModeEarthAndSpaceAlbumEnabled{
 constexpr base::FeatureParam<bool> kAmbientModeStreetArtAlbumEnabled{
     &kAmbientModeFeature, "StreetArtAlbumEnabled", false};
 
+constexpr base::FeatureParam<bool> kAmbientModeDefaultFeedEnabled{
+    &kAmbientModeFeature, "DefaultFeedEnabled", false};
+
+constexpr base::FeatureParam<bool> kAmbientModePersonalPhotosEnabled{
+    &kAmbientModeFeature, "PersonalPhotosEnabled", false};
+
+constexpr base::FeatureParam<bool> kAmbientModeFeaturedPhotosEnabled{
+    &kAmbientModeFeature, "FeaturedPhotosEnabled", false};
+
+constexpr base::FeatureParam<bool> kAmbientModeGeoPhotosEnabled{
+    &kAmbientModeFeature, "GeoPhotosEnabled", false};
+
+constexpr base::FeatureParam<bool> kAmbientModeCulturalInstitutePhotosEnabled{
+    &kAmbientModeFeature, "CulturalInstitutePhotosEnabled", false};
+
+constexpr base::FeatureParam<bool> kAmbientModeRssPhotosEnabled{
+    &kAmbientModeFeature, "RssPhotosEnabled", false};
+
+constexpr base::FeatureParam<bool> kAmbientModeCapturedOnPixelPhotosEnabled{
+    &kAmbientModeFeature, "CapturedOnPixelPhotosEnabled", false};
+
 // Controls whether to enable Ambient mode album selection with photo previews.
 const base::Feature kAmbientModePhotoPreviewFeature{
     "ChromeOSAmbientModePhotoPreview", base::FEATURE_ENABLED_BY_DEFAULT};

chromeos/constants/chromeos_features.h

@@ -31,6 +31,21 @@ extern const base::FeatureParam<bool> kAmbientModeEarthAndSpaceAlbumEnabled;
 COMPONENT_EXPORT(CHROMEOS_CONSTANTS)
 extern const base::FeatureParam<bool> kAmbientModeStreetArtAlbumEnabled;
 COMPONENT_EXPORT(CHROMEOS_CONSTANTS)
+extern const base::FeatureParam<bool> kAmbientModeDefaultFeedEnabled;
+COMPONENT_EXPORT(CHROMEOS_CONSTANTS)
+extern const base::FeatureParam<bool> kAmbientModePersonalPhotosEnabled;
+COMPONENT_EXPORT(CHROMEOS_CONSTANTS)
+extern const base::FeatureParam<bool> kAmbientModeFeaturedPhotosEnabled;
+COMPONENT_EXPORT(CHROMEOS_CONSTANTS)
+extern const base::FeatureParam<bool> kAmbientModeGeoPhotosEnabled;
+COMPONENT_EXPORT(CHROMEOS_CONSTANTS)
+extern const base::FeatureParam<bool>
+    kAmbientModeCulturalInstitutePhotosEnabled;
+COMPONENT_EXPORT(CHROMEOS_CONSTANTS)
+extern const base::FeatureParam<bool> kAmbientModeRssPhotosEnabled;
+COMPONENT_EXPORT(CHROMEOS_CONSTANTS)
+extern const base::FeatureParam<bool> kAmbientModeCapturedOnPixelPhotosEnabled;
+COMPONENT_EXPORT(CHROMEOS_CONSTANTS)
 extern const base::Feature kAmbientModePhotoPreviewFeature;
 COMPONENT_EXPORT(CHROMEOS_CONSTANTS)
 extern const base::Feature kAmbientModeDevUseProdFeature;