[webkit-reviews] review granted: [Bug 234112] [WebAuthn] Provide SPI to export/import local credentials : [Attachment 449987] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has granted j_pascoe at apple.com
<j_pascoe at apple.com>'s request for review:
Bug 234112: [WebAuthn] Provide SPI to export/import local credentials
https://bugs.webkit.org/show_bug.cgi?id=234112

Attachment 449987: Patch

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




--- Comment #7 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 449987
  --> https://bugs.webkit.org/attachment.cgi?id=449987
Patch

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

r=me

> Source/WebCore/Modules/webauthn/WebAuthenticationConstants.h:80
> +constexpr const char LocalAuthenticatorAccessGroup[] =
"com.apple.webkit.webauthn";

Thank you for fixing that typo!

> Source/WebKit/UIProcess/API/Cocoa/_WKWebAuthenticationPanel.mm:397
> +	       *error = [NSError errorWithDomain:WKErrorDomain
code:WKErrorMalformedCredential userInfo:nil];

This little check and assignment is repeated so many times it might be nice as
a static function:

static void createMalformedCredentialErrorIfNecessary(NSError **error)

> Source/WebKit/UIProcess/API/Cocoa/_WKWebAuthenticationPanel.mm:488
> +	       *error = [NSError errorWithDomain:WKErrorDomain
code:WKErrorDuplicateCredential userInfo:nil];

... or, perhaps a helper that takes the WKError type:

static void createNSErrorFromWKErrorIfNecessary(NSError **error, WKErrorCode
errorCode)
{
    if (error)
	 *error = [NSError errorWithDomain:WKErrorDomain code: errorCode
userInfo:nil];
}