Make enum values consistent in ApplicationManifest::Icon::Purpose and _WKApplicationManifestIconPurpose

• Component: WebCore Misc.
• Status: RESOLVED FIXED

`Icon::Purpose` values are 1 (`1 << 0`), 2 (`1 << 1`), and 4 (`1 << 2`), but the default values for an enum are 0, 1, 2, which is used in `_WKApplicationManifestIcon`. This results in the purpose values being mis-translated in `_WKApplicationManifest`. In `WKApplicationManifest` an icon with the `purpose` set to `Any` in the manifest file, will have it's `purposes` set to `_WKApplicationManifestIconPurposeMonochrome`.

[Radar WebKit Bug Importer]

<rdar://problem/85976270>

Created attachment 445744 [details]
Patch

[Brent Fulgham]

Comment on attachment 445744 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=445744&action=review

This looks fine, but I suggest following the model of the underling WebCore type and use the bitwise-shift to show the mask values.

> Source/WebKit/UIProcess/API/Cocoa/_WKApplicationManifest.h:46
> +    _WKApplicationManifestIconPurposeAny = 1,

We should follow the behavior of the WebCore::ApplicationManifest::Icon::Purpose enum:

_WKApplicationManifestIconPurposeAny =  (1 << 0),

> Source/WebKit/UIProcess/API/Cocoa/_WKApplicationManifest.h:47
> +    _WKApplicationManifestIconPurposeMonochrome = 2,

_WKApplicationManifestIconPurposeMonochrome = (1 << 1),

> Source/WebKit/UIProcess/API/Cocoa/_WKApplicationManifest.h:48
> +    _WKApplicationManifestIconPurposeMaskable = 4,

_WKApplicationManifestIconPurposeMaskable = (1 << 2),

Created attachment 445778 [details]
Patch

(In reply to Brent Fulgham from comment #3)
> Comment on attachment 445744 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=445744&action=review
> 
> This looks fine, but I suggest following the model of the underling WebCore
> type and use the bitwise-shift to show the mask values.
> 
> > Source/WebKit/UIProcess/API/Cocoa/_WKApplicationManifest.h:46
> > +    _WKApplicationManifestIconPurposeAny = 1,
> 
> We should follow the behavior of the
> WebCore::ApplicationManifest::Icon::Purpose enum:
> 
> _WKApplicationManifestIconPurposeAny =  (1 << 0),
> 
> > Source/WebKit/UIProcess/API/Cocoa/_WKApplicationManifest.h:47
> > +    _WKApplicationManifestIconPurposeMonochrome = 2,
> 
> _WKApplicationManifestIconPurposeMonochrome = (1 << 1),
> 
> > Source/WebKit/UIProcess/API/Cocoa/_WKApplicationManifest.h:48
> > +    _WKApplicationManifestIconPurposeMaskable = 4,
> 
> _WKApplicationManifestIconPurposeMaskable = (1 << 2),

Thanks! Just uploaded a patch that uses this model

[Brent Fulgham]

Comment on attachment 445778 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=445778&action=review

r=me

> Source/WebKit/UIProcess/API/Cocoa/_WKApplicationManifest.h:48
> +    _WKApplicationManifestIconPurposeMaskable = (1 << 2),

Thank you for adjusting this!

[Brent Fulgham]

I'll cq+ this change once EWS finishes.

[EWS]

Committed r286470 (244810@main): <https://commits.webkit.org/244810@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 445778 [details].