WKWebView: Added cert verification API to web controller.

ios/web/net/cert_verifier_block_adapter.h

Index: ios/web/net/cert_verifier_block_adapter.h
diff --git a/ios/web/net/cert_verifier_block_adapter.h b/ios/web/net/cert_verifier_block_adapter.h
index ee9829e68c4db76564129da3192602ef7b3d9b6c..76ccaa114feb58122a65de488119ea2bbccf86b5 100644
--- a/ios/web/net/cert_verifier_block_adapter.h
+++ b/ios/web/net/cert_verifier_block_adapter.h
@@ -6,34 +6,39 @@
 #define IOS_WEB_NET_CERT_VERIFIER_BLOCK_ADAPTER_H_
 
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/scoped_vector.h"
+#include "base/threading/thread_checker.h"
 #include "net/cert/cert_verifier.h"
-#include "net/log/net_log.h"
+#include "net/cert/cert_verify_result.h"
 
 namespace net {
 
-class CertVerifyResult;
 class CRLSet;
+class NetLog;
 class X509Certificate;
 
-// Provides block-based interface for net::CertVerifier.
+// Provides block-based interface for net::CertVerifier. This class can be
+// created and used on any thread as long as it's the same thread where
+// |CertVerifier| was created.
 class CertVerifierBlockAdapter {
  public:
-  CertVerifierBlockAdapter();
-  // Constructs adapter with given |CertVerifier| which can not be null.
-  CertVerifierBlockAdapter(scoped_ptr<CertVerifier> cert_verifier);
+  // Constructs adapter with given |CertVerifier| and |NetLog|, both can not be
+  // null. CertVerifierBlockAdapter does NOT take ownership over |cert_verifier|

[davidben, 2015/08/19 18:51:45]

Nit: over -> of?

[Eugene But (OOO till 7-30), 2015/08/20 01:14:41]

Done.

+  // and |net_log|.
+  CertVerifierBlockAdapter(CertVerifier* cert_verifier, NetLog* net_log);
 
   // When the verifier is destroyed, all certificate verification requests are
   // canceled, and their completion handlers will not be called.
   ~CertVerifierBlockAdapter();
 
-  // Encapsulates verification parms. |cert| and |hostname| are mandatory, the
+  // Encapsulates verification params. |cert| and |hostname| are mandatory, the
   // other params are optional. If either of mandatory arguments is null or
   // empty then verification |CompletionHandler| will be called with
-  // ERR_INVALID_ARGUMENT status.
+  // ERR_INVALID_ARGUMENT status_result.
   struct Params {
     // Constructs Params from X509 cert and hostname, which are mandatory for
     // verification.
-    Params(scoped_refptr<net::X509Certificate> cert,
+    Params(const scoped_refptr<net::X509Certificate>& cert,

[davidben, 2015/08/19 18:51:45]

Nit: Stray net:: prefix?

(Arguably having something in //ios/web squat in //net's namespace is already
kinda weird, but I'm not going to insist you unnamespace it or anything.)

[Eugene But (OOO till 7-30), 2015/08/20 01:14:41]

This lives in //ios/web/net, and it's more net than web, I think.

[davidben, 2015/08/20 01:25:27]

That's true, but stomping in another module's namespace is very much not
reasonable. Anyway, this is an existing bug, so we should leave it as it is for
this CL. But I do not think this is correct.

[Eugene But (OOO till 7-30), 2015/08/20 03:08:01]

Changed to web:: namespace, because all ios/web/net symbols are in web::. 

            const std::string& hostname);
     ~Params();
 
@@ -47,28 +52,34 @@ class CertVerifierBlockAdapter {
     std::string ocsp_response;
 
     // Bitwise OR of CertVerifier::VerifyFlags.
-    CertVerifier::VerifyFlags flags;
+    int flags;
 
     // An optional CRLSet structure which can be used to avoid revocation checks
     // over the network.
     scoped_refptr<CRLSet> crl_set;
   };
 
-  // Type of verification completion block. On success CertVerifyResult is not
-  // null and status is OK, otherwise CertVerifyResult is null and status is a
-  // net error code.
-  typedef void (^CompletionHandler)(scoped_ptr<CertVerifyResult>, int status);
+  // Type of verification completion block. If cert is successfully validated
+  // |status_result| is OK, otherwise |status_result| is a net error code.
+  typedef void (^CompletionHandler)(CertVerifyResult cert_verify_result,
+                                    int status_result);

[davidben, 2015/08/19 18:51:45]

This name tripped me up rather badly because of the word "status" and the
overlap with cert_status. Why not just result? status_result is a pretty
unhelpful name I think. Or maybe just error. (Ditto throughout the tests and
elsewhere.)

[Eugene But (OOO till 7-30), 2015/08/20 01:14:41]

Just |result| will overlap with CertVerifyResult. Changed to |error| (as far as
I can see this name is used in other places as well).

 
   // Verifies certificate with given |params|. |completion_handler| must not be
-  // null and call be called either syncronously (in the same runloop) or
-  // asyncronously.
+  // null and can be called either synchronously (in the same runloop) or
+  // asynchronously.
   void Verify(const Params& params, CompletionHandler completion_handler);
 
  private:
-  // Underlying CertVerifier.
-  scoped_ptr<CertVerifier> cert_verifier_;
-  // Net Log required by CertVerifier.
-  BoundNetLog net_log_;
+  // Pending verification requests. Request must be alive until verification is
+  // completed, otherwise verification operation will be cancelled.
+  ScopedVector<CertVerifier::Request> pending_requests_;
+  // Underlying unowned CertVerifier.
+  CertVerifier* cert_verifier_;
+  // Unowned NetLog required by CertVerifier.
+  NetLog* net_log_;
+  // CertVerifierBlockAdapter should be used on the same thread where it was
+  // created.
+  base::ThreadChecker thread_checker_;
 };
 
 }  // net