Skip to content

Commit

Permalink
Renamed Manifest::Purpose::BADGE to MONOCHROME to match the spec.
Browse files Browse the repository at this point in the history 
Web App Manifest spec term was updated recently:
w3c/manifest#833

Bug: 1096388
Fixes: 1096388
Change-Id: I6fe6da6abb8d4e06391c46d4054a746bc49e6e1f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2291512
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Mugdha Lakhani <nator@chromium.org>
Commit-Queue: Glen Robertson <glenrob@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#788482}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 41d3c0fcaf1fe115f85ee3ca304ed9847417829b
  • Loading branch information
Glen Robertson authored and Commit Bot committed Jul 15, 2020
1 parent 9c68cb0 commit 798be74
Show file tree
Hide file tree
Showing 10 changed files with 43 additions and 40 deletions.
11 changes: 6 additions & 5 deletions blink/common/manifest/manifest_icon_selector_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,23 +140,24 @@ TEST_P(ManifestIconSelectorTest, PurposeFiltering) {
sizes_144.push_back(gfx::Size(width_to_height_ratio() * 144, 144));

std::vector<blink::Manifest::ImageResource> icons;
icons.push_back(
CreateIcon("http://foo.com/icon_48.png", "", sizes_48, Purpose::BADGE));
icons.push_back(CreateIcon("http://foo.com/icon_48.png", "", sizes_48,
Purpose::MONOCHROME));
icons.push_back(
CreateIcon("http://foo.com/icon_96.png", "", sizes_96, Purpose::ANY));
icons.push_back(
CreateIcon("http://foo.com/icon_144.png", "", sizes_144, Purpose::ANY));

GURL url = FindBestMatchingIcon(icons, 48, kMinimumIconSize, Purpose::BADGE);
GURL url =
FindBestMatchingIcon(icons, 48, kMinimumIconSize, Purpose::MONOCHROME);
EXPECT_EQ("http://foo.com/icon_48.png", url.spec());

url = FindBestMatchingIcon(icons, 48, kMinimumIconSize, Purpose::ANY);
EXPECT_EQ("http://foo.com/icon_96.png", url.spec());

url = FindBestMatchingIcon(icons, 96, kMinimumIconSize, Purpose::BADGE);
url = FindBestMatchingIcon(icons, 96, kMinimumIconSize, Purpose::MONOCHROME);
EXPECT_EQ("http://foo.com/icon_48.png", url.spec());

url = FindBestMatchingIcon(icons, 96, 96, Purpose::BADGE);
url = FindBestMatchingIcon(icons, 96, 96, Purpose::MONOCHROME);
EXPECT_TRUE(url.is_empty());

url = FindBestMatchingIcon(icons, 144, kMinimumIconSize, Purpose::ANY);
Expand Down
2 changes: 1 addition & 1 deletion blink/public/common/manifest/manifest.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ struct BLINK_COMMON_EXPORT Manifest {
struct BLINK_COMMON_EXPORT ImageResource {
enum class Purpose {
ANY = 0,
BADGE,
MONOCHROME,
MASKABLE,
IMAGE_RESOURCE_PURPOSE_LAST = MASKABLE,
};
Expand Down
8 changes: 4 additions & 4 deletions blink/public/common/manifest/manifest_mojom_traits.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -328,8 +328,8 @@ struct BLINK_COMMON_EXPORT
switch (purpose) {
case ::blink::Manifest::ImageResource::Purpose::ANY:
return blink::mojom::ManifestImageResource_Purpose::ANY;
case ::blink::Manifest::ImageResource::Purpose::BADGE:
return blink::mojom::ManifestImageResource_Purpose::BADGE;
case ::blink::Manifest::ImageResource::Purpose::MONOCHROME:
return blink::mojom::ManifestImageResource_Purpose::MONOCHROME;
case ::blink::Manifest::ImageResource::Purpose::MASKABLE:
return blink::mojom::ManifestImageResource_Purpose::MASKABLE;
}
Expand All @@ -342,8 +342,8 @@ struct BLINK_COMMON_EXPORT
case blink::mojom::ManifestImageResource_Purpose::ANY:
*out = ::blink::Manifest::ImageResource::Purpose::ANY;
return true;
case blink::mojom::ManifestImageResource_Purpose::BADGE:
*out = ::blink::Manifest::ImageResource::Purpose::BADGE;
case blink::mojom::ManifestImageResource_Purpose::MONOCHROME:
*out = ::blink::Manifest::ImageResource::Purpose::MONOCHROME;
return true;
case blink::mojom::ManifestImageResource_Purpose::MASKABLE:
*out = ::blink::Manifest::ImageResource::Purpose::MASKABLE;
Expand Down
2 changes: 1 addition & 1 deletion blink/public/mojom/manifest/manifest.mojom
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ struct ManifestShortcutItem {
struct ManifestImageResource {
enum Purpose {
ANY = 0,
BADGE,
MONOCHROME,
MASKABLE,
};

Expand Down
2 changes: 1 addition & 1 deletion blink/renderer/modules/manifest/fuzzer_seed_corpus/manifest_minimal-ui.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"src": "launcher-icon-4x.png",
"sizes": "192x192",
"type": "image/png",
"purpose": "badge"
"purpose": "monochrome"
}
],
"start_url": "/",
Expand Down
11 changes: 6 additions & 5 deletions blink/renderer/modules/manifest/image_resource_type_converters.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ WTF::Vector<Purpose> ParsePurpose(const WTF::String& purpose) {
Purpose purpose_enum;
if (lowercase_purpose == "any") {
purpose_enum = Purpose::ANY;
} else if (lowercase_purpose == "badge") {
purpose_enum = Purpose::BADGE;
} else if (lowercase_purpose == "monochrome") {
purpose_enum = Purpose::MONOCHROME;
} else if (lowercase_purpose == "maskable") {
purpose_enum = Purpose::MASKABLE;
} else {
Expand Down Expand Up @@ -122,16 +122,17 @@ Manifest::ImageResource ConvertManifestImageResource(

// Parse 'purpose'
const auto purposes = mojo::ParsePurpose(icon->purpose());
// ParsePurpose() would've weeded out any purposes that're not ANY or BADGE.
// ParsePurpose() would've weeded out any purposes that're not ANY or
// MONOCHROME.
for (auto purpose : purposes) {
switch (purpose) {
case mojo::Purpose::ANY:
manifest_icon.purpose.emplace_back(
Manifest::ImageResource::Purpose::ANY);
break;
case mojo::Purpose::BADGE:
case mojo::Purpose::MONOCHROME:
manifest_icon.purpose.emplace_back(
Manifest::ImageResource::Purpose::BADGE);
Manifest::ImageResource::Purpose::MONOCHROME);
break;
case mojo::Purpose::MASKABLE:
manifest_icon.purpose.emplace_back(
Expand Down
20 changes: 10 additions & 10 deletions blink/renderer/modules/manifest/image_resource_type_converters_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,22 +119,22 @@ TEST(ImageResourceConverter, ValidPurposeTest) {
ASSERT_EQ(1u, converted->purpose.size());
ASSERT_EQ(Purpose::ANY, converted->purpose.front());

resource->setPurpose(" Badge");
resource->setPurpose(" Monochrome");
converted = ManifestImageResource::From(resource);
ASSERT_EQ(1u, converted->purpose.size());
ASSERT_EQ(Purpose::BADGE, converted->purpose.front());
ASSERT_EQ(Purpose::MONOCHROME, converted->purpose.front());

resource->setPurpose(" Badge AnY");
resource->setPurpose(" Monochrome AnY");
converted = ManifestImageResource::From(resource);
ASSERT_EQ(2u, converted->purpose.size());
ASSERT_EQ(Purpose::BADGE, converted->purpose.front());
ASSERT_EQ(Purpose::MONOCHROME, converted->purpose.front());
ASSERT_EQ(Purpose::ANY, converted->purpose.back());

resource->setPurpose("any badge AnY");
resource->setPurpose("any monochrome AnY");
converted = ManifestImageResource::From(resource);
ASSERT_EQ(2u, converted->purpose.size());
ASSERT_EQ(Purpose::ANY, converted->purpose.front());
ASSERT_EQ(Purpose::BADGE, converted->purpose.back());
ASSERT_EQ(Purpose::MONOCHROME, converted->purpose.back());
}

TEST(ImageResourceConverter, InvalidPurposeTest) {
Expand Down Expand Up @@ -181,13 +181,13 @@ TEST(ImageResourceConverter, ExampleValueTest) {
blink::ManifestImageResource* resource =
blink::ManifestImageResource::Create();
resource->setSrc("http://example.com/lolcat.jpg");
resource->setPurpose("BADGE");
resource->setPurpose("MONOCHROME");
resource->setSizes("32x32 64x64 128x128");
resource->setType("image/jpeg");

auto expected_resource = ManifestImageResource::New();
expected_resource->src = blink::KURL("http://example.com/lolcat.jpg");
expected_resource->purpose = {Purpose::BADGE};
expected_resource->purpose = {Purpose::MONOCHROME};
expected_resource->sizes = {{32, 32}, {64, 64}, {128, 128}};
expected_resource->type = "image/jpeg";

Expand All @@ -197,7 +197,7 @@ TEST(ImageResourceConverter, ExampleValueTest) {
TEST(ImageResourceConverter, BlinkToMojoTypeTest) {
blink::ManifestImageResource* icon = blink::ManifestImageResource::Create();
icon->setSrc("http://example.com/lolcat.jpg");
icon->setPurpose("BADGE");
icon->setPurpose("MONOCHROME");
icon->setSizes("32x32 64x64 128x128");
icon->setType("image/jpeg");

Expand All @@ -207,7 +207,7 @@ TEST(ImageResourceConverter, BlinkToMojoTypeTest) {
EXPECT_EQ(mojo_icon.type, blink::WebString("image/jpeg").Utf16());
EXPECT_EQ(mojo_icon.sizes[1], gfx::Size(64, 64));
EXPECT_EQ(mojo_icon.purpose[0],
blink::Manifest::ImageResource::Purpose::BADGE);
blink::Manifest::ImageResource::Purpose::MONOCHROME);
}

} // namespace
Expand Down
5 changes: 3 additions & 2 deletions blink/renderer/modules/manifest/manifest_parser.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -375,8 +375,9 @@ ManifestParser::ParseIconPurpose(const JSONObject* icon) {

if (!CodeUnitCompareIgnoringASCIICase(keyword, "any")) {
purposes.push_back(mojom::blink::ManifestImageResource::Purpose::ANY);
} else if (!CodeUnitCompareIgnoringASCIICase(keyword, "badge")) {
purposes.push_back(mojom::blink::ManifestImageResource::Purpose::BADGE);
} else if (!CodeUnitCompareIgnoringASCIICase(keyword, "monochrome")) {
purposes.push_back(
mojom::blink::ManifestImageResource::Purpose::MONOCHROME);
} else if (!CodeUnitCompareIgnoringASCIICase(keyword, "maskable")) {
purposes.push_back(
mojom::blink::ManifestImageResource::Purpose::MASKABLE);
Expand Down
18 changes: 9 additions & 9 deletions blink/renderer/modules/manifest/manifest_parser_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1004,15 +1004,15 @@ TEST_F(ManifestParserTest, IconPurposeParseRules) {
{
auto& manifest = ParseManifest(
"{ \"icons\": [ {\"src\": \"\","
"\"purpose\": \"Any Badge Maskable\" } ] }");
"\"purpose\": \"Any Monochrome Maskable\" } ] }");
EXPECT_FALSE(manifest->icons.IsEmpty());

auto& icons = manifest->icons;
ASSERT_EQ(icons[0]->purpose.size(), 3u);
EXPECT_EQ(icons[0]->purpose[0],
mojom::blink::ManifestImageResource::Purpose::ANY);
EXPECT_EQ(icons[0]->purpose[1],
mojom::blink::ManifestImageResource::Purpose::BADGE);
mojom::blink::ManifestImageResource::Purpose::MONOCHROME);
EXPECT_EQ(icons[0]->purpose[2],
mojom::blink::ManifestImageResource::Purpose::MASKABLE);
EXPECT_EQ(0u, GetErrorCount());
Expand All @@ -1022,45 +1022,45 @@ TEST_F(ManifestParserTest, IconPurposeParseRules) {
{
auto& manifest = ParseManifest(
"{ \"icons\": [ {\"src\": \"\","
"\"purpose\": \" Any Badge \" } ] }");
"\"purpose\": \" Any Monochrome \" } ] }");
EXPECT_FALSE(manifest->icons.IsEmpty());

auto& icons = manifest->icons;
ASSERT_EQ(icons[0]->purpose.size(), 2u);
EXPECT_EQ(icons[0]->purpose[0],
mojom::blink::ManifestImageResource::Purpose::ANY);
EXPECT_EQ(icons[0]->purpose[1],
mojom::blink::ManifestImageResource::Purpose::BADGE);
mojom::blink::ManifestImageResource::Purpose::MONOCHROME);
EXPECT_EQ(0u, GetErrorCount());
}

// Twice the same value is parsed twice.
{
auto& manifest = ParseManifest(
"{ \"icons\": [ {\"src\": \"\","
"\"purpose\": \"badge badge\" } ] }");
"\"purpose\": \"monochrome monochrome\" } ] }");
EXPECT_FALSE(manifest->icons.IsEmpty());

auto& icons = manifest->icons;
ASSERT_EQ(icons[0]->purpose.size(), 2u);
EXPECT_EQ(icons[0]->purpose[0],
mojom::blink::ManifestImageResource::Purpose::BADGE);
mojom::blink::ManifestImageResource::Purpose::MONOCHROME);
EXPECT_EQ(icons[0]->purpose[1],
mojom::blink::ManifestImageResource::Purpose::BADGE);
mojom::blink::ManifestImageResource::Purpose::MONOCHROME);
EXPECT_EQ(0u, GetErrorCount());
}

// Invalid icon purpose is ignored.
{
auto& manifest = ParseManifest(
"{ \"icons\": [ {\"src\": \"\","
"\"purpose\": \"badge fizzbuzz\" } ] }");
"\"purpose\": \"monochrome fizzbuzz\" } ] }");
EXPECT_FALSE(manifest->icons.IsEmpty());

auto& icons = manifest->icons;
ASSERT_EQ(icons[0]->purpose.size(), 1u);
EXPECT_EQ(icons[0]->purpose[0],
mojom::blink::ManifestImageResource::Purpose::BADGE);
mojom::blink::ManifestImageResource::Purpose::MONOCHROME);
ASSERT_EQ(1u, GetErrorCount());
EXPECT_EQ(kSomeInvalidPurposeError, errors()[0]);
}
Expand Down
4 changes: 2 additions & 2 deletions blink/web_tests/http/tests/payments/payment-instruments.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
'src': './resources/icon-2x.png',
'sizes': '96x96',
'type': 'image/png',
'purpose': 'any badge'
'purpose': 'any monochrome'
}
],
method: 'basic-card',
Expand Down Expand Up @@ -300,7 +300,7 @@
'src': './resources/icon-2x.png',
'sizes': '96x96',
'type': 'image/png',
'purpose': 'any badge'
'purpose': 'any monochrome'
}
],
method: 'basic-card',
Expand Down

0 comments on commit 798be74

Please sign in to comment.