Bug 240318 - [WebAuthn] Clean up WebAuthenticationModern and WebAuthnProcess
Summary: [WebAuthn] Clean up WebAuthenticationModern and WebAuthnProcess
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: pascoe@apple.com
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-05-11 14:45 PDT by pascoe@apple.com
Modified: 2022-05-18 14:59 PDT (History)
2 users (show)

See Also:


Attachments
Patch (229.00 KB, patch)
2022-05-11 14:54 PDT, pascoe@apple.com
bfulgham: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description pascoe@apple.com 2022-05-11 14:45:21 PDT
These are no longer necessary for ToT.


Previously landed in https://bugs.webkit.org/show_bug.cgi?id=239073, but was rolled out.
Comment 1 Radar WebKit Bug Importer 2022-05-11 14:45:41 PDT
<rdar://problem/93130490>
Comment 2 pascoe@apple.com 2022-05-11 14:54:50 PDT
Created attachment 459183 [details]
Patch
Comment 3 Brent Fulgham 2022-05-12 09:54:01 PDT
Comment on attachment 459183 [details]
Patch

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

So much cleanup! r=me

> Tools/ChangeLog:-1977
> -        (TestWebKitAPI::WebCore::webAuthenticationModernExperimentalFeature): Deleted.

We don't need this ChangeLog comment removal

> Tools/ChangeLog:2011
> +        (TestWebKitAPI::WebCore::webAuthenticationModernExperimentalFeature): Deleted.

This seems to be in the wrong part of the file (is perhaps a dupe of the 5/11 comment)

> Tools/TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm:-1437
> -}

Are these two tests really not needed anymore? It doesn't seem like disallowing the local authenticator is specifically related to the WebAuthn process, is it?
Comment 4 Brent Fulgham 2022-05-12 09:55:18 PDT
It doesn't seem like the Mac-AS-debug-wk2 failures are related. I have re-ran them.
Comment 5 pascoe@apple.com 2022-05-12 10:08:06 PDT
(In reply to Brent Fulgham from comment #3)
> > Tools/TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm:-1437
> > -}
> 
> Are these two tests really not needed anymore? It doesn't seem like
> disallowing the local authenticator is specifically related to the WebAuthn
> process, is it?

These tests are from when the WebAuthenticationPanel used a UI delegate method (decidePolicyForLocalAuthenticator) to ask if using the platform authenticator is permitted. This is superseded by requestLAContextForUserVerification, which effectively does the same thing, but does not return ExceptionData { UnknownError, "Disallow local authenticator."_s } (the standard request cancelled error is used).
Comment 6 pascoe@apple.com 2022-05-18 13:30:15 PDT
Pull request: https://github.com/WebKit/WebKit/pull/744
Comment 7 EWS 2022-05-18 14:59:13 PDT
Committed r294438 (250716@main): <https://commits.webkit.org/250716@main>

Reviewed commits have been landed. Closing PR #744 and removing active labels.