Add support for walking and modifying the CachingCertVerifier

net/cert/caching_cert_verifier.h

Index: net/cert/caching_cert_verifier.h
diff --git a/net/cert/caching_cert_verifier.h b/net/cert/caching_cert_verifier.h
index 523f72c2982b4d1b0ed04a31209fdaf992184ce3..b36b72689754f7f98a1ab3836442ff143a9bfc62 100644
--- a/net/cert/caching_cert_verifier.h
+++ b/net/cert/caching_cert_verifier.h
@@ -36,6 +36,21 @@ class CertTrustAnchorProvider;
 class NET_EXPORT CachingCertVerifier : public CertVerifier,
                                        public CertDatabase::Observer {
  public:
+  // Visitor class to allow read-only inspection of the verification cache.

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

optional: Either here or in VisitEntries() mention that the CachingCertVerifier
should not be mutated, as it may invalidate iteration.

+  class NET_EXPORT CacheVisitor {
+   public:
+    virtual ~CacheVisitor() {}

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

optional: Move to an out-of-line dtor in .cc file, and use "= default"

+
+    // Called once for each entry in the cache, providing details about the
+    // cached entry.
+    // Returns true to continue iteration, or false to abort.
+    virtual bool VisitEntry(const RequestParams& params,

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

optional: Name this VisitCertificateCacheEntry(). The more specific name may be
convenient if you want to smash it into an existing class.

Or alternately instead of a visitor class use a callback -- this might be
convenient for callers wanting to writing an inline visitor through lambda, or
base::Bind()ing stuff.

At any rate, I am happy with the visitor class as written.

[Ryan Sleevi, 2016/06/16 00:44:34]

I'm pretty anti-smashing of pure interfaces :)
They wouldn't be able to mix a lambda with a callback at present, and
callbacks-called-multiple-times are an anti-pattern (e.g. if you're curring an
owned pointer, everything gets wrong).
SG, opted for no change.

+                            int error,
+                            const CertVerifyResult& verify_result,
+                            base::Time verification_time,
+                            base::Time expiration_time) = 0;
+  };
+
   // Creates a CachingCertVerifier that will use |verifier| to perform the
   // actual verifications if they're not already cached or if the cached
   // item has expired.
@@ -59,8 +74,27 @@ class NET_EXPORT CachingCertVerifier : public CertVerifier,
              const BoundNetLog& net_log) override;
   bool SupportsOCSPStapling() override;
 
+  // Opportunistically attempts to add |error| and |verify_result| as the
+  // 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).
+  bool AddEntry(const RequestParams& params,
+                int error,
+                const CertVerifyResult& verify_result,
+                base::Time verification_time);
+
+  // Iterates through all of the non-expired entries in the cache, calling
+  // VisitEntry on |visitor| for each, until either all entries are
+  // iterated through or the |visitor| aborts.
+  void VisitEntries(CacheVisitor* visitor);

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

Can this method be "const" ?

[Ryan Sleevi, 2016/06/16 00:44:34]

I saw no reason to impose that contract on implementations, since from the POV
of this interface, the const-ness is irrelevant, but imposing it would be
limiting (e.g. if you wanted to build up a serialized version in a member field)

[eroman, 2016/06/16 00:54:51]

I think you misunderstood. What I am asking for is:

  void VisitEntries(CacheVisitor* visitor) const;

+
  private:
+  friend class CacheVisitor;

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

Is this needed?

[Ryan Sleevi, 2016/06/16 00:44:34]

Nope

   FRIEND_TEST_ALL_PREFIXES(CachingCertVerifierTest, CacheHit);
+  FRIEND_TEST_ALL_PREFIXES(CachingCertVerifierTest, Visitor);
+  FRIEND_TEST_ALL_PREFIXES(CachingCertVerifierTest, AddsEntries);
   FRIEND_TEST_ALL_PREFIXES(CachingCertVerifierTest, DifferentCACerts);
   FRIEND_TEST_ALL_PREFIXES(CachingCertVerifierTest, CertTrustAnchorProvider);