[webkit-reviews] review granted: [Bug 60595] Mac WebKit2 WebProcess needs a shim to make prompts appear to be from the UIProcess : [Attachment 93796] Patch #3 - Add CoreIPC stuff for when we marshall to the UIProcess and back

[bugzilla-daemon at webkit.org]

Anders Carlsson <andersca at apple.com> has granted Brady Eidson
<beidson at apple.com>'s request for review:
Bug 60595: Mac WebKit2 WebProcess needs a shim to make prompts appear to be
from the UIProcess
https://bugs.webkit.org/show_bug.cgi?id=60595

Attachment 93796: Patch #3 - Add CoreIPC stuff for when we marshall to the
UIProcess and back
https://bugs.webkit.org/attachment.cgi?id=93796&action=review

------- Additional Comments from Anders Carlsson <andersca at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=93796&action=review

> Source/WebKit2/Shared/mac/SecItemRequestData.cpp:71
> +    if (!expectAttributes)
> +	   return true;
> +    
> +    if (!CoreIPC::decode(decoder, secItemRequestData.m_attributesToMatch))
> +	   return false;

Could you change this to

if (expectAttributes && !CoreIPC::decode(decoder,
secItemRequestData.m_attributesToMatch)
    return false;

We prefer having all the if statements in decoders return false and then a
single return true at the end.

> Source/WebKit2/Shared/mac/SecItemResponseData.h:46
> +    CFTypeRef leakResultObject() { return m_resultObject.leakRef(); }

Any reason why this couldn't return a RetainPtr<CFTypeRef> instead? (and be
called releaseResultObject).