Re-enable the omitted tests in path_builder_unittest.cc.

net/cert/internal/path_builder_unittest.cc

 // Copyright 2016 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.
 
 #include "net/cert/internal/path_builder.h"
 
 #include "base/base_paths.h"
 #include "base/files/file_util.h"
 #include "base/path_service.h"
 #include "net/cert/internal/cert_issuer_source_static.h"
@@ skipping 863 matching lines @@
   EXPECT_FALSE(result.paths[0]->valid);
   const auto& path = result.paths[0]->path;
   ASSERT_EQ(2U, path.certs.size());
   EXPECT_EQ(target_, path.certs[0]);
   EXPECT_EQ(oldintermediate_, path.certs[1]);
   // Compare the DER instead of ParsedCertificate pointer, don't care which copy
   // of newroot was used in the path.
   EXPECT_EQ(newroot_->der_cert(), path.trust_anchor->cert()->der_cert());
 }
 
-// TODO(eroman): Re-enable these tests
-#if 0
 class MockCertIssuerSourceRequest : public CertIssuerSource::Request {
  public:
   MOCK_METHOD1(GetNext, void(ParsedCertificateList*));
 };
 
 class MockCertIssuerSource : public CertIssuerSource {
  public:
   MOCK_METHOD2(SyncGetIssuersOf,
                void(const ParsedCertificate*, ParsedCertificateList*));
-  MOCK_METHOD3(AsyncGetIssuersOf,
-               void(const ParsedCertificate*,
-                    std::unique_ptr<Request>*));
+  MOCK_METHOD2(AsyncGetIssuersOf,
+               void(const ParsedCertificate*, std::unique_ptr<Request>*));
 };
 
 // Helper class to pass the Request to the PathBuilder when it calls
 // AsyncGetIssuersOf. (GoogleMock has a ByMove helper, but it apparently can
 // only be used with Return, not SetArgPointee.)
 class CertIssuerSourceRequestMover {
  public:
   CertIssuerSourceRequestMover(std::unique_ptr<CertIssuerSource::Request> req)
       : request_(std::move(req)) {}
   void MoveIt(const ParsedCertificate* cert,
-              const CertIssuerSource::IssuerCallback& issuers_callback,
               std::unique_ptr<CertIssuerSource::Request>* out_req) {
     *out_req = std::move(request_);
   }
 
  private:
   std::unique_ptr<CertIssuerSource::Request> request_;
 };
 
+// Functor that when called with a ParsedCertificateList* will append the
+// specified certificate.
+class AppendCertToList {
+ public:
+  explicit AppendCertToList(const scoped_refptr<ParsedCertificate>& cert)
+      : cert_(cert) {}
+
+  void operator()(ParsedCertificateList* out) { out->push_back(cert_); }
+
+ private:
+  scoped_refptr<ParsedCertificate> cert_;
+};
+
 // Test that a single CertIssuerSource returning multiple async batches of
 // issuers is handled correctly. Due to the StrictMocks, it also tests that path
 // builder does not request issuers of certs that it shouldn't.
-TEST_F(PathBuilderKeyRolloverTest, TestMultipleAsyncCallbacksFromSingleSource) {
+TEST_F(PathBuilderKeyRolloverTest, TestMultipleAsyncIssuersFromSingleSource) {
   StrictMock<MockCertIssuerSource> cert_issuer_source;
 
   // Only newroot is a trusted root.
   TrustStoreInMemory trust_store;
   AddTrustedCertificate(newroot_, &trust_store);
 
   CertPathBuilder::Result result;
   CertPathBuilder path_builder(target_, &trust_store, &signature_policy_, time_,
                                &result);
   path_builder.AddCertIssuerSource(&cert_issuer_source);
 
-  CertIssuerSource::IssuerCallback target_issuers_callback;
   // Create the mock CertIssuerSource::Request...
   std::unique_ptr<StrictMock<MockCertIssuerSourceRequest>>
       target_issuers_req_owner(new StrictMock<MockCertIssuerSourceRequest>());
   // Keep a raw pointer to the Request...
   StrictMock<MockCertIssuerSourceRequest>* target_issuers_req =
       target_issuers_req_owner.get();
   // Setup helper class to pass ownership of the Request to the PathBuilder when
   // it calls AsyncGetIssuersOf.
   CertIssuerSourceRequestMover req_mover(std::move(target_issuers_req_owner));
   {
     ::testing::InSequence s;
     EXPECT_CALL(cert_issuer_source, SyncGetIssuersOf(target_.get(), _));
-    EXPECT_CALL(cert_issuer_source, AsyncGetIssuersOf(target_.get(), _, _))
-        .WillOnce(
-            DoAll(SaveArg<1>(&target_issuers_callback),
-                  Invoke(&req_mover, &CertIssuerSourceRequestMover::MoveIt)));
+    EXPECT_CALL(cert_issuer_source, AsyncGetIssuersOf(target_.get(), _))
+        .WillOnce(Invoke(&req_mover, &CertIssuerSourceRequestMover::MoveIt));
   }
 
-  TestClosure callback;
-  CompletionStatus rv = path_builder.Run(callback.closure());
-  ASSERT_EQ(CompletionStatus::ASYNC, rv);
-
-  ASSERT_FALSE(target_issuers_callback.is_null());
-
-  ::testing::Mock::VerifyAndClearExpectations(&cert_issuer_source);
-
-  // First async batch: return oldintermediate_.
   EXPECT_CALL(*target_issuers_req, GetNext(_))
-      .WillOnce(DoAll(SetArgPointee<0>(oldintermediate_),
-                      Return(CompletionStatus::SYNC)))
-      .WillOnce(
-          DoAll(SetArgPointee<0>(nullptr), Return(CompletionStatus::ASYNC)));
+      // First async batch: return oldintermediate_.
+      .WillOnce(Invoke(AppendCertToList(oldintermediate_)))
+      // Second async batch: return newintermediate_.
+      .WillOnce(Invoke(AppendCertToList(newintermediate_)));
   {
     ::testing::InSequence s;
     // oldintermediate_ does not create a valid path, so both sync and async
     // lookups are expected.
     EXPECT_CALL(cert_issuer_source,
                 SyncGetIssuersOf(oldintermediate_.get(), _));
     EXPECT_CALL(cert_issuer_source,
-                AsyncGetIssuersOf(oldintermediate_.get(), _, _));
+                AsyncGetIssuersOf(oldintermediate_.get(), _));
   }
-  target_issuers_callback.Run(target_issuers_req);
-  ::testing::Mock::VerifyAndClearExpectations(target_issuers_req);
-  ::testing::Mock::VerifyAndClearExpectations(&cert_issuer_source);
 
-  // Second async batch: return newintermediate_.
-  EXPECT_CALL(*target_issuers_req, GetNext(_))
-      .WillOnce(DoAll(SetArgPointee<0>(newintermediate_),
-                      Return(CompletionStatus::SYNC)))
-      .WillOnce(
-          DoAll(SetArgPointee<0>(nullptr), Return(CompletionStatus::ASYNC)));
   // newroot_ is in the trust store, so this path will be completed
   // synchronously. AsyncGetIssuersOf will not be called on newintermediate_.
   EXPECT_CALL(cert_issuer_source, SyncGetIssuersOf(newintermediate_.get(), _));
-  target_issuers_callback.Run(target_issuers_req);
+
+  // Ensure pathbuilder finished and filled result.
+  path_builder.Run();
+
   // Note that VerifyAndClearExpectations(target_issuers_req) is not called
   // here. PathBuilder could have destroyed it already, so just let the
   // expectations get checked by the destructor.
   ::testing::Mock::VerifyAndClearExpectations(&cert_issuer_source);
 
-  // Ensure pathbuilder finished and filled result.
-  callback.WaitForResult();
-
   EXPECT_TRUE(result.HasValidPath());
   ASSERT_EQ(2U, result.paths.size());
 
   // Path builder first attempts: target <- oldintermediate <- newroot
   // but it will fail since oldintermediate is signed by oldroot.
   EXPECT_FALSE(result.paths[0]->valid);
   const auto& path0 = result.paths[0]->path;
   ASSERT_EQ(2U, path0.certs.size());
   EXPECT_EQ(target_, path0.certs[0]);
   EXPECT_EQ(oldintermediate_, path0.certs[1]);
@@ skipping 16 matching lines @@
 
   // Only newroot is a trusted root.
   TrustStoreInMemory trust_store;
   AddTrustedCertificate(newroot_, &trust_store);
 
   CertPathBuilder::Result result;
   CertPathBuilder path_builder(target_, &trust_store, &signature_policy_, time_,
                                &result);
   path_builder.AddCertIssuerSource(&cert_issuer_source);
 
-  CertIssuerSource::IssuerCallback target_issuers_callback;
   // Create the mock CertIssuerSource::Request...
   std::unique_ptr<StrictMock<MockCertIssuerSourceRequest>>
       target_issuers_req_owner(new StrictMock<MockCertIssuerSourceRequest>());
   // Keep a raw pointer to the Request...
   StrictMock<MockCertIssuerSourceRequest>* target_issuers_req =
       target_issuers_req_owner.get();
   // Setup helper class to pass ownership of the Request to the PathBuilder when
   // it calls AsyncGetIssuersOf.
   CertIssuerSourceRequestMover req_mover(std::move(target_issuers_req_owner));
   {
     ::testing::InSequence s;
     EXPECT_CALL(cert_issuer_source, SyncGetIssuersOf(target_.get(), _));
-    EXPECT_CALL(cert_issuer_source, AsyncGetIssuersOf(target_.get(), _, _))
-        .WillOnce(
-            DoAll(SaveArg<1>(&target_issuers_callback),
-                  Invoke(&req_mover, &CertIssuerSourceRequestMover::MoveIt)));
+    EXPECT_CALL(cert_issuer_source, AsyncGetIssuersOf(target_.get(), _))
+        .WillOnce(Invoke(&req_mover, &CertIssuerSourceRequestMover::MoveIt));
   }
 
-  TestClosure callback;
-  CompletionStatus rv = path_builder.Run(callback.closure());
-  ASSERT_EQ(CompletionStatus::ASYNC, rv);
+  scoped_refptr<ParsedCertificate> oldintermediate_dupe(
+      ParsedCertificate::Create(oldintermediate_->der_cert().AsStringPiece(),
+                                {}, nullptr));
 
-  ASSERT_FALSE(target_issuers_callback.is_null());
+  EXPECT_CALL(*target_issuers_req, GetNext(_))
+      // First async batch: return oldintermediate_.
+      .WillOnce(Invoke(AppendCertToList(oldintermediate_)))
+      // Second async batch: return a different copy of oldintermediate_ again.
+      .WillOnce(Invoke(AppendCertToList(oldintermediate_dupe)))
+      // Third async batch: return newintermediate_.
+      .WillOnce(Invoke(AppendCertToList(newintermediate_)));
 
-  ::testing::Mock::VerifyAndClearExpectations(&cert_issuer_source);
-
-  // First async batch: return oldintermediate_.
-  EXPECT_CALL(*target_issuers_req, GetNext(_))
-      .WillOnce(DoAll(SetArgPointee<0>(oldintermediate_),
-                      Return(CompletionStatus::SYNC)))
-      .WillOnce(
-          DoAll(SetArgPointee<0>(nullptr), Return(CompletionStatus::ASYNC)));
   {
     ::testing::InSequence s;
     // oldintermediate_ does not create a valid path, so both sync and async
     // lookups are expected.
     EXPECT_CALL(cert_issuer_source,
                 SyncGetIssuersOf(oldintermediate_.get(), _));
     EXPECT_CALL(cert_issuer_source,
-                AsyncGetIssuersOf(oldintermediate_.get(), _, _));
+                AsyncGetIssuersOf(oldintermediate_.get(), _));
   }
-  target_issuers_callback.Run(target_issuers_req);
-  ::testing::Mock::VerifyAndClearExpectations(target_issuers_req);
-  ::testing::Mock::VerifyAndClearExpectations(&cert_issuer_source);
 
-  // Second async batch: return a different copy of oldintermediate_ again.
-  scoped_refptr<ParsedCertificate> oldintermediate_dupe(
-      ParsedCertificate::Create(oldintermediate_->der_cert().AsStringPiece(),
-                                {}, nullptr));
-  EXPECT_CALL(*target_issuers_req, GetNext(_))
-      .WillOnce(DoAll(SetArgPointee<0>(oldintermediate_dupe),
-                      Return(CompletionStatus::SYNC)))
-      .WillOnce(
-          DoAll(SetArgPointee<0>(nullptr), Return(CompletionStatus::ASYNC)));
-  target_issuers_callback.Run(target_issuers_req);
-  // oldintermediate was already processed above, it should not generate any
-  // more requests.
-  ::testing::Mock::VerifyAndClearExpectations(target_issuers_req);
-  ::testing::Mock::VerifyAndClearExpectations(&cert_issuer_source);
-
-  // Third async batch: return newintermediate_.
-  EXPECT_CALL(*target_issuers_req, GetNext(_))
-      .WillOnce(DoAll(SetArgPointee<0>(newintermediate_),
-                      Return(CompletionStatus::SYNC)))
-      .WillOnce(
-          DoAll(SetArgPointee<0>(nullptr), Return(CompletionStatus::ASYNC)));
   // newroot_ is in the trust store, so this path will be completed
   // synchronously. AsyncGetIssuersOf will not be called on newintermediate_.
   EXPECT_CALL(cert_issuer_source, SyncGetIssuersOf(newintermediate_.get(), _));
-  target_issuers_callback.Run(target_issuers_req);
-  // Note that VerifyAndClearExpectations(target_issuers_req) is not called
-  // here. PathBuilder could have destroyed it already, so just let the
-  // expectations get checked by the destructor.
-  ::testing::Mock::VerifyAndClearExpectations(&cert_issuer_source);
 
   // Ensure pathbuilder finished and filled result.
-  callback.WaitForResult();
+  path_builder.Run();
+
+  ::testing::Mock::VerifyAndClearExpectations(&cert_issuer_source);
 
   EXPECT_TRUE(result.HasValidPath());
   ASSERT_EQ(2U, result.paths.size());
 
   // Path builder first attempts: target <- oldintermediate <- newroot
   // but it will fail since oldintermediate is signed by oldroot.
   EXPECT_FALSE(result.paths[0]->valid);
   const auto& path0 = result.paths[0]->path;
   ASSERT_EQ(2U, path0.certs.size());
   EXPECT_EQ(target_, path0.certs[0]);
   EXPECT_EQ(oldintermediate_, path0.certs[1]);
   EXPECT_EQ(newroot_, path0.trust_anchor->cert());
 
   // The second async result does not generate any path.
 
   // After the third batch of async results, path builder will attempt:
   // target <- newintermediate <- newroot which will succeed.
   EXPECT_TRUE(result.paths[1]->valid);
   const auto& path1 = result.paths[1]->path;
   ASSERT_EQ(2U, path1.certs.size());
   EXPECT_EQ(target_, path1.certs[0]);
   EXPECT_EQ(newintermediate_, path1.certs[1]);
   EXPECT_EQ(newroot_, path1.trust_anchor->cert());
 }
 
-#endif
-
 }  // namespace
 
 }  // namespace net