[webkit-reviews] review granted: [Bug 234770] [Cocoa] Simplify some FontAttributes API tests : [Attachment 448135] Depends on #234757

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Wenson Hsieh
<wenson_hsieh at apple.com>'s request for review:
Bug 234770: [Cocoa] Simplify some FontAttributes API tests
https://bugs.webkit.org/show_bug.cgi?id=234770

Attachment 448135: Depends on #234757

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




--- Comment #2 from Darin Adler <darin at apple.com> ---
Comment on attachment 448135
  --> https://bugs.webkit.org/attachment.cgi?id=448135
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.