Create stubs for system certificate validation.

net/base/multi_threaded_cert_verifier.h

Index: net/base/multi_threaded_cert_verifier.h
diff --git a/net/base/multi_threaded_cert_verifier.h b/net/base/multi_threaded_cert_verifier.h
index 78f372ea34a5267eaddd9c8feb1f56000e7c0098..fbd385892a9057988d71d68be024477e213b2b9f 100644
--- a/net/base/multi_threaded_cert_verifier.h
+++ b/net/base/multi_threaded_cert_verifier.h
@@ -11,6 +11,7 @@
 
 #include "base/basictypes.h"
 #include "base/gtest_prod_util.h"
+#include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/threading/non_thread_safe.h"
 #include "net/base/cert_database.h"
@@ -25,12 +26,38 @@ namespace net {
 
 // MultiThreadedCertVerifier is a CertVerifier implementation that runs
 // synchronous CertVerifier implementations on worker threads.
-class NET_EXPORT MultiThreadedCertVerifier :
+class MultiThreadedCertVerifier :
     public CertVerifier,
     NON_EXPORTED_BASE(public base::NonThreadSafe),

[wtc, 2012/03/06 23:10:14]

Maybe NON_EXPORTED_BASE can be removed now?

[Ryan Sleevi, 2012/03/10 03:09:12]

The removal of NET_EXPORT was a mistake. It should be NET_EXPORT_PRIVATE so that
it can be unit tested.

As a result, per
http://src.chromium.org/viewvc/chrome/trunk/src/base/threading/non_thread_saf...
, this is still needed.

     public CertDatabase::Observer {
  public:
-  MultiThreadedCertVerifier();
+  // Class to perform the actual certificate validation on a worker thread.
+  // This class MUST be thread-safe, as it will be called concurrently on
+  // multiple worker threads.
+  // Note: Because these worker threads run within a worker pool, VerifyProc
+  // implementations must be careful about using other objects, such as
+  // MessageLoops, Singletons, etc, as these objects may no longer exist
+  // during shutdown.
+  class VerifyProc : public base::RefCountedThreadSafe<VerifyProc> {

[wtc, 2012/03/06 23:10:14]

Why does VerifyProc need to be a class?  Isn't it a
function?  None of the concrete subclasses of VerifyProc
in this CL have data members.

Why does the VerifyProc class need to be ref-counted?

[Ryan Sleevi, 2012/03/10 03:09:12]

Using function pointers outside of PPAPI and unittests seems to be exceedingly
rare in Chromium code. Further, by making something a function pointer, you
guarantee that no state can ever be used. It's one thing to say a function must
be thread safe and not touch MessageLoops - but quite another to say it can
never use any variables.

That said, considering the end goal is to have all the Mocking logic at the
CertVerifier layer, and not at the VerifyProc layer, I went ahead and split it
into function pointers.
VerifyProc's class lifetime needs to be as long as any of the worker threads -
as the worker threads need to be able to call into it. The alternative to making
the VerifyProc thread-safe would have been to have an inner "Core" class that
could be detached from the main IO thread when the MTCV went out of scope. While
functionally equivalent, forcing VerifyProc to be RCTS is a way to ensure
clients' consider thread-safety as inherent to the class.

That said, in light of the above comments, it's a non-issue.

Note: See also HostResolverProc, which is a class that is RCTS.

+   public:
+    // Performs a synchronous verification of |cert| for the specified
+    // |hostname|.
+    // The arguments mirror those of CertVerifier::Verify().
+    // Note: Multiple calls to Verify() may be running in parallel, so
+    // any state inside of |this| should not mutate.
+    virtual int Verify(X509Certificate* cert,
+                       const std::string& hostname,
+                       int flags,
+                       CRLSet* crl_set,
+                       CertVerifyResult* verify_result) = 0;
+
+   protected:
+    friend class base::RefCountedThreadSafe<VerifyProc>;
+
+    virtual ~VerifyProc() {}
+  };
+
+  explicit MultiThreadedCertVerifier(VerifyProc* verifier);
 
   // When the verifier is destroyed, all certificate verifications requests are
   // canceled, and their completion callbacks will not be called.
@@ -126,6 +153,8 @@ class NET_EXPORT MultiThreadedCertVerifier :
   // place.
   std::map<RequestParams, CertVerifierJob*> inflight_;
 
+  scoped_refptr<VerifyProc> verifier_;

[wtc, 2012/03/06 23:10:14]

Please name this member verify_proc_.  verifier_ suggests
it's a CertVerifier.

Make the same change to the "VerifyProc* verifier" argument
to the constructor on line 60 above.

+
   uint64 requests_;
   uint64 cache_hits_;
   uint64 inflight_joins_;