Ensure X509Certificate::OSCertHandles are safe to be used on both UI and IO threads on Win

net/base/x509_util_win.h

+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef NET_BASE_X509_UTIL_WIN_H_
+#define NET_BASE_X509_UTIL_WIN_H_
+#pragma once
+
+#include <windows.h>
+#include <wincrypt.h>
+
+namespace net {
+
+class X509Certificate;
+
+namespace x509_util {
+
+// Returns a new PCCERT_CONTEXT containing |cert| and its intermediates, or
+// NULL on failure.

[wtc, 2011/10/16 14:55:49]

Nit: intermediates => intermediate certificates

In the rest of this comment block, you may use "intermediates"
now that you have defined it in the first paragraph.

Please add a heading "Remarks", "Notes", or "Rationale"
after this paragraph to indicate that the subsequent
paragraphs merely provide background info rather than
describing how to use this function.  The only important
info from the subsequent paragraphs regarding usage is
"This function is only needed when the HCERTSTORE of the os_cert_handle()
will be accessed".  It may be a good idea to repeat that
in the first paragraph.

In any case, I just wanted to make the first paragraph
contain complete info for using this function correctly,
even if in a terse way, and mark the rest of this comment
block as providing additional details.  Please do what you
think is right to accomplish these goals.

+//
+// Depending on the CryptoAPI function, Windows may need to access the
+// HCERTSTORE that the passed-in PCCERT_CONTEXT belongs to, such as to locate
+// additional intermediates or access certificate properties. However, in the
+// current implementation on Windows, all X509Certificate::OSCertHandles

[wtc, 2011/10/16 14:55:49]

Nit: current implementation => current X509Certificate implementation

+// belong to the same HCERTSTORE - X509Certificate::cert_store(). If CryptoAPI
+// accesses this shared store on multiple threads, it may return inconsistent
+// results if the store is modified while enumerating.
+//
+// To avoid this, a new in-memory HCERTSTORE is created containing just |cert|
+// and its optional intermediates. The handle to the primary certificate of
+// |cert| in this new HCERTSTORE is then returned, and the new HCERTSTORE will
+// be automatically freed when the returned certificate is released.
+//
+// This function is only needed when the HCERTSTORE of the os_cert_handle()
+// will be accessed, which is generally only during certificate validation or
+// display. While the returned PCCERT_CONTEXT and its HCERTSTORE can safely
+// be used on multiple threads if no further modifications happen, it is
+// generally preferable for each thread that needs such a context to obtain
+// its own, rather than risk thread-safety issues by sharing.
+//
+// Additionally, because of how X509Certificate caching is implemented, the
+// returned PCCERT_CONTEXT *SHOULD NOT* be stored in an X509Certificate, as

[wtc, 2011/10/16 14:55:49]

Nit: SHOULD => MUST?

+// the returned os_cert_handle() may differ from the one originally created by
+// this function.
+PCCERT_CONTEXT CreateOSCertChainForCert(const X509Certificate* cert);
+
+} // namespace x509_util
+
+} // namespace net
+
+#endif  // NET_BASE_X509_UTIL_WIN_H_