[webkit-reviews] review granted: [Bug 183527] [WebAuthN] Implement authenticatorMakeCredential : [Attachment 335986] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has granted Jiewen Tan
<jiewen_tan at apple.com>'s request for review:
Bug 183527: [WebAuthN] Implement authenticatorMakeCredential
https://bugs.webkit.org/show_bug.cgi?id=183527

Attachment 335986: Patch

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




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

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

Looks good, please clean up the comments I marked up.

> Source/WebCore/Modules/webauthn/PublicKeyCredentialCreationOptions.h:74
> +    // Warning: Do not call this at least no other ways around.

This comment is hard to understand. Do you just mean no one should use this
because it isn’t currently a complete copy?

> Source/WebCore/Modules/webauthn/PublicKeyCredentialCreationOptions.h:147
> +// Not every member are copied.

Not every member IS copied.

> Source/WebCore/Modules/webauthn/cocoa/LocalAuthenticator.mm:51
> +// FIXM(rdar://problem/38320512): Define Apple AAGUID.

FIXME

> Source/WebCore/Modules/webauthn/cocoa/LocalAuthenticator.mm:123
> +    [context evaluatePolicy:LAPolicyDeviceOwnerAuthenticationWithBiometrics
localizedReason:reason reply:BlockPtr<void(BOOL, NSError
*)>::fromCallable([weakThis = m_weakFactory.createWeakPtr(*this), callback =
WTFMove(callback), exceptionCallback = WTFMove(exceptionCallback), options =
crossThreadCopy(options), hash] (BOOL success, NSError *error) mutable {

Nice!!!

> Source/WebCore/Modules/webauthn/cocoa/LocalAuthenticator.mm:243
> +		   // L

Is this comment correct? What does L mean here?