[webkit-reviews] review granted: [Bug 240143] [WebAuthn] Get rid of ASCAgentProxy instance after success/error/cancel : [Attachment 458927] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri May 6 09:42:16 PDT 2022


Brent Fulgham <bfulgham at webkit.org> has granted j_pascoe at apple.com
<j_pascoe at apple.com>'s request for review:
Bug 240143: [WebAuthn] Get rid of ASCAgentProxy instance after
success/error/cancel
https://bugs.webkit.org/show_bug.cgi?id=240143

Attachment 458927: Patch

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




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

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

r=me

> Source/WebKit/ChangeLog:12
> +	   *
UIProcess/WebAuthentication/Cocoa/WebAuthenticatorCoordinatorProxy.mm:

Nit: This file should probably be called
WebAuthenticatorCoordinatorProxyCocoa.mm (but that's an issue for another day).

>
Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticatorCoordinatorProx
y.mm:450
> +		   weakThis->m_proxy.clear();

Do we need to clear m_proxy in in the case of 'ASCCredentialRequestTypeNone' on
line 438?

>
Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticatorCoordinatorProx
y.mm:475
> +		   weakThis->m_proxy.clear();

What if we do have a weakThis, but we do not have a daemonEndPoint. Do we need
to clear weakThis->m_proxy on line 465, above? Are they guaranteed to pass
through the 'cancel' logic?


More information about the webkit-reviews mailing list