Initial implementation for CertNetFetcher.

net/cert_net/cert_net_fetcher_impl_unittest.cc

+// 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.
+
+#include "net/cert_net/cert_net_fetcher_impl.h"
+
+#include <string>
+
+#include "base/compiler_specific.h"
+#include "base/run_loop.h"
+#include "net/cert/mock_cert_verifier.h"
+#include "net/dns/mock_host_resolver.h"
+#include "net/http/http_server_properties_impl.h"
+#include "net/test/spawned_test_server/spawned_test_server.h"
+#include "net/url_request/url_request_job_factory_impl.h"
+#include "net/url_request/url_request_test_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "testing/platform_test.h"
+
+// TODO(eroman): Test that cookies aren't sent.
+
+using base::ASCIIToUTF16;
+
+namespace net {
+
+namespace {
+
+const base::FilePath::CharType kDocRoot[] =
+    FILE_PATH_LITERAL("net/data/cert_net_fetcher_impl_unittest");
+
+// A non-mock URLRequestContext which can access http:// urls.
+class RequestContext : public URLRequestContext {
+ public:
+  RequestContext() : storage_(this) {
+    ProxyConfig no_proxy;
+    storage_.set_host_resolver(scoped_ptr<HostResolver>(new MockHostResolver));
+    storage_.set_cert_verifier(new MockCertVerifier);
+    storage_.set_transport_security_state(new TransportSecurityState);
+    storage_.set_proxy_service(ProxyService::CreateFixed(no_proxy));
+    storage_.set_ssl_config_service(new SSLConfigServiceDefaults);
+    storage_.set_http_server_properties(
+        scoped_ptr<HttpServerProperties>(new HttpServerPropertiesImpl()));
+
+    HttpNetworkSession::Params params;
+    params.host_resolver = host_resolver();
+    params.cert_verifier = cert_verifier();
+    params.transport_security_state = transport_security_state();
+    params.proxy_service = proxy_service();
+    params.ssl_config_service = ssl_config_service();
+    params.http_server_properties = http_server_properties();
+    scoped_refptr<HttpNetworkSession> network_session(
+        new HttpNetworkSession(params));
+    storage_.set_http_transaction_factory(new HttpCache(
+        network_session.get(), HttpCache::DefaultBackend::InMemory(0)));
+    URLRequestJobFactoryImpl* job_factory = new URLRequestJobFactoryImpl();
+    storage_.set_job_factory(job_factory);
+  }
+
+  ~RequestContext() override { AssertNoURLRequests(); }
+
+ private:
+  URLRequestContextStorage storage_;
+};
+
+class FetchResult {
+ public:
+  FetchResult(Error net_error, const std::vector<uint8_t>& response_body)
+      : net_error_(net_error), response_body_(response_body) {}
+
+  void VerifySuccess(const std::string& expected_body) {
+    EXPECT_EQ(OK, net_error_);
+    EXPECT_EQ(expected_body,
+              std::string(response_body_.begin(), response_body_.end()));
+  }
+
+  void VerifyFailure(Error expected_error) {
+    EXPECT_EQ(expected_error, net_error_);
+    EXPECT_EQ(0u, response_body_.size());
+  }
+
+ private:
+  const Error net_error_;
+  const std::vector<uint8_t> response_body_;
+};
+
+// Helper to synchronously wait for the fetch completion. This is similar to
+// net's TestCompletionCallback, but built around FetchCallback.
+class TestFetchCallback {
+ public:
+  TestFetchCallback()
+      : callback_(base::Bind(&TestFetchCallback::OnCallback,
+                             base::Unretained(this))) {}
+
+  const CertNetFetcher::FetchCallback& callback() const { return callback_; }
+
+  scoped_ptr<FetchResult> WaitForResult() {
+    DCHECK(quit_closure_.is_null());
+    while (!HasResult()) {
+      base::RunLoop run_loop;
+      quit_closure_ = run_loop.QuitClosure();
+      run_loop.Run();
+      quit_closure_.Reset();
+    }
+    return result_.Pass();
+  }
+
+  bool HasResult() const { return result_.get(); }
+
+  // Sets an extra action (in addition to recording the result) that is run when
+  // the FetchCallback is invoked.
+  void set_extra_closure(const base::Closure& closure) {
+    extra_closure_ = closure;
+  }
+
+ private:
+  void OnCallback(Error net_error, const std::vector<uint8_t>& response_body) {
+    DCHECK(!HasResult());
+    result_.reset(new FetchResult(net_error, response_body));
+
+    if (!extra_closure_.is_null())
+      extra_closure_.Run();
+
+    if (!quit_closure_.is_null())
+      quit_closure_.Run();
+  }
+
+  CertNetFetcher::FetchCallback callback_;
+  scoped_ptr<FetchResult> result_;
+  base::Closure quit_closure_;
+  base::Closure extra_closure_;
+};
+
+}  // namespace
+
+class CertNetFetcherImplTest : public PlatformTest {
+ public:
+  CertNetFetcherImplTest()
+      : test_server_(SpawnedTestServer::TYPE_HTTP,
+                     net::SpawnedTestServer::kLocalhost,
+                     base::FilePath(kDocRoot)) {
+    context_.set_network_delegate(&network_delegate_);
+  }
+
+ protected:
+  SpawnedTestServer test_server_;
+  TestNetworkDelegate network_delegate_;
+  RequestContext context_;
+};
+
+// Helper to start an AIA fetch using default parameters.
+CertNetFetcher::RequestId StartRequest(const GURL& url,
+                                       const TestFetchCallback& callback,
+                                       CertNetFetcher* fetcher) {

[davidben, 2015/03/31 20:52:25]

Nit: Probably makes more sense for the fetcher to be the first argument since
it's the context of sorts?

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

I left it last because I thought it was more style guide compliant
(out-parameters go at the end; this is kind of an in/out parameter).

I haven't changed this yet; let me know how you want me to proceed.

[davidben, 2015/04/07 19:22:25]

Hrm. I see it as more a context than an in/out parameter (you aren't really
writing to fetcher, you're doing something with it). I guess the style guide
doesn't actually say anything about "context parameters" or that they're a
thing. Anyway, I don't feel terribly strongly either way.

(I'm mostly getting that wording from BoringSSL, which does context/out/in. And
since I wrote that style guide, I may as well have made it up. But I think I
borrowed that word from agl, so...)

[eroman, 2015/04/07 20:22:40]

Done.

+  return fetcher->FetchCaIssuers(url, CertNetFetcher::DEFAULT,
+                                 CertNetFetcher::DEFAULT, callback.callback());
+}
+
+// Fetch a few unique URLs using GET in parallel. Each URL has a different body
+// and Content-Type.
+TEST_F(CertNetFetcherImplTest, ParallelFetchNoDupes) {
+  ASSERT_TRUE(test_server_.Start());
+
+  CertNetFetcherImpl fetcher(&context_);
+  TestFetchCallback callback1;
+  TestFetchCallback callback2;
+  TestFetchCallback callback3;
+
+  // Request a URL with Content-Type "application/pkix-cert"
+  GURL url1 = test_server_.GetURL("files/cert.crt");
+  StartRequest(url1, callback1, &fetcher);
+
+  // Request a URL with Content-Type "application/pkix-crl"
+  GURL url2 = test_server_.GetURL("files/root.crl");
+  StartRequest(url2, callback2, &fetcher);
+
+  // Request a URL with Content-Type "application/pkcs7-mime"
+  GURL url3 = test_server_.GetURL("files/certs.p7c");
+  StartRequest(url3, callback3, &fetcher);
+
+  // Wait for all of the requests to complete.
+  scoped_ptr<FetchResult> result1 = callback1.WaitForResult();
+  scoped_ptr<FetchResult> result2 = callback2.WaitForResult();
+  scoped_ptr<FetchResult> result3 = callback3.WaitForResult();
+
+  // Verify the fetch results.
+  result1->VerifySuccess("-cert.crt-\n");
+  result2->VerifySuccess("-root.crl-\n");
+  result3->VerifySuccess("-certs.p7c-\n");
+
+  EXPECT_EQ(3, network_delegate_.created_requests());
+}
+
+// Fetch a caIssuers URL which has an unexpected extension and Content-Type.
+// The extension is .txt and the Content-Type is text/plain. Despite being
+// unusual this succeeds as the extension and Content-Type are not required to
+// be meaningful.
+TEST_F(CertNetFetcherImplTest, ContentTypeDoesntMatter) {
+  ASSERT_TRUE(test_server_.Start());
+
+  CertNetFetcherImpl fetcher(&context_);
+
+  TestFetchCallback callback;
+  GURL url = test_server_.GetURL("files/foo.txt");
+  StartRequest(url, callback, &fetcher);
+  scoped_ptr<FetchResult> result = callback.WaitForResult();
+  result->VerifySuccess("-foo.txt-\n");
+}
+
+// Fetch a URLs whose HTTP response code is not 200. These are considered
+// failures.
+TEST_F(CertNetFetcherImplTest, HttpStatusCode) {
+  ASSERT_TRUE(test_server_.Start());
+
+  CertNetFetcherImpl fetcher(&context_);
+
+  // Response was HTTP status 404.
+  {
+    TestFetchCallback callback;
+    GURL url = test_server_.GetURL("files/404.html");
+    StartRequest(url, callback, &fetcher);
+    scoped_ptr<FetchResult> result = callback.WaitForResult();
+    result->VerifyFailure(ERR_FAILED);
+  }
+
+  // Response was HTTP status 500.
+  {
+    TestFetchCallback callback;
+    GURL url = test_server_.GetURL("files/500.html");
+    StartRequest(url, callback, &fetcher);
+    scoped_ptr<FetchResult> result = callback.WaitForResult();
+    result->VerifyFailure(ERR_FAILED);
+  }
+}
+
+// Fetching a URL with a Content-Disposition header should have no effect.
+TEST_F(CertNetFetcherImplTest, ContentDisposition) {
+  ASSERT_TRUE(test_server_.Start());
+
+  CertNetFetcherImpl fetcher(&context_);
+
+  TestFetchCallback callback;
+  GURL url = test_server_.GetURL("files/downloadable.js");
+  StartRequest(url, callback, &fetcher);
+  scoped_ptr<FetchResult> result = callback.WaitForResult();
+  result->VerifySuccess("-downloadable.js-\n");
+}
+
+// Verifies that a cachable request will be served from the HTTP cache the
+// second time it is requested.
+TEST_F(CertNetFetcherImplTest, Cache) {
+  ASSERT_TRUE(test_server_.Start());
+
+  CertNetFetcherImpl fetcher(&context_);
+
+  // Fetch a URL whose HTTP headers make it cacheable for 1 hour.
+  GURL url(test_server_.GetURL("files/cacheable_1hr.crt"));
+  {
+    TestFetchCallback callback;
+    StartRequest(url, callback, &fetcher);
+    scoped_ptr<FetchResult> result = callback.WaitForResult();
+    result->VerifySuccess("-cacheable_1hr.crt-\n");
+  }
+
+  EXPECT_EQ(1, network_delegate_.created_requests());
+
+  // Kill the HTTP server.
+  ASSERT_TRUE(test_server_.Stop());
+
+  // Fetch again -- will fail unless served from cache.
+  {
+    TestFetchCallback callback;
+    StartRequest(url, callback, &fetcher);
+    scoped_ptr<FetchResult> result = callback.WaitForResult();
+    result->VerifySuccess("-cacheable_1hr.crt-\n");
+  }
+
+  EXPECT_EQ(2, network_delegate_.created_requests());
+}
+
+// Verify that the maximum response body constraints are enforced by fetching a
+// resource that is larger than the limit.
+TEST_F(CertNetFetcherImplTest, TooLarge) {
+  ASSERT_TRUE(test_server_.Start());
+
+  CertNetFetcherImpl fetcher(&context_);
+
+  // This file has a response body 12 bytes long. So setting the maximum to 11
+  // bytes will cause it to fail.
+  GURL url(test_server_.GetURL("files/certs.p7c"));
+  TestFetchCallback callback;
+  fetcher.FetchCaIssuers(url, CertNetFetcher::DEFAULT, 11, callback.callback());
+
+  scoped_ptr<FetchResult> result = callback.WaitForResult();
+  result->VerifyFailure(ERR_FILE_TOO_BIG);
+}
+
+// Set the timeout to 10 milliseconds, and try fetching a URL that takes 5
+// seconds to complete. It should fail due to a timeout.
+TEST_F(CertNetFetcherImplTest, Hang) {
+  ASSERT_TRUE(test_server_.Start());
+
+  CertNetFetcherImpl fetcher(&context_);
+
+  GURL url(test_server_.GetURL("slow/certs.p7c?5.1"));

[davidben, 2015/03/31 20:52:25]

Why 5.1? The comment says 5.

(I think 5.1 doesn't work right now anyway.
https://codereview.chromium.org/1049343002/ )

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

Changed all the floating points to integers.

I had copied the code from elsewhere and wasn't sure if non-floats would work so
just conservatively used floats everywhere.

... And based on your bugfix, turns out even that was wrong ;)

+  TestFetchCallback callback;
+  fetcher.FetchCaIssuers(url, 10, CertNetFetcher::DEFAULT, callback.callback());
+  scoped_ptr<FetchResult> result = callback.WaitForResult();
+  result->VerifyFailure(ERR_TIMED_OUT);
+}
+
+// Verify that if a response is gzip-encoded it gets inflated before being
+// returned to the caller.
+TEST_F(CertNetFetcherImplTest, Gzip) {
+  ASSERT_TRUE(test_server_.Start());
+
+  CertNetFetcherImpl fetcher(&context_);
+
+  GURL url(test_server_.GetURL("files/gzipped_crl"));
+  TestFetchCallback callback;
+  StartRequest(url, callback, &fetcher);
+  scoped_ptr<FetchResult> result = callback.WaitForResult();
+  result->VerifySuccess("-gzipped_crl-\n");
+}
+
+// Try fetching an unsupported URL scheme (https).
+TEST_F(CertNetFetcherImplTest, HttpsNotAllowed) {
+  ASSERT_TRUE(test_server_.Start());
+
+  CertNetFetcherImpl fetcher(&context_);
+
+  GURL url("https://foopy/foo.crt");
+  TestFetchCallback callback;
+  StartRequest(url, callback, &fetcher);
+  // Should NOT complete synchronously despite being a test that could be done
+  // immediately.
+  EXPECT_FALSE(callback.HasResult());
+  scoped_ptr<FetchResult> result = callback.WaitForResult();
+  result->VerifyFailure(ERR_DISALLOWED_URL_SCHEME);
+
+  // No request was created because the URL scheme was unsupported.
+  EXPECT_EQ(0, network_delegate_.created_requests());
+}
+
+// Try fetching a URL which redirects to https.
+TEST_F(CertNetFetcherImplTest, RedirectToHttpsNotAllowed) {
+  ASSERT_TRUE(test_server_.Start());
+
+  CertNetFetcherImpl fetcher(&context_);
+
+  GURL url(test_server_.GetURL("files/redirect_https"));
+  TestFetchCallback callback;
+  StartRequest(url, callback, &fetcher);
+  scoped_ptr<FetchResult> result = callback.WaitForResult();
+  result->VerifyFailure(ERR_DISALLOWED_URL_SCHEME);
+
+  EXPECT_EQ(1, network_delegate_.created_requests());
+}
+
+// Try fetching an unsupported URL scheme (https) and then immediately
+// cancelling. This is a bit special because this codepath needs to post a task.
+TEST_F(CertNetFetcherImplTest, CancelHttpsNotAllowed) {
+  ASSERT_TRUE(test_server_.Start());
+
+  CertNetFetcherImpl fetcher(&context_);
+
+  GURL url("https://foopy/foo.crt");
+  TestFetchCallback callback;
+  CertNetFetcher::RequestId id = StartRequest(url, callback, &fetcher);
+
+  // Should NOT complete synchronously despite being a test that could be done
+  // immediately.
+  EXPECT_FALSE(callback.HasResult());
+
+  EXPECT_EQ(0, network_delegate_.created_requests());
+  fetcher.CancelRequest(id);

[davidben, 2015/03/31 20:52:24]

This probably wants a RunUntilIdle and still assert that callback.HasResult is
false, to assert that the callback never runs.

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

Done.

+}
+
+// Start a few requests, and cancel one of them before running the message loop
+// again.
+TEST_F(CertNetFetcherImplTest, CancelBeforeRunningMessageLoop) {
+  ASSERT_TRUE(test_server_.Start());
+
+  CertNetFetcherImpl fetcher(&context_);
+  TestFetchCallback callback1;
+  TestFetchCallback callback2;
+  TestFetchCallback callback3;
+
+  GURL url1 = test_server_.GetURL("files/cert.crt");
+  StartRequest(url1, callback1, &fetcher);
+
+  GURL url2 = test_server_.GetURL("files/root.crl");
+  CertNetFetcher::RequestId id2 = StartRequest(url2, callback2, &fetcher);
+
+  GURL url3 = test_server_.GetURL("files/certs.p7c");
+  StartRequest(url3, callback3, &fetcher);
+
+  EXPECT_EQ(3, network_delegate_.created_requests());
+  EXPECT_FALSE(callback1.HasResult());
+  EXPECT_FALSE(callback2.HasResult());
+  EXPECT_FALSE(callback3.HasResult());
+
+  // Cancel the second request.
+  fetcher.CancelRequest(id2);
+
+  // Wait for the non-cancelled requests to complete.
+  scoped_ptr<FetchResult> result1 = callback1.WaitForResult();
+  scoped_ptr<FetchResult> result3 = callback3.WaitForResult();
+
+  // Verify the fetch results.
+  result1->VerifySuccess("-cert.crt-\n");
+  result3->VerifySuccess("-certs.p7c-\n");
+
+  EXPECT_FALSE(callback2.HasResult());
+}
+
+// Start several requests, and cancel one of them after the first has completed.
+// NOTE: The python test server is single threaded and can only service one
+// request at a time. After a socket is opened by the server it waits for it to
+// be completed, and any subsequent request will hang until the first socket is
+// closed.
+// Cancelling the first request can therefore be problematic, since if
+// cancellation is done after the socket is opened but before reading/writing,
+// then the socket is re-cycled and things will be stalled until the cleanup
+// timer (10 seconds) closes it.
+// To work around this, the last request is cancelled, and hope that the
+// requests are given opened sockets in a FIFO order.

[davidben, 2015/03/31 20:52:25]

Perhaps if we configured the socket pool allow a maximum of one socket per
group. That would force us to serialize on our end and work around the server
bug. Or we could CloseIdleSockets() right after the CancelRequest (though that
depends on CancelRequest synchronously releasing the socket).

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

I took a quick look on how to configure that and didn't find it. Will leave a
TODO to make this more robust.

+TEST_F(CertNetFetcherImplTest, CancelAfterRunningMessageLoop) {
+  ASSERT_TRUE(test_server_.Start());
+
+  CertNetFetcherImpl fetcher(&context_);
+  TestFetchCallback callback1;
+  TestFetchCallback callback2;
+  TestFetchCallback callback3;
+
+  GURL url1 = test_server_.GetURL("files/cert.crt");
+  StartRequest(url1, callback1, &fetcher);
+
+  GURL url2 = test_server_.GetURL("files/certs.p7c");
+  CertNetFetcher::RequestId id2 = StartRequest(url2, callback2, &fetcher);
+
+  GURL url3("ftp://www.not.supported.com/foo");
+  StartRequest(url3, callback3, &fetcher);
+
+  EXPECT_FALSE(callback1.HasResult());
+  EXPECT_FALSE(callback2.HasResult());
+  EXPECT_FALSE(callback3.HasResult());
+
+  // Wait for the fast request to complete.

[davidben, 2015/03/31 20:52:23]

fast -> last?

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

Fast was intentional -- The last request is for an FTP URL, so it will fail
immediately. Whereas the other requests will try to talk to the test server. So
it is expected that the last request will be the first to complete.

Reworded the comment.

+  scoped_ptr<FetchResult> result3 = callback3.WaitForResult();
+  result3->VerifyFailure(ERR_DISALLOWED_URL_SCHEME);
+
+  // Cancel the second outstanding request.
+  fetcher.CancelRequest(id2);

[davidben, 2015/03/31 20:52:24]

This assumes that callback2 hasn't fired yet. (Realistically this won't happen,
but you have waited on a WaitForResult already.)

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

With the new scoped_ptr<Request> approach this is no longer a problem (since the
Request is owned by caller).

+
+  // Wait for the first request to complete.
+  scoped_ptr<FetchResult> result2 = callback1.WaitForResult();
+
+  // Verify the fetch results.
+  result2->VerifySuccess("-cert.crt-\n");
+}
+
+// Delete a CertNetFetcherImpl with outstanding requests on it.
+TEST_F(CertNetFetcherImplTest, DeleteCancels) {
+  ASSERT_TRUE(test_server_.Start());
+
+  CertNetFetcherImpl fetcher(&context_);
+
+  GURL url(test_server_.GetURL("slow/certs.p7c?20.1"));

[davidben, 2015/03/31 20:52:26]

Why 20.1 instead of 20?

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

Done.

+  TestFetchCallback callback;
+  StartRequest(url, callback, &fetcher);
+
+  // Note that the request is never completed, nor cancelled.
+}
+
+// Fetch the same URLs in parallel and verify that only 1 request is made per
+// URL.
+TEST_F(CertNetFetcherImplTest, ParallelFetchDupes) {

[davidben, 2015/03/31 20:52:25]

Nit: Dupes is a strange word. Duplicates? :-)

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

Done.

+  ASSERT_TRUE(test_server_.Start());
+
+  CertNetFetcherImpl fetcher(&context_);
+
+  GURL url1 = test_server_.GetURL("files/cert.crt");
+  GURL url2 = test_server_.GetURL("files/root.crl");
+
+  // Issue 3 requests for url1, and 3 requests for url2
+  TestFetchCallback callback1;
+  CertNetFetcher::RequestId request1 = StartRequest(url1, callback1, &fetcher);
+
+  TestFetchCallback callback2;
+  StartRequest(url2, callback2, &fetcher);
+
+  TestFetchCallback callback3;
+  CertNetFetcher::RequestId request3 = StartRequest(url1, callback3, &fetcher);
+
+  TestFetchCallback callback4;
+  StartRequest(url2, callback4, &fetcher);
+
+  TestFetchCallback callback5;
+  StartRequest(url2, callback5, &fetcher);
+
+  TestFetchCallback callback6;
+  StartRequest(url1, callback6, &fetcher);
+
+  // Cancel all but one of the requests for url1.
+  fetcher.CancelRequest(request1);
+  fetcher.CancelRequest(request3);
+
+  // Wait for the remaining requests to finish.
+  scoped_ptr<FetchResult> result2 = callback2.WaitForResult();
+  scoped_ptr<FetchResult> result4 = callback4.WaitForResult();
+  scoped_ptr<FetchResult> result5 = callback5.WaitForResult();
+  scoped_ptr<FetchResult> result6 = callback6.WaitForResult();
+
+  // Verify that none of the cancelled requests for url1 completed (since they
+  // were cancelled).
+  EXPECT_FALSE(callback1.HasResult());
+  EXPECT_FALSE(callback3.HasResult());
+
+  // Verify the fetch results.
+  result2->VerifySuccess("-root.crl-\n");
+  result4->VerifySuccess("-root.crl-\n");
+  result5->VerifySuccess("-root.crl-\n");
+  result6->VerifySuccess("-cert.crt-\n");
+
+  // Verify that only 2 URLRequests were started even though 6 requests were
+  // issued.
+  EXPECT_EQ(2, network_delegate_.created_requests());
+}
+
+// Cancel a request and then start another one for the same URL.
+TEST_F(CertNetFetcherImplTest, CancelThenStart) {
+  ASSERT_TRUE(test_server_.Start());
+
+  CertNetFetcherImpl fetcher(&context_);
+  TestFetchCallback callback1;
+  CertNetFetcher::RequestId request1;
+
+  TestFetchCallback callback2;
+  CertNetFetcher::RequestId request2;
+
+  TestFetchCallback callback3;
+  CertNetFetcher::RequestId request3;
+
+  GURL url = test_server_.GetURL("files/cert.crt");
+
+  request1 = StartRequest(url, callback1, &fetcher);
+  fetcher.CancelRequest(request1);
+
+  request2 = StartRequest(url, callback2, &fetcher);
+
+  request3 = StartRequest(url, callback3, &fetcher);
+  fetcher.CancelRequest(request3);
+
+  // All but |request2| were canceled.
+  scoped_ptr<FetchResult> result = callback2.WaitForResult();
+
+  result->VerifySuccess("-cert.crt-\n");
+
+  EXPECT_FALSE(callback1.HasResult());
+  EXPECT_FALSE(callback3.HasResult());
+
+  // One URLRequest that was cancelled, then another right afterwards.
+  EXPECT_EQ(2, network_delegate_.created_requests());
+}
+
+// Start duplicate requests and then cancel all of them.
+TEST_F(CertNetFetcherImplTest, CancelAll) {
+  ASSERT_TRUE(test_server_.Start());
+
+  CertNetFetcherImpl fetcher(&context_);
+  TestFetchCallback callback[3];
+  CertNetFetcher::RequestId request[3];
+
+  GURL url = test_server_.GetURL("files/cert.crt");
+
+  for (size_t i = 0; i < arraysize(callback); ++i) {
+    request[i] = StartRequest(url, callback[i], &fetcher);
+  }

[davidben, 2015/03/31 20:52:25]

Nit: This line has curlies while the rest don't.

(Curly-less for loops actually really really confuse me, but apparently we do
that in //net, so okay.)

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

Done.

+
+  for (size_t i = 0; i < arraysize(request); ++i)
+    fetcher.CancelRequest(request[i]);
+
+  EXPECT_EQ(1, network_delegate_.created_requests());
+
+  for (size_t i = 0; i < arraysize(request); ++i)
+    EXPECT_FALSE(callback[i].HasResult());
+}
+
+void DeleteCertNetFetcher(CertNetFetcher* fetcher) {
+  delete fetcher;
+}
+
+// Delete the CertNetFetcherImpl within a request callback.
+TEST_F(CertNetFetcherImplTest, DeleteWithinCallback) {
+  ASSERT_TRUE(test_server_.Start());
+
+  // Deleted by callback2.
+  CertNetFetcher* fetcher = new CertNetFetcherImpl(&context_);
+
+  GURL url = test_server_.GetURL("files/cert.crt");
+
+  TestFetchCallback callback[4];
+  callback[1].set_extra_closure(base::Bind(DeleteCertNetFetcher, fetcher));
+
+  for (size_t i = 0; i < arraysize(callback); ++i)
+    StartRequest(url, callback[i], fetcher);
+
+  EXPECT_EQ(1, network_delegate_.created_requests());
+
+  callback[1].WaitForResult();
+
+  // Assume requests for the same URL are executed in FIFO order.
+  EXPECT_TRUE(callback[0].HasResult());
+  EXPECT_FALSE(callback[2].HasResult());
+  EXPECT_FALSE(callback[3].HasResult());
+}
+
+void FetchRequest(CertNetFetcher* fetcher,
+                  const GURL& url,
+                  TestFetchCallback* callback) {
+  StartRequest(url, *callback, fetcher);
+}
+
+// Make a request during callback for the same URL.
+TEST_F(CertNetFetcherImplTest, FetchWithinCallback) {
+  ASSERT_TRUE(test_server_.Start());
+
+  CertNetFetcherImpl fetcher(&context_);
+
+  GURL url = test_server_.GetURL("files/cert.crt");
+
+  TestFetchCallback callback[5];
+  callback[1].set_extra_closure(
+      base::Bind(FetchRequest, &fetcher, url, &callback[4]));
+
+  for (size_t i = 0; i < arraysize(callback) - 1; ++i)
+    StartRequest(url, callback[i], &fetcher);
+
+  EXPECT_EQ(1, network_delegate_.created_requests());
+
+  for (size_t i = 0; i < arraysize(callback); ++i) {
+    scoped_ptr<FetchResult> result = callback[i].WaitForResult();
+    result->VerifySuccess("-cert.crt-\n");
+  }
+
+  // The fetch started within a callback should have started a new request
+  // rather than attaching to the current job.
+  EXPECT_EQ(2, network_delegate_.created_requests());
+}
+
+void CancelRequest(CertNetFetcher* fetcher, CertNetFetcher::RequestId request) {
+  fetcher->CancelRequest(request);
+}
+
+// Cancel a request while executing a callback for the same job.
+TEST_F(CertNetFetcherImplTest, CancelWithinCallback) {
+  ASSERT_TRUE(test_server_.Start());
+
+  CertNetFetcherImpl fetcher(&context_);
+
+  GURL url = test_server_.GetURL("files/cert.crt");
+
+  TestFetchCallback callback[4];
+  CertNetFetcher::RequestId request[4];
+
+  for (size_t i = 0; i < arraysize(callback); ++i) {
+    request[i] = StartRequest(url, callback[i], &fetcher);
+  }
+
+  // Cancel request[2] when the callback for request[1] runs.
+  callback[1].set_extra_closure(
+      base::Bind(CancelRequest, &fetcher, request[2]));
+
+  EXPECT_EQ(1, network_delegate_.created_requests());
+
+  for (size_t i = 0; i < arraysize(request); ++i) {
+    if (i == 2)
+      continue;
+
+    scoped_ptr<FetchResult> result = callback[i].WaitForResult();
+    result->VerifySuccess("-cert.crt-\n");
+  }
+
+  // request[2] was cancelled.
+  EXPECT_FALSE(callback[2].HasResult());
+}
+
+}  // namespace net