Bug 172140 – Migrate Font constructor from bools to enums

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Myles C. Maxfield]

Migrate Font constructor from bools to enums

[Myles C. Maxfield]

Created attachment 310183 [details]
Patch

[Tim Horton]

Comment on attachment 310183 [details]
Patch

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

> Source/WebCore/platform/graphics/Font.h:137
> +    OrientationFallback isTextOrientationFallback() const { return m_orientationFallback ? OrientationFallback::Fallback : OrientationFallback::NoFallback; }

Ditto, I don't love this.

> Source/WebCore/platform/graphics/Font.h:185
> +    Interstitial isInterstitial() const { return m_interstitial ? Interstitial::Yes : Interstitial::No; }

If this is an "is", it should probably return a bool?

[Simon Fraser (smfr)]

Comment on attachment 310183 [details]
Patch

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

> Source/WebCore/platform/graphics/Font.h:90
> +    enum class Origin {
> +        Remote,
> +        Local
> +    };
> +    enum class Interstitial {
> +        Yes,
> +        No
> +    };
> +    enum class OrientationFallback {
> +        Fallback,
> +        NoFallback
> +    };

Could these all be in an OptionSet?

[Myles C. Maxfield]

Created attachment 310191 [details]
Patch

[Tim Horton]

Comment on attachment 310191 [details]
Patch

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

> Source/WebCore/platform/graphics/Font.h:308
> +    unsigned m_origin : 1; // Whether or not we are custom font loaded via @font-face
> +    unsigned m_isInterstitial : 1; // Whether or not this custom font is the last resort placeholder for a loading font

These could totally be your enum if you wanted. But this is much better.

[WebKit Commit Bot]

Comment on attachment 310191 [details]
Patch

Clearing flags on attachment: 310191

Committed r216896: <http://trac.webkit.org/changeset/216896>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.