Reverting 8868.

This relands wtc's original CL for working around not caching the intermediate CA certificates. We believe the original commit failed on buildbot because of a bad incremental build, and will be resolved by doing a clobber
Review URL: http://codereview.chromium.org/19463

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@8870 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 301 insertions, 256 deletions

diff --git a/net/base/ssl_client_socket_mac.cc b/net/base/ssl_client_socket_mac.cc
index f4b636e..24d33e5 100644
--- a/net/base/ssl_client_socket_mac.cc
+++ b/net/base/ssl_client_socket_mac.cc
@@ -380,7 +380,8 @@ void SSLClientSocketMac::GetSSLInfo(SSLInfo* ssl_info) {
         static_cast<SecCertificateRef>(
           const_cast<void*>(CFArrayGetValueAtIndex(certs, 0)));
     CFRetain(client_cert);
-    ssl_info->cert = X509Certificate::CreateFromHandle(client_cert);
+    ssl_info->cert = X509Certificate::CreateFromHandle(
+        client_cert, X509Certificate::SOURCE_FROM_NETWORK);
     CFRelease(certs);
   }
   
diff --git a/net/base/ssl_client_socket_win.cc b/net/base/ssl_client_socket_win.cc
index beeaba9..1f4c314 100644
--- a/net/base/ssl_client_socket_win.cc
+++ b/net/base/ssl_client_socket_win.cc
@@ -359,7 +359,8 @@ void SSLClientSocketWin::GetSSLInfo(SSLInfo* ssl_info) {
   if (status == SEC_E_OK) {
     DCHECK(server_cert_);
     PCCERT_CONTEXT dup_cert = CertDuplicateCertificateContext(server_cert_);
-    ssl_info->cert = X509Certificate::CreateFromHandle(dup_cert);
+    ssl_info->cert = X509Certificate::CreateFromHandle(
+        dup_cert, X509Certificate::SOURCE_FROM_NETWORK);
   }
   SecPkgContext_ConnectionInfo connection_info;
   status = QueryContextAttributes(&ctxt_,
diff --git a/net/base/x509_certificate.cc b/net/base/x509_certificate.cc
index d62f963..443ef81 100644
--- a/net/base/x509_certificate.cc
+++ b/net/base/x509_certificate.cc
@@ -4,6 +4,7 @@
 
 #include "net/base/x509_certificate.h"
 
+#include "base/histogram.h"
 #include "base/logging.h"
 
 namespace net {
@@ -122,5 +123,66 @@ void X509Certificate::Policy::Deny(X509Certificate* cert) {
   denied_.insert(cert->fingerprint());
 }
 
+// static
+X509Certificate* X509Certificate::CreateFromHandle(OSCertHandle cert_handle,
+                                                   Source source) {
+  DCHECK(cert_handle);
+  DCHECK(source != SOURCE_UNUSED);
+
+  // Check if we already have this certificate in memory.
+  X509Certificate::Cache* cache = X509Certificate::Cache::GetInstance();
+  X509Certificate* cached_cert =
+      cache->Find(CalculateFingerprint(cert_handle));
+  if (cached_cert) {
+    DCHECK(cached_cert->source_ != SOURCE_UNUSED);
+    if (cached_cert->source_ >= source) {
+      // We've found a certificate with the same fingerprint in our cache.  We
+      // own the |cert_handle|, which makes it our job to free it.
+      FreeOSCertHandle(cert_handle);
+      DHISTOGRAM_COUNTS(L"X509CertificateReuseCount", 1);
+      return cached_cert;
+    }
+    // Kick out the old certificate from our cache.  The new one is better.
+    cache->Remove(cached_cert);
+  }
+  // Otherwise, allocate a new object.
+  return new X509Certificate(cert_handle, source);
+}
+
+// static
+X509Certificate* X509Certificate::CreateFromBytes(const char* data,
+                                                  int length) {
+  OSCertHandle cert_handle = CreateOSCertHandleFromBytes(data, length);
+  if (!cert_handle)
+    return NULL;
+
+  return CreateFromHandle(cert_handle, SOURCE_LONE_CERT_IMPORT);
+}
+
+X509Certificate::X509Certificate(OSCertHandle cert_handle, Source source)
+    : cert_handle_(cert_handle), source_(source) {
+  Initialize();
+}
+
+X509Certificate::X509Certificate(const std::string& subject,
+                                 const std::string& issuer,
+                                 base::Time start_date,
+                                 base::Time expiration_date)
+    : subject_(subject),
+      issuer_(issuer),
+      valid_start_(start_date),
+      valid_expiry_(expiration_date),
+      cert_handle_(NULL),
+      source_(SOURCE_UNUSED) {
+  memset(fingerprint_.data, 0, sizeof(fingerprint_.data));
+}
+
+X509Certificate::~X509Certificate() {
+  // We might not be in the cache, but it is safe to remove ourselves anyway.
+  X509Certificate::Cache::GetInstance()->Remove(this);
+  if (cert_handle_)
+    FreeOSCertHandle(cert_handle_);
+}
+
 }  // namespace net
 
diff --git a/net/base/x509_certificate.h b/net/base/x509_certificate.h
index 4e41570..9127cc6 100644
--- a/net/base/x509_certificate.h
+++ b/net/base/x509_certificate.h
@@ -13,6 +13,7 @@
 #include "base/ref_counted.h"
 #include "base/singleton.h"
 #include "base/time.h"
+#include "testing/gtest/include/gtest/gtest_prod.h"
 
 #if defined(OS_WIN)
 #include <windows.h>
@@ -112,10 +113,25 @@ class X509Certificate : public base::RefCountedThreadSafe<X509Certificate> {
     std::set<Fingerprint, FingerprintLessThan> denied_;
   };
 
+  // Where the certificate comes from.  The enumeration constants are
+  // listed in increasing order of preference.
+  enum Source {
+    SOURCE_UNUSED = 0,            // The source_ member is not used.
+    SOURCE_LONE_CERT_IMPORT = 1,  // From importing a certificate without
+                                  // its intermediate CA certificates.
+    SOURCE_FROM_NETWORK = 2,      // From the network.
+  };
+
   // Create an X509Certificate from a handle to the certificate object
   // in the underlying crypto library. This is a transfer of ownership;
   // X509Certificate will properly dispose of |cert_handle| for you.
-  static X509Certificate* CreateFromHandle(OSCertHandle cert_handle);
+  // |source| specifies where |cert_handle| comes from.  Given two
+  // certificate handles for the same certificate, our certificate cache
+  // prefers the handle from the network because our HTTP cache isn't
+  // caching the corresponding intermediate CA certificates yet
+  // (http://crbug.com/7065).
+  static X509Certificate* CreateFromHandle(OSCertHandle cert_handle,
+                                           Source source);
 
   // Create an X509Certificate from the BER-encoded representation.
   // Returns NULL on failure.
@@ -130,7 +146,7 @@ class X509Certificate : public base::RefCountedThreadSafe<X509Certificate> {
 
   // Creates a X509Certificate from the ground up.  Used by tests that simulate
   // SSL connections.
-  X509Certificate(std::string subject, std::string issuer,
+  X509Certificate(const std::string& subject, const std::string& issuer,
                   base::Time start_date, base::Time expiration_date);
 
   // Appends a representation of this object to the given pickle.
@@ -172,6 +188,9 @@ class X509Certificate : public base::RefCountedThreadSafe<X509Certificate> {
   OSCertHandle os_cert_handle() const { return cert_handle_; }
 
  private:
+  friend class base::RefCountedThreadSafe<X509Certificate>;
+  FRIEND_TEST(X509CertificateTest, Cache);
+
   // A cache of X509Certificate objects.
   class Cache {
    public:
@@ -200,14 +219,25 @@ class X509Certificate : public base::RefCountedThreadSafe<X509Certificate> {
 
   // Construct an X509Certificate from a handle to the certificate object
   // in the underlying crypto library.
-  explicit X509Certificate(OSCertHandle cert_handle);
+  X509Certificate(OSCertHandle cert_handle, Source source);
 
-  friend class base::RefCountedThreadSafe<X509Certificate>;
   ~X509Certificate();
 
   // Common object initialization code.  Called by the constructors only.
   void Initialize();
 
+  // Creates an OS certificate handle from the BER-encoded representation.
+  // Returns NULL on failure.
+  static OSCertHandle CreateOSCertHandleFromBytes(const char* data,
+                                                  int length);
+
+  // Frees an OS certificate handle.
+  static void FreeOSCertHandle(OSCertHandle cert_handle);
+
+  // Calculates the SHA-1 fingerprint of the certificate.  Returns an empty
+  // (all zero) fingerprint on failure.
+  static Fingerprint CalculateFingerprint(OSCertHandle cert_handle);
+
   // The subject of the certificate.
   Principal subject_;
 
@@ -226,6 +256,9 @@ class X509Certificate : public base::RefCountedThreadSafe<X509Certificate> {
   // A handle to the certificate object in the underlying crypto library.
   OSCertHandle cert_handle_;
 
+  // Where the certificate comes from.
+  Source source_;
+
   DISALLOW_COPY_AND_ASSIGN(X509Certificate);
 };
 
diff --git a/net/base/x509_certificate_mac.cc b/net/base/x509_certificate_mac.cc
index 11191bd..ac60645 100644
--- a/net/base/x509_certificate_mac.cc
+++ b/net/base/x509_certificate_mac.cc
@@ -7,7 +7,6 @@
 #include <CommonCrypto/CommonDigest.h>
 #include <time.h>
 
-#include "base/histogram.h"
 #include "base/logging.h"
 #include "base/pickle.h"
 #include "net/base/cert_status_flags.h"
@@ -19,26 +18,6 @@ namespace net {
 
 namespace {
 
-// Calculates the SHA-1 fingerprint of the certificate.  Returns an empty
-// (all zero) fingerprint on failure.
-X509Certificate::Fingerprint CalculateFingerprint(
-    X509Certificate::OSCertHandle cert) {
-  X509Certificate::Fingerprint sha1;
-  memset(sha1.data, 0, sizeof(sha1.data));
-  
-  CSSM_DATA cert_data;
-  OSStatus status = SecCertificateGetData(cert, &cert_data);
-  if (status)
-    return sha1;
-  
-  DCHECK(NULL != cert_data.Data);
-  DCHECK(0 != cert_data.Length);
-  
-  CC_SHA1(cert_data.Data, cert_data.Length, sha1.data);
-
-  return sha1;
-}
-
 inline bool CSSMOIDEqual(const CSSM_OID* oid1, const CSSM_OID* oid2) {
   return oid1->Length == oid2->Length &&
       (memcmp(oid1->Data, oid2->Data, oid1->Length) == 0);
@@ -242,42 +221,6 @@ void X509Certificate::Initialize() {
 }
 
 // static
-X509Certificate* X509Certificate::CreateFromHandle(OSCertHandle cert_handle) {
-  DCHECK(cert_handle);
-
-  // Check if we already have this certificate in memory.
-  X509Certificate::Cache* cache = X509Certificate::Cache::GetInstance();
-  X509Certificate* cert = cache->Find(CalculateFingerprint(cert_handle));
-  if (cert) {
-    // We've found a certificate with the same fingerprint in our cache.  We own
-    // the |cert_handle|, which makes it our job to free it.
-    CFRelease(cert_handle);
-    DHISTOGRAM_COUNTS(L"X509CertificateReuseCount", 1);
-    return cert;
-  }
-  // Otherwise, allocate a new object.
-  return new X509Certificate(cert_handle);
-}
-
-// static
-X509Certificate* X509Certificate::CreateFromBytes(const char* data,
-                                                  int length) {
-  CSSM_DATA cert_data;
-  cert_data.Data = const_cast<uint8*>(reinterpret_cast<const uint8*>(data));
-  cert_data.Length = length;
-
-  OSCertHandle cert_handle = NULL;
-  OSStatus status = SecCertificateCreateFromData(&cert_data,
-                                                 CSSM_CERT_X_509v3,
-                                                 CSSM_CERT_ENCODING_BER,
-                                                 &cert_handle);
-  if (status)
-    return NULL;
-
-  return CreateFromHandle(cert_handle);
-}
-
-// static
 X509Certificate* X509Certificate::CreateFromPickle(const Pickle& pickle,
                                                    void** pickle_iter) {
   const char* data;
@@ -288,21 +231,6 @@ X509Certificate* X509Certificate::CreateFromPickle(const Pickle& pickle,
   return CreateFromBytes(data, length);
 }
 
-X509Certificate::X509Certificate(OSCertHandle cert_handle)
-    : cert_handle_(cert_handle) {
-  Initialize();
-}
-
-X509Certificate::X509Certificate(std::string subject, std::string issuer,
-                                 Time start_date, Time expiration_date)
-    : subject_(subject),
-      issuer_(issuer),
-      valid_start_(start_date),
-      valid_expiry_(expiration_date),
-      cert_handle_(NULL) {
-  memset(fingerprint_.data, 0, sizeof(fingerprint_.data));
-}
-
 void X509Certificate::Persist(Pickle* pickle) {
   CSSM_DATA cert_data;
   OSStatus status = SecCertificateGetData(cert_handle_, &cert_data);
@@ -314,13 +242,6 @@ void X509Certificate::Persist(Pickle* pickle) {
   pickle->WriteData(reinterpret_cast<char*>(cert_data.Data), cert_data.Length);
 }
 
-X509Certificate::~X509Certificate() {
-  // We might not be in the cache, but it is safe to remove ourselves anyway.
-  X509Certificate::Cache::GetInstance()->Remove(this);
-  if (cert_handle_)
-    CFRelease(cert_handle_);
-}
-
 void X509Certificate::GetDNSNames(std::vector<std::string>* dns_names) const {
   dns_names->clear();
   
@@ -345,4 +266,46 @@ bool X509Certificate::IsEV(int cert_status) const {
   return false;
 }
 
+// static
+X509Certificate::OSCertHandle X509Certificate::CreateOSCertHandleFromBytes(
+    const char* data, int length) {
+  CSSM_DATA cert_data;
+  cert_data.Data = const_cast<uint8*>(reinterpret_cast<const uint8*>(data));
+  cert_data.Length = length;
+
+  OSCertHandle cert_handle = NULL;
+  OSStatus status = SecCertificateCreateFromData(&cert_data,
+                                                 CSSM_CERT_X_509v3,
+                                                 CSSM_CERT_ENCODING_BER,
+                                                 &cert_handle);
+  if (status)
+    return NULL;
+
+  return cert_handle;
+}
+
+// static
+void X509Certificate::FreeOSCertHandle(OSCertHandle cert_handle) {
+  CFRelease(cert_handle);
+}
+
+// static
+X509Certificate::Fingerprint X509Certificate::CalculateFingerprint(
+    OSCertHandle cert) {
+  Fingerprint sha1;
+  memset(sha1.data, 0, sizeof(sha1.data));
+
+  CSSM_DATA cert_data;
+  OSStatus status = SecCertificateGetData(cert, &cert_data);
+  if (status)
+    return sha1;
+
+  DCHECK(NULL != cert_data.Data);
+  DCHECK(0 != cert_data.Length);
+
+  CC_SHA1(cert_data.Data, cert_data.Length, sha1.data);
+
+  return sha1;
+}
+
 }  // namespace net
diff --git a/net/base/x509_certificate_nss.cc b/net/base/x509_certificate_nss.cc
index ef4b8cd..2ca1255 100644
--- a/net/base/x509_certificate_nss.cc
+++ b/net/base/x509_certificate_nss.cc
@@ -13,30 +13,12 @@
 #include <sechash.h>
 #undef Lock
 
-#include "base/histogram.h"
 #include "base/logging.h"
 #include "base/time.h"
 #include "base/nss_init.h"
 
 namespace net {
 
-// Calculates the SHA-1 fingerprint of the certificate.  Returns an empty
-// (all zero) fingerprint on failure.
-X509Certificate::Fingerprint CalculateFingerprint(
-    X509Certificate::OSCertHandle cert) {
-  X509Certificate::Fingerprint sha1;
-  memset(sha1.data, 0, sizeof(sha1.data));
-  
-  DCHECK(NULL != cert->derCert.data);
-  DCHECK(0 != cert->derCert.len);
-  
-  SECStatus rv = HASH_HashBuf(HASH_AlgSHA1, sha1.data, 
-                              cert->derCert.data, cert->derCert.len);
-  DCHECK(rv == SECSuccess);
-
-  return sha1;
-}
-
 namespace {
 
 // TODO(port): Implement this more simply, and put it in the right place
@@ -57,11 +39,11 @@ base::Time PRTimeToBaseTime(PRTime prtime) {
   return base::Time::FromUTCExploded(exploded);
 }
 
-void ParsePrincipal(SECItem *der_name,
+void ParsePrincipal(SECItem* der_name,
                     X509Certificate::Principal* principal) {
 
   CERTName name;
-  PRArenaPool *arena = NULL;
+  PRArenaPool* arena = NULL;
 
   arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
   DCHECK(arena != NULL);
@@ -110,7 +92,7 @@ void ParsePrincipal(SECItem *der_name,
           SECItem* decode_item = CERT_DecodeAVAValue(&avas[pair]->value);
           if (!decode_item)
             break;
-          std::string value(reinterpret_cast<char *>(decode_item->data), 
+          std::string value(reinterpret_cast<char*>(decode_item->data), 
                             decode_item->len);
           values[oid]->push_back(value);
           SECITEM_FreeItem(decode_item, PR_TRUE);
@@ -165,9 +147,9 @@ void GetCertSubjectAltNamesOfType(X509Certificate::OSCertHandle cert_handle,
            name->type == certDNSName ||
            name->type == certURI);
     if (name->type == name_type) {
-      unsigned char *p = name->name.other.data;
+      unsigned char* p = name->name.other.data;
       int len = name->name.other.len;
-      std::string value = std::string(reinterpret_cast<char *>(p), len);
+      std::string value = std::string(reinterpret_cast<char*>(p), len);
       result->push_back(value);
     }
     name = CERT_GetNextGeneralName(name);
@@ -193,63 +175,16 @@ void X509Certificate::Initialize() {
 }
 
 // static
-X509Certificate* X509Certificate::CreateFromHandle(OSCertHandle cert_handle) {
-  DCHECK(cert_handle);
-
-  // Check if we already have this certificate in memory.
-  X509Certificate::Cache* cache = X509Certificate::Cache::GetInstance();
-  X509Certificate* cert = cache->Find(CalculateFingerprint(cert_handle));
-  if (cert) {
-    // We've found a certificate with the same fingerprint in our cache.  We own
-    // the |cert_handle|, which makes it our job to free it.
-    CERT_DestroyCertificate(cert_handle);
-    DHISTOGRAM_COUNTS(L"X509CertificateReuseCount", 1);
-    return cert;
-  }
-  // Otherwise, allocate a new object.
-  return new X509Certificate(cert_handle);
-}
-
-// static
-X509Certificate* X509Certificate::CreateFromBytes(const char* data,
-                                                  int length) {
-  base::EnsureNSSInit();
-
-  SECItem der_cert;
-  der_cert.data = reinterpret_cast<unsigned char *>(const_cast<char *>(data));
-  der_cert.len = length;
-  OSCertHandle cert_handle = 
-      CERT_NewTempCertificate(CERT_GetDefaultCertDB(), &der_cert, 
-                              NULL, PR_FALSE, PR_TRUE);
-  if (!cert_handle)
-    return NULL;
-
-  return CreateFromHandle(cert_handle);
-}
-
-// static
 X509Certificate* X509Certificate::CreateFromPickle(const Pickle& pickle,
                                                    void** pickle_iter) {
   NOTIMPLEMENTED();
   return NULL;
 }
 
-X509Certificate::X509Certificate(OSCertHandle cert_handle)
-    : cert_handle_(cert_handle) {
-  Initialize();
-}
-
 void X509Certificate::Persist(Pickle* pickle) {
   NOTIMPLEMENTED();
 }
 
-X509Certificate::~X509Certificate() {
-  // We might not be in the cache, but it is safe to remove ourselves anyway.
-  X509Certificate::Cache::GetInstance()->Remove(this);
-  if (cert_handle_)
-    CERT_DestroyCertificate(cert_handle_);
-}
-
 void X509Certificate::GetDNSNames(std::vector<std::string>* dns_names) const {
   dns_names->clear();
 
@@ -263,6 +198,39 @@ void X509Certificate::GetDNSNames(std::vector<std::string>* dns_names) const {
   if (dns_names->empty())
     dns_names->push_back(subject_.common_name);
 }
+
+// static
+X509Certificate::OSCertHandle X509Certificate::CreateOSCertHandleFromBytes(
+    const char* data, int length) {
+  base::EnsureNSSInit();
+
+  SECItem der_cert;
+  der_cert.data = reinterpret_cast<unsigned char*>(const_cast<char*>(data));
+  der_cert.len = length;
+  return CERT_NewTempCertificate(CERT_GetDefaultCertDB(), &der_cert,
+                                 NULL, PR_FALSE, PR_TRUE);
+}
+
+// static
+void X509Certificate::FreeOSCertHandle(OSCertHandle cert_handle) {
+  CERT_DestroyCertificate(cert_handle);
+}
+
+// static
+X509Certificate::Fingerprint X509Certificate::CalculateFingerprint(
+    OSCertHandle cert) {
+  Fingerprint sha1;
+  memset(sha1.data, 0, sizeof(sha1.data));
+
+  DCHECK(NULL != cert->derCert.data);
+  DCHECK(0 != cert->derCert.len);
+
+  SECStatus rv = HASH_HashBuf(HASH_AlgSHA1, sha1.data,
+                              cert->derCert.data, cert->derCert.len);
+  DCHECK(rv == SECSuccess);
+
+  return sha1;
+}
   
 }  // namespace net
 
diff --git a/net/base/x509_certificate_unittest.cc b/net/base/x509_certificate_unittest.cc
index 499b914..bf98b5c 100644
--- a/net/base/x509_certificate_unittest.cc
+++ b/net/base/x509_certificate_unittest.cc
@@ -18,11 +18,8 @@ using base::Time;
 
 namespace {
 
-class X509CertificateTest : public testing::Test {
-};
-
 // Certificates for test data. They're obtained with:
-// 
+//
 // $ openssl s_client -connect [host]:443 -showcerts
 // $ openssl x509 -inform PEM -outform DER > /tmp/host.der
 // $ xxd -i /tmp/host.der
@@ -332,13 +329,13 @@ unsigned char thawte_fingerprint[] = {
 
 }  // namespace
 
-using net::X509Certificate;
+namespace net {
 
 TEST(X509CertificateTest, GoogleCertParsing) {
   scoped_refptr<X509Certificate> google_cert = X509Certificate::CreateFromBytes(
       reinterpret_cast<const char*>(google_der), sizeof(google_der));
-  
-  ASSERT_NE(static_cast<X509Certificate *>(NULL), google_cert);
+
+  ASSERT_NE(static_cast<X509Certificate*>(NULL), google_cert);
 
   const X509Certificate::Principal& subject = google_cert->subject();
   EXPECT_EQ("www.google.com", subject.common_name);
@@ -350,7 +347,7 @@ TEST(X509CertificateTest, GoogleCertParsing) {
   EXPECT_EQ("Google Inc", subject.organization_names[0]);
   EXPECT_EQ(0U, subject.organization_unit_names.size());
   EXPECT_EQ(0U, subject.domain_components.size());
-  
+
   const X509Certificate::Principal& issuer = google_cert->issuer();
   EXPECT_EQ("Thawte SGC CA", issuer.common_name);
   EXPECT_EQ("", issuer.locality_name);
@@ -361,14 +358,14 @@ TEST(X509CertificateTest, GoogleCertParsing) {
   EXPECT_EQ("Thawte Consulting (Pty) Ltd.", issuer.organization_names[0]);
   EXPECT_EQ(0U, issuer.organization_unit_names.size());
   EXPECT_EQ(0U, issuer.domain_components.size());
-  
+
   // Use DoubleT because its epoch is the same on all platforms
   const Time& valid_start = google_cert->valid_start();
   EXPECT_EQ(1209747775, valid_start.ToDoubleT());
-  
+
   const Time& valid_expiry = google_cert->valid_expiry();
   EXPECT_EQ(1241283775, valid_expiry.ToDoubleT());
-  
+
   const X509Certificate::Fingerprint& fingerprint = google_cert->fingerprint();
   for (size_t i = 0; i < 20; ++i)
     EXPECT_EQ(google_fingerprint[i], fingerprint.data[i]);
@@ -377,7 +374,7 @@ TEST(X509CertificateTest, GoogleCertParsing) {
   google_cert->GetDNSNames(&dns_names);
   EXPECT_EQ(1U, dns_names.size());
   EXPECT_EQ("www.google.com", dns_names[0]);
-  
+
 #if ALLOW_EXTERNAL_ACCESS && defined(OS_WIN)
   // TODO(avi): turn this on for the Mac once EV checking is implemented.
   EXPECT_EQ(false, google_cert->IsEV(net::CERT_STATUS_REV_CHECKING_ENABLED));
@@ -387,8 +384,8 @@ TEST(X509CertificateTest, GoogleCertParsing) {
 TEST(X509CertificateTest, WebkitCertParsing) {
   scoped_refptr<X509Certificate> webkit_cert = X509Certificate::CreateFromBytes(
       reinterpret_cast<const char*>(webkit_der), sizeof(webkit_der));
-  
-  ASSERT_NE(static_cast<X509Certificate *>(NULL), webkit_cert);
+
+  ASSERT_NE(static_cast<X509Certificate*>(NULL), webkit_cert);
 
   const X509Certificate::Principal& subject = webkit_cert->subject();
   EXPECT_EQ("Cupertino", subject.locality_name);
@@ -400,7 +397,7 @@ TEST(X509CertificateTest, WebkitCertParsing) {
   EXPECT_EQ(1U, subject.organization_unit_names.size());
   EXPECT_EQ("Mac OS Forge", subject.organization_unit_names[0]);
   EXPECT_EQ(0U, subject.domain_components.size());
-  
+
   const X509Certificate::Principal& issuer = webkit_cert->issuer();
   EXPECT_EQ("Go Daddy Secure Certification Authority", issuer.common_name);
   EXPECT_EQ("Scottsdale", issuer.locality_name);
@@ -413,14 +410,14 @@ TEST(X509CertificateTest, WebkitCertParsing) {
   EXPECT_EQ("http://certificates.godaddy.com/repository",
       issuer.organization_unit_names[0]);
   EXPECT_EQ(0U, issuer.domain_components.size());
-  
+
   // Use DoubleT because its epoch is the same on all platforms
   const Time& valid_start = webkit_cert->valid_start();
   EXPECT_EQ(1205883319, valid_start.ToDoubleT());
-  
+
   const Time& valid_expiry = webkit_cert->valid_expiry();
   EXPECT_EQ(1300491319, valid_expiry.ToDoubleT());
-  
+
   const X509Certificate::Fingerprint& fingerprint = webkit_cert->fingerprint();
   for (size_t i = 0; i < 20; ++i)
     EXPECT_EQ(webkit_fingerprint[i], fingerprint.data[i]);
@@ -430,7 +427,7 @@ TEST(X509CertificateTest, WebkitCertParsing) {
   EXPECT_EQ(2U, dns_names.size());
   EXPECT_EQ("*.webkit.org", dns_names[0]);
   EXPECT_EQ("webkit.org", dns_names[1]);
-  
+
 #if ALLOW_EXTERNAL_ACCESS && defined(OS_WIN)
   EXPECT_EQ(false, webkit_cert->IsEV(net::CERT_STATUS_REV_CHECKING_ENABLED));
 #endif
@@ -439,8 +436,8 @@ TEST(X509CertificateTest, WebkitCertParsing) {
 TEST(X509CertificateTest, ThawteCertParsing) {
   scoped_refptr<X509Certificate> thawte_cert = X509Certificate::CreateFromBytes(
       reinterpret_cast<const char*>(thawte_der), sizeof(thawte_der));
-  
-  ASSERT_NE(static_cast<X509Certificate *>(NULL), thawte_cert);
+
+  ASSERT_NE(static_cast<X509Certificate*>(NULL), thawte_cert);
 
   const X509Certificate::Principal& subject = thawte_cert->subject();
   EXPECT_EQ("www.thawte.com", subject.common_name);
@@ -452,7 +449,7 @@ TEST(X509CertificateTest, ThawteCertParsing) {
   EXPECT_EQ("Thawte Inc", subject.organization_names[0]);
   EXPECT_EQ(0U, subject.organization_unit_names.size());
   EXPECT_EQ(0U, subject.domain_components.size());
-  
+
   const X509Certificate::Principal& issuer = thawte_cert->issuer();
   EXPECT_EQ("thawte Extended Validation SSL CA", issuer.common_name);
   EXPECT_EQ("", issuer.locality_name);
@@ -465,14 +462,14 @@ TEST(X509CertificateTest, ThawteCertParsing) {
   EXPECT_EQ("Terms of use at https://www.thawte.com/cps (c)06",
             issuer.organization_unit_names[0]);
   EXPECT_EQ(0U, issuer.domain_components.size());
-  
+
   // Use DoubleT because its epoch is the same on all platforms
   const Time& valid_start = thawte_cert->valid_start();
   EXPECT_EQ(1169078400, valid_start.ToDoubleT());
-  
+
   const Time& valid_expiry = thawte_cert->valid_expiry();
   EXPECT_EQ(1232236799, valid_expiry.ToDoubleT());
-  
+
   const X509Certificate::Fingerprint& fingerprint = thawte_cert->fingerprint();
   for (size_t i = 0; i < 20; ++i)
     EXPECT_EQ(thawte_fingerprint[i], fingerprint.data[i]);
@@ -481,7 +478,7 @@ TEST(X509CertificateTest, ThawteCertParsing) {
   thawte_cert->GetDNSNames(&dns_names);
   EXPECT_EQ(1U, dns_names.size());
   EXPECT_EQ("www.thawte.com", dns_names[0]);
-  
+
 #if ALLOW_EXTERNAL_ACCESS && defined(OS_WIN)
   // EV cert verification requires revocation checking.
   EXPECT_EQ(true, thawte_cert->IsEV(net::CERT_STATUS_REV_CHECKING_ENABLED));
@@ -490,3 +487,57 @@ TEST(X509CertificateTest, ThawteCertParsing) {
   EXPECT_EQ(false, thawte_cert->IsEV(0));
 #endif
 }
+
+// Tests X509Certificate::Cache via X509Certificate::CreateFromHandle.  We
+// call X509Certificate::CreateFromHandle several times and observe whether
+// it returns a cached or new X509Certificate object.
+//
+// All the OS certificate handles in this test are actually from the same
+// source (the bytes of a lone certificate), but we pretend that some of them
+// come from the network.
+TEST(X509CertificateTest, Cache) {
+  X509Certificate::OSCertHandle google_cert_handle;
+
+  // Add a certificate from the source SOURCE_LONE_CERT_IMPORT to our
+  // certificate cache.
+  google_cert_handle = X509Certificate::CreateOSCertHandleFromBytes(
+      reinterpret_cast<const char*>(google_der), sizeof(google_der));
+  scoped_refptr<X509Certificate> cert1 = X509Certificate::CreateFromHandle(
+      google_cert_handle, X509Certificate::SOURCE_LONE_CERT_IMPORT);
+
+  // Add a certificate from the same source (SOURCE_LONE_CERT_IMPORT).  This
+  // should return the cached certificate (cert1).
+  google_cert_handle = X509Certificate::CreateOSCertHandleFromBytes(
+      reinterpret_cast<const char*>(google_der), sizeof(google_der));
+  scoped_refptr<X509Certificate> cert2 = X509Certificate::CreateFromHandle(
+      google_cert_handle, X509Certificate::SOURCE_LONE_CERT_IMPORT);
+
+  EXPECT_EQ(cert1, cert2);
+
+  // Add a certificate from the network.  This should kick out the original
+  // cached certificate (cert1) and return a new certificate.
+  google_cert_handle = X509Certificate::CreateOSCertHandleFromBytes(
+      reinterpret_cast<const char*>(google_der), sizeof(google_der));
+  scoped_refptr<X509Certificate> cert3 = X509Certificate::CreateFromHandle(
+      google_cert_handle, X509Certificate::SOURCE_FROM_NETWORK);
+
+  EXPECT_NE(cert1, cert3);
+
+  // Add one certificate from each source.  Both should return the new cached
+  // certificate (cert3).
+  google_cert_handle = X509Certificate::CreateOSCertHandleFromBytes(
+      reinterpret_cast<const char*>(google_der), sizeof(google_der));
+  scoped_refptr<X509Certificate> cert4 = X509Certificate::CreateFromHandle(
+      google_cert_handle, X509Certificate::SOURCE_FROM_NETWORK);
+
+  EXPECT_EQ(cert3, cert4);
+
+  google_cert_handle = X509Certificate::CreateOSCertHandleFromBytes(
+      reinterpret_cast<const char*>(google_der), sizeof(google_der));
+  scoped_refptr<X509Certificate> cert5 = X509Certificate::CreateFromHandle(
+      google_cert_handle, X509Certificate::SOURCE_FROM_NETWORK);
+
+  EXPECT_EQ(cert3, cert5);
+}
+
+}  // namespace net
diff --git a/net/base/x509_certificate_win.cc b/net/base/x509_certificate_win.cc
index 4e7c4ba..57ce947 100644
--- a/net/base/x509_certificate_win.cc
+++ b/net/base/x509_certificate_win.cc
@@ -4,7 +4,6 @@
 
 #include "net/base/x509_certificate.h"
 
-#include "base/histogram.h"
 #include "base/logging.h"
 #include "base/pickle.h"
 #include "base/string_tokenizer.h"
@@ -21,23 +20,6 @@ namespace net {
 
 namespace {
 
-// Calculates the SHA-1 fingerprint of the certificate.  Returns an empty
-// (all zero) fingerprint on failure.
-X509Certificate::Fingerprint CalculateFingerprint(PCCERT_CONTEXT cert) {
-  DCHECK(NULL != cert->pbCertEncoded);
-  DCHECK(0 != cert->cbCertEncoded);
-
-  BOOL rv;
-  X509Certificate::Fingerprint sha1;
-  DWORD sha1_size = sizeof(sha1.data);
-  rv = CryptHashCertificate(NULL, CALG_SHA1, 0, cert->pbCertEncoded,
-                            cert->cbCertEncoded, sha1.data, &sha1_size);
-  DCHECK(rv && sha1_size == sizeof(sha1.data));
-  if (!rv)
-    memset(sha1.data, 0, sizeof(sha1.data));
-  return sha1;
-}
-
 // Wrappers of malloc and free for CRYPT_DECODE_PARA, which requires the
 // WINAPI calling convention.
 void* WINAPI MyCryptAlloc(size_t size) {
@@ -255,39 +237,6 @@ void X509Certificate::Initialize() {
 }
 
 // static
-X509Certificate* X509Certificate::CreateFromHandle(OSCertHandle cert_handle) {
-  DCHECK(cert_handle);
-
-  // Check if we already have this certificate in memory.
-  X509Certificate::Cache* cache = X509Certificate::Cache::GetInstance();
-  X509Certificate* cert = cache->Find(CalculateFingerprint(cert_handle));
-  if (cert) {
-    // We've found a certificate with the same fingerprint in our cache.  We own
-    // the |cert_handle|, which makes it our job to free it.
-    CertFreeCertificateContext(cert_handle);
-    DHISTOGRAM_COUNTS(L"X509CertificateReuseCount", 1);
-    return cert;
-  }
-  // Otherwise, allocate a new object.
-  return new X509Certificate(cert_handle);
-}
-
-// static
-X509Certificate* X509Certificate::CreateFromBytes(const char* data,
-                                                  int length) {
-  OSCertHandle cert_handle = NULL;
-  if (!CertAddEncodedCertificateToStore(
-      NULL,  // the cert won't be persisted in any cert store
-      X509_ASN_ENCODING,
-      reinterpret_cast<const BYTE*>(data), length,
-      CERT_STORE_ADD_USE_EXISTING,
-      &cert_handle))
-    return NULL;
-
-  return CreateFromHandle(cert_handle);
-}
-
-// static
 X509Certificate* X509Certificate::CreateFromPickle(const Pickle& pickle,
                                                    void** pickle_iter) {
   const char* data;
@@ -303,22 +252,7 @@ X509Certificate* X509Certificate::CreateFromPickle(const Pickle& pickle,
       NULL, reinterpret_cast<const void **>(&cert_handle)))
     return NULL;
 
-  return CreateFromHandle(cert_handle);
-}
-
-X509Certificate::X509Certificate(OSCertHandle cert_handle)
-    : cert_handle_(cert_handle) {
-  Initialize();
-}
-
-X509Certificate::X509Certificate(std::string subject, std::string issuer,
-                                 Time start_date, Time expiration_date)
-    : subject_(subject),
-      issuer_(issuer),
-      valid_start_(start_date),
-      valid_expiry_(expiration_date),
-      cert_handle_(NULL) {
-  memset(fingerprint_.data, 0, sizeof(fingerprint_.data));
+  return CreateFromHandle(cert_handle, SOURCE_LONE_CERT_IMPORT);
 }
 
 void X509Certificate::Persist(Pickle* pickle) {
@@ -337,13 +271,6 @@ void X509Certificate::Persist(Pickle* pickle) {
   pickle->TrimWriteData(length);
 }
 
-X509Certificate::~X509Certificate() {
-  // We might not be in the cache, but it is safe to remove ourselves anyway.
-  X509Certificate::Cache::GetInstance()->Remove(this);
-  if (cert_handle_)
-    CertFreeCertificateContext(cert_handle_);
-}
-
 void X509Certificate::GetDNSNames(std::vector<std::string>* dns_names) const {
   dns_names->clear();
   scoped_ptr_malloc<CERT_ALT_NAME_INFO> alt_name_info;
@@ -406,7 +333,7 @@ bool X509Certificate::IsEV(int cert_status) const {
 
   // Look up the EV policy OID of the root CA.
   PCCERT_CONTEXT root_cert = element[num_elements - 1]->pCertContext;
-  X509Certificate::Fingerprint fingerprint = CalculateFingerprint(root_cert);
+  Fingerprint fingerprint = CalculateFingerprint(root_cert);
   std::string ev_policy_oid;
   if (!metadata->GetPolicyOID(fingerprint, &ev_policy_oid))
     return false;
@@ -422,5 +349,42 @@ bool X509Certificate::IsEV(int cert_status) const {
   return ContainsPolicy(policies_info.get(), ev_policy_oid.c_str());
 }
 
+// static
+X509Certificate::OSCertHandle X509Certificate::CreateOSCertHandleFromBytes(
+    const char* data, int length) {
+  OSCertHandle cert_handle = NULL;
+  if (!CertAddEncodedCertificateToStore(
+      NULL,  // the cert won't be persisted in any cert store
+      X509_ASN_ENCODING,
+      reinterpret_cast<const BYTE*>(data), length,
+      CERT_STORE_ADD_USE_EXISTING,
+      &cert_handle))
+    return NULL;
+
+  return cert_handle;
+}
+
+// static
+void X509Certificate::FreeOSCertHandle(OSCertHandle cert_handle) {
+  CertFreeCertificateContext(cert_handle);
+}
+
+// static
+X509Certificate::Fingerprint X509Certificate::CalculateFingerprint(
+    OSCertHandle cert) {
+  DCHECK(NULL != cert->pbCertEncoded);
+  DCHECK(0 != cert->cbCertEncoded);
+
+  BOOL rv;
+  Fingerprint sha1;
+  DWORD sha1_size = sizeof(sha1.data);
+  rv = CryptHashCertificate(NULL, CALG_SHA1, 0, cert->pbCertEncoded,
+                            cert->cbCertEncoded, sha1.data, &sha1_size);
+  DCHECK(rv && sha1_size == sizeof(sha1.data));
+  if (!rv)
+    memset(sha1.data, 0, sizeof(sha1.data));
+  return sha1;
+}
+
 }  // namespace net
 
diff --git a/net/http/http_cache.cc b/net/http/http_cache.cc
index fe0a130..accd7da 100644
--- a/net/http/http_cache.cc
+++ b/net/http/http_cache.cc
@@ -855,6 +855,8 @@ void HttpCache::Transaction::OnNetworkInfoAvailable(int result) {
       if (mode_ == READ_WRITE) {
         if (new_response->headers->response_code() == 304) {
           // Update cached response based on headers in new_response
+          // TODO(wtc): should we update cached certificate
+          // (response_.ssl_info), too?
           response_.headers->Update(*new_response->headers);
           if (response_.headers->HasHeaderValue("cache-control", "no-store")) {
             cache_->DoomEntry(cache_key_);