[Webkit-unassigned] [Bug 230906] Adopt platform UI for WebAuthn

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 12 16:20:17 PDT 2021


https://bugs.webkit.org/show_bug.cgi?id=230906

--- Comment #13 from David Kilzer (:ddkilzer) <ddkilzer at webkit.org> ---
Comment on attachment 440851
  --> https://bugs.webkit.org/attachment.cgi?id=440851
Patch

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

> Source/WebKit/SourcesCocoa.txt:595
> +UIProcess/WebAuthentication/Cocoa/WebAuthenticatorCoordinatorProxy.mm

If you added a new file to the project, that file still needs to show up in the Xcode project (otherwise searches will ignore the file).

That means you'll have to add the file to the Xcode project, but not add it to any build targets (or it will get built twice and link failures will occur).

So there should be a corresponding change to add the file to the Xcode project (but not build it since it's built by the Unified Sources mechanism).

This must be fixed before landing the patch.

>> Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticatorCoordinatorProxy.mm:287
>> +                ASCPlatformPublicKeyCredentialRegistration *registrationCredential = credential;
> 
> There is a new checked_objc_cast<> function in the new <wtf/cocoa/TypeCastsCocoa.h> header since you started writing this patch.  It would probably be a good idea to do that here to make sure `credentials` is the type of object you expect it to be:
> 
>                 auto *registrationCredential = checked_objc_cast<ASCPlatformPublicKeyCredentialRegistration>(credential);
> 
> Note that if you nil-check the result of the cast, you can use `dynamic_objc_cast<>` instead:
> 
>                 auto *registrationCredential = dynamic_objc_cast<ASCPlatformPublicKeyCredentialRegistration>(credential);
>                 if (!registrationCredential) {
>                     /* return early with an error, possibly calling the handler() function */
>                 }
> 
> Since this code deals with authentication, I would strongly suggest using one or the other of these cast methods here and the other places where `credential` is assigned a specific type below.

Garrett pointed out that the -isKindOfClass: check is done above, so no need to do it again using checked_objc_cast<> or dynamic_objc_cast<>, and those won't work with soft-linked classes anyway.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20211012/0b7a32c7/attachment-0001.htm>


More information about the webkit-unassigned mailing list