Bug 240143 – [WebAuthn] Get rid of ASCAgentProxy instance after success/error/cancel

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[[e-mail]]

^

[Radar WebKit Bug Importer]

<rdar://problem/92825715>

[[e-mail]]

Created attachment 458921 [details]
Patch

[[e-mail]]

Created attachment 458926 [details]
Patch

[[e-mail]]

Created attachment 458927 [details]
Patch

[[e-mail]]

rdar://91393607

[Brent Fulgham]

Comment on attachment 458927 [details]
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/WebAuthenticatorCoordinatorProxy.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/WebAuthenticatorCoordinatorProxy.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?

[[e-mail]]

Comment on attachment 458927 [details]
Patch

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

>> Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticatorCoordinatorProxy.mm:450
>> +                weakThis->m_proxy.clear();
> 
> Do we need to clear m_proxy in in the case of 'ASCCredentialRequestTypeNone' on line 438?

We haven't set the m_proxy for the request yet here, so it shouldn't be necessary.

>> Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticatorCoordinatorProxy.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?

If we don't get a daemonEndpoint it's likely something is wrong with the daemon. It does make sense to clear it here, will update.

[[e-mail]]

Created attachment 458960 [details]
Patch for landing

[EWS]

Committed r293907 (250362@main): <https://commits.webkit.org/250362@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 458960 [details].