Cancel client auth requests when not promptable.

android_webview/native/aw_contents_client_bridge.cc

Index: android_webview/native/aw_contents_client_bridge.cc
diff --git a/android_webview/native/aw_contents_client_bridge.cc b/android_webview/native/aw_contents_client_bridge.cc
index 9d6860378e57ef682ec8a46d01ff367b9f225252..c1d4ca55a3e68b6f6fdc81ed1d1410b2c4e0dc68 100644
--- a/android_webview/native/aw_contents_client_bridge.cc
+++ b/android_webview/native/aw_contents_client_bridge.cc
@@ -11,6 +11,7 @@
 #include "base/android/jni_string.h"
 #include "base/callback_helpers.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/client_certificate_delegate.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
@@ -52,6 +53,12 @@ AwContentsClientBridge::AwContentsClientBridge(JNIEnv* env, jobject obj)
   DCHECK(obj);
   Java_AwContentsClientBridge_setNativeContentsClientBridge(
       env, obj, reinterpret_cast<intptr_t>(this));
+
+  for (IDMap<content::ClientCertificateDelegate>::iterator iter(
+           &pending_client_cert_request_delegates_);
+       !iter.IsAtEnd(); iter.Advance()) {
+    delete iter.GetCurrentValue();
+  }

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

BUG:  This should go in the destructor.

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

*facepalm* Done.

 }
 
 AwContentsClientBridge::~AwContentsClientBridge() {
@@ -114,13 +121,13 @@ void AwContentsClientBridge::ProceedSslError(JNIEnv* env, jobject obj,
 // This method is inspired by SelectClientCertificate() in
 // chrome/browser/ui/android/ssl_client_certificate_request.cc
 void AwContentsClientBridge::SelectClientCertificate(
-      net::SSLCertRequestInfo* cert_request_info,
-      const SelectCertificateCallback& callback) {
+    net::SSLCertRequestInfo* cert_request_info,
+    scoped_ptr<content::ClientCertificateDelegate> delegate) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   // Add the callback to id map.
-  int request_id = pending_client_cert_request_callbacks_.Add(
-      new SelectCertificateCallback(callback));
+  int request_id =
+      pending_client_cert_request_delegates_.Add(delegate.release());
   // Make sure callback is run on error.
   base::ScopedClosureRunner guard(base::Bind(
       &AwContentsClientBridge::HandleErrorInClientCertificateResponse,
@@ -196,9 +203,9 @@ void AwContentsClientBridge::ProvideClientCertificateResponse(
     jobject private_key_ref) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  SelectCertificateCallback* callback =
-      pending_client_cert_request_callbacks_.Lookup(request_id);
-  DCHECK(callback);
+  content::ClientCertificateDelegate* delegate =
+      pending_client_cert_request_delegates_.Lookup(request_id);
+  DCHECK(delegate);
 
   // Make sure callback is run on error.
   base::ScopedClosureRunner guard(base::Bind(
@@ -236,20 +243,20 @@ void AwContentsClientBridge::ProvideClientCertificateResponse(
     return;
   }
 
+  // Release the guard and |pending_client_cert_request_delegates_| references
+  // to |delegate|.
+  pending_client_cert_request_delegates_.Remove(request_id);
+  ignore_result(guard.Release());

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

Need to include base/macros.h for ignore_result.  Header isn't using it, since
it doesn't use DISALLOW_COPY_AND_ASSIGN (Erroneously, presumably)

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

Done.

+
   // RecordClientCertificateKey() must be called on the I/O thread,
-  // before the callback is called with the selected certificate on
+  // before the delegate is called with the selected certificate on
   // the UI thread.
   content::BrowserThread::PostTaskAndReply(
-      content::BrowserThread::IO,
-      FROM_HERE,
-      base::Bind(&RecordClientCertificateKey,
-                 client_cert,
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&RecordClientCertificateKey, client_cert,
                  base::Passed(&private_key)),
-      base::Bind(*callback, client_cert));
-  pending_client_cert_request_callbacks_.Remove(request_id);
-
-  // Release the guard.
-  ignore_result(guard.Release());
+      base::Bind(&content::ClientCertificateDelegate::ContinueWithCertificate,
+                 base::Owned(delegate), client_cert));
 }
 
 void AwContentsClientBridge::RunJavaScriptDialog(
@@ -377,10 +384,12 @@ void AwContentsClientBridge::CancelJsResult(JNIEnv*, jobject, int id) {
 // Use to cleanup if there is an error in client certificate response.
 void AwContentsClientBridge::HandleErrorInClientCertificateResponse(
     int request_id) {
-  SelectCertificateCallback* callback =
-      pending_client_cert_request_callbacks_.Lookup(request_id);
-  callback->Run(nullptr);
-  pending_client_cert_request_callbacks_.Remove(request_id);
+  content::ClientCertificateDelegate* delegate =
+      pending_client_cert_request_delegates_.Lookup(request_id);
+  pending_client_cert_request_delegates_.Remove(request_id);
+
+  delegate->ContinueWithCertificate(nullptr);

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

You should probably have someone with an opinion on such things review when we
continue and when we cancel.  I really don't have sufficient context to review
these decisions.

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

Yeah, this is partly why I left it alone for the non-Chrome uses.
(ContinueWithCertificate(nullptr) the no-op option.)

+  delete delegate;
 }
 
 bool RegisterAwContentsClientBridge(JNIEnv* env) {