Define X509Certificate::intermediate_ca_certs_ as a std::vector of...

Define X509Certificate::intermediate_ca_certs_ as a std::vector of
OSCertHandle so that we can also use it on Windows.

Remove the unused SSLClientSocketMac::intermediate_certs_ member.

R=hawk
BUG=28744
TEST=Can visit good HTTPS sites with no certificate errors.  Clicking
the "Certificate information" button in the page security information
window should show a complete certificate chain (as opposed to just
the server certificate).

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=34175

[wtc, 2009-12-02 03:39:22]

http://codereview.chromium.org/452042/diff/1/4
File net/base/x509_certificate.cc (left):

http://codereview.chromium.org/452042/diff/1/4#oldcode203
net/base/x509_certificate.cc:203: CFRelease(intermediate_ca_certs_);
Does CFRelease release the array ref recursively?
That is, does it release the sec certificate refs stored in
the array, and then release the array ref itself?

http://codereview.chromium.org/452042/diff/1/5
File net/base/x509_certificate.h (right):

http://codereview.chromium.org/452042/diff/1/5#newcode220
net/base/x509_certificate.h:220: // Ownership follows the "get" rule: it is the
caller's responsibility to
Does this comment on "ownership" still make sense now that
we're using a std::vector instead of CFArray?

http://codereview.chromium.org/452042/diff/1/6
File net/base/x509_certificate_mac.cc (left):

http://codereview.chromium.org/452042/diff/1/6#oldcode663
net/base/x509_certificate_mac.cc:663: CFArrayAppendValue(intermediate_ca_certs_,
cert);
Does CFArrayAppendValue do a CFRetain on |cert| ?

http://codereview.chromium.org/452042/diff/1/2
File net/socket/ssl_client_socket_mac.cc (right):

http://codereview.chromium.org/452042/diff/1/2#newcode289
net/socket/ssl_client_socket_mac.cc:289: // TODO(wtc): Since
X509Certificate::CreateFromHandle may return a cached
Let me know if you agree that this could be a problem.

http://codereview.chromium.org/452042/diff/1/2#newcode296
net/socket/ssl_client_socket_mac.cc:296: CFRetain(cert_ref);
I had to add a CFRetain call here.  This is related to my
question on whether CFArrayAppendValue does a CFRetain
on its second argument.

[hawk, 2009-12-08 01:49:58]

LGTM

http://codereview.chromium.org/452042/diff/1/4
File net/base/x509_certificate.cc (left):

http://codereview.chromium.org/452042/diff/1/4#oldcode203
net/base/x509_certificate.cc:203: CFRelease(intermediate_ca_certs_);
On 2009/12/02 03:39:22, wtc wrote:
> Does CFRelease release the array ref recursively?
> That is, does it release the sec certificate refs stored in
> the array, and then release the array ref itself?

Yes, it calls CFRelease() on all objects in the in array (and conversely called
CFRetain() on every object as they were added to the array).

http://codereview.chromium.org/452042/diff/1/5
File net/base/x509_certificate.h (right):

http://codereview.chromium.org/452042/diff/1/5#newcode220
net/base/x509_certificate.h:220: // Ownership follows the "get" rule: it is the
caller's responsibility to
On 2009/12/02 03:39:22, wtc wrote:
> Does this comment on "ownership" still make sense now that
> we're using a std::vector instead of CFArray?

Yes, since we're not mucking with the returned object's retain count. The
wording could be changed a little to reflect the fact that you can't retain the
result, since it's not a CFType any more. How about "it is the caller's
responsibility to retain the elements of the result" or something like that.

http://codereview.chromium.org/452042/diff/1/6
File net/base/x509_certificate_mac.cc (left):

http://codereview.chromium.org/452042/diff/1/6#oldcode663
net/base/x509_certificate_mac.cc:663: CFArrayAppendValue(intermediate_ca_certs_,
cert);
On 2009/12/02 03:39:22, wtc wrote:
> Does CFArrayAppendValue do a CFRetain on |cert| ?

Yes -- adding to an array retains, and removing from an array releases.

http://codereview.chromium.org/452042/diff/1/2
File net/socket/ssl_client_socket_mac.cc (right):

http://codereview.chromium.org/452042/diff/1/2#newcode289
net/socket/ssl_client_socket_mac.cc:289: // TODO(wtc): Since
X509Certificate::CreateFromHandle may return a cached
On 2009/12/02 03:39:22, wtc wrote:
> Let me know if you agree that this could be a problem.

It will waste some memory, but it should be OK in terms of verifying the
certificate. SecTrustEvaluate() will just ignore the duplicates

http://codereview.chromium.org/452042/diff/1/2#newcode296
net/socket/ssl_client_socket_mac.cc:296: CFRetain(cert_ref);
On 2009/12/02 03:39:22, wtc wrote:
> I had to add a CFRetain call here.  This is related to my
> question on whether CFArrayAppendValue does a CFRetain
> on its second argument.

Since adding an element to a CFArray retains that element, you do indeed need
the CFRetain here when adding to a vector instead.