[webkit-reviews] review granted: [Bug 57195] Include certificate when sending a WebCore::ResourceError to UI process on Windows : [Attachment 87164] Patch

[bugzilla-daemon at webkit.org]

Adam Roben (:aroben) <aroben at apple.com> has granted Jeff Miller
<jeffm at apple.com>'s request for review:
Bug 57195: Include certificate when sending a WebCore::ResourceError to UI
process on Windows
https://bugs.webkit.org/show_bug.cgi?id=57195

Attachment 87164: Patch
https://bugs.webkit.org/attachment.cgi?id=87164&action=review

------- Additional Comments from Adam Roben (:aroben) <aroben at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=87164&action=review

> Source/WebKit2/Shared/win/WebCoreArgumentCodersWin.cpp:130
> +    CFErrorRef cfError = resourceError.cfError();
> +    if (!cfError) {
> +	   encoder->encode(WebKit::PlatformCertificateInfo());
> +	   return;
> +    }

I don't think this is needed anymore.

> Source/WebKit2/Shared/win/WebCoreArgumentCodersWin.cpp:144
> +    CFDataRef certificateData = resourceError.certificate();
> +    if (!certificateData) {
> +	   encoder->encode(WebKit::PlatformCertificateInfo());
> +	   return;
> +    }
> +
> +    PCCERT_CONTEXT certificate =
reinterpret_cast<PCCERT_CONTEXT>(CFDataGetBytePtr(certificateData));
> +    if (!certificate) {
> +	   encoder->encode(WebKit::PlatformCertificateInfo());
> +	   return;
> +    }
> +    
> +    encoder->encode(WebKit::PlatformCertificateInfo(certificate));

Maybe ResourceError should have an accessor that returns a PCCERT_CONTEXT?
And/or maybe we should have a function that returns a PlatformCertificateInfo
given a ResourceError? What you have now seems fine, too, but these other
functions might make it cleaner.