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.
+  // 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());

[pneubeck (no reviews), 2014/12/14 16:45:12]

how do you ensure that the raw_ptr_to_store is still valid (same for all similar
tests)? wouldn't it be more reliable if the ClientCertStoreStub would write its
results to some variables that outlive the previous calls for sure?

[davidben, 2015/01/23 21:05:35]

Yeah, that does seem better. There's a lot of other raw_ptr_to_*s in this file,
but let's resolve that one to start with. Done.

+  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