Cancel client auth requests when not promptable.

content/browser/loader/resource_loader_unittest.cc

Index: content/browser/loader/resource_loader_unittest.cc
diff --git a/content/browser/loader/resource_loader_unittest.cc b/content/browser/loader/resource_loader_unittest.cc
index 92241b68d0beab40f4d03c6512289017c9578e5b..757fe414c94b964222e759178512ebf50c6297ee 100644
--- a/content/browser/loader/resource_loader_unittest.cc
+++ b/content/browser/loader/resource_loader_unittest.cc
@@ -11,11 +11,15 @@
 #include "content/browser/browser_thread_impl.h"
 #include "content/browser/loader/redirect_to_file_resource_handler.h"
 #include "content/browser/loader/resource_loader_delegate.h"
+#include "content/public/browser/client_certificate_delegate.h"
 #include "content/public/browser/resource_request_info.h"
 #include "content/public/common/resource_response.h"
 #include "content/public/test/mock_resource_context.h"
+#include "content/public/test/test_browser_context.h"
 #include "content/public/test/test_browser_thread_bundle.h"
+#include "content/public/test/test_renderer_host.h"
 #include "content/test/test_content_browser_client.h"
+#include "content/test/test_web_contents.h"
 #include "ipc/ipc_message.h"
 #include "net/base/io_buffer.h"
 #include "net/base/mock_file_stream.h"
@@ -239,10 +243,9 @@ class SelectCertificateBrowserClient : public TestContentBrowserClient {
   SelectCertificateBrowserClient() : call_count_(0) {}
 
   void SelectClientCertificate(
-      int render_process_id,
-      int render_view_id,
+      WebContents* web_contents,
       net::SSLCertRequestInfo* cert_request_info,
-      const base::Callback<void(net::X509Certificate*)>& callback) override {
+      scoped_ptr<ClientCertificateDelegate> delegate) override {
     ++call_count_;
     passed_certs_ = cert_request_info->client_certs;
   }
@@ -317,8 +320,12 @@ class ResourceLoaderTest : public testing::Test,
   }
 
   void SetUp() override {
-    const int kRenderProcessId = 1;
-    const int kRenderViewId = 2;
+    browser_context_.reset(new TestBrowserContext());
+    scoped_refptr<SiteInstance> site_instance =
+        SiteInstance::Create(browser_context_.get());
+    web_contents_.reset(
+        TestWebContents::Create(browser_context_.get(), site_instance.get()));

[mmenke, 2015/02/10 17:19:10]

I'm confused.  Why are these needed?  Aren't we just using the default
ContentBrowserClient, which will cancel any cert requests it sees, anyways?

[davidben, 2015/02/10 20:28:49]

There's a handful of tests that install a custom one, so I need there to be an
actual WebContents on it. See SelectCertificateBrowserClient up top.

[mmenke, 2015/02/11 17:09:16]

Ah, somehow I was missing SelectCertificateBrowserClient.  Should we have a
tests in this file where SelectClientCertificate cancels the request, and a
request where there's no WebContents?  Yes, we're adding browser tests for those
cases, but it seems like since they're both loader behavior (And the code is all
in content/), we should have tests for both cases here, too.  Also always nice
to have unit tests, as they're easier to debug.

[davidben, 2015/02/12 22:55:15]

Done.

+    RenderFrameHost* rfh = web_contents_->GetMainFrame();
 
     scoped_ptr<net::URLRequest> request(
         resource_context_.GetRequestContext()->CreateRequest(
@@ -330,9 +337,9 @@ class ResourceLoaderTest : public testing::Test,
     ResourceRequestInfo::AllocateForTesting(request.get(),
                                             RESOURCE_TYPE_MAIN_FRAME,
                                             &resource_context_,
-                                            kRenderProcessId,
-                                            kRenderViewId,
+                                            rfh->GetProcess()->GetID(),
                                             MSG_ROUTING_NONE,
+                                            rfh->GetRoutingID(),

[mmenke, 2015/02/10 17:19:10]

Think you messed up the order here

[davidben, 2015/02/10 20:28:49]

I'm passing in RFH ID, and not RVH ID. Getting the RVH ID for completeness is
probably a good plan though...

                                             true,    // is_main_frame
                                             false,   // parent_is_main_frame
                                             true,    // allow_download
@@ -346,6 +353,11 @@ class ResourceLoaderTest : public testing::Test,
         this));
   }
 
+  void TearDown() override {
+    web_contents_.reset();
+    base::RunLoop().RunUntilIdle();
+  }
+
   // ResourceLoaderDelegate:
   ResourceDispatcherHostLoginDelegate* CreateLoginDelegate(
       ResourceLoader* loader,
@@ -362,11 +374,14 @@ class ResourceLoaderTest : public testing::Test,
   void DidReceiveResponse(ResourceLoader* loader) override {}
   void DidFinishLoading(ResourceLoader* loader) override {}
 
-  content::TestBrowserThreadBundle thread_bundle_;
+  TestBrowserThreadBundle thread_bundle_;
+  RenderViewHostTestEnabler rvh_test_enabler_;
 
   net::URLRequestJobFactoryImpl job_factory_;
   net::TestURLRequestContext test_url_request_context_;
   ResourceContextStub resource_context_;
+  scoped_ptr<TestBrowserContext> browser_context_;
+  scoped_ptr<TestWebContents> web_contents_;
 
   // The ResourceLoader owns the URLRequest and the ResourceHandler.
   ResourceHandlerStub* raw_ptr_resource_handler_;