[Zine] Sends display capability to server when requesting cards.

Bug: 984787
Change-Id: Id4210690f5a09e91af106df70c58965bbaee8898
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1709702
Commit-Queue: Justin DeWitt <dewittj@chromium.org>
Auto-Submit: Justin DeWitt <dewittj@chromium.org>
Reviewed-by: Patrick Noland <pnoland@chromium.org>
Reviewed-by: Dmitry Titov <dimich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#680498}

components/ntp_snippets/features.cc

@@ -34,10 +34,9 @@ const base::Feature kRemoteSuggestionsBackendFeature{
 
 // Keep sorted, and keep nullptr at the end.
 const base::Feature* const kAllFeatures[] = {
-    &kArticleSuggestionsFeature,
-    &kKeepPrefetchedContentSuggestions,
-    &kNotificationsFeature,
-    &kRemoteSuggestionsBackendFeature};
+    &kArticleSuggestionsFeature, &kKeepPrefetchedContentSuggestions,
+    &kNotificationsFeature, &kRemoteSuggestionsBackendFeature,
+    &kOptionalImagesEnabledFeature};
 
 const base::Feature kArticleSuggestionsFeature{
     "NTPArticleSuggestions", base::FEATURE_ENABLED_BY_DEFAULT};
@@ -72,6 +71,9 @@ const char kNotificationsIgnoredLimitParam[] = "ignored_limit";
 const base::Feature kKeepPrefetchedContentSuggestions{
     "KeepPrefetchedContentSuggestions", base::FEATURE_ENABLED_BY_DEFAULT};
 
+const base::Feature kOptionalImagesEnabledFeature{
+    "NTPRemoteSuggestionsOptionalImages", base::FEATURE_DISABLED_BY_DEFAULT};
+
 std::vector<const base::Feature*> GetAllFeatures() {
   // Skip the last feature as it's a nullptr.
   return std::vector<const base::Feature*>(

components/ntp_snippets/features.h

@@ -79,6 +79,9 @@ constexpr int kNotificationsIgnoredDefaultLimit = 3;
 // have been fetched.
 extern const base::Feature kKeepPrefetchedContentSuggestions;
 
+// Whether this version of the client supports responses without an image.
+extern const base::Feature kOptionalImagesEnabledFeature;
+
 // Return all the features as a vector.
 std::vector<const base::Feature*> GetAllFeatures();
 

components/ntp_snippets/remote/json_request.cc

@@ -10,6 +10,7 @@
 
 #include "base/bind.h"
 #include "base/command_line.h"
+#include "base/feature_list.h"
 #include "base/json/json_writer.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/metrics/histogram_macros.h"
@@ -63,6 +64,11 @@ bool IsSendingUserClassEnabled() {
       /*default_value=*/true);
 }
 
+bool IsSendingOptionalImagesCapabilityEnabled() {
+  return base::FeatureList::IsEnabled(
+      ntp_snippets::kOptionalImagesEnabledFeature);
+}
+
 // Translate the BCP 47 |language_code| into a posix locale string.
 std::string PosixLocaleFromBCP47Language(const std::string& language_code) {
   char locale[ULOC_FULLNAME_CAPACITY];
@@ -272,6 +278,14 @@ JsonRequest::Builder& JsonRequest::Builder::SetUserClassifier(
   return *this;
 }
 
+JsonRequest::Builder& JsonRequest::Builder::SetOptionalImagesCapability(
+    bool supports_optional_images) {
+  if (supports_optional_images && IsSendingOptionalImagesCapabilityEnabled()) {
+    display_capability_ = "CAPABILITY_OPTIONAL_IMAGES";
+  }
+  return *this;
+}
+
 std::unique_ptr<network::ResourceRequest>
 JsonRequest::Builder::BuildResourceRequest() const {
   auto resource_request = std::make_unique<network::ResourceRequest>();
@@ -312,6 +326,10 @@ std::string JsonRequest::Builder::BuildBody() const {
     request->SetString("userActivenessClass", user_class_);
   }
 
+  if (!display_capability_.empty()) {
+    request->SetString("displayCapability", display_capability_);
+  }
+
   language::UrlLanguageHistogram::LanguageInfo ui_language;
   language::UrlLanguageHistogram::LanguageInfo other_top_language;
   PrepareLanguages(&ui_language, &other_top_language);

components/ntp_snippets/remote/json_request.h

@@ -90,6 +90,7 @@ class JsonRequest {
     Builder& SetUrlLoaderFactory(
         scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory);
     Builder& SetUserClassifier(const UserClassifier& user_classifier);
+    Builder& SetOptionalImagesCapability(bool supports_optional_images);
 
     // These preview methods allow to inspect the Request without exposing it
     // publicly.
@@ -127,6 +128,7 @@ class JsonRequest {
     // Optional properties.
     std::string obfuscated_gaia_id_;
     std::string user_class_;
+    std::string display_capability_;
     const language::UrlLanguageHistogram* language_histogram_;
 
     DISALLOW_COPY_AND_ASSIGN(Builder);

components/ntp_snippets/remote/json_request_unittest.cc

@@ -105,6 +105,17 @@ class JsonRequestTest : public testing::Test {
     return builder;
   }
 
+  std::unique_ptr<base::test::ScopedFeatureList> ForceOptionalImagesSupport(
+      bool supported) {
+    auto feature_list = std::make_unique<base::test::ScopedFeatureList>();
+    if (supported) {
+      feature_list->InitWithFeatures({kOptionalImagesEnabledFeature}, {});
+    } else {
+      feature_list->InitWithFeatures({}, {kOptionalImagesEnabledFeature});
+    }
+    return feature_list;
+  }
+
  private:
   base::test::ScopedFeatureList scoped_feature_list_;
   std::unique_ptr<TestingPrefServiceSimple> pref_service_;
@@ -160,6 +171,58 @@ TEST_F(JsonRequestTest, BuildRequestUnauthenticated) {
                          "}"));
 }
 
+TEST_F(JsonRequestTest, BuildRequestDisplayCapabilityDisabledByFeature) {
+  auto optional_images_feature_list = ForceOptionalImagesSupport(false);
+
+  JsonRequest::Builder builder;
+  builder.SetOptionalImagesCapability(true);
+
+  EXPECT_THAT(builder.PreviewRequestHeadersForTesting(),
+              StrEq("Content-Type: application/json; charset=UTF-8\r\n"
+                    "\r\n"));
+
+  // The JSON should not contain any mention of displayCapability.
+  EXPECT_THAT(builder.PreviewRequestBodyForTesting(),
+              EqualsJSON("{"
+                         "  \"excludedSuggestionIds\": [],"
+                         "  \"priority\": \"BACKGROUND_PREFETCH\""
+                         "}"));
+}
+
+TEST_F(JsonRequestTest, BuildRequestDisplayCapabilityUnspecified) {
+  auto optional_images_feature_list = ForceOptionalImagesSupport(true);
+
+  JsonRequest::Builder builder;
+  builder.SetOptionalImagesCapability(false);
+
+  EXPECT_THAT(builder.PreviewRequestHeadersForTesting(),
+              StrEq("Content-Type: application/json; charset=UTF-8\r\n"
+                    "\r\n"));
+  EXPECT_THAT(builder.PreviewRequestBodyForTesting(),
+              EqualsJSON("{"
+                         "  \"excludedSuggestionIds\": [],"
+                         "  \"priority\": \"BACKGROUND_PREFETCH\""
+                         "}"));
+}
+
+TEST_F(JsonRequestTest, BuildRequestOptionalImages) {
+  auto optional_images_feature_list = ForceOptionalImagesSupport(true);
+
+  JsonRequest::Builder builder;
+  builder.SetOptionalImagesCapability(true);
+
+  EXPECT_THAT(builder.PreviewRequestHeadersForTesting(),
+              StrEq("Content-Type: application/json; charset=UTF-8\r\n"
+                    "\r\n"));
+  EXPECT_THAT(
+      builder.PreviewRequestBodyForTesting(),
+      EqualsJSON("{"
+                 "  \"displayCapability\": \"CAPABILITY_OPTIONAL_IMAGES\","
+                 "  \"excludedSuggestionIds\": [],"
+                 "  \"priority\": \"BACKGROUND_PREFETCH\""
+                 "}"));
+}
+
 TEST_F(JsonRequestTest, ShouldNotTruncateExcludedIdsList) {
   JsonRequest::Builder builder;
   RequestParams params;

components/ntp_snippets/remote/remote_suggestions_fetcher_impl.cc

@@ -215,7 +215,8 @@ void RemoteSuggestionsFetcherImpl::FetchSnippets(
       .SetParseJsonCallback(parse_json_callback_)
       .SetClock(clock_)
       .SetUrlLoaderFactory(url_loader_factory_)
-      .SetUserClassifier(*user_classifier_);
+      .SetUserClassifier(*user_classifier_)
+      .SetOptionalImagesCapability(true);
 
   if (identity_manager_->HasPrimaryAccount()) {
     // Signed-in: get OAuth token --> fetch suggestions.