Cancel client auth requests when not promptable.

android_webview/native/aw_contents_client_bridge_unittest.cc

Index: android_webview/native/aw_contents_client_bridge_unittest.cc
diff --git a/android_webview/native/aw_contents_client_bridge_unittest.cc b/android_webview/native/aw_contents_client_bridge_unittest.cc
index b301f9199276f1923f676564ef3054181b0ed6f8..5fce9be206585f06db02bdd765d15ad9a624df44 100644
--- a/android_webview/native/aw_contents_client_bridge_unittest.cc
+++ b/android_webview/native/aw_contents_client_bridge_unittest.cc
@@ -8,8 +8,10 @@
 #include "base/android/jni_array.h"
 #include "base/android/scoped_java_ref.h"
 #include "base/bind.h"
+#include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/run_loop.h"
+#include "content/public/browser/client_certificate_delegate.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "jni/MockAwContentsClientBridge_jni.h"
 #include "net/android/net_jni_registrar.h"
@@ -33,11 +35,10 @@ namespace {
 // Tests the android_webview contents client bridge.
 class AwContentsClientBridgeTest : public Test {
  public:
-  typedef AwContentsClientBridge::SelectCertificateCallback
-      SelectCertificateCallback;
-
   AwContentsClientBridgeTest() { }
 
+  scoped_ptr<content::ClientCertificateDelegate> CreateDelegate();
+
   // Callback method called when a cert is selected.
   void CertSelected(X509Certificate* cert);
  protected:
@@ -53,6 +54,26 @@ class AwContentsClientBridgeTest : public Test {
   JNIEnv* env_;
 };
 
+class TestClientCertificateDelegate
+    : public content::ClientCertificateDelegate {
+ public:
+  TestClientCertificateDelegate(AwContentsClientBridgeTest* test)

[mmenke, 2015/02/17 19:50:14]

explicit

[davidben, 2015/02/18 22:31:44]

Done.

+      : test_(test) {}
+
+  // content::ClientCertificateDelegate.
+  void ContinueWithCertificate(net::X509Certificate* cert) override {
+    test_->CertSelected(cert);
+  }
+
+  void CancelCertificateSelection() override {
+    ADD_FAILURE() << "Selection canceled";
+  }
+
+ private:
+  AwContentsClientBridgeTest* test_;
+  DISALLOW_COPY_AND_ASSIGN(TestClientCertificateDelegate);

[mmenke, 2015/02/17 19:50:14]

nit:  Blank line before DISALLOW_...

[davidben, 2015/02/18 22:31:44]

Done.

+};
+
 }   // namespace
 
 void AwContentsClientBridgeTest::SetUp() {
@@ -69,6 +90,12 @@ void AwContentsClientBridgeTest::SetUp() {
   cert_request_info_ = new net::SSLCertRequestInfo;
 }
 
+scoped_ptr<content::ClientCertificateDelegate>
+AwContentsClientBridgeTest::CreateDelegate() {
+  return scoped_ptr<content::ClientCertificateDelegate>(
+      new TestClientCertificateDelegate(this));
+}

[mmenke, 2015/02/17 19:50:14]

I don't think the saved code is worth the ugliness of having a circular
dependency.  Fine however you choose to unwrangle it (Make this a static method
of TestClientCertificateDelegate that takes the test fixture, make the delegate
take a callback, give the Delegate a RunLoop and then pull the chain from that,
whatever).

[davidben, 2015/02/18 22:31:44]

Switched it to use make_scoped_ptr which I just discovered existed.

+
 void AwContentsClientBridgeTest::CertSelected(X509Certificate* cert) {
   selected_cert_ = cert;
   cert_selected_callbacks_++;
@@ -88,11 +115,7 @@ void AwContentsClientBridgeTest::TestCertType(SSLClientCertType type,
       const std::string& expected_name) {
   cert_request_info_->cert_key_types.clear();
   cert_request_info_->cert_key_types.push_back(type);
-  bridge_->SelectClientCertificate(
-      cert_request_info_.get(),
-      base::Bind(
-          &AwContentsClientBridgeTest::CertSelected,
-          base::Unretained(static_cast<AwContentsClientBridgeTest*>(this))));
+  bridge_->SelectClientCertificate(cert_request_info_.get(), CreateDelegate());
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(0, cert_selected_callbacks_);
   ScopedJavaLocalRef<jobjectArray> key_types =
@@ -111,11 +134,7 @@ TEST_F(AwContentsClientBridgeTest,
     TestProvideClientCertificateResponseCallsCallbackOnNullKey) {
   // Call SelectClientCertificate to create a callback id that mock java object
   // can call on.
-  bridge_->SelectClientCertificate(
-    cert_request_info_.get(),
-    base::Bind(
-        &AwContentsClientBridgeTest::CertSelected,
-        base::Unretained(static_cast<AwContentsClientBridgeTest*>(this))));
+  bridge_->SelectClientCertificate(cert_request_info_.get(), CreateDelegate());
   bridge_->ProvideClientCertificateResponse(env_, jbridge_.obj(),
       Java_MockAwContentsClientBridge_getRequestId(env_, jbridge_.obj()),
       Java_MockAwContentsClientBridge_createTestCertChain(
@@ -132,11 +151,7 @@ TEST_F(AwContentsClientBridgeTest,
     TestProvideClientCertificateResponseCallsCallbackOnNullChain) {
   // Call SelectClientCertificate to create a callback id that mock java object
   // can call on.
-  bridge_->SelectClientCertificate(
-    cert_request_info_.get(),
-    base::Bind(
-        &AwContentsClientBridgeTest::CertSelected,
-        base::Unretained(static_cast<AwContentsClientBridgeTest*>(this))));
+  bridge_->SelectClientCertificate(cert_request_info_.get(), CreateDelegate());
   int requestId =
     Java_MockAwContentsClientBridge_getRequestId(env_, jbridge_.obj());
   bridge_->ProvideClientCertificateResponse(env_, jbridge_.obj(),