Add webapp manifest parsing code for three new fields.

New fields: 'description', 'categories' and 'screenshots'.

Bug: 1146450
Change-Id: I8d104ed2ccc38e67cc84c873026b6f24a82a75b0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2523178
Commit-Queue: Finnur Thorarinsson <finnur@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Rayan Kanso <rayankans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826234}

third_party/blink/common/manifest/manifest_mojom_traits.cc

@@ -55,6 +55,13 @@ bool StructTraits<blink::mojom::ManifestDataView, ::blink::Manifest>::Read(
     return false;
   out->short_name = std::move(string.string);
 
+  if (!data.ReadDescription(&string))
+    return false;
+  out->description = std::move(string.string);
+
+  if (!data.ReadCategories(&out->categories))
+    return false;
+
   if (!data.ReadGcmSenderId(&string))
     return false;
   out->gcm_sender_id = std::move(string.string);
@@ -65,6 +72,9 @@ bool StructTraits<blink::mojom::ManifestDataView, ::blink::Manifest>::Read(
   if (!data.ReadIcons(&out->icons))
     return false;
 
+  if (!data.ReadScreenshots(&out->screenshots))
+    return false;
+
   if (!data.ReadShortcuts(&out->shortcuts))
     return false;
 

third_party/blink/public/common/manifest/manifest.h

@@ -154,6 +154,13 @@ struct BLINK_COMMON_EXPORT Manifest {
   // Null if the parsing failed or the field was not present.
   base::Optional<base::string16> short_name;
 
+  // Null if the parsing failed or the field was not present.
+  base::Optional<base::string16> description;
+
+  // Empty if the parsing failed, the field was not present, or all the
+  // values inside the JSON array were invalid.
+  std::vector<base::string16> categories;
+
   // Empty if the parsing failed or the field was not present.
   GURL start_url;
 
@@ -174,6 +181,10 @@ struct BLINK_COMMON_EXPORT Manifest {
   // icons inside the JSON array were invalid.
   std::vector<ImageResource> icons;
 
+  // Empty if the parsing failed, the field was not present, or all the
+  // screenshots inside the JSON array were invalid.
+  std::vector<ImageResource> screenshots;
+
   // Empty if the parsing failed, the field was not present, or all the
   // icons inside the JSON array were invalid.
   std::vector<ShortcutItem> shortcuts;

third_party/blink/public/common/manifest/manifest_mojom_traits.h

@@ -55,6 +55,16 @@ struct BLINK_COMMON_EXPORT
     return internal::TruncateOptionalString16(manifest.short_name);
   }
 
+  static base::Optional<base::StringPiece16> description(
+      const ::blink::Manifest& manifest) {
+    return internal::TruncateOptionalString16(manifest.description);
+  }
+
+  static const std::vector<base::string16>& categories(
+      const ::blink::Manifest& manifest) {
+    return manifest.categories;
+  }
+
   static base::Optional<base::StringPiece16> gcm_sender_id(
       const ::blink::Manifest& manifest) {
     return internal::TruncateOptionalString16(manifest.gcm_sender_id);
@@ -103,6 +113,11 @@ struct BLINK_COMMON_EXPORT
     return manifest.icons;
   }
 
+  static const std::vector<::blink::Manifest::ImageResource>& screenshots(
+      const ::blink::Manifest& manifest) {
+    return manifest.screenshots;
+  }
+
   static const std::vector<::blink::Manifest::ShortcutItem>& shortcuts(
       const ::blink::Manifest& manifest) {
     return manifest.shortcuts;

third_party/blink/public/mojom/manifest/manifest.mojom

@@ -21,6 +21,10 @@ struct Manifest {
 
   mojo_base.mojom.String16? short_name;
 
+  mojo_base.mojom.String16? description;
+
+  array<mojo_base.mojom.String16> categories;
+
   url.mojom.Url start_url;
 
   DisplayMode display;
@@ -31,6 +35,8 @@ struct Manifest {
 
   array<ManifestImageResource> icons;
 
+  array<ManifestImageResource> screenshots;
+
   array<ManifestShortcutItem> shortcuts;
 
   ManifestShareTarget? share_target;

third_party/blink/renderer/modules/manifest/manifest_parser.cc

@@ -82,12 +82,15 @@ void ManifestParser::Parse() {
 
   manifest_->name = ParseName(root_object.get());
   manifest_->short_name = ParseShortName(root_object.get());
+  manifest_->description = ParseDescription(root_object.get());
+  manifest_->categories = ParseCategories(root_object.get());
   manifest_->start_url = ParseStartURL(root_object.get());
   manifest_->scope = ParseScope(root_object.get(), manifest_->start_url);
   manifest_->display = ParseDisplay(root_object.get());
   manifest_->display_override = ParseDisplayOverride(root_object.get());
   manifest_->orientation = ParseOrientation(root_object.get());
   manifest_->icons = ParseIcons(root_object.get());
+  manifest_->screenshots = ParseScreenshots(root_object.get());
 
   auto share_target = ParseShareTarget(root_object.get());
   if (share_target.has_value())
@@ -267,6 +270,33 @@ String ManifestParser::ParseShortName(const JSONObject* object) {
   return short_name.has_value() ? *short_name : String();
 }
 
+String ManifestParser::ParseDescription(const JSONObject* object) {
+  base::Optional<String> description = ParseString(object, "description", Trim);
+  return description.has_value() ? *description : String();
+}
+
+Vector<String> ManifestParser::ParseCategories(const JSONObject* object) {
+  Vector<String> categories;
+
+  JSONValue* json_value = object->Get("categories");
+  if (!json_value)
+    return categories;
+
+  JSONArray* categories_list = object->GetArray("categories");
+  if (!categories_list) {
+    AddErrorInfo("property 'categories' ignored, type array expected.");
+    return categories;
+  }
+
+  for (wtf_size_t i = 0; i < categories_list->size(); ++i) {
+    String category_string;
+    categories_list->at(i)->AsString(&category_string);
+    categories.push_back(category_string.StripWhiteSpace().LowerASCII());
+  }
+
+  return categories;
+}
+
 KURL ManifestParser::ParseStartURL(const JSONObject* object) {
   return ParseURL(object, "start_url", manifest_url_,
                   ParseURLRestrictions::kSameOriginOnly);
@@ -457,14 +487,25 @@ ManifestParser::ParseIconPurpose(const JSONObject* icon) {
 
 Vector<mojom::blink::ManifestImageResourcePtr> ManifestParser::ParseIcons(
     const JSONObject* object) {
+  return ParseImageResource("icons", object);
+}
+
+Vector<mojom::blink::ManifestImageResourcePtr> ManifestParser::ParseScreenshots(
+    const JSONObject* object) {
+  return ParseImageResource("screenshots", object);
+}
+
+Vector<mojom::blink::ManifestImageResourcePtr>
+ManifestParser::ParseImageResource(const String& key,
+                                   const JSONObject* object) {
   Vector<mojom::blink::ManifestImageResourcePtr> icons;
-  JSONValue* json_value = object->Get("icons");
+  JSONValue* json_value = object->Get(key);
   if (!json_value)
     return icons;
 
-  JSONArray* icons_list = object->GetArray("icons");
+  JSONArray* icons_list = object->GetArray(key);
   if (!icons_list) {
-    AddErrorInfo("property 'icons' ignored, type array expected.");
+    AddErrorInfo("property '" + key + "' ignored, type array expected.");
     return icons;
   }
 

third_party/blink/renderer/modules/manifest/manifest_parser.h

@@ -114,6 +114,17 @@ class MODULES_EXPORT ManifestParser {
   // Returns the parsed string if any, a null string if the parsing failed.
   String ParseShortName(const JSONObject* object);
 
+  // Parses the 'description' field of the manifest, as defined in:
+  // https://w3c.github.io/manifest/#description-member-0
+  // Returns the parsed string if any, a null string if the parsing failed.
+  String ParseDescription(const JSONObject* object);
+
+  // Parses the 'categories' field of the manifest, as defined in:
+  // https://w3c.github.io/manifest/#dfn-processing-the-categories-member
+  // Returns a vector of the parsed strings if any, or empty if the parsing
+  // failed.
+  Vector<String> ParseCategories(const JSONObject* object);
+
   // Parses the 'scope' field of the manifest, as defined in:
   // https://w3c.github.io/manifest/#scope-member. Returns the parsed KURL if
   // any, or start URL (falling back to document URL) without filename, path,
@@ -177,6 +188,18 @@ class MODULES_EXPORT ManifestParser {
   Vector<mojom::blink::ManifestImageResourcePtr> ParseIcons(
       const JSONObject* object);
 
+  // Parses the 'screenshots' field of a Manifest, as defined in:
+  // https://w3c.github.io/manifest/#screenshots-member
+  // Returns a vector of ManifestImageResourcePtr with the successfully parsed
+  // screenshots, if any. An empty vector if the field was not present or empty.
+  Vector<mojom::blink::ManifestImageResourcePtr> ParseScreenshots(
+      const JSONObject* object);
+
+  // A helper function for parsing ImageResources under |key| in the manifest.
+  Vector<mojom::blink::ManifestImageResourcePtr> ParseImageResource(
+      const String& key,
+      const JSONObject* object);
+
   // Parses the 'name' field of a shortcut, as defined in:
   // https://w3c.github.io/manifest/#shortcuts-member
   // Returns the parsed string if any, a null string if the parsing failed.

third_party/blink/renderer/modules/manifest/manifest_parser_unittest.cc

@@ -171,6 +171,89 @@ TEST_F(ManifestParserTest, NameParseRules) {
   }
 }
 
+TEST_F(ManifestParserTest, DescriptionParseRules) {
+  // Smoke test.
+  {
+    auto& manifest =
+        ParseManifest(R"({ "description": "foo is the new black" })");
+    ASSERT_EQ(manifest->description, "foo is the new black");
+    ASSERT_FALSE(IsManifestEmpty(manifest));
+    EXPECT_EQ(0u, GetErrorCount());
+  }
+
+  // Trim whitespaces.
+  {
+    auto& manifest = ParseManifest(R"({ "description": "  foo  " })");
+    ASSERT_EQ(manifest->description, "foo");
+    EXPECT_EQ(0u, GetErrorCount());
+  }
+
+  // Don't parse if description isn't a string.
+  {
+    auto& manifest = ParseManifest(R"({ "description": {} })");
+    ASSERT_TRUE(manifest->description.IsNull());
+    ASSERT_EQ(1u, GetErrorCount());
+    EXPECT_EQ("property 'description' ignored, type string expected.",
+              errors()[0]);
+  }
+
+  // Don't parse if description isn't a string.
+  {
+    auto& manifest = ParseManifest(R"({ "description": 42 })");
+    ASSERT_TRUE(manifest->description.IsNull());
+    ASSERT_EQ(1u, GetErrorCount());
+    EXPECT_EQ("property 'description' ignored, type string expected.",
+              errors()[0]);
+  }
+}
+
+TEST_F(ManifestParserTest, CategoriesParseRules) {
+  // Smoke test.
+  {
+    auto& manifest = ParseManifest(R"({ "categories": ["cats", "memes"] })");
+    ASSERT_EQ(2u, manifest->categories.size());
+    ASSERT_EQ(manifest->categories[0], "cats");
+    ASSERT_EQ(manifest->categories[1], "memes");
+    ASSERT_FALSE(IsManifestEmpty(manifest));
+    EXPECT_EQ(0u, GetErrorCount());
+  }
+
+  // Trim whitespaces.
+  {
+    auto& manifest =
+        ParseManifest(R"({ "categories": ["  cats  ", "  memes  "] })");
+    ASSERT_EQ(2u, manifest->categories.size());
+    ASSERT_EQ(manifest->categories[0], "cats");
+    ASSERT_EQ(manifest->categories[1], "memes");
+    EXPECT_EQ(0u, GetErrorCount());
+  }
+
+  // Categories should be lower-cased.
+  {
+    auto& manifest = ParseManifest(R"({ "categories": ["CaTs", "Memes"] })");
+    ASSERT_EQ(2u, manifest->categories.size());
+    ASSERT_EQ(manifest->categories[0], "cats");
+    ASSERT_EQ(manifest->categories[1], "memes");
+    EXPECT_EQ(0u, GetErrorCount());
+  }
+
+  // Empty array.
+  {
+    auto& manifest = ParseManifest(R"({ "categories": [] })");
+    ASSERT_EQ(0u, manifest->categories.size());
+    EXPECT_EQ(0u, GetErrorCount());
+  }
+
+  // Detect error if categories isn't an array.
+  {
+    auto& manifest = ParseManifest(R"({ "categories": {} })");
+    ASSERT_EQ(0u, manifest->categories.size());
+    ASSERT_EQ(1u, GetErrorCount());
+    EXPECT_EQ("property 'categories' ignored, type array expected.",
+              errors()[0]);
+  }
+}
+
 TEST_F(ManifestParserTest, ShortNameParseRules) {
   // Smoke test.
   {
@@ -877,6 +960,55 @@ TEST_F(ManifestParserTest, IconsParseRules) {
   }
 }
 
+TEST_F(ManifestParserTest, ScreenshotsParseRules) {
+  // Smoke test: if no screenshot, no value.
+  {
+    auto& manifest = ParseManifest(R"({ "screenshots": [] })");
+    EXPECT_TRUE(manifest->screenshots.IsEmpty());
+    EXPECT_EQ(0u, GetErrorCount());
+  }
+
+  // Smoke test: if empty screenshot, no value.
+  {
+    auto& manifest = ParseManifest(R"({ "screenshots": [ {} ] })");
+    EXPECT_TRUE(manifest->screenshots.IsEmpty());
+    EXPECT_EQ(0u, GetErrorCount());
+  }
+
+  // Smoke test: screenshot with invalid src, no value.
+  {
+    auto& manifest =
+        ParseManifest(R"({ "screenshots": [ { "screenshots": [] } ] })");
+    EXPECT_TRUE(manifest->screenshots.IsEmpty());
+    EXPECT_EQ(0u, GetErrorCount());
+  }
+
+  // Smoke test: if screenshot with empty src, it will be present in the list.
+  {
+    auto& manifest = ParseManifest(R"({ "screenshots": [ { "src": "" } ] })");
+    EXPECT_FALSE(manifest->screenshots.IsEmpty());
+
+    auto& screenshots = manifest->screenshots;
+    EXPECT_EQ(screenshots.size(), 1u);
+    EXPECT_EQ(screenshots[0]->src.GetString(), "http://foo.com/manifest.json");
+    EXPECT_FALSE(IsManifestEmpty(manifest));
+    EXPECT_EQ(0u, GetErrorCount());
+  }
+
+  // Smoke test: if one icons has valid src, it will be present in the list.
+  {
+    auto& manifest =
+        ParseManifest(R"({ "screenshots": [{ "src": "foo.jpg" }] })");
+    EXPECT_FALSE(manifest->screenshots.IsEmpty());
+
+    auto& screenshots = manifest->screenshots;
+    EXPECT_EQ(screenshots.size(), 1u);
+    EXPECT_EQ(screenshots[0]->src.GetString(), "http://foo.com/foo.jpg");
+    EXPECT_FALSE(IsManifestEmpty(manifest));
+    EXPECT_EQ(0u, GetErrorCount());
+  }
+}
+
 TEST_F(ManifestParserTest, IconSrcParseRules) {
   // Smoke test.
   {
@@ -1679,6 +1811,7 @@ TEST_F(ManifestParserTest, ShortcutIconsParseRules) {
     EXPECT_EQ(0u, GetErrorCount());
   }
 }
+
 TEST_F(ManifestParserTest, FileHandlerParseRules) {
   // Does not contain file_handlers field.
   {