Cancel client auth requests when not promptable.

chrome/browser/ui/views/ssl_client_certificate_selector.cc

Index: chrome/browser/ui/views/ssl_client_certificate_selector.cc
diff --git a/chrome/browser/ui/views/ssl_client_certificate_selector.cc b/chrome/browser/ui/views/ssl_client_certificate_selector.cc
index 097195fc71714aa5cbe7c1e4b24fa41ee0f81f30..09e71a3d10d74d3afba0caccb24dbf276159b5bd 100644
--- a/chrome/browser/ui/views/ssl_client_certificate_selector.cc
+++ b/chrome/browser/ui/views/ssl_client_certificate_selector.cc
@@ -9,7 +9,9 @@
 #include "base/logging.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/grit/generated_resources.h"
+#include "components/web_modal/popup_manager.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/client_certificate_delegate.h"
 #include "content/public/browser/web_contents.h"
 #include "net/cert/x509_certificate.h"
 #include "net/ssl/ssl_cert_request_info.h"
@@ -23,11 +25,11 @@
 SSLClientCertificateSelector::SSLClientCertificateSelector(
     content::WebContents* web_contents,
     const scoped_refptr<net::SSLCertRequestInfo>& cert_request_info,
-    const chrome::SelectCertificateCallback& callback)
+    scoped_ptr<content::ClientCertificateDelegate> delegate)
     : CertificateSelector(cert_request_info->client_certs, web_contents),
       SSLClientAuthObserver(web_contents->GetBrowserContext(),
                             cert_request_info,
-                            callback) {
+                            delegate.Pass()) {
   DVLOG(1) << __FUNCTION__;

[Ryan Sleevi, 2015/02/25 06:31:36]

Any idea why all this debug spam is here? Can you nuke it in a follow-up CL

[sky, 2015/02/25 15:57:27]

If you don't like this spam, why are we adding more on 82?

[davidben, 2015/02/26 18:09:41]

Copy-pasta mostly. I've removed it from the newly added function and will remove
the rest in a follow-up.

 }
 
@@ -76,6 +78,13 @@ bool SSLClientCertificateSelector::Accept() {
   return false;
 }
 
+bool SSLClientCertificateSelector::Close() {
+  DVLOG(1) << __FUNCTION__;
+  StopObserving();

[sky, 2015/02/25 15:57:27]

Is there a reason we need StopObserving every where and it can't be done only in
the constructor?

[davidben, 2015/02/26 18:09:41]

Constructor wouldn't work since this is Stop, not Start. But that confused me
too. The two completion hooks hooks already StopObserving(), so there's really
no need to do it here, except possibly in Accept[*]. I've removed it from the
new method and will remove it from Cancel() in the follow-up as the DVLOG spew.

Also added a comment that Close() sorely needs.[**]

[*] That code also has other issues anyway. https://crbug.com/460215

[**] Yeah, this UI is awful and making the Cancel button take an action is
screwy. We should have an explicit entry for 'Send no certificate' that you hit
OK for maybe. I'm intentionally not trying to resolve that half here and just
trying to get the plumbing needed for the crash and close-tab cases. Having an
explicit "no certificate" option would still need this same plumbing.

+  CancelCertificateSelection();
+  return true;
+}
+
 void SSLClientCertificateSelector::Unlocked(net::X509Certificate* cert) {
   DVLOG(1) << __FUNCTION__;
   CertificateSelected(cert);
@@ -87,11 +96,19 @@ namespace chrome {
 void ShowSSLClientCertificateSelector(
     content::WebContents* contents,
     net::SSLCertRequestInfo* cert_request_info,
-    const chrome::SelectCertificateCallback& callback) {
+    scoped_ptr<content::ClientCertificateDelegate> delegate) {
   DVLOG(1) << __FUNCTION__ << " " << contents;
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  SSLClientCertificateSelector* selector =
-      new SSLClientCertificateSelector(contents, cert_request_info, callback);
+
+  // Not all WebContentses can show modal dialogs.
+  //
+  // TODO(davidben): Move this hook to the WebContentsDelegate and only try to
+  // show a dialog in Browser's implementation. https://crbug.com/456255
+  if (web_modal::PopupManager::FromWebContents(contents) == nullptr)
+    return;
+
+  SSLClientCertificateSelector* selector = new SSLClientCertificateSelector(
+      contents, cert_request_info, delegate.Pass());
   selector->Init();
   selector->Show();
 }