Add support for walking and modifying the CachingCertVerifier

net/cert/caching_cert_verifier.h

 // Copyright 2016 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_CERT_CACHING_CERT_VERIFIER_H_
 #define NET_CERT_CACHING_CERT_VERIFIER_H_
 
 #include <memory>
 
 #include "net/base/expiring_cache.h"
 #include "net/base/net_export.h"
 #include "net/cert/cert_database.h"
 #include "net/cert/cert_verifier.h"
 #include "net/cert/cert_verify_result.h"
 
 namespace net {
 
 class CertTrustAnchorProvider;
 
 class NET_EXPORT CachingCertVerifier : public CertVerifier,
                                        public CertDatabase::Observer {
  public:
+  // Provides a read-only iterator over items in the cache.
+  //
+  // This iterator is invalidated by any modifications to the cache, such
+  // as by a call to Verify() or to AddEntry().
+  class NET_EXPORT Iterator {
+   public:
+    explicit Iterator(const CachingCertVerifier& verifier);

[eroman, 2016/06/13 22:49:35]

I would have said this is clearer as a pointer rather than reference type given
how this is coupled to the lifetime of |verifier|.... But looks like the
underlying cache iterator using this same interface.

Can you  add a comment explaining that |verifier| needs to outlive |this| ?
(Technically a bit stronger requirement, but easier to explain).

+    ~Iterator();
+
+    bool HasNext() const;

[eroman, 2016/06/13 22:49:35]

Iteration currently exposes expired entries right?

Should a compaction be done before iterating, or is the expectation on the
client to not bother serializing expired entries?

+    void Advance();
+
+    const RequestParams& params() const;
+    int error() const;
+    const CertVerifyResult& verify_result() const;
+    base::Time verification_time() const;
+    base::Time expiration_time() const;
+
+   private:
+    void* iter_;
+  };

[eroman, 2016/06/13 22:49:35]

Disallow copy and assign.

+
   explicit CachingCertVerifier(std::unique_ptr<CertVerifier> verifier);
 
   ~CachingCertVerifier() override;
 
   // Configures a source of additional certificates that should be treated as
   // trust anchors during verification, provided that the underlying
   // CertVerifyProc supports additional trust beyond the default implementation.
   // It must outlive the CachingCertVerifier.
   void SetCertTrustAnchorProvider(
       CertTrustAnchorProvider* trust_anchor_provider);
 
   // CertVerifier implementation:
   int Verify(const RequestParams& params,
              CRLSet* crl_set,
              CertVerifyResult* verify_result,
              const CompletionCallback& callback,
              std::unique_ptr<Request>* out_req,
              const BoundNetLog& net_log) override;
   bool SupportsOCSPStapling() override;
 
+  // Opportunistically attempt to add |error| and |verify_result| as the

[eroman, 2016/06/13 22:49:35]

style nit: use descriptive rather than imperative comment. i.e. "attempt" -->
"attempts"

+  // result for |params|, which was obtained at |verification_time| and
+  // expires at |expiration_time|.
+  // This is opportunistic because it is not guaranteed that the entry
+  // will be added (such as if the cache is full or an entry already
+  // exists).

[eroman, 2016/06/13 22:49:35]

nit: Mention how how this relates to the return value.

[eroman, 2016/06/16 00:25:04]

ping? (My request was to comment that returns true if an entry was added)

+  bool AddEntry(const RequestParams& params,
+                int error,
+                const CertVerifyResult& verify_result,
+                base::Time verification_time);
+
  private:
+  friend class Iterator;
   FRIEND_TEST_ALL_PREFIXES(CachingCertVerifierTest, CacheHit);
   FRIEND_TEST_ALL_PREFIXES(CachingCertVerifierTest, DifferentCACerts);
   FRIEND_TEST_ALL_PREFIXES(CachingCertVerifierTest, CertTrustAnchorProvider);
 
   // CachedResult contains the result of a certificate verification.
   struct NET_EXPORT_PRIVATE CachedResult {
     CachedResult();
     ~CachedResult();
 
     int error;                // The return value of CertVerifier::Verify.
@@ skipping 61 matching lines @@
 
   uint64_t requests_;
   uint64_t cache_hits_;
 
   DISALLOW_COPY_AND_ASSIGN(CachingCertVerifier);
 };
 
 }  // namespace net
 
 #endif  // NET_CERT_CACHING_CERT_VERIFIER_H_