Bug 223424 - Update Keychain queries according to internal needs
Summary: Update Keychain queries according to internal needs
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:
 
Reported: 2021-03-18 01:13 PDT by Jiewen Tan
Modified: 2021-03-18 16:46 PDT (History)
3 users (show)

See Also:


Attachments
Patch (9.37 KB, patch)
2021-03-18 01:18 PDT, Jiewen Tan
no flags Details | Formatted Diff | Diff
Patch (9.46 KB, patch)
2021-03-18 16:18 PDT, Jiewen Tan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jiewen Tan 2021-03-18 01:13:36 PDT
Update Keychain queries according to internal needs.
Comment 1 Jiewen Tan 2021-03-18 01:13:49 PDT
rdar://75434045
Comment 2 Jiewen Tan 2021-03-18 01:18:00 PDT
Created attachment 423571 [details]
Patch
Comment 3 Brent Fulgham 2021-03-18 12:47:51 PDT
Comment on attachment 423571 [details]
Patch

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

Can you try the simpler approach I suggested?

> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm:53
> +static void updateQuery(NSMutableDictionary *)

This should probably be called ‘updateQueryIfNeeded’

> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm:-132
> -    NSDictionary *query = @{

Could this just be NSMutableDictionary *query = @{ ...

> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm:188
> +    [query setDictionary:@{

Ditto

> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm:397
> +        auto query = adoptNS([[NSMutableDictionary alloc] init]);

Ditto

> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm:603
> +        [query setDictionary:@{

Ditto

> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm:673
> +        [query setDictionary:@{

Ditto
Comment 4 Jiewen Tan 2021-03-18 14:23:51 PDT
Comment on attachment 423571 [details]
Patch

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

Thanks Brent for reviewing the patch.

>> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm:53
>> +static void updateQuery(NSMutableDictionary *)
> 
> This should probably be called ‘updateQueryIfNeeded’

Good call! Let me change that.

>> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm:-132
>> -    NSDictionary *query = @{
> 
> Could this just be NSMutableDictionary *query = @{ ...

I don't think this could work.
Comment 5 Brent Fulgham 2021-03-18 15:46:45 PDT
Comment on attachment 423571 [details]
Patch

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

>>> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm:-132
>>> -    NSDictionary *query = @{
>> 
>> Could this just be NSMutableDictionary *query = @{ ...
> 
> I don't think this could work.

auto query = [NSMutableDictionary dictionaryWithObjectsAndKeys:
    @"value1", @"key1", @"value2", @"key2", nil];
Comment 6 Jiewen Tan 2021-03-18 15:54:42 PDT
(In reply to Brent Fulgham from comment #5)
> Comment on attachment 423571 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=423571&action=review
> 
> >>> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm:-132
> >>> -    NSDictionary *query = @{
> >> 
> >> Could this just be NSMutableDictionary *query = @{ ...
> > 
> > I don't think this could work.
> 
> auto query = [NSMutableDictionary dictionaryWithObjectsAndKeys:
>     @"value1", @"key1", @"value2", @"key2", nil];

Thanks Brent for r+ this patch.
Comment 7 Jiewen Tan 2021-03-18 16:18:00 PDT
Created attachment 423667 [details]
Patch
Comment 8 EWS 2021-03-18 16:46:27 PDT
Committed r274689: <https://commits.webkit.org/r274689>

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