Remove dependence on a message loop for net::PathBuilder.

net/cert/internal/path_builder.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 <set>
 #include <unordered_set>
 
-#include "base/callback_helpers.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "net/base/net_errors.h"
 #include "net/cert/internal/cert_issuer_source.h"
 #include "net/cert/internal/parse_certificate.h"
 #include "net/cert/internal/parse_name.h"  // For CertDebugString.
 #include "net/cert/internal/signature_policy.h"
 #include "net/cert/internal/trust_store.h"
 #include "net/cert/internal/verify_certificate_chain.h"
 #include "net/cert/internal/verify_name_match.h"
@@ skipping 43 matching lines @@
 // CertIssuersIter iterates through the intermediates from |cert_issuer_sources|
 // which may be issuers of |cert|.
 class CertIssuersIter {
  public:
   // Constructs the CertIssuersIter. |*cert_issuer_sources| and |*trust_store|
   // must be valid for the lifetime of the CertIssuersIter.
   CertIssuersIter(scoped_refptr<ParsedCertificate> cert,
                   CertIssuerSources* cert_issuer_sources,
                   const TrustStore* trust_store);
 
-  // Gets the next candidate issuer. If an issuer is ready synchronously, SYNC
-  // is returned and the cert is stored in |*cert|.  If an issuer is not
-  // ready, ASYNC is returned and |callback| will be called once |*out_cert| has
-  // been set. If |callback| is null, always completes synchronously.
-  //
-  // In either case, if all issuers have been exhausted, |*out| is cleared.
-  CompletionStatus GetNextIssuer(CertificateOrTrustAnchor* out,
-                                 const base::Closure& callback);
+  // Gets the next candidate issuer, or clears |*out| when all issuers have been
+  // exhausted.
+  void GetNextIssuer(CertificateOrTrustAnchor* out);
 
   // Returns the |cert| for which issuers are being retrieved.
   const ParsedCertificate* cert() const { return cert_.get(); }
   scoped_refptr<ParsedCertificate> reference_cert() const { return cert_; }
 
  private:
+  void AddIssuers(ParsedCertificateList issuers);
   void DoAsyncIssuerQuery();
-  void GotAsyncAnchors(TrustAnchors anchors);
-  void GotAsyncCerts(CertIssuerSource::Request* request);
-  void NotifyIfNecessary();
 
   scoped_refptr<ParsedCertificate> cert_;
   CertIssuerSources* cert_issuer_sources_;
   const TrustStore* trust_store_;
 
   // The list of trust anchors that match the issuer name for |cert_|.
   TrustAnchors anchors_;
   // The index of the next trust anchor in |anchors_| to return.
   size_t cur_anchor_ = 0;
 
@@ skipping 10 matching lines @@
 
   // Set of DER-encoded values for the certs in |issuers_|. Used to prevent
   // duplicates. This is based on the full DER of the cert to allow different
   // versions of the same certificate to be tried in different candidate paths.
   // This points to data owned by |issuers_|.
   std::unordered_set<base::StringPiece, base::StringPieceHash> present_issuers_;
 
   // Tracks which requests have been made yet.
   bool did_initial_query_ = false;
   bool did_async_issuer_query_ = false;
-  // If asynchronous requests were made, how many of them are still outstanding?
-  size_t pending_async_results_;
+  // If asynchronous requests were made, which ones are still outstanding?

[mattm, 2016/10/28 02:11:55]

comment is a bit confusing / indirect. maybe mention it is an index into
pending_async_requests_, and that it is next outstanding one to process, or
something?

[eroman, 2016/11/23 22:10:21]

Done.

+  size_t cur_async_request_ = 0;
   // Owns the Request objects for any asynchronous requests so that they will be
   // cancelled if CertIssuersIter is destroyed.
   std::vector<std::unique_ptr<CertIssuerSource::Request>>
       pending_async_requests_;
-  std::unique_ptr<TrustStore::Request> pending_anchor_request_;
-
-  // When GetNextIssuer was called and returned asynchronously, |*out_| is
-  // where the result will be stored, and |callback_| will be run when the
-  // result is ready.
-  CertificateOrTrustAnchor* out_;
-  base::Closure callback_;
 
   DISALLOW_COPY_AND_ASSIGN(CertIssuersIter);
 };
 
 CertIssuersIter::CertIssuersIter(scoped_refptr<ParsedCertificate> in_cert,
                                  CertIssuerSources* cert_issuer_sources,
                                  const TrustStore* trust_store)
     : cert_(in_cert),
       cert_issuer_sources_(cert_issuer_sources),
       trust_store_(trust_store) {
   DVLOG(1) << "CertIssuersIter(" << CertDebugString(cert()) << ") created";
 }
 
-CompletionStatus CertIssuersIter::GetNextIssuer(CertificateOrTrustAnchor* out,
-                                                const base::Closure& callback) {
-  // Should not be called again while already waiting for an async result.
-  DCHECK(callback_.is_null());
-
+void CertIssuersIter::GetNextIssuer(CertificateOrTrustAnchor* out) {
   if (!did_initial_query_) {
     did_initial_query_ = true;
-    trust_store_->FindTrustAnchorsForCert(
-        cert_,
-        callback.is_null() ? TrustStore::TrustAnchorsCallback()
-                           : base::Bind(&CertIssuersIter::GotAsyncAnchors,
-                                        base::Unretained(this)),
-        &anchors_, &pending_anchor_request_);
+    trust_store_->FindTrustAnchorsForCert(cert_, &anchors_);
 
     for (auto* cert_issuer_source : *cert_issuer_sources_) {
       ParsedCertificateList new_issuers;
       cert_issuer_source->SyncGetIssuersOf(cert(), &new_issuers);
-      for (scoped_refptr<ParsedCertificate>& issuer : new_issuers) {
-        if (present_issuers_.find(issuer->der_cert().AsStringPiece()) !=
-            present_issuers_.end())
-          continue;
-        present_issuers_.insert(issuer->der_cert().AsStringPiece());
-        issuers_.push_back(std::move(issuer));
-      }
+      AddIssuers(std::move(new_issuers));
     }
     DVLOG(1) << anchors_.size() << " sync anchors, " << issuers_.size()
              << " sync issuers";
     // TODO(mattm): sort by notbefore, etc (eg if cert issuer matches a trust
     // anchor subject (or is a trust anchor), that should be sorted higher too.
     // See big list of possible sorting hints in RFC 4158.)
     // (Update PathBuilderKeyRolloverTest.TestRolloverBothRootsTrusted once that
     // is done)
   }
 
   // Return possible trust anchors first.
   if (cur_anchor_ < anchors_.size()) {
     DVLOG(1) << "CertIssuersIter(" << CertDebugString(cert())
              << "): returning anchor " << cur_anchor_ << " of "
              << anchors_.size();
     // Still have anchors that haven't been returned yet, return one of them.
     *out = CertificateOrTrustAnchor(anchors_[cur_anchor_++]);
-    return CompletionStatus::SYNC;
+    return;
   }
 
-  if (pending_anchor_request_) {
-    DVLOG(1) << "CertIssuersIter(" << CertDebugString(cert())
-             << ") Still waiting for async trust anchor results.";
-    out_ = out;
-    callback_ = callback;
-    return CompletionStatus::ASYNC;
+  // If there aren't any issuers left, block until async results are ready.
+  if (cur_issuer_ >= issuers_.size()) {
+    if (!did_async_issuer_query_) {
+      // Now issue request(s) for async ones (AIA, etc).
+      DoAsyncIssuerQuery();
+    }
+
+    // TODO(eroman): Rather than blocking on the async requests in FIFO order,
+    // consume in the order they become ready.
+    while (cur_async_request_ < pending_async_requests_.size()) {
+      ParsedCertificateList new_issuers;
+      pending_async_requests_[cur_async_request_]->GetNext(&new_issuers);
+      if (new_issuers.empty()) {
+        // Request is exhausted, no more results pending from that
+        // CertIssuerSource.
+        pending_async_requests_[cur_async_request_++].reset();
+        continue;
+      }
+
+      AddIssuers(std::move(new_issuers));
+      break;
+    }
   }
 
   if (cur_issuer_ < issuers_.size()) {
     DVLOG(1) << "CertIssuersIter(" << CertDebugString(cert())
              << "): returning issuer " << cur_issuer_ << " of "
              << issuers_.size();
     // Still have issuers that haven't been returned yet, return one of them.
     // A reference to the returned issuer is retained, since |present_issuers_|
     // points to data owned by it.
     *out = CertificateOrTrustAnchor(issuers_[cur_issuer_++]);
-    return CompletionStatus::SYNC;
-  }
-
-  if (did_async_issuer_query_) {
-    if (pending_async_results_ == 0) {
-      DVLOG(1) << "CertIssuersIter(" << CertDebugString(cert())
-               << ") Reached the end of all available issuers.";
-      // Reached the end of all available issuers.
-      *out = CertificateOrTrustAnchor();
-      return CompletionStatus::SYNC;
-    }
-
-    DVLOG(1) << "CertIssuersIter(" << CertDebugString(cert())
-             << ") Still waiting for async results from other "
-                "CertIssuerSources.";
-    // Still waiting for async results from other CertIssuerSources.
-    out_ = out;
-    callback_ = callback;
-    return CompletionStatus::ASYNC;
-  }
-  // Reached the end of synchronously gathered issuers.
-
-  if (callback.is_null()) {
-    // Synchronous-only mode, don't try to query async sources.
-    *out = CertificateOrTrustAnchor();
-    return CompletionStatus::SYNC;
-  }
-
-  // Now issue request(s) for async ones (AIA, etc).
-  DoAsyncIssuerQuery();
-
-  if (pending_async_results_ == 0) {
-    DVLOG(1) << "CertIssuersIter(" << CertDebugString(cert())
-             << ") No cert sources have async results.";
-    // No cert sources have async results.
-    *out = CertificateOrTrustAnchor();
-    return CompletionStatus::SYNC;
+    return;
   }
 
   DVLOG(1) << "CertIssuersIter(" << CertDebugString(cert())
-           << ") issued AsyncGetIssuersOf call(s) (n=" << pending_async_results_
-           << ")";
-  out_ = out;
-  callback_ = callback;
-  return CompletionStatus::ASYNC;
+           << ") Reached the end of all available issuers.";
+  // Reached the end of all available issuers.
+  *out = CertificateOrTrustAnchor();
+}
+
+void CertIssuersIter::AddIssuers(ParsedCertificateList new_issuers) {
+  for (scoped_refptr<ParsedCertificate>& issuer : new_issuers) {
+    if (present_issuers_.find(issuer->der_cert().AsStringPiece()) !=
+        present_issuers_.end())
+      continue;
+    present_issuers_.insert(issuer->der_cert().AsStringPiece());
+    issuers_.push_back(std::move(issuer));
+  }
 }
 
 void CertIssuersIter::DoAsyncIssuerQuery() {
   DCHECK(!did_async_issuer_query_);
   did_async_issuer_query_ = true;
-  pending_async_results_ = 0;
+  cur_async_request_ = 0;
   for (auto* cert_issuer_source : *cert_issuer_sources_) {
     std::unique_ptr<CertIssuerSource::Request> request;
-    cert_issuer_source->AsyncGetIssuersOf(
-        cert(),
-        base::Bind(&CertIssuersIter::GotAsyncCerts, base::Unretained(this)),
-        &request);
+    cert_issuer_source->AsyncGetIssuersOf(cert(), &request);
     if (request) {
       DVLOG(1) << "AsyncGetIssuersOf(" << CertDebugString(cert())
                << ") pending...";
-      pending_async_results_++;
       pending_async_requests_.push_back(std::move(request));
     }
   }
 }
 
-void CertIssuersIter::GotAsyncAnchors(TrustAnchors anchors) {
-  DVLOG(1) << "CertIssuersIter::GotAsyncAnchors(" << CertDebugString(cert())
-           << "): " << anchors.size() << " anchors";
-  for (scoped_refptr<TrustAnchor>& anchor : anchors)
-    anchors_.push_back(std::move(anchor));
-  pending_anchor_request_.reset();
-
-  NotifyIfNecessary();
-}
-
-void CertIssuersIter::GotAsyncCerts(CertIssuerSource::Request* request) {
-  DVLOG(1) << "CertIssuersIter::GotAsyncCerts(" << CertDebugString(cert())
-           << ")";
-  while (true) {
-    scoped_refptr<ParsedCertificate> cert;
-    CompletionStatus status = request->GetNext(&cert);
-    if (!cert) {
-      if (status == CompletionStatus::SYNC) {
-        // Request is exhausted, no more results pending from that
-        // CertIssuerSource.
-        DCHECK_GT(pending_async_results_, 0U);
-        pending_async_results_--;
-      }
-      break;
-    }
-    DCHECK_EQ(status, CompletionStatus::SYNC);
-    if (present_issuers_.find(cert->der_cert().AsStringPiece()) !=
-        present_issuers_.end())
-      continue;
-    present_issuers_.insert(cert->der_cert().AsStringPiece());
-    issuers_.push_back(std::move(cert));
-  }
-
-  // TODO(mattm): re-sort remaining elements of issuers_ (remaining elements may
-  // be more than the ones just inserted, depending on |cur_| value).
-
-  NotifyIfNecessary();
-}
-
-void CertIssuersIter::NotifyIfNecessary() {
-  // Notify that more results are available, if necessary.
-  if (!callback_.is_null()) {
-    if (cur_anchor_ < anchors_.size()) {
-      DVLOG(1) << "CertIssuersIter(" << CertDebugString(cert())
-               << "): async returning anchor " << cur_anchor_ << " of "
-               << anchors_.size();
-      *out_ = CertificateOrTrustAnchor(std::move(anchors_[cur_anchor_++]));
-      base::ResetAndReturn(&callback_).Run();
-      return;
-    }
-    if (cur_issuer_ < issuers_.size()) {
-      DCHECK(!pending_anchor_request_);
-      DVLOG(1) << "CertIssuersIter(" << CertDebugString(cert())
-               << "): async returning issuer " << cur_issuer_ << " of "
-               << issuers_.size();
-      *out_ = CertificateOrTrustAnchor(std::move(issuers_[cur_issuer_++]));
-      base::ResetAndReturn(&callback_).Run();
-      return;
-    }
-
-    if (!did_async_issuer_query_)
-      DoAsyncIssuerQuery();
-
-    if (pending_async_results_ == 0) {
-      DVLOG(1) << "CertIssuersIter(" << CertDebugString(cert())
-               << "): async returning empty result";
-      *out_ = CertificateOrTrustAnchor();
-      base::ResetAndReturn(&callback_).Run();
-      return;
-    }
-    DVLOG(1) << "CertIssuersIter(" << CertDebugString(cert())
-             << "): empty result, but other async results "
-                "pending, waiting..";
-  }
-}
-
 // CertIssuerIterPath tracks which certs are present in the path and prevents
 // paths from being built which repeat any certs (including different versions
 // of the same cert, based on Subject+SubjectAltName+SPKI).
 class CertIssuerIterPath {
  public:
   // Returns true if |cert| is already present in the path.
   bool IsPresent(const ParsedCertificate* cert) const {
     return present_certs_.find(GetKey(cert)) != present_certs_.end();
   }
 
@@ skipping 79 matching lines @@
 class CertPathIter {
  public:
   CertPathIter(scoped_refptr<ParsedCertificate> cert,
                const TrustStore* trust_store);
 
   // Adds a CertIssuerSource to provide intermediates for use in path building.
   // The |*cert_issuer_source| must remain valid for the lifetime of the
   // CertPathIter.
   void AddCertIssuerSource(CertIssuerSource* cert_issuer_source);
 
-  // Gets the next candidate path. If a path is ready synchronously, SYNC is
-  // returned and the path is stored in |*path|.  If a path is not ready,
-  // ASYNC is returned and |callback| will be called once |*path| has been set.
-  // In either case, if all paths have been exhausted, |*path| is cleared.
-  CompletionStatus GetNextPath(CertPath* path, const base::Closure& callback);
+  // Gets the next candidate path, or clears |*path| when all paths have been
+  // exhausted.
+  void GetNextPath(CertPath* path);
 
  private:
   enum State {
     STATE_NONE,
     STATE_GET_NEXT_ISSUER,
     STATE_GET_NEXT_ISSUER_COMPLETE,
     STATE_RETURN_A_PATH,
     STATE_BACKTRACK,
   };
 
-  CompletionStatus DoLoop(bool allow_async);
-
-  CompletionStatus DoGetNextIssuer(bool allow_async);
-  CompletionStatus DoGetNextIssuerComplete();
-  CompletionStatus DoBackTrack();
-
-  void HandleGotNextIssuer(void);
+  void DoGetNextIssuer();
+  void DoGetNextIssuerComplete();
+  void DoBackTrack();
 
   // Stores the next candidate issuer, until it is used during the
   // STATE_GET_NEXT_ISSUER_COMPLETE step.
   CertificateOrTrustAnchor next_issuer_;
   // The current path being explored, made up of CertIssuerIters. Each node
   // keeps track of the state of searching for issuers of that cert, so that
   // when backtracking it can resume the search where it left off.
   CertIssuerIterPath cur_path_;
   // The CertIssuerSources for retrieving candidate issuers.
   CertIssuerSources cert_issuer_sources_;
   // The TrustStore for checking if a path ends in a trust anchor.
   const TrustStore* trust_store_;
   // The output variable for storing the next candidate path, which the client
   // passes in to GetNextPath. Only used for a single path output.
   CertPath* out_path_;
-  // The callback to be called if an async lookup generated a candidate path.
-  base::Closure callback_;
   // Current state of the state machine.
   State next_state_;
 
   DISALLOW_COPY_AND_ASSIGN(CertPathIter);
 };
 
 CertPathIter::CertPathIter(scoped_refptr<ParsedCertificate> cert,
                            const TrustStore* trust_store)
     : next_issuer_(std::move(cert)),
       trust_store_(trust_store),
       next_state_(STATE_GET_NEXT_ISSUER_COMPLETE) {}
 
 void CertPathIter::AddCertIssuerSource(CertIssuerSource* cert_issuer_source) {
   cert_issuer_sources_.push_back(cert_issuer_source);
 }
 
-CompletionStatus CertPathIter::GetNextPath(CertPath* path,
-                                           const base::Closure& callback) {
+// TODO(eroman): Simplify (doesn't need to use the "DoLoop" pattern).
+void CertPathIter::GetNextPath(CertPath* path) {
   out_path_ = path;
   out_path_->Clear();
-  CompletionStatus rv = DoLoop(!callback.is_null());
-  if (rv == CompletionStatus::ASYNC) {
-    callback_ = callback;
-  } else {
-    // Clear the reference to the output parameter as a precaution.
-    out_path_ = nullptr;
-  }
-  return rv;
-}
-
-CompletionStatus CertPathIter::DoLoop(bool allow_async) {
-  CompletionStatus result = CompletionStatus::SYNC;
   do {
     State state = next_state_;
     next_state_ = STATE_NONE;
     switch (state) {
       case STATE_NONE:
         NOTREACHED();
         break;
       case STATE_GET_NEXT_ISSUER:
-        result = DoGetNextIssuer(allow_async);
+        DoGetNextIssuer();
         break;
       case STATE_GET_NEXT_ISSUER_COMPLETE:
-        result = DoGetNextIssuerComplete();
+        DoGetNextIssuerComplete();
         break;
       case STATE_RETURN_A_PATH:
         // If the returned path did not verify, keep looking for other paths
         // (the trust root is not part of cur_path_, so don't need to
         // backtrack).
         next_state_ = STATE_GET_NEXT_ISSUER;
-        result = CompletionStatus::SYNC;
         break;
       case STATE_BACKTRACK:
-        result = DoBackTrack();
+        DoBackTrack();
         break;
     }
-  } while (result == CompletionStatus::SYNC && next_state_ != STATE_NONE &&
-           next_state_ != STATE_RETURN_A_PATH);
+  } while (next_state_ != STATE_NONE && next_state_ != STATE_RETURN_A_PATH);
 
-  return result;
+  out_path_ = nullptr;
 }
 
-CompletionStatus CertPathIter::DoGetNextIssuer(bool allow_async) {
+void CertPathIter::DoGetNextIssuer() {
   next_state_ = STATE_GET_NEXT_ISSUER_COMPLETE;
-  CompletionStatus rv = cur_path_.back()->GetNextIssuer(
-      &next_issuer_, allow_async
-                         ? base::Bind(&CertPathIter::HandleGotNextIssuer,
-                                      base::Unretained(this))
-                         : base::Closure());
-  return rv;
+  cur_path_.back()->GetNextIssuer(&next_issuer_);
 }
 
-CompletionStatus CertPathIter::DoGetNextIssuerComplete() {
+void CertPathIter::DoGetNextIssuerComplete() {
   // If the issuer is a trust anchor signal readiness.
   if (next_issuer_.IsTrustAnchor()) {
     DVLOG(1) << "CertPathIter got anchor("
              << CertDebugString(next_issuer_.anchor->cert().get());
     next_state_ = STATE_RETURN_A_PATH;
     cur_path_.CopyPath(&out_path_->certs);
     out_path_->trust_anchor = std::move(next_issuer_.anchor);
     next_issuer_ = CertificateOrTrustAnchor();
-    return CompletionStatus::SYNC;
+    return;
   }
 
   if (next_issuer_.IsCertificate()) {
     // Skip this cert if it is already in the chain.
     if (cur_path_.IsPresent(next_issuer_.cert.get())) {
       next_state_ = STATE_GET_NEXT_ISSUER;
-      return CompletionStatus::SYNC;
+      return;
     }
 
     cur_path_.Append(base::MakeUnique<CertIssuersIter>(
         std::move(next_issuer_.cert), &cert_issuer_sources_, trust_store_));
     next_issuer_ = CertificateOrTrustAnchor();
     DVLOG(1) << "CertPathIter cur_path_ = " << cur_path_.PathDebugString();
     // Continue descending the tree.
     next_state_ = STATE_GET_NEXT_ISSUER;
   } else {
     // TODO(mattm): should also include such paths in CertPathBuilder::Result,
     // maybe with a flag to enable it. Or use a visitor pattern so the caller
     // can decide what to do with any failed paths.
     // No more issuers for current chain, go back up and see if there are any
     // more for the previous cert.
     next_state_ = STATE_BACKTRACK;
   }
-  return CompletionStatus::SYNC;
 }
 
-CompletionStatus CertPathIter::DoBackTrack() {
+void CertPathIter::DoBackTrack() {
   DVLOG(1) << "CertPathIter backtracking...";
   cur_path_.Pop();
   if (cur_path_.Empty()) {
     // Exhausted all paths.
     next_state_ = STATE_NONE;
   } else {
     // Continue exploring issuers of the previous path.
     next_state_ = STATE_GET_NEXT_ISSUER;
   }
-  return CompletionStatus::SYNC;
-}
-
-void CertPathIter::HandleGotNextIssuer(void) {
-  DCHECK(!callback_.is_null());
-  CompletionStatus rv = DoLoop(true /* allow_async */);
-  if (rv == CompletionStatus::SYNC) {
-    // Clear the reference to the output parameter as a precaution.
-    out_path_ = nullptr;
-    base::ResetAndReturn(&callback_).Run();
-  }
 }
 
 CertPathBuilder::ResultPath::ResultPath() = default;
 CertPathBuilder::ResultPath::~ResultPath() = default;
 CertPathBuilder::Result::Result() = default;
 CertPathBuilder::Result::~Result() = default;
 
 const CertPathBuilder::ResultPath* CertPathBuilder::Result::GetBestValidPath()
     const {
   DCHECK((paths.empty() && best_result_index == 0) ||
@@ skipping 24 matching lines @@
       next_state_(STATE_NONE),
       out_result_(result) {}
 
 CertPathBuilder::~CertPathBuilder() {}
 
 void CertPathBuilder::AddCertIssuerSource(
     CertIssuerSource* cert_issuer_source) {
   cert_path_iter_->AddCertIssuerSource(cert_issuer_source);
 }
 
-CompletionStatus CertPathBuilder::Run(const base::Closure& callback) {
+// TODO(eroman): Simplify (doesn't need to use the "DoLoop" pattern).
+void CertPathBuilder::Run() {
   DCHECK_EQ(STATE_NONE, next_state_);
   next_state_ = STATE_GET_NEXT_PATH;
-  CompletionStatus rv = DoLoop(!callback.is_null());
-
-  if (rv == CompletionStatus::ASYNC)
-    callback_ = callback;
-
-  return rv;
-}
-
-CompletionStatus CertPathBuilder::DoLoop(bool allow_async) {
-  CompletionStatus result = CompletionStatus::SYNC;
 
   do {
     State state = next_state_;
     next_state_ = STATE_NONE;
     switch (state) {
       case STATE_NONE:
         NOTREACHED();
         break;
       case STATE_GET_NEXT_PATH:
-        result = DoGetNextPath(allow_async);
+        DoGetNextPath();
         break;
       case STATE_GET_NEXT_PATH_COMPLETE:
-        result = DoGetNextPathComplete();
+        DoGetNextPathComplete();
         break;
     }
-  } while (result == CompletionStatus::SYNC && next_state_ != STATE_NONE);
-
-  return result;
+  } while (next_state_ != STATE_NONE);
 }
 
-CompletionStatus CertPathBuilder::DoGetNextPath(bool allow_async) {
+void CertPathBuilder::DoGetNextPath() {
   next_state_ = STATE_GET_NEXT_PATH_COMPLETE;
-  CompletionStatus rv = cert_path_iter_->GetNextPath(
-      &next_path_, allow_async ? base::Bind(&CertPathBuilder::HandleGotNextPath,
-                                            base::Unretained(this))
-                               : base::Closure());
-  return rv;
+  cert_path_iter_->GetNextPath(&next_path_);
 }
 
-void CertPathBuilder::HandleGotNextPath() {
-  DCHECK(!callback_.is_null());
-  CompletionStatus rv = DoLoop(true /* allow_async */);
-  if (rv == CompletionStatus::SYNC)
-    base::ResetAndReturn(&callback_).Run();
-}
-
-CompletionStatus CertPathBuilder::DoGetNextPathComplete() {
+void CertPathBuilder::DoGetNextPathComplete() {
   if (next_path_.IsEmpty()) {
     // No more paths to check, signal completion.
     next_state_ = STATE_NONE;
-    return CompletionStatus::SYNC;
+    return;
   }
 
   // Verify the entire certificate chain.
   auto result_path = base::MakeUnique<ResultPath>();
   bool verify_result =
       VerifyCertificateChain(next_path_.certs, next_path_.trust_anchor.get(),
                              signature_policy_, time_, &result_path->errors);
   DVLOG(1) << "CertPathBuilder VerifyCertificateChain result = "
            << result_path->valid;
   result_path->path = next_path_;
   result_path->valid = verify_result;
   AddResultPath(std::move(result_path));
 
   if (verify_result) {
     // Found a valid path, return immediately.
     // TODO(mattm): add debug/test mode that tries all possible paths.
     next_state_ = STATE_NONE;
-    return CompletionStatus::SYNC;
+    return;
   }
 
   // Path did not verify. Try more paths. If there are no more paths, the result
   // will be returned next time DoGetNextPathComplete is called with next_path_
   // empty.
   next_state_ = STATE_GET_NEXT_PATH;
-  return CompletionStatus::SYNC;
 }
 
 void CertPathBuilder::AddResultPath(std::unique_ptr<ResultPath> result_path) {
   // TODO(mattm): set best_result_index based on number or severity of errors.
   if (result_path->valid)
     out_result_->best_result_index = out_result_->paths.size();
   // TODO(mattm): add flag to only return a single path or all attempted paths?
   out_result_->paths.push_back(std::move(result_path));
 }
 
 }  // namespace net