Allways call didReceiveAuthenticationChallenge: completion handler.

ios/web/net/cert_verifier_block_adapter.cc

Index: ios/web/net/cert_verifier_block_adapter.cc
diff --git a/ios/web/net/cert_verifier_block_adapter.cc b/ios/web/net/cert_verifier_block_adapter.cc
index b56b414351dc19d806f198c60b9c004866c99831..baba2cccdc122abc99c7b9180d83b4a19673ebff 100644
--- a/ios/web/net/cert_verifier_block_adapter.cc
+++ b/ios/web/net/cert_verifier_block_adapter.cc
@@ -26,8 +26,8 @@ struct VerificationContext
         net_log(net::BoundNetLog::Make(
             net_log,
             net::NetLog::SOURCE_IOS_WEB_VIEW_CERT_VERIFIER)) {}
-  // Unowned verification request.
-  net::CertVerifier::Request* request;
+  // Verification request must be alive until verification is completed.

[Ryan Sleevi, 2015/10/28 18:32:27]

I find this comment a bit confusing. You're making a statement about a
requirement, but it's unclear where this requirement is coming from or why.

Put differently, you're not describing what this member is, but why it is, and
that's a bit... odd.

// Stores the current verification request. The request must outlive the
VerificationContext, so that the verification
// request is not cancelled, [because why]

[Eugene But (OOO till 7-30), 2015/10/29 15:43:38]

Done.

+  scoped_ptr<net::CertVerifier::Request> request;
   // The result of certificate verification.
   net::CertVerifyResult result;
   // Certificate being verified.
@@ -74,12 +74,6 @@ void CertVerifierBlockAdapter::Verify(
   scoped_refptr<VerificationContext> context(
       new VerificationContext(params.cert, net_log_));
   net::CompletionCallback callback = base::BindBlock(^(int error) {
-    // Remove pending request.
-    auto request_iterator = std::find(
-        pending_requests_.begin(), pending_requests_.end(), context->request);
-    DCHECK(pending_requests_.end() != request_iterator);
-    pending_requests_.erase(request_iterator);
-
     completion_handler(context->result, error);
   });
   scoped_ptr<net::CertVerifier::Request> request;
@@ -88,10 +82,11 @@ void CertVerifierBlockAdapter::Verify(
                                      params.crl_set.get(), &(context->result),
                                      callback, &request, context->net_log);
   if (error == net::ERR_IO_PENDING) {
-    // Make sure that |net::CertVerifier::Request| is alive until either
-    // verification is completed or CertVerifierBlockAdapter is destroyed.
-    pending_requests_.push_back(request.Pass());
-    context->request = pending_requests_.back();
+    // Make sure that |net::CertVerifier::Request| is alive until verification
+    // is completed. It means that cert verification request will not be
+    // cancellable but it will guarantee that |completion_handler| callback is
+    // always called by this method, which is a part of API contract.

[Ryan Sleevi, 2015/10/28 18:32:27]

This still reads a bit... weird.

// Keep the |net::CertVerifier::Request| alive until verification completes.
Because |context| is kept alive by
// |callback| (through base::BindBlock), this means that the cert verification
request cannot be cancelled.
// However, it guarantees that |callback| - and thus |completion_handler| - will
always be called, which is
// a necessary part of the API contract of |CertVerifierBlockAdapter::Verify()|.

[Eugene But (OOO till 7-30), 2015/10/29 15:43:38]

Done.

+    context->request = request.Pass();
     // Completion handler will be called from |callback| when verification
     // request is completed.
     return;