[webkit-reviews] review granted: [Bug 207664] WebKit support for Apple Pay Buttons with custom corner radii : [Attachment 390712] Patch

[bugzilla-daemon at webkit.org]

Andy Estes <aestes at apple.com> has granted Nikos Mouchtaris
<nmouchtaris at apple.com>'s request for review:
Bug 207664: WebKit support for Apple Pay Buttons with custom corner radii
https://bugs.webkit.org/show_bug.cgi?id=207664

Attachment 390712: Patch

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




--- Comment #4 from Andy Estes <aestes at apple.com> ---
Comment on attachment 390712
  --> https://bugs.webkit.org/attachment.cgi?id=390712
Patch

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

> Source/WebCore/PAL/pal/cocoa/PassKitSoftLink.h:50
>  SOFT_LINK_FUNCTION_FOR_HEADER(PAL, PassKit, PKDrawApplePayButton, void,
(CGContextRef context, CGRect drawRect, CGFloat scale, PKPaymentButtonType
type, PKPaymentButtonStyle style, NSString *languageCode), (context, drawRect,
scale, type, style, languageCode))

We can remove this now.

> Source/WebCore/PAL/pal/cocoa/PassKitSoftLink.mm:54
>  SOFT_LINK_FUNCTION_FOR_SOURCE(PAL, PassKit, PKDrawApplePayButton, void,
(CGContextRef context, CGRect drawRect, CGFloat scale, PKPaymentButtonType
type, PKPaymentButtonStyle style, NSString *languageCode), (context, drawRect,
scale, type, style, languageCode))

Ditto.

> Source/WebCore/PAL/pal/spi/cocoa/PassKitSPI.h:366
>  extern "C"
>  void PKDrawApplePayButton(_Nonnull CGContextRef, CGRect drawRect, CGFloat
scale, PKPaymentButtonType, PKPaymentButtonStyle, NSString * _Nullable
languageCode);

Ditto.

> Source/WebCore/rendering/RenderThemeCocoa.mm:103
> +static float getMaxBorderRadius(const RenderStyle& renderStyle)
> +{
> +    return renderStyle.hasBorderRadius() ?
std::max({renderStyle.borderTopLeftRadius().height.value(),
renderStyle.borderTopLeftRadius().width.value(),
renderStyle.borderTopRightRadius().height.value(),
renderStyle.borderTopRightRadius().width.value(),
renderStyle.borderBottomLeftRadius().height.value(),
renderStyle.borderBottomLeftRadius().width.value(),
renderStyle.borderBottomRightRadius().height.value(),
renderStyle.borderBottomRightRadius().width.value()}) :
PAL::get_PassKit_PKApplePayButtonDefaultCornerRadius();
> +}

We should return a CGFloat, and we typically don't prefix functions like this
with "get".

A suggestion for even easier reading:

static CGFloat largestCornerRadius(const RenderStyle& style)
{
    if (!renderStyle.hasBorderRadius())
	return PAL::get_PassKit_PKApplePayButtonDefaultCornerRadius();

    return std::max<CGFloat>({
	style.borderTopLeftRadius().height.value(),
	style.borderTopLeftRadius().width.value(),
	style.borderTopRightRadius().height.value(),
	style.borderTopRightRadius().width.value(),
	style.borderBottomLeftRadius().height.value(),
	style.borderBottomLeftRadius().width.value(),
	style.borderBottomRightRadius().height.value(),
	style.borderBottomRightRadius().width.value()
    });
}

> LayoutTests/http/tests/ssl/applepay/ApplePayButtonV4.html:46
> +	   for (let style of ["white-outline", "black"]) {

Did you mean to remove the "white" style?

Maybe we should render all the buttons over a neither-white-nor-black
background so that it's easy to see the corner radius in all cases.