Update CertVerifier::Verify to use RequestParams instead

net/cert/cert_verifier.h

Index: net/cert/cert_verifier.h
diff --git a/net/cert/cert_verifier.h b/net/cert/cert_verifier.h
index 4e36688c5a188d29c453b216955325dec804ee82..092b8a9cd501f22ad07eb66635924539e5321484 100644
--- a/net/cert/cert_verifier.h
+++ b/net/cert/cert_verifier.h
@@ -10,6 +10,7 @@
 #include <vector>
 
 #include "base/macros.h"
+#include "base/memory/ref_counted.h"
 #include "net/base/completion_callback.h"
 #include "net/base/hash_value.h"
 #include "net/base/net_export.h"
@@ -76,30 +77,49 @@ class NET_EXPORT CertVerifier {
     VERIFY_REV_CHECKING_REQUIRED_LOCAL_ANCHORS = 1 << 4,
   };
 
-  // The parameters for doing a Verify(). |certificate|, |hostname|, and
-  // |flags| are required. The rest are optional.
+  // Verification parameters to verify |certificate| against the supplied

[eroman, 2016/05/20 00:41:18]

nit: "Verification parameter to verify" --> Parameters for verifying
|certificate|

(or other re-wording to not say "verify" twice).

+  // |hostname| as an SSL server.

[eroman, 2016/05/20 00:41:18]

Should this comment document expectation around the format of |hostname| ? (is
it expected to be canonicalized, does it not matter).

+  //
+  // |flags| is a bitwise OR of VerifyFlags.
+  //
+  // |ocsp_response| is optional, but if non-empty, should contain an OCSP
+  // response obtained via OCSP stapling. It may be ignored by the
+  // CertVerifier.
+  //
+  // |additional_trust_anchors| is optional, but if non-empty, should contain
+  // additional certificates to be treated as trust anchors. It may be ignored
+  // by the CertVerifier.
   class NET_EXPORT RequestParams {
    public:
-    RequestParams(X509Certificate* certificate,
+    RequestParams(scoped_refptr<X509Certificate> certificate,
                   const std::string& hostname,
                   int flags,
                   const std::string& ocsp_response,
-                  const CertificateList& additional_trust_anchors);
+                  CertificateList additional_trust_anchors);
     RequestParams(const RequestParams& other);
     ~RequestParams();
 
+    const scoped_refptr<X509Certificate>& certificate() const {
+      return certificate_;
+    }
     const std::string& hostname() const { return hostname_; }
     int flags() const { return flags_; }
-    const std::vector<SHA1HashValue> request_data() const {
-      return request_data_;
+    const std::string& ocsp_response() const { return ocsp_response_; }
+    const CertificateList& additional_trust_anchors() const {
+      return additional_trust_anchors_;
     }
 
     bool operator<(const RequestParams& other) const;
 
    private:
+    scoped_refptr<X509Certificate> certificate_;
     std::string hostname_;

[eroman, 2016/05/20 00:41:18]

I suggest marking these all as  const.

     int flags_;
-    std::vector<SHA1HashValue> request_data_;
+    std::string ocsp_response_;
+    CertificateList additional_trust_anchors_;
+
+    // Used to optimize sorting/indexing comparisons.
+    std::string key_;
   };
 
   // When the verifier is destroyed, all certificate verification requests are
@@ -115,17 +135,6 @@ class NET_EXPORT CertVerifier {
   // |verify_result->cert_status|, and the error code for the most serious
   // error is returned.
   //
-  // |ocsp_response|, if non-empty, is a stapled OCSP response to use.
-  //
-  // |flags| is bitwise OR'd of VerifyFlags.
-  // If VERIFY_REV_CHECKING_ENABLED is set in |flags|, certificate revocation
-  // checking is performed.
-  //
-  // If VERIFY_EV_CERT is set in |flags| too, EV certificate verification is
-  // performed.  If |flags| is VERIFY_EV_CERT (that is,
-  // VERIFY_REV_CHECKING_ENABLED is not set), EV certificate verification will
-  // not be performed.
-  //
   // |crl_set| points to an optional CRLSet structure which can be used to
   // avoid revocation checks over the network.
   //
@@ -140,12 +149,7 @@ class NET_EXPORT CertVerifier {
   // If Verify() completes synchronously then |out_req| *may* be reset to
   // nullptr. However it is not guaranteed that all implementations will reset
   // it in this case.
-  //
-  // TODO(rsleevi): Update this to use RequestParams as part of the signature.
-  virtual int Verify(X509Certificate* cert,
-                     const std::string& hostname,
-                     const std::string& ocsp_response,
-                     int flags,
+  virtual int Verify(const RequestParams& params,
                      CRLSet* crl_set,
                      CertVerifyResult* verify_result,
                      const CompletionCallback& callback,