[webkit-reviews] review granted: [Bug 184428] Remove access to keychain from the WebContent process : [Attachment 338265] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has granted Jiewen Tan
<jiewen_tan at apple.com>'s request for review:
Bug 184428: Remove access to keychain from the WebContent process
https://bugs.webkit.org/show_bug.cgi?id=184428

Attachment 338265: Patch

https://bugs.webkit.org/attachment.cgi?id=338265&action=review




--- Comment #14 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 338265
  --> https://bugs.webkit.org/attachment.cgi?id=338265
Patch

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

Looks good!

> Source/WebKit/Shared/mac/WebCoreArgumentCodersMac.mm:340
> +	       if (CFGetTypeID(object) == SecIdentityGetTypeID()) {

This might be clearer as an early return:

if (CFGetTypeID(object) != SecIdentityGetTypeID()) {
    [clientCertificates addObject:object];
    continue;
}

> Source/WebKit/Shared/mac/WebCoreArgumentCodersMac.mm:354
> +	       [clientCertificates addObject:object];

.. Then this line would go away (move above line 341).