Bug 221125 - [WebAuthn] Optimize _WKWebAuthenticationPanelDelegate for native API
Summary: [WebAuthn] Optimize _WKWebAuthenticationPanelDelegate for native API
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: Jiewen Tan
URL:
Keywords: InRadar
Depends on:
Blocks: 181943
  Show dependency tree / graph
 
Reported: 2021-01-29 00:41 PST by Jiewen Tan
Modified: 2021-02-01 18:50 PST (History)
3 users (show)

See Also:

Attachments
Patch (37.45 KB, patch)
2021-01-29 00:54 PST, Jiewen Tan 		no flags Details | Formatted Diff | Diff
Patch (37.54 KB, patch)
2021-01-29 17:05 PST, Jiewen Tan 		no flags Details | Formatted Diff | Diff
Patch (43.04 KB, patch)
2021-01-29 22:19 PST, Jiewen Tan 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (43.38 KB, patch)
2021-01-29 22:26 PST, Jiewen Tan 		bfulgham: review+
Details | Formatted Diff | Diff
Patch for landing (43.36 KB, patch)
2021-02-01 16:02 PST, Jiewen Tan 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Jiewen Tan 2021-01-29 00:41:21 PST 
Optimize _WKWebAuthenticationPanelDelegate for native API.
Comment 1 Jiewen Tan 2021-01-29 00:41:36 PST 
<rdar://problem/71509141>
Comment 2 Jiewen Tan 2021-01-29 00:54:46 PST 
Created attachment 418711 [details]
Patch
Comment 3 Jiewen Tan 2021-01-29 17:05:08 PST 
Created attachment 418789 [details]
Patch
Comment 4 Jiewen Tan 2021-01-29 22:19:02 PST 
Created attachment 418808 [details]
Patch
Comment 5 Jiewen Tan 2021-01-29 22:26:38 PST 
Created attachment 418809 [details]
Patch
Comment 6 Brent Fulgham 2021-02-01 15:38:24 PST 
Comment on attachment 418809 [details]
Patch

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

r=me

> Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:509
> +    RunLoop::main().dispatch([weakPanel, call = WTFMove(call)] () {

Should we WTFMove() the weakPanel? We don't use it again outside the dispatch.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm:155
> +        [responses[0] setLAContext:laContext.get()];

Why not responses.firstObject?
Comment 7 Jiewen Tan 2021-02-01 15:47:24 PST 
Comment on attachment 418809 [details]
Patch

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

Thanks Brent for r+ this patch.

>> Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:509
>> +    RunLoop::main().dispatch([weakPanel, call = WTFMove(call)] () {
> 
> Should we WTFMove() the weakPanel? We don't use it again outside the dispatch.

Good catch. We can totally move that.

>> Tools/TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm:155
>> +        [responses[0] setLAContext:laContext.get()];
> 
> Why not responses.firstObject?

Sure, it's totally doable. Since we check the count before, [0] shouldn't cause any issue.
Comment 8 Jiewen Tan 2021-02-01 16:02:15 PST 
Created attachment 418937 [details]
Patch for landing
Comment 9 EWS 2021-02-01 18:50:06 PST 
Committed r272184: <https://trac.webkit.org/changeset/272184>

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