[webkit-reviews] review canceled: [Bug 187102] [Curl] Embed certificate information into ResourceResponse. : [Attachment 343741] Fix style

[bugzilla-daemon at webkit.org]

Basuke Suzuki <Basuke.Suzuki at sony.com> has canceled Basuke Suzuki
<Basuke.Suzuki at sony.com>'s request for review:
Bug 187102: [Curl] Embed certificate information into ResourceResponse.
https://bugs.webkit.org/show_bug.cgi?id=187102

Attachment 343741: Fix style

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




--- Comment #8 from Basuke Suzuki <Basuke.Suzuki at sony.com> ---
Comment on attachment 343741
  --> https://bugs.webkit.org/attachment.cgi?id=343741
Fix style

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

>> Source/WebCore/platform/network/curl/CurlRequest.h:91
>> +	CertificateInfo getCertificateInfo() const { return
m_certificateInfo.isolatedCopy(); }
> 
> Why is this getter prefixed with `get`?
> Why do you use `isolatedCopy`?

isolatedCopy is used because it basically live in the background thread and
information will be gathered from main thread. But if these are moved to
CurlResponse, which is more data carrying object, individual isolatedCopy()
won't needed. Also rename will be done on that timing. Thanks.

>> Source/WebCore/platform/network/curl/CurlRequest.h:198
>> +	CertificateInfo m_certificateInfo;
> 
> It's weird for me CertificateInfo is added to CurlRequest.
> It think CertificateInfo should be added to CurlResponse instead of
CurlRequest. Right?

Humm. Good point. I guess this class has a long history and CurlResponse was
introduced lately. Maybe these are left over at that time or added after that.
Anyway, your point is completely make sense. I'll consider this tomorrow.