Bug 234770 – [Cocoa] Simplify some FontAttributes API tests

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Wenson Hsieh]

• Instead of defining PlatformColor/PlatformFont, use CocoaColor/CocoaFont from WebCore.
• Remove the Nullity flag in ShadowExpectation and ColorExpectation (and simply std::optional instead).

[Wenson Hsieh]

Created attachment 448135 [details]
Depends on #234757

[Darin Adler]

Comment on attachment 448135 [details]
Depends on #234757

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

> Tools/TestWebKitAPI/Tests/WebKitCocoa/FontAttributes.mm:106
> +    CGFloat red;
> +    CGFloat green;
> +    CGFloat blue;
> +    CGFloat alpha;

I suggest these still be initialized to zero, even though this is a simple structure. No reason to let them be uninitialized.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/FontAttributes.mm:111
> +    CGFloat opacity;
> +    CGFloat blurRadius;

Ditto. Same for the other structures below.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/FontAttributes.mm:119
> +static void checkColor(WebCore::CocoaColor *color, std::optional<ColorExpectation>&& expectation)

No reason to use an rvalue reference for the argument. It’s not important to move something full of a lot of scalars. I’d just use a value, not a reference at all.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/FontAttributes.mm:139
> +static void checkShadow(NSShadow *shadow, std::optional<ShadowExpectation>&& expectation)

Ditto.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/FontAttributes.mm:154
> +static void checkFont(WebCore::CocoaFont *font, FontExpectation&& expectation)

Ditto.

[Wenson Hsieh]

Comment on attachment 448135 [details]
Depends on #234757

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

Thanks for the review!

>> Tools/TestWebKitAPI/Tests/WebKitCocoa/FontAttributes.mm:106
>> +    CGFloat alpha;
> 
> I suggest these still be initialized to zero, even though this is a simple structure. No reason to let them be uninitialized.

Makes sense — added initial values to ColorExpectation, ShadowExpectation, FontExpectation and ParagraphStyleExpectation (specifically the NSTextAlignment member).

>> Tools/TestWebKitAPI/Tests/WebKitCocoa/FontAttributes.mm:119
>> +static void checkColor(WebCore::CocoaColor *color, std::optional<ColorExpectation>&& expectation)
> 
> No reason to use an rvalue reference for the argument. It’s not important to move something full of a lot of scalars. I’d just use a value, not a reference at all.

Sounds good! Changed this (and the other places below) to pass by value instead.

[Wenson Hsieh]

Created attachment 448159 [details]
For EWS

[EWS]

Committed r287503 (245638@main): <https://commits.webkit.org/245638@main>

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

[Radar WebKit Bug Importer]

<rdar://problem/87041744>