Bug 206248 – [WebAuthn] Move the async code from WebAuthenticationPanelClient to AuthenticatorManager

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

[Jiewen Tan]

Move the async code from WebAuthenticationPanelClient to AuthenticatorManager. The async code is supposed to protect the state of the authenticator manager from reentrance. It shouldn't sit at the API layer given the protection is needed for all different kinds of API.

[Jiewen Tan]

Created attachment 390156 [details]
Patch

[Alex Christensen]

Comment on attachment 390156 [details]
Patch

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

> Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:281
> +    dispatchPanelClientCall([retries, completionHandler = WTFMove(completionHandler)] (const API::WebAuthenticationPanel& panel) mutable {

If m_pendingRequestData.panel.get() returns null, this CompletionHandler will not be called.  That's a problem that will cause hangs.

> Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:289
> +    responseVector.reserveCapacity(responses.size());

reserveInitialCapacity

[Jiewen Tan]

Comment on attachment 390156 [details]
Patch

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

Thanks for reviewing my patch, Alex.

>> Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:281
>> +    dispatchPanelClientCall([retries, completionHandler = WTFMove(completionHandler)] (const API::WebAuthenticationPanel& panel) mutable {
> 
> If m_pendingRequestData.panel.get() returns null, this CompletionHandler will not be called.  That's a problem that will cause hangs.

Well, the panel should always exist once the operations starts, see: handleRequest() -> runPanel() -> startDiscovery() -> operation starts if there is available authenticators. That's a simple null check. Also, there is a timer (m_requestTimeOutTimer) for every operation, therefore there should be no hangs.

>> Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:289
>> +    responseVector.reserveCapacity(responses.size());
> 
> reserveInitialCapacity

Fixed.

[Jiewen Tan]

Created attachment 390277 [details]
Patch

[Jiewen Tan]

Comment on attachment 390277 [details]
Patch

Thanks Alex for r+ this patch. I don't think EWS failures are related. cq+ regardless.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 390277 [details]:

editing/spelling/spellcheck-attribute.html bug 206178 (authors: [e-mail], [e-mail], and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 390277 [details]
Patch

Clearing flags on attachment: 390277

Committed r256238: <https://trac.webkit.org/changeset/256238>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/59335837>