Initial implementation for CertNetFetcher.

net/cert/cert_net_fetcher.h

Index: net/cert/cert_net_fetcher.h
diff --git a/net/cert/cert_net_fetcher.h b/net/cert/cert_net_fetcher.h
new file mode 100644
index 0000000000000000000000000000000000000000..bb7d9d58493e798f9d6fad675c38e75e066a0248
--- /dev/null
+++ b/net/cert/cert_net_fetcher.h
@@ -0,0 +1,107 @@
+// Copyright 2015 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.
+
+#ifndef NET_CERT_CERT_NET_FETCHER_H_
+#define NET_CERT_CERT_NET_FETCHER_H_
+
+#include <vector>
+
+#include "base/callback.h"
+#include "net/base/net_errors.h"
+#include "net/base/net_export.h"
+
+class GURL;
+
+namespace net {
+
+class URLRequestContext;
+
+// CertNetFetcher is an asynchronous interface for fetching AIA URLs and CRL
+// URLs. Implementations are responsible for issuing and managing the requests.
+//
+// -------------------------
+// Cancellation of requests
+// -------------------------
+//
+//  * Network requests started by the CertNetFetcher can be cancelled using
+//    CancelRequest(). Cancellation means the callback will not be invoked.
+//
+//  * If the CertNetFetcher is deleted then any outstanding
+//    requests are automatically cancelled.
+//
+//  * Cancelling a request more than once is an error (and may result in a
+//    use-after-free).
+//
+//  * Cancelling a request within the execution of a callback is allowed.
+//
+//  * Deleting the CertNetFetcher from within the execution of a callback is
+//    allowed.
+//
+// -------------------------
+// Threading
+// -------------------------
+//
+// The CertNetFetcher is expected to be operated from a single thread, which has
+// an IO message loop. The URLRequestContext will be accessed from this same
+// thread, and callbacks will be posted to this message loop.
+//
+// For more details see the design document:
+//   https://docs.google.com/a/chromium.org/document/d/1CdS9YOnPdAyVZBJqHY7ZJ6tUlU71OCvX8kHnaVhf144/edit
+class NET_EXPORT CertNetFetcher {
+ public:
+  // Callback invoked on request completion. If the Error is OK, then the
+  // vector contains the response bytes.
+  using FetchCallback =
+      base::Callback<void(Error, const std::vector<uint8_t>&)>;
+
+  // Opaque handled used to cancel requests.
+  using RequestId = void*;
+
+  // This value can be used in place of timeout or max size limits.
+  enum { DEFAULT = -1 };
+
+  CertNetFetcher() {}
+
+  // Deletion implicitly cancels any outstanding requests.
+  virtual ~CertNetFetcher() {}
+
+  // Cancels the indicated request. It is an error to call this function on a
+  // request which has already completed (including one that was already
+  // cancelled).
+  virtual void CancelRequest(RequestId request) = 0;

[davidben, 2015/03/27 23:34:31]

Rather than CancelRequest, perhaps:

class Request {
  virtual ~Request() = 0;
};

virtual scoped_ptr<Request> FetchCaIssuers(...)

So the caller can just toss it around and stuff. Though it does mean that the
destructor needs to check if the request already completed first. I suspect it'd
also make the ownership around Jobs and Requests less fiddly. Matches how
HttpStreamFactory works too I think.

(I didn't look too carefully at the interaction, under the assumption that this
would change some things. If you prefer the current scheme, I'll review the
current logic more carefully.)

[eroman, 2015/04/04 21:57:14]

Done.

I agree this is cleaner, I think we should do use this approach elsewhere in the
network stack. I filed
https://code.google.com/p/chromium/issues/detail?id=473980 to address that.

+
+  // The Fetch*() methods start an asynchronous request which can be cancelled
+  // using RequestId. Here is the meaning of the common parameters:
+  //
+  //   * url -- The http:// URL to fetch.
+  //   * timeout_seconds -- The maximum allowed duration for the fetch job. If
+  //         this delay is exceeded then the request will fail. To use a default
+  //         timeout pass DEFAULT.
+  //   * max_response_bytes -- The maximum size of the response body. If this
+  //     size is exceeded then the request will fail. To use a default timeout
+  //     pass DEFAULT.
+  //   * callback -- The callback that will be invoked on completion of the job.
+
+  virtual RequestId FetchCaIssuers(const GURL& url,
+                                   int timeout_milliseconds,
+                                   int max_response_bytes,
+                                   const FetchCallback& callback) = 0;

[davidben, 2015/03/27 23:34:31]

Maybe for a followup, but since we want to be able to debug circular
dependencies, it might be prudent to pass in a BoundNetLog bits into these
functions so the CERT_VERIFIER_JOB in net-internals can link to the URL_REQUEST.
(Hrm. There isn't a reverse pointer right now from CERT_VERIFIER_JOB to SOCKET.
That seems worth fixing.)

Though how that interacts with this potential movement from net/cert to
crypto/cert, I have no idea. I suspect the NetLog integration will be valuable.

[eroman, 2015/04/04 21:57:14]

+1 on NetLog integration. I will add that as a followup.
Added a TODO at the top of cert_net_fetcher_impl.cc

+
+  virtual RequestId FetchCrl(const GURL& url,
+                             int timeout_milliseconds,
+                             int max_response_bytes,
+                             const FetchCallback& callback) = 0;
+
+  virtual RequestId FetchOcsp(const GURL& url,
+                              int timeout_milliseconds,
+                              int max_response_bytes,
+                              const FetchCallback& callback) = 0;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(CertNetFetcher);
+};
+
+}  // namespace net
+
+#endif  // NET_CERT_NET_CERT_NET_FETCHER_H_