Update CertVerifier::Verify to use RequestParams instead

net/cert/cert_verifier.h

 // Copyright (c) 2012 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_CERT_VERIFIER_H_
 #define NET_CERT_CERT_VERIFIER_H_
 
 #include <memory>
 #include <string>
 #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"
 #include "net/cert/x509_certificate.h"
 
 namespace net {
 
 class BoundNetLog;
 class CertVerifyResult;
 class CRLSet;
@@ skipping 46 matching lines @@
     // for certificates issued by non-public trust anchors. Failure to check
     // revocation is treated as a hard failure.
     // Note: If VERIFY_CERT_IO_ENABLE is not also supplied, certificates
     // that chain to local trust anchors will likely fail - for example, due to
     // lacking fresh cached revocation issue (Windows) or because OCSP stapling
     // can only provide information for the leaf, and not for any
     // intermediates.
     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
   // canceled, and their completion callbacks will not be called.
   virtual ~CertVerifier() {}
 
   // Verifies the given certificate against the given hostname as an SSL server.
   // Returns OK if successful or an error code upon failure.
   //
   // The |*verify_result| structure, including the |verify_result->cert_status|
   // bitmask, is always filled out regardless of the return value.  If the
   // certificate has multiple errors, the corresponding status flags are set in
   // |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.
   //
   // |callback| must not be null.  ERR_IO_PENDING is returned if the operation
   // could not be completed synchronously, in which case the result code will
   // be passed to the callback when available.
   //
   // On asynchronous completion (when Verify returns ERR_IO_PENDING) |out_req|
   // will be reset with a pointer to the request. Freeing this pointer before
   // the request has completed will cancel it.
   //
   // 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,
                      std::unique_ptr<Request>* out_req,
                      const BoundNetLog& net_log) = 0;
 
   // Returns true if this CertVerifier supports stapled OCSP responses.
   virtual bool SupportsOCSPStapling();
 
   // Creates a CertVerifier implementation that verifies certificates using
   // the preferred underlying cryptographic libraries.
   static std::unique_ptr<CertVerifier> CreateDefault();
 };
 
 }  // namespace net
 
 #endif  // NET_CERT_CERT_VERIFIER_H_