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..c8d3d8fd582a466e59a58ed1de47d398c1f86533
--- /dev/null
+++ b/net/cert/cert_net_fetcher.h
@@ -0,0 +1,165 @@
+// 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 <set>
+#include <vector>
+
+#include "base/callback.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/threading/thread_checker.h"
+#include "base/time/time.h"

[Ryan Sleevi, 2015/02/23 20:25:22]

This appears to be unused now?

[eroman, 2015/02/23 23:36:58]

Done.

+#include "net/base/net_export.h"
+#include "url/gurl.h"
+
+namespace net {
+
+class URLRequestContext;

[Ryan Sleevi, 2015/02/23 20:25:22]

Hrm. From the perspective of //net/cert, this is a layering violation

That is, //net/cert is used by //net/socket which is used by //net/url_request,
so this is a circular dependency.

For NSS, we have explicitly setup a top level directory in //net, //net/ocsp,
that might otherwise be called //net/cert_fetching, that exists to resolve this
conflict.

It implements an interface specified by NSS (conceptually, a //net/cert
interface) and is injected down.

This suggests to me that we need to pursue an Interface/Impl split here in
//net/cert - //net/cert defines a pure virtual interface, while //net/ocsp
(probably renamed) implements it using //net/url_request

I apologize for not having thought about this sooner.

[eroman, 2015/02/23 23:36:58]

I can certainly split up as Interface/Impl if needed, however I would like to
better understand what your concern is with using forward declarations.

//net is compiled as a whole and contains plenty of other circular dependencies.
For instance NetworkDelegate. Or in the same vein as the one you mentioned,
URLRequestContext contains a ProxyService which contains a ProxyScriptFetcher
which contains a URLRequest/URLRequestContext.

If we were to compile //net/cert in isolation then we would have an issue. It
doesn't seem that we currently do, but I imagine that is a future goal.

I am fine with making the change, just want to understand exactly which problem
I am fixing.

+
+// CertNetFetcher is a helper for fetching AIA URLs and CRL URLs.
+//
+// It manages the outstanding requests, and applies policies specific to
+// certificate network fetches. For instance:
+//   * Enforces a maximum timeout on responses
+//   * Enforces a maximum size on responses
+//   * Restricts URLs to http://

[Ryan Sleevi, 2015/02/23 20:25:22]

A thought occurred to me - how will we handle HSTS? We can't disable it (and
that's a good thing, generally, from an API perspective), and we'll
automatically inherit any HSTS settings.

[eroman, 2015/02/23 23:36:58]

Good point I hadn't considered HSTS. I responded on the other comment, perhaps
we can surface it as its own error rather than try to make it work
automagically.

+//
+// The CertNetFetcher is initialized by giving it a URLRequestContext for the
+// network dependencies. Note that it does not maintain its own in-memory cache
+// for responses. Instead it is reliant on the URLRequestContext providing an
+// HTTP cache if this is desired.
+//
+// -------------------------
+// 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 early design document:
+//   https://docs.google.com/a/chromium.org/document/d/1CdS9YOnPdAyVZBJqHY7ZJ6tUlU71OCvX8kHnaVhf144/edit
+class NET_EXPORT CertNetFetcher {
+ private:
+  struct Request;
+
+ public:
+  typedef base::Callback<void(int, const std::vector<uint8_t>&)> FetchCallback;
+
+  typedef Request* RequestId;
+
+  // This value can be used in place of timeout or max size limits.
+  static const int DEFAULT = -1;
+
+  // Initializes CertNetFetcher using the specified URLRequestContext for
+  // issuing requests. |context| must remain valid for the entire lifetime of
+  // the CertNetFetcher.
+  explicit CertNetFetcher(URLRequestContext* context);
+
+  // Deletion implicitly cancels any outstanding requests.
+  ~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).
+  void CancelRequest(RequestId request);
+
+  // 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.
+
+  RequestId FetchCaIssuers(const GURL& url,
+                           int timeout_milliseconds,
+                           int max_response_bytes,
+                           const FetchCallback& callback);
+
+  RequestId FetchCrl(const GURL& url,
+                     int timeout_milliseconds,
+                     int max_response_bytes,
+                     const FetchCallback& callback);
+
+  RequestId FetchOcsp(const GURL& url,
+                      int timeout_milliseconds,
+                      int max_response_bytes,
+                      const FetchCallback& callback);
+
+ private:
+  class Job;
+  struct JobToRequestParamsComparator;
+  struct RequestParams;
+
+  struct JobComparator {
+    bool operator()(const Job* job1, const Job* job2) const;
+  };
+
+  typedef std::set<Job*, JobComparator> JobSet;

[Ryan Sleevi, 2015/02/23 20:25:22]

Per https://chromium-cpp.appspot.com/ , prefer "using ="

[eroman, 2015/02/23 23:36:58]

Done. Changed all the typedefs to using=

+
+  // Starts an asynchronous request to fetch the given URL. On completion
+  // |callback| will be invoked.
+  //
+  // Completion of the request will never occur synchronously. In other words it
+  // is guaranteed that |callback| will only be invoked once the Fetch*() method
+  // has returned.
+  RequestId Fetch(scoped_ptr<RequestParams> request_params,
+                  const FetchCallback& callback);
+
+  // Finds a job with a matching RequestPararms or returns NULL if there was no
+  // match.
+  Job* FindJob(const RequestParams& params);
+
+  // Removes |job| from the in progress jobs and transfers ownership to the
+  // caller.
+  scoped_ptr<Job> RemoveJob(Job* job);
+
+  // Indicates which Job is currently executing inside of OnJobCompleted().
+  void SetCurrentlyCompletingJob(Job* job);
+
+  // The in-progress jobs. This set does not contain the job which is actively
+  // invoking callbacks (OnJobCompleted). Instead that is tracked by
+  // |currently_completing_job_|.
+  JobSet jobs_;
+
+  // The Job that is currently executing OnJobCompleted(). There can be at most
+  // one such job. This pointer is not owned.
+  Job* currently_completing_job_;
+
+  // Not owned. CertNetFetcher must outlive the URLRequestContext.
+  URLRequestContext* context_;
+
+  base::ThreadChecker thread_checker_;
+
+  DISALLOW_COPY_AND_ASSIGN(CertNetFetcher);
+};
+
+}  // namespace net
+
+#endif  // NET_CERT_CERT_NET_FETCHER_H_