Refactor the API for CertVerifier::Verify() and the implementation of MultiThreadedCertVerifier::Ver

net/cert/multi_threaded_cert_verifier_unittest.cc

Index: net/cert/multi_threaded_cert_verifier_unittest.cc
diff --git a/net/cert/multi_threaded_cert_verifier_unittest.cc b/net/cert/multi_threaded_cert_verifier_unittest.cc
index 142e65a8f648156e29b12d5387c1c0657213964d..cb58291e69dbb998d790fc7d820231534c5f9bb8 100644
--- a/net/cert/multi_threaded_cert_verifier_unittest.cc
+++ b/net/cert/multi_threaded_cert_verifier_unittest.cc
@@ -85,7 +85,7 @@ TEST_F(MultiThreadedCertVerifierTest, CacheHit) {
   int error;
   CertVerifyResult verify_result;
   TestCompletionCallback callback;
-  CertVerifier::RequestHandle request_handle;
+  scoped_ptr<CertVerifier::Request> request_handle;
 
   error = verifier_.Verify(test_cert.get(), "www.example.com", std::string(), 0,
                            NULL, &verify_result, callback.callback(),
@@ -145,7 +145,7 @@ TEST_F(MultiThreadedCertVerifierTest, DifferentCACerts) {
   int error;
   CertVerifyResult verify_result;
   TestCompletionCallback callback;
-  CertVerifier::RequestHandle request_handle;
+  scoped_ptr<CertVerifier::Request> request_handle;
 
   error = verifier_.Verify(cert_chain1.get(), "www.example.com", std::string(),
                            0, NULL, &verify_result, callback.callback(),
@@ -182,10 +182,10 @@ TEST_F(MultiThreadedCertVerifierTest, InflightJoin) {
   int error;
   CertVerifyResult verify_result;
   TestCompletionCallback callback;
-  CertVerifier::RequestHandle request_handle;
+  scoped_ptr<CertVerifier::Request> request_handle;
   CertVerifyResult verify_result2;
   TestCompletionCallback callback2;
-  CertVerifier::RequestHandle request_handle2;
+  scoped_ptr<CertVerifier::Request> request_handle2;
 
   error = verifier_.Verify(test_cert.get(), "www.example.com", std::string(), 0,
                            NULL, &verify_result, callback.callback(),
@@ -196,7 +196,7 @@ TEST_F(MultiThreadedCertVerifierTest, InflightJoin) {
                            NULL, &verify_result2, callback2.callback(),
                            &request_handle2, BoundNetLog());
   EXPECT_EQ(ERR_IO_PENDING, error);
-  EXPECT_TRUE(request_handle2 != NULL);
+  EXPECT_TRUE(request_handle2);
   error = callback.WaitForResult();
   EXPECT_TRUE(IsCertificateError(error));
   error = callback2.WaitForResult();
@@ -215,14 +215,14 @@ TEST_F(MultiThreadedCertVerifierTest, CancelRequest) {
 
   int error;
   CertVerifyResult verify_result;
-  CertVerifier::RequestHandle request_handle;
+  scoped_ptr<CertVerifier::Request> request_handle;
 
   error = verifier_.Verify(test_cert.get(), "www.example.com", std::string(), 0,
                            NULL, &verify_result, base::Bind(&FailTest),
                            &request_handle, BoundNetLog());
   ASSERT_EQ(ERR_IO_PENDING, error);
-  ASSERT_TRUE(request_handle != NULL);
-  verifier_.CancelRequest(request_handle);
+  ASSERT_TRUE(request_handle);
+  request_handle.reset();
 
   // Issue a few more requests to the worker pool and wait for their
   // completion, so that the task of the canceled request (which runs on a
@@ -249,7 +249,7 @@ TEST_F(MultiThreadedCertVerifierTest, CancelRequestThenQuit) {
   int error;
   CertVerifyResult verify_result;
   TestCompletionCallback callback;
-  CertVerifier::RequestHandle request_handle;
+  scoped_ptr<CertVerifier::Request> request_handle;
 
   {
     // Because shutdown intentionally doesn't join worker threads, a
@@ -262,7 +262,7 @@ TEST_F(MultiThreadedCertVerifierTest, CancelRequestThenQuit) {
   }
   ASSERT_EQ(ERR_IO_PENDING, error);
   EXPECT_TRUE(request_handle);
-  verifier_.CancelRequest(request_handle);
+  request_handle.reset();
   // Destroy |verifier| by going out of scope.
 }
 
@@ -393,7 +393,7 @@ TEST_F(MultiThreadedCertVerifierTest, CertTrustAnchorProvider) {
   int error;
   CertVerifyResult verify_result;
   TestCompletionCallback callback;
-  CertVerifier::RequestHandle request_handle;
+  scoped_ptr<CertVerifier::Request> request_handle;
   EXPECT_CALL(trust_provider, GetAdditionalTrustAnchors())
       .WillOnce(ReturnRef(empty_cert_list));
   error = verifier_.Verify(test_cert.get(), "www.example.com", std::string(), 0,
@@ -435,4 +435,78 @@ TEST_F(MultiThreadedCertVerifierTest, CertTrustAnchorProvider) {
   ASSERT_EQ(1u, verifier_.cache_hits());
 }
 
+// Tests de-duplication of requests.
+// Starts up 5 requests, of which 3 are unique.
+TEST_F(MultiThreadedCertVerifierTest, MultipleInflightJoin) {
+  base::FilePath certs_dir = GetTestCertsDirectory();
+  scoped_refptr<X509Certificate> test_cert(
+      ImportCertFromFile(certs_dir, "ok_cert.pem"));
+  ASSERT_NE(static_cast<X509Certificate*>(nullptr), test_cert.get());
+
+  int error;
+  CertVerifyResult verify_result1;
+  TestCompletionCallback callback1;
+  scoped_ptr<CertVerifier::Request> request_handle1;
+  CertVerifyResult verify_result2;
+  TestCompletionCallback callback2;
+  scoped_ptr<CertVerifier::Request> request_handle2;
+  CertVerifyResult verify_result3;
+  TestCompletionCallback callback3;
+  scoped_ptr<CertVerifier::Request> request_handle3;
+  CertVerifyResult verify_result4;
+  TestCompletionCallback callback4;
+  scoped_ptr<CertVerifier::Request> request_handle4;
+  CertVerifyResult verify_result5;
+  TestCompletionCallback callback5;
+  scoped_ptr<CertVerifier::Request> request_handle5;
+
+  const char domain1[] = "www.example1.com";
+  const char domain2[] = "www.exampleB.com";
+  const char domain3[] = "www.example3.com";
+
+  // Start 3 unique requests.
+  error = verifier_.Verify(test_cert.get(), domain2, std::string(), 0, nullptr,
+                           &verify_result1, callback1.callback(),
+                           &request_handle1, BoundNetLog());
+  ASSERT_EQ(ERR_IO_PENDING, error);
+  EXPECT_TRUE(request_handle1);
+
+  error = verifier_.Verify(test_cert.get(), domain2, std::string(), 0, nullptr,
+                           &verify_result2, callback2.callback(),
+                           &request_handle2, BoundNetLog());
+  EXPECT_EQ(ERR_IO_PENDING, error);
+  EXPECT_TRUE(request_handle2);
+
+  error = verifier_.Verify(test_cert.get(), domain3, std::string(), 0, nullptr,
+                           &verify_result3, callback3.callback(),
+                           &request_handle3, BoundNetLog());
+  EXPECT_EQ(ERR_IO_PENDING, error);
+  EXPECT_TRUE(request_handle3);
+
+  // Start duplicate requests (which should join to existing jobs).
+  error = verifier_.Verify(test_cert.get(), domain1, std::string(), 0, nullptr,
+                           &verify_result4, callback4.callback(),
+                           &request_handle4, BoundNetLog());
+  EXPECT_EQ(ERR_IO_PENDING, error);
+  EXPECT_TRUE(request_handle4);
+
+  error = verifier_.Verify(test_cert.get(), domain2, std::string(), 0, nullptr,
+                           &verify_result5, callback5.callback(),
+                           &request_handle5, BoundNetLog());
+  EXPECT_EQ(ERR_IO_PENDING, error);
+  EXPECT_TRUE(request_handle5);
+
+  error = callback1.WaitForResult();
+  EXPECT_TRUE(IsCertificateError(error));
+  error = callback2.WaitForResult();
+  ASSERT_TRUE(IsCertificateError(error));
+  error = callback4.WaitForResult();
+  ASSERT_TRUE(IsCertificateError(error));
+
+  // Let the other requests automatically cancel.
+  ASSERT_EQ(5u, verifier_.requests());
+  ASSERT_EQ(0u, verifier_.cache_hits());
+  ASSERT_EQ(2u, verifier_.inflight_joins());
+}
+
 }  // namespace net