Un-refcount SSLClientAuthHandler.

content/browser/loader/resource_loader_unittest.cc

 // Copyright (c) 2013 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 "content/browser/loader/resource_loader.h"
 
 #include "base/files/file.h"
 #include "base/files/file_util.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/run_loop.h"
@@ skipping 23 matching lines @@
 
 namespace content {
 namespace {
 
 // Stub client certificate store that returns a preset list of certificates for
 // each request and records the arguments of the most recent request for later
 // inspection.
 class ClientCertStoreStub : public net::ClientCertStore {
  public:
   ClientCertStoreStub(const net::CertificateList& certs)
-      : response_(certs),
-        request_count_(0) {}
+      : response_(certs), async_(false), request_count_(0) {}
 
   ~ClientCertStoreStub() override {}
 
+  // Configures whether the certificates are returned asynchronously or not.
+  void set_async(bool async) { async_ = async; }
+
   // Returns |cert_authorities| field of the certificate request passed in the
   // most recent call to GetClientCerts().
   // TODO(ppi): Make the stub independent from the internal representation of
   // SSLCertRequestInfo. For now it seems that we cannot neither save the
   // scoped_refptr<> (since it is never passed to us) nor copy the entire
   // CertificateRequestInfo (since there is no copy constructor).
   std::vector<std::string> requested_authorities() {
     return requested_authorities_;
   }
 
   // Returns the number of calls to GetClientCerts().
   int request_count() {
     return request_count_;
   }
 
   // net::ClientCertStore:
   void GetClientCerts(const net::SSLCertRequestInfo& cert_request_info,
                       net::CertificateList* selected_certs,
                       const base::Closure& callback) override {
     ++request_count_;
     requested_authorities_ = cert_request_info.cert_authorities;
     *selected_certs = response_;
-    callback.Run();
+    if (async_) {
+      base::MessageLoop::current()->PostTask(FROM_HERE, callback);
+    } else {
+      callback.Run();
+    }
   }
 
  private:
   const net::CertificateList response_;
+  bool async_;
   int request_count_;
   std::vector<std::string> requested_authorities_;
 };
 
 // Arbitrary read buffer size.
 const int kReadBufSize = 1024;
 
 // Dummy implementation of ResourceHandler, instance of which is needed to
 // initialize ResourceLoader.
 class ResourceHandlerStub : public ResourceHandler {
@@ skipping 344 matching lines @@
 
   // Restore the original content browser client.
   SetBrowserClientForTesting(old_client);
 
   // Check if the SelectClientCertificate was called on the content browser
   // client.
   EXPECT_EQ(1, test_client.call_count());
   EXPECT_EQ(net::CertificateList(), test_client.passed_certs());
 }
 
+TEST_F(ResourceLoaderTest, ClientCertStoreAsyncCancel) {
+  // Set up the test client cert store.
+  scoped_ptr<ClientCertStoreStub> test_store(
+      new ClientCertStoreStub(net::CertificateList()));
+  test_store->set_async(true);
+  EXPECT_EQ(0, test_store->request_count());
+
+  // Ownership of the |test_store| is about to be turned over to ResourceLoader.

[mmenke, 2014/12/11 21:02:20]

Could you add some docs to ClientCertStore?  The name sounds to me like "this is
the store of all client certs", which would imply a global object, not something
that should be owned by an individual request.

Makes sense to do that in another CL.

[davidben, 2014/12/11 22:08:10]

https://codereview.chromium.org/802563002

+  // We need to keep raw pointer copies to access these objects later.
+  ClientCertStoreStub* raw_ptr_to_store = test_store.get();
+  resource_context_.SetClientCertStore(test_store.Pass());
+
+  // Prepare a dummy certificate request.
+  scoped_refptr<net::SSLCertRequestInfo> cert_request_info(
+      new net::SSLCertRequestInfo());
+  std::vector<std::string> dummy_authority(1, "dummy");
+  cert_request_info->cert_authorities = dummy_authority;
+
+  // Everything is set up. Trigger the resource loader certificate request
+  // event.
+  loader_->OnCertificateRequested(raw_ptr_to_request_, cert_request_info.get());
+
+  // Check if the test store was queried against correct |cert_authorities|.
+  EXPECT_EQ(1, raw_ptr_to_store->request_count());
+  EXPECT_EQ(dummy_authority, raw_ptr_to_store->requested_authorities());
+
+  // Cancel the request before the store calls the callback.
+  loader_.reset();
+
+  // Pump the event loop. There shouldn't be a crash when the callback is run.
+  base::RunLoop().RunUntilIdle();
+}
+
 TEST_F(ResourceLoaderTest, ResumeCancelledRequest) {
   raw_ptr_resource_handler_->set_defer_request_on_will_start(true);
 
   loader_->StartRequest();
   loader_->CancelRequest(true);
   static_cast<ResourceController*>(loader_.get())->Resume();
 }
 
 // Tests that no invariants are broken if a ResourceHandler cancels during
 // OnReadCompleted.
@@ skipping 271 matching lines @@
   ASSERT_TRUE(base::ReadFileToString(temp_path(), &contents));
   EXPECT_EQ(test_data(), contents);
 
   // Release the loader. The file should be gone now.
   ReleaseLoader();
   base::RunLoop().RunUntilIdle();
   EXPECT_FALSE(base::PathExists(temp_path()));
 }
 
 }  // namespace content