[webkit-reviews] review granted: [Bug 219708] [WebAuthn] Adopt new UI for the Security Key makeCredential flow : [Attachment 415807] 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 219708: [WebAuthn] Adopt new UI for the Security Key makeCredential flow
https://bugs.webkit.org/show_bug.cgi?id=219708

Attachment 415807: Patch

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




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

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

It looks like the bots aren't happy with this yet, but I think the overall
approach is fine. Please answer my questions about whether it is okay to remove
the USB-related code for existing WebAuthn flows. r=me if you fix the EWS
failures.

> Source/WebKit/Scripts/process-entitlements.sh:264
> +    plistbuddy Add :com.apple.frontboard.launchapplications bool YES

Do we need this for Catalyst apps, too?

> Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:-345
> -	       continue;

Why is it okay to remove this code now? If we aren't using the new UI, don't we
still need to do this?

> Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:-392
> -	   m_pendingRequestData.panelResult = result;

Ditto.

>
Source/WebKit/UIProcess/WebAuthentication/Cocoa/AuthenticatorPresenterCoordinat
or.mm:76
> +#endif // PLATFORM(IOS)

Is there ever going to be a macOS version of this? If we do intend to implement
it, maybe we should add a 'notImplemented()' call in an #else clause?