Update CertVerifier::Verify to use RequestParams instead

net/cert/multi_threaded_cert_verifier.cc

Index: net/cert/multi_threaded_cert_verifier.cc
diff --git a/net/cert/multi_threaded_cert_verifier.cc b/net/cert/multi_threaded_cert_verifier.cc
index 9b38160286d403f900e101becb6ac79e4b843fc5..82fa6a4d5e5c808bce2d1a9f399dbe413511354d 100644
--- a/net/cert/multi_threaded_cert_verifier.cc
+++ b/net/cert/multi_threaded_cert_verifier.cc
@@ -258,7 +258,6 @@ class CertVerifierJob {
  public:
   CertVerifierJob(const CertVerifier::RequestParams& key,
                   NetLog* net_log,
-                  X509Certificate* cert,
                   MultiThreadedCertVerifier* cert_verifier)
       : key_(key),
         start_time_(base::TimeTicks::Now()),
@@ -267,9 +266,9 @@ class CertVerifierJob {
         cert_verifier_(cert_verifier),
         is_first_job_(false),
         weak_ptr_factory_(this) {
-    net_log_.BeginEvent(
-        NetLog::TYPE_CERT_VERIFIER_JOB,
-        base::Bind(&NetLogX509CertificateCallback, base::Unretained(cert)));
+    net_log_.BeginEvent(NetLog::TYPE_CERT_VERIFIER_JOB,
+                        base::Bind(&NetLogX509CertificateCallback,
+                                   base::Unretained(key.certificate().get())));
   }
 
   // Indicates whether this was the first job started by the CertVerifier. This
@@ -282,12 +281,7 @@ class CertVerifierJob {
   // verification has completed on the worker thread, it will call
   // OnJobCompleted() on the origin thread.
   bool Start(const scoped_refptr<CertVerifyProc>& verify_proc,
-             const scoped_refptr<X509Certificate>& cert,
-             const std::string& hostname,
-             const std::string& ocsp_response,
-             int flags,
-             const scoped_refptr<CRLSet>& crl_set,
-             const CertificateList& additional_trust_anchors) {
+             const scoped_refptr<CRLSet>& crl_set) {
     // Owned by the bound reply callback.
     std::unique_ptr<MultiThreadedCertVerifier::CachedResult> owned_result(
         new MultiThreadedCertVerifier::CachedResult());
@@ -298,9 +292,10 @@ class CertVerifierJob {
 
     return base::WorkerPool::PostTaskAndReply(
         FROM_HERE,
-        base::Bind(&DoVerifyOnWorkerThread, verify_proc, cert, hostname,
-                   ocsp_response, flags, crl_set, additional_trust_anchors,
-                   &result->error, &result->result),
+        base::Bind(&DoVerifyOnWorkerThread, verify_proc, key_.certificate(),
+                   key_.hostname(), key_.ocsp_response(), key_.flags(), crl_set,
+                   key_.additional_trust_anchors(), &result->error,
+                   &result->result),
         base::Bind(&CertVerifierJob::OnJobCompleted,
                    weak_ptr_factory_.GetWeakPtr(), base::Passed(&owned_result)),
         true /* task is slow */);
@@ -423,10 +418,7 @@ void MultiThreadedCertVerifier::SetCertTrustAnchorProvider(
   trust_anchor_provider_ = trust_anchor_provider;
 }
 
-int MultiThreadedCertVerifier::Verify(X509Certificate* cert,
-                                      const std::string& hostname,
-                                      const std::string& ocsp_response,
-                                      int flags,
+int MultiThreadedCertVerifier::Verify(const CertVerifier::RequestParams& params,
                                       CRLSet* crl_set,
                                       CertVerifyResult* verify_result,
                                       const CompletionCallback& callback,
@@ -436,18 +428,22 @@ int MultiThreadedCertVerifier::Verify(X509Certificate* cert,
 
   DCHECK(CalledOnValidThread());
 
-  if (callback.is_null() || !verify_result || hostname.empty())
+  if (callback.is_null() || !verify_result || params.hostname().empty())
     return ERR_INVALID_ARGUMENT;
 
   requests_++;
 
-  const CertificateList empty_cert_list;
-  const CertificateList& additional_trust_anchors =
-      trust_anchor_provider_ ?
-          trust_anchor_provider_->GetAdditionalTrustAnchors() : empty_cert_list;
+  CertificateList new_trust_anchors(params.additional_trust_anchors());
+  if (trust_anchor_provider_) {
+    new_trust_anchors.insert(
+        new_trust_anchors.end(),
+        trust_anchor_provider_->GetAdditionalTrustAnchors().begin(),

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

My expectation is for GetAdditionalTrustAnchors() to return a CertificateList.
But it does in fact return a |const CertificateList&| so this could be right.

But certainly it looks very fishy when reading it.

Do you have a suggestion for improving this? Or just kill off TrustAnchor
Provider.

[Ryan Sleevi, 2016/05/20 02:39:43]

Why? It's the same as GetIntermediateCertificates() on X509Certificate.
Killing off TAP is the next cleanup, once this lands. Well, ok, second to next -
killing off fingerprint/ca_fingerprint is next, then killing off TAP as it's
unnecessary once PolicyCertVerifier can just do the injection (that is, moving
this entirely out of MTCV, or CachingCertVerifier where I next move it up to -
that is, I'm gently shuffling the TAP into the higher layer with each CL)

+        trust_anchor_provider_->GetAdditionalTrustAnchors().end());
+  }
 
-  const CertVerifier::RequestParams key(cert, hostname, flags, ocsp_response,
-                                        additional_trust_anchors);
+  const CertVerifier::RequestParams key(params.certificate(), params.hostname(),
+                                        params.flags(), params.ocsp_response(),
+                                        new_trust_anchors);
   const CertVerifierCache::value_type* cached_entry =
       cache_.Get(key, CacheValidityPeriod(base::Time::Now()));
   if (cached_entry) {
@@ -465,10 +461,9 @@ int MultiThreadedCertVerifier::Verify(X509Certificate* cert,
   } else {
     // Need to make a new job.
     std::unique_ptr<CertVerifierJob> new_job(
-        new CertVerifierJob(key, net_log.net_log(), cert, this));
+        new CertVerifierJob(key, net_log.net_log(), this));
 
-    if (!new_job->Start(verify_proc_, cert, hostname, ocsp_response, flags,
-                        crl_set, additional_trust_anchors)) {
+    if (!new_job->Start(verify_proc_, crl_set)) {
       // TODO(wtc): log to the NetLog.
       LOG(ERROR) << "CertVerifierJob couldn't be started.";
       return ERR_INSUFFICIENT_RESOURCES;  // Just a guess.