Bug 202563

Summary: [WebAuthn] Supply FrameInfo in -[WKUIDelegatePrivate _webView:runWebAuthenticationPanel:initiatedByFrame:completionHandler:]
Product: WebKit Reporter: Jiewen Tan <jiewen_tan>
Component: WebKit Misc.Assignee: Jiewen Tan <jiewen_tan>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, alex.gaynor, bfulgham, jiewen_tan, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 202559    
Bug Blocks: 181943    
Attachments:
Description Flags
Patch
none
Patch
none
Patch bfulgham: review+

Description Jiewen Tan 2019-10-03 22:42:24 PDT 
Supply FrameInfo in -[WKUIDelegatePrivate webView:runWebAuthenticationPanel:initiatedByFrame:completionHandler:].
Comment 1 Radar WebKit Bug Importer 2019-10-03 22:42:48 PDT 
<rdar://problem/55973968>
Comment 2 Jiewen Tan 2019-10-22 03:14:33 PDT 
Created attachment 381534 [details]
Patch
Comment 3 Jiewen Tan 2019-10-22 16:30:10 PDT 
Created attachment 381625 [details]
Patch
Comment 4 Jiewen Tan 2019-10-22 22:49:27 PDT 
This patch here conflicts with Bug 202561 on some minor ways. Will rebase it depending on which one get landed first.
Comment 5 Jiewen Tan 2019-10-22 23:08:20 PDT 
Created attachment 381659 [details]
Patch
Comment 6 Brent Fulgham 2019-10-23 08:44:21 PDT 
Comment on attachment 381659 [details]
Patch

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

I have some minor suggestions, but overall looks good! r=me

> Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.cpp:74
> +        }, [&](const  ExceptionData& exception) {

It looks like you have two spaces between 'const' and 'ExceptionData&'.

> Source/WebKit/WebProcess/WebAuthentication/WebAuthenticatorCoordinator.cpp:47
> +void WebAuthenticatorCoordinator::makeCredential(const Frame& frame, const SecurityOrigin& origin, const Vector<uint8_t>& hash, const PublicKeyCredentialCreationOptions& options, RequestCompletionHandler&& handler)

Since origin and hash are coming from the message passing, I wonder if they could be WTFMove'd? Surely the hash would be worth moving.
Comment 7 Jiewen Tan 2019-10-23 12:33:00 PDT 
Comment on attachment 381659 [details]
Patch

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

Thanks Brent for r+ this patch.

>> Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.cpp:74
>> +        }, [&](const  ExceptionData& exception) {
> 
> It looks like you have two spaces between 'const' and 'ExceptionData&'.

Fixed.

>> Source/WebKit/WebProcess/WebAuthentication/WebAuthenticatorCoordinator.cpp:47
>> +void WebAuthenticatorCoordinator::makeCredential(const Frame& frame, const SecurityOrigin& origin, const Vector<uint8_t>& hash, const PublicKeyCredentialCreationOptions& options, RequestCompletionHandler&& handler)
> 
> Since origin and hash are coming from the message passing, I wonder if they could be WTFMove'd? Surely the hash would be worth moving.

This is actually for sending the message, and therefore && will do nothing here.
Comment 8 Jiewen Tan 2019-10-23 12:36:50 PDT 
Committed r251489: <https://trac.webkit.org/changeset/251489>