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 <[email protected]>
Reviewed-by: Dominick Ng <[email protected]>
Reviewed-by: Mugdha Lakhani <[email protected]>
Commit-Queue: Glen Robertson <[email protected]>
Cr-Commit-Position: refs/heads/master@{#788482}
  • Loading branch information
Glen Robertson authored and Commit Bot committed Jul 15, 2020
1 parent 6de14c0 commit 41d3c0f
Show file tree
Hide file tree
Showing 26 changed files with 78 additions and 75 deletions.
2 changes: 1 addition & 1 deletion chrome/android/java/res/values/dimens.xml
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@
83dp -->
<dimen name="webapk_adaptive_icon_size">83dp</dimen>

<dimen name="webapk_badge_icon_size">24dp</dimen>
<dimen name="webapk_monochrome_icon_size">24dp</dimen>

<!-- Toolbar dimensions -->
<dimen name="toolbar_tab_count_text_size_1_digit">12dp</dimen>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -678,12 +678,12 @@ public static int getMinimumSplashImageSizeInPx(Context context) {
}

/**
* Returns the ideal size for a badge icon of a WebAPK.
* Returns the ideal size for a monochrome icon of a WebAPK.
* @param context Context to pull resources from.
* @return the dimensions in pixels which the badge icon should have.
* @return the dimensions in pixels which the monochrome icon should have.
*/
public static int getIdealBadgeIconSizeInPx(Context context) {
return getSizeFromResourceInPx(context, R.dimen.webapk_badge_icon_size);
public static int getIdealMonochromeIconSizeInPx(Context context) {
return getSizeFromResourceInPx(context, R.dimen.webapk_monochrome_icon_size);
}

/**
Expand Down Expand Up @@ -747,7 +747,7 @@ private static int[] getIconSizes() {
// This ordering must be kept up to date with the C++ ShortcutHelper.
return new int[] {getIdealHomescreenIconSizeInPx(context),
getMinimumHomescreenIconSizeInPx(context), getIdealSplashImageSizeInPx(context),
getMinimumSplashImageSizeInPx(context), getIdealBadgeIconSizeInPx(context),
getMinimumSplashImageSizeInPx(context), getIdealMonochromeIconSizeInPx(context),
getIdealAdaptiveLauncherIconSizeInPx(context),
ViewUtils.dpToPx(context, SHORTCUT_ICON_IDEAL_SIZE_DP)};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -977,10 +977,11 @@ public void testPrimaryIconUrlChangeShouldUpgrade() {
* AND
* - "best" icon URL for the primary icon did not change.
* AND
* - "best" icon URL for the badge icon did not change.
* - "best" icon URL for the monochrome icon did not change.
*/
@Test
public void testIconUrlsChangeShouldNotUpgradeIfPrimaryIconUrlAndBadgeIconUrlDoNotChange() {
public void
testIconUrlsChangeShouldNotUpgradeIfPrimaryIconUrlAndMonochromeIconUrlDoNotChange() {
ManifestData fetchedData = defaultManifestData();
fetchedData.iconUrlToMurmur2HashMap.put("/icon2.png", null);
assertFalse(checkUpdateNeededForFetchedManifest(defaultManifestData(), fetchedData));
Expand All @@ -998,8 +999,8 @@ public void testIconUrlsChangeShouldNotUpgradeIfPrimaryIconUrlAndBadgeIconUrlDoN
public void testWebManifestSameButBestIconUrlChangedShouldNotUpgrade() {
String iconUrl1 = "/icon1.png";
String iconUrl2 = "/icon2.png";
String badgeUrl1 = "/badge1.png";
String badgeUrl2 = "/badge2.pgn";
String monochromeUrl1 = "/monochrome1.png";
String monochromeUrl2 = "/monochrome2.png";
String hash1 = "11";
String hash2 = "22";
String hash3 = "33";
Expand All @@ -1010,16 +1011,16 @@ public void testWebManifestSameButBestIconUrlChangedShouldNotUpgrade() {
androidManifestData.iconUrlToMurmur2HashMap.clear();
androidManifestData.iconUrlToMurmur2HashMap.put(iconUrl1, hash1);
androidManifestData.iconUrlToMurmur2HashMap.put(iconUrl2, hash2);
androidManifestData.iconUrlToMurmur2HashMap.put(badgeUrl1, hash3);
androidManifestData.iconUrlToMurmur2HashMap.put(badgeUrl2, hash4);
androidManifestData.iconUrlToMurmur2HashMap.put(monochromeUrl1, hash3);
androidManifestData.iconUrlToMurmur2HashMap.put(monochromeUrl2, hash4);

ManifestData fetchedManifestData = defaultManifestData();
fetchedManifestData.primaryIconUrl = iconUrl2;
fetchedManifestData.iconUrlToMurmur2HashMap.clear();
fetchedManifestData.iconUrlToMurmur2HashMap.put(iconUrl1, null);
fetchedManifestData.iconUrlToMurmur2HashMap.put(iconUrl2, hash2);
fetchedManifestData.iconUrlToMurmur2HashMap.put(badgeUrl1, null);
fetchedManifestData.iconUrlToMurmur2HashMap.put(badgeUrl2, hash4);
fetchedManifestData.iconUrlToMurmur2HashMap.put(monochromeUrl1, null);
fetchedManifestData.iconUrlToMurmur2HashMap.put(monochromeUrl2, hash4);

assertFalse(checkUpdateNeededForFetchedManifest(androidManifestData, fetchedManifestData));
}
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/android/shortcut_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ int g_ideal_homescreen_icon_size = -1;
int g_minimum_homescreen_icon_size = -1;
int g_ideal_splash_image_size = -1;
int g_minimum_splash_image_size = -1;
int g_ideal_badge_icon_size = -1;
int g_ideal_monochrome_icon_size = -1;
int g_ideal_adaptive_launcher_icon_size = -1;
int g_ideal_shortcut_icon_size = -1;

Expand All @@ -68,7 +68,7 @@ void GetIconSizes() {
g_minimum_homescreen_icon_size = sizes[1];
g_ideal_splash_image_size = sizes[2];
g_minimum_splash_image_size = sizes[3];
g_ideal_badge_icon_size = sizes[4];
g_ideal_monochrome_icon_size = sizes[4];
g_ideal_adaptive_launcher_icon_size = sizes[5];
g_ideal_shortcut_icon_size = sizes[6];

Expand Down
5 changes: 2 additions & 3 deletions chrome/browser/android/webapk/webapk.proto
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,8 @@ message Image {
// The icon can be displayed in any context.
ANY = 1;

// The icon can be used where space constraints and/or color requirements
// differ from those of the application icon.
// BADGE = 2; We do not plan to support Badge purpose, and will ignore it.
// The icon can be used where a monochrome icon with a solid fill is needed.
// MONOCHROME = 2; This is not currently used, so ignore it.

// The icon is designed with the intention to be masked.
MASKABLE = 3;
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/installable/installable_manager_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ TEST_F(InstallableManagerUnitTest,
EXPECT_EQ(NO_ERROR_DETECTED, GetErrorCode());

// The icon MUST have IconPurpose::ANY or IconPurpose::Maskable.
manifest.icons[0].purpose[0] = IconPurpose::BADGE;
manifest.icons[0].purpose[0] = IconPurpose::MONOCHROME;
EXPECT_FALSE(IsManifestValid(manifest, true, true));
EXPECT_EQ(MANIFEST_MISSING_SUITABLE_ICON, GetErrorCode());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,12 @@ TEST(WebAppInstallUtils, UpdateWebAppInfoFromManifest) {
blink::Manifest::ImageResource icon;
icon.src = AppIcon2();
icon.purpose = {blink::Manifest::ImageResource::Purpose::ANY,
blink::Manifest::ImageResource::Purpose::BADGE};
blink::Manifest::ImageResource::Purpose::MONOCHROME};
manifest.icons.push_back(icon);
icon.src = AppIcon3();
manifest.icons.push_back(icon);
// Add an icon without purpose ANY (expect to be ignored).
icon.purpose = {blink::Manifest::ImageResource::Purpose::BADGE};
icon.purpose = {blink::Manifest::ImageResource::Purpose::MONOCHROME};
manifest.icons.push_back(icon);

UpdateWebAppInfoFromManifest(manifest, &web_app_info);
Expand Down Expand Up @@ -241,12 +241,12 @@ TEST_F(WebAppInstallUtilsWithShortcutsMenu,
blink::Manifest::ImageResource icon;
icon.src = AppIcon2();
icon.purpose = {blink::Manifest::ImageResource::Purpose::ANY,
blink::Manifest::ImageResource::Purpose::BADGE};
blink::Manifest::ImageResource::Purpose::MONOCHROME};
manifest.icons.push_back(icon);
icon.src = AppIcon3();
manifest.icons.push_back(icon);
// Add an icon without purpose ANY (expect to be ignored).
icon.purpose = {blink::Manifest::ImageResource::Purpose::BADGE};
icon.purpose = {blink::Manifest::ImageResource::Purpose::MONOCHROME};
manifest.icons.push_back(icon);

// Test that shortcuts in the manifest replace those in |web_app_info|.
Expand Down
2 changes: 1 addition & 1 deletion chrome/test/data/banners/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"src": "launcher-icon-2x.png",
"sizes": "96x96",
"type": "image/png",
"purpose": "any badge"
"purpose": "any monochrome"
},
{
"src": "launcher-icon-3x.png",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"src": "launcher-icon-2x.png",
"sizes": "96x96",
"type": "image/png",
"purpose": "any badge"
"purpose": "any monochrome"
},
{
"src": "launcher-icon-3x.png",
Expand Down
2 changes: 1 addition & 1 deletion chrome/test/data/banners/manifest_too_small_icon.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"src": "launcher-icon-1x.png",
"sizes": "48x48",
"type": "image/png",
"purpose": "any badge"
"purpose": "any monochrome"
}
],
"start_url": "manifest_test_page.html",
Expand Down
2 changes: 1 addition & 1 deletion chrome/test/data/banners/manifest_with_file_handlers.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"src": "launcher-icon-2x.png",
"sizes": "96x96",
"type": "image/png",
"purpose": "any badge"
"purpose": "any monochrome"
},
{
"src": "launcher-icon-3x.png",
Expand Down
2 changes: 1 addition & 1 deletion chrome/test/data/banners/release_notes_manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"src": "launcher-icon-2x.png",
"sizes": "96x96",
"type": "image/png",
"purpose": "any badge"
"purpose": "any monochrome"
},
{
"src": "launcher-icon-3x.png",
Expand Down
2 changes: 1 addition & 1 deletion content/browser/background_fetch/background_fetch.proto
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ message BackgroundFetchOptions {
// https://w3c.github.io/manifest/#purpose-member
enum Purpose {
ANY = 1;
BADGE = 2;
MONOCHROME = 2;
MASKABLE = 3;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,9 +264,9 @@ void CreateMetadataTask::InitializeMetadataProto() {
image_resource_proto->add_purpose(
proto::BackgroundFetchOptions_ImageResource_Purpose_ANY);
break;
case blink::Manifest::ImageResource::Purpose::BADGE:
case blink::Manifest::ImageResource::Purpose::MONOCHROME:
image_resource_proto->add_purpose(
proto::BackgroundFetchOptions_ImageResource_Purpose_BADGE);
proto::BackgroundFetchOptions_ImageResource_Purpose_MONOCHROME);
break;
case blink::Manifest::ImageResource::Purpose::MASKABLE:
image_resource_proto->add_purpose(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -361,9 +361,9 @@ class FillFromMetadataTask : public InitializationSubTask {
case proto::BackgroundFetchOptions_ImageResource_Purpose_ANY:
ir.purpose.push_back(blink::Manifest::ImageResource::Purpose::ANY);
break;
case proto::BackgroundFetchOptions_ImageResource_Purpose_BADGE:
case proto::BackgroundFetchOptions_ImageResource_Purpose_MONOCHROME:
ir.purpose.push_back(
blink::Manifest::ImageResource::Purpose::BADGE);
blink::Manifest::ImageResource::Purpose::MONOCHROME);
break;
}
}
Expand Down
2 changes: 1 addition & 1 deletion content/test/data/payments/payment_app_invocation.html
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
'src': 'icon-2x.png',
'sizes': '96x96',
'type': 'image/png',
'purpose': 'any badge'
'purpose': 'any monochrome'
}
]
}),
Expand Down
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 third_party/blink/public/common/manifest/manifest.h
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
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 third_party/blink/public/mojom/manifest/manifest.mojom
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
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
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
Loading

0 comments on commit 41d3c0f

Please sign in to comment.