WKWebView: Added cert verification API to web controller.

ios/web/net/cert_verifier_block_adapter_unittest.cc

Index: ios/web/net/cert_verifier_block_adapter_unittest.cc
diff --git a/ios/web/net/cert_verifier_block_adapter_unittest.cc b/ios/web/net/cert_verifier_block_adapter_unittest.cc
index 1380045e16e79eee8386043d1cbf9474262a776e..d4e90163d04a40fa3cf00d0bd2725598e94fbdaf 100644
--- a/ios/web/net/cert_verifier_block_adapter_unittest.cc
+++ b/ios/web/net/cert_verifier_block_adapter_unittest.cc
@@ -4,11 +4,18 @@
 
 #include "ios/web/net/cert_verifier_block_adapter.h"
 
+#include "base/location.h"
 #include "base/test/ios/wait_util.h"
+#include "ios/web/public/test/test_web_thread_bundle.h"
+#include "ios/web/public/web_thread.h"
 #include "net/base/net_errors.h"
+#include "net/base/test_data_directory.h"
+#include "net/cert/cert_verifier.h"
 #include "net/cert/cert_verify_result.h"
 #include "net/cert/crl_set.h"
 #include "net/cert/x509_certificate.h"
+#include "net/log/net_log.h"
+#include "net/test/cert_test_util.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/platform_test.h"
 
@@ -17,9 +24,10 @@ namespace net {
 using testing::_;
 
 namespace {
-
+// Test cert filename.
+const char kCertFileName[] = "2029_globalsign_com_cert.pem";
 // Test hostname for CertVerifier.
-const char kHostName[] = "chromium.org";
+const char kHostName[] = "2029.globalsign.com";

[Ryan Sleevi, 2015/08/14 02:29:44]

use ok_cert.pem, if you're just looking for a random file.

The README in that directory is meant to guide to well-supported files for
different purposes :)

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

Done. ok_cert.pem works for me.

 // Test OCSP response for CertVerifier.
 const char kOcspResponse[] = "ocsp";
 
@@ -44,8 +52,9 @@ ACTION_P(SetVerifyResult, result) {
 }
 
 // Calls CompletionCallback to emulate CertVerifier behavior.
-ACTION(RunCallback) {
-  arg6.Run(0);
+ACTION_P(RunCallback, status_result) {
+  web::WebThread::PostTask(web::WebThread::IO, FROM_HERE,

[Ryan Sleevi, 2015/08/14 02:29:44]

This doesn't seem right. Nothing in your test asserts it's on the IO thread
(e.g. the test starting at line 124).

base::ThreadTaskRunnerHandle::Get()->PostTask(...) runs the callback on the same
thread you called Verify() on, and is consistent from an API contract.

Put differently, what if someone made the WebThread::IO a different thread than
your main test? It would reveal that your test is encoding the wrong
expectations.

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

Done.

+                           base::Bind(arg6, status_result));
 }
 
 }  // namespace
@@ -53,154 +62,184 @@ ACTION(RunCallback) {
 // Test fixture to test CertVerifierBlockAdapter class.
 class CertVerifierBlockAdapterTest : public PlatformTest {
  protected:
-  void SetUp() override {
-    PlatformTest::SetUp();
-
-    cert_ = new X509Certificate("test", "test", base::Time(), base::Time());
-    scoped_ptr<CertVerifierMock> cert_verifier_mock(new CertVerifierMock());
-    cert_verifier_mock_ = cert_verifier_mock.get();
-    test_adapter_.reset(
-        new CertVerifierBlockAdapter(cert_verifier_mock.Pass()));
-  }
+  CertVerifierBlockAdapterTest()
+      : thread_bundle_(web::TestWebThreadBundle::REAL_IO_THREAD),
+        cert_(ImportCertFromFile(GetTestCertsDirectory(), kCertFileName)),

[Ryan Sleevi, 2015/08/14 02:29:43]

Shouldn't this be in SetUp() so that you can ASSERT_TRUE(cert_) to make sure
you're not going to crash the test harness?

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

I added DCHECK to constructor. In GTest Constructors are more preferable than
SetUp:
https://code.google.com/p/googletest/wiki/FAQ#Should_I_use_the_constructor/de...

[Ryan Sleevi, 2015/08/14 21:43:53]

I specifically said SetUp because that's what it's for. DCHECK should _never_ be
used in unit tests.

[Eugene But (OOO till 7-30), 2015/08/19 17:57:36]

Done.

+        test_adapter_(
+            new CertVerifierBlockAdapter(&cert_verifier_mock_, &net_log_)) {}
 
   // Performs synchronous verification.
   void Verify(CertVerifierBlockAdapter::Params params,
-              scoped_ptr<net::CertVerifyResult>* result,
-              int* status) {
+              net::CertVerifyResult* cert_verify_result,
+              int* status_result) {
     __block bool verification_completed = false;
     test_adapter_->Verify(params,
-                          ^(scoped_ptr<net::CertVerifyResult> callback_result,
-                            int callback_status) {
-                            *result = callback_result.Pass();
-                            *status = callback_status;
+                          ^(net::CertVerifyResult callback_cert_verify_result,
+                            int callback_status_result) {
+                            *cert_verify_result = callback_cert_verify_result;
+                            *status_result = callback_status_result;
                             verification_completed = true;
                           });
     base::test::ios::WaitUntilCondition(^{
       return verification_completed;
     });
   }
-
+  // IO Thread bundle.

[Ryan Sleevi, 2015/08/14 02:29:44]

Seems unnecessarily verbose

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

Done.

+  web::TestWebThreadBundle thread_bundle_;
   // Fake certificate created for testing.

[Ryan Sleevi, 2015/08/14 02:29:43]

It's not really a fake certificate, is it?

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

Removed.

   scoped_refptr<X509Certificate> cert_;
+  // CertVerifier mock.

[Ryan Sleevi, 2015/08/14 02:29:44]

As does this

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

Done.

+  CertVerifierMock cert_verifier_mock_;
+  // NetLog object required by CertVerifierBlockAdapter.

[Ryan Sleevi, 2015/08/14 02:29:43]

As does this

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

Done.

+  NetLog net_log_;
   // Testable |CertVerifierBlockAdapter| object.

[Ryan Sleevi, 2015/08/14 02:29:44]

As does this

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

Done.

   scoped_ptr<CertVerifierBlockAdapter> test_adapter_;
-  // CertVerifier mock owned by |test_adapter_|.
-  CertVerifierMock* cert_verifier_mock_;
 };
 
 // Tests |Verify| with default params and synchronous verification.
 TEST_F(CertVerifierBlockAdapterTest, DefaultParamsAndSync) {
   // Set up expectation.
-  net::CertVerifyResult expectedResult;
-  expectedResult.cert_status = net::CERT_STATUS_AUTHORITY_INVALID;
-  const int kExpectedStatus = 0;
-  EXPECT_CALL(*cert_verifier_mock_,
-              Verify(cert_.get(), kHostName, "", 0, nullptr, _, _, _, _))
+  net::CertVerifyResult expected_cert_verify_result;
+  expected_cert_verify_result.cert_status = net::CERT_STATUS_AUTHORITY_INVALID;
+  const int kExpectedStatus = OK;
+  EXPECT_CALL(cert_verifier_mock_, Verify(cert_.get(), kHostName, std::string(),
+                                          0, nullptr, _, _, _, _))
       .Times(1)
-      .WillOnce(testing::DoAll(SetVerifyResult(expectedResult),
+      .WillOnce(testing::DoAll(SetVerifyResult(expected_cert_verify_result),
                                testing::Return(kExpectedStatus)));
 
   // Call |Verify|.
-  scoped_ptr<CertVerifyResult> actualResult;
+  CertVerifyResult actual_cert_verify_result;
   int actualStatus = -1;
   CertVerifierBlockAdapter::Params params(cert_.get(), kHostName);
-  Verify(params, &actualResult, &actualStatus);
+  Verify(params, &actual_cert_verify_result, &actualStatus);
 
   // Ensure that Verification results are correct.
   EXPECT_EQ(kExpectedStatus, actualStatus);
-  EXPECT_EQ(expectedResult.cert_status, actualResult->cert_status);
+  EXPECT_EQ(expected_cert_verify_result.cert_status,
+            actual_cert_verify_result.cert_status);
 }
 
 // Tests |Verify| with default params and asynchronous verification.
 TEST_F(CertVerifierBlockAdapterTest, DefaultParamsAndAsync) {
   // Set up expectation.
-  net::CertVerifyResult expectedResult;
-  expectedResult.is_issued_by_known_root = true;
-  const int kExpectedStatus = 0;
-  EXPECT_CALL(*cert_verifier_mock_,
-              Verify(cert_.get(), kHostName, "", 0, nullptr, _, _, _, _))
+  net::CertVerifyResult expected_cert_verify_result;
+  expected_cert_verify_result.is_issued_by_known_root = true;
+  const int kExpectedStatus = OK;
+  EXPECT_CALL(cert_verifier_mock_, Verify(cert_.get(), kHostName, std::string(),
+                                          0, nullptr, _, _, _, _))
       .Times(1)
-      .WillOnce(testing::DoAll(SetVerifyResult(expectedResult), RunCallback(),
+      .WillOnce(testing::DoAll(SetVerifyResult(expected_cert_verify_result),
+                               RunCallback(kExpectedStatus),
                                testing::Return(ERR_IO_PENDING)));
 
   // Call |Verify|.
-  scoped_ptr<CertVerifyResult> actualResult;
+  CertVerifyResult actual_cert_verify_result;
   int actualStatus = -1;
   CertVerifierBlockAdapter::Params params(cert_.get(), kHostName);
-  Verify(params, &actualResult, &actualStatus);
+  Verify(params, &actual_cert_verify_result, &actualStatus);
 
   // Ensure that Verification results are correct.
   EXPECT_EQ(kExpectedStatus, actualStatus);
-  EXPECT_EQ(expectedResult.is_issued_by_known_root,
-            actualResult->is_issued_by_known_root);
+  EXPECT_EQ(expected_cert_verify_result.is_issued_by_known_root,
+            actual_cert_verify_result.is_issued_by_known_root);
+}
+
+// Tests |Verify| with invalid cert argument.
+TEST_F(CertVerifierBlockAdapterTest, InvalidCert) {
+  // Call |Verify|.
+  CertVerifyResult actual_cert_verify_result;
+  int actualStatus = -1;

[Ryan Sleevi, 2015/08/14 02:29:44]

naming (throughout this file) doesn't follow the C++ style guide.

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

Fixed actual_status name. I believe other names are correct.

+  CertVerifierBlockAdapter::Params params(nullptr, kHostName);
+  Verify(params, &actual_cert_verify_result, &actualStatus);
+
+  // Ensure that Verification results are correct.
+  EXPECT_EQ(ERR_INVALID_ARGUMENT, actualStatus);
+}
+
+// Tests |Verify| with invalid hostname argument.
+TEST_F(CertVerifierBlockAdapterTest, InvalidHostname) {
+  // Call |Verify|.
+  CertVerifyResult actual_cert_verify_result;
+  int actualStatus = -1;
+  CertVerifierBlockAdapter::Params params(cert_.get(), std::string());
+  Verify(params, &actual_cert_verify_result, &actualStatus);
+
+  // Ensure that Verification results are correct.
+  EXPECT_EQ(ERR_INVALID_ARGUMENT, actualStatus);
 }
 
-// Tests |Verify| with invalid arguments.
-TEST_F(CertVerifierBlockAdapterTest, InvalidParamsAndError) {
+// Tests |Verify| with synchronous error.
+TEST_F(CertVerifierBlockAdapterTest, DefaultParamsAndSyncError) {
   // Set up expectation.
-  const int kExpectedStatus = ERR_INVALID_ARGUMENT;
-  EXPECT_CALL(*cert_verifier_mock_,
-              Verify(nullptr, "", "", 0, nullptr, _, _, _, _))
+  const int kExpectedStatus = ERR_INSUFFICIENT_RESOURCES;
+  EXPECT_CALL(cert_verifier_mock_, Verify(cert_.get(), kHostName, std::string(),
+                                          0, nullptr, _, _, _, _))
       .Times(1)
       .WillOnce(testing::Return(kExpectedStatus));
 
   // Call |Verify|.
-  scoped_ptr<CertVerifyResult> actualResult;
+  CertVerifyResult actual_cert_verify_result;
   int actualStatus = -1;
-  CertVerifierBlockAdapter::Params params(nullptr, "");
-  Verify(params, &actualResult, &actualStatus);
+  CertVerifierBlockAdapter::Params params(cert_.get(), kHostName);
+  Verify(params, &actual_cert_verify_result, &actualStatus);
 
   // Ensure that Verification results are correct.
   EXPECT_EQ(kExpectedStatus, actualStatus);
-  EXPECT_FALSE(actualResult);
 }
 
-// Tests |Verify| with error.
-TEST_F(CertVerifierBlockAdapterTest, DefaultParamsAndError) {
+// Tests |Verify| with asynchronous error.
+TEST_F(CertVerifierBlockAdapterTest, DefaultParamsAndAsyncError) {
   // Set up expectation.
-  const int kExpectedStatus = ERR_INSUFFICIENT_RESOURCES;
-  EXPECT_CALL(*cert_verifier_mock_,
-              Verify(cert_.get(), kHostName, "", 0, nullptr, _, _, _, _))
+  net::CertVerifyResult expected_cert_verify_result;
+  expected_cert_verify_result.is_issued_by_known_root = true;
+  const int kExpectedStatus = ERR_ACCESS_DENIED;
+  EXPECT_CALL(cert_verifier_mock_, Verify(cert_.get(), kHostName, std::string(),
+                                          0, nullptr, _, _, _, _))
       .Times(1)
-      .WillOnce(testing::Return(kExpectedStatus));
+      .WillOnce(testing::DoAll(SetVerifyResult(expected_cert_verify_result),
+                               RunCallback(kExpectedStatus),
+                               testing::Return(ERR_IO_PENDING)));
 
   // Call |Verify|.
-  scoped_ptr<CertVerifyResult> actualResult;
+  CertVerifyResult actual_cert_verify_result;
   int actualStatus = -1;
   CertVerifierBlockAdapter::Params params(cert_.get(), kHostName);
-  Verify(params, &actualResult, &actualStatus);
+  Verify(params, &actual_cert_verify_result, &actualStatus);
 
   // Ensure that Verification results are correct.
   EXPECT_EQ(kExpectedStatus, actualStatus);
-  EXPECT_FALSE(actualResult);
+  EXPECT_EQ(expected_cert_verify_result.is_issued_by_known_root,
+            actual_cert_verify_result.is_issued_by_known_root);
 }
 
 // Tests |Verify| with all params and synchronous verification.
 TEST_F(CertVerifierBlockAdapterTest, AllParamsAndSync) {
   // Set up expectation.
-  net::CertVerifyResult expectedResult;
-  expectedResult.verified_cert = cert_;
-  const int kExpectedStatus = 0;
+  net::CertVerifyResult expected_cert_verify_result;
+  expected_cert_verify_result.verified_cert = cert_;
+  const int kExpectedStatus = OK;
   scoped_refptr<CRLSet> crl_set(CRLSet::EmptyCRLSetForTesting());
-  EXPECT_CALL(*cert_verifier_mock_,
+  EXPECT_CALL(cert_verifier_mock_,
               Verify(cert_.get(), kHostName, kOcspResponse,
                      CertVerifier::VERIFY_EV_CERT, crl_set.get(), _, _, _, _))
       .Times(1)
-      .WillOnce(testing::DoAll(SetVerifyResult(expectedResult),
+      .WillOnce(testing::DoAll(SetVerifyResult(expected_cert_verify_result),
                                testing::Return(kExpectedStatus)));
 
   // Call |Verify|.
-  scoped_ptr<CertVerifyResult> actualResult;
+  CertVerifyResult actual_cert_verify_result;
   int actualStatus = -1;
   CertVerifierBlockAdapter::Params params(cert_.get(), kHostName);
   params.ocsp_response = kOcspResponse;
   params.flags = CertVerifier::VERIFY_EV_CERT;
   params.crl_set = crl_set;
-  Verify(params, &actualResult, &actualStatus);
+  Verify(params, &actual_cert_verify_result, &actualStatus);
 
   // Ensure that Verification results are correct.
   EXPECT_EQ(kExpectedStatus, actualStatus);
-  EXPECT_EQ(expectedResult.verified_cert, actualResult->verified_cert);
+  EXPECT_EQ(expected_cert_verify_result.verified_cert,
+            actual_cert_verify_result.verified_cert);
 }
 
 }  // namespace