Cancel client auth requests when not promptable.

chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa.mm

Index: chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa.mm
diff --git a/chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa.mm b/chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa.mm
index 9d471b923de6fe2903f327656efcdfd7df25dc2e..474b8367d21c4d4a12482600afbaa779a3b1e84d 100644
--- a/chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa.mm
+++ b/chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa.mm
@@ -14,7 +14,9 @@
 #include "chrome/browser/ssl/ssl_client_auth_observer.h"
 #import "chrome/browser/ui/cocoa/constrained_window/constrained_window_mac.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/cert/x509_util_mac.h"
@@ -42,11 +44,12 @@ class SSLClientAuthObserverCocoaBridge : public SSLClientAuthObserver,
   SSLClientAuthObserverCocoaBridge(
       const content::BrowserContext* browser_context,
       net::SSLCertRequestInfo* cert_request_info,
-      const chrome::SelectCertificateCallback& callback,
+      scoped_ptr<content::ClientCertificateDelegate> delegate,
       SSLClientCertificateSelectorCocoa* controller)
-      : SSLClientAuthObserver(browser_context, cert_request_info, callback),
-        controller_(controller) {
-  }
+      : SSLClientAuthObserver(browser_context,
+                              cert_request_info,
+                              delegate.Pass()),
+        controller_(controller) {}
 
   // SSLClientAuthObserver implementation:
   void OnCertSelectedByNotification() override {
@@ -72,14 +75,24 @@ namespace chrome {
 void ShowSSLClientCertificateSelector(
     content::WebContents* contents,
     net::SSLCertRequestInfo* cert_request_info,
-    const SelectCertificateCallback& callback) {
+    scoped_ptr<content::ClientCertificateDelegate> delegate) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  // 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) {
+    delegate->CancelCertificateSelection();
+    return;
+  }
+
   // The dialog manages its own lifetime.
   SSLClientCertificateSelectorCocoa* selector =
       [[SSLClientCertificateSelectorCocoa alloc]
           initWithBrowserContext:contents->GetBrowserContext()
                  certRequestInfo:cert_request_info
-                        callback:callback];
+                        delegate:delegate.Pass()];
   [selector displayForWebContents:contents];
 }
 
@@ -88,13 +101,14 @@ void ShowSSLClientCertificateSelector(
 @implementation SSLClientCertificateSelectorCocoa
 
 - (id)initWithBrowserContext:(const content::BrowserContext*)browserContext
-    certRequestInfo:(net::SSLCertRequestInfo*)certRequestInfo
-           callback:(const chrome::SelectCertificateCallback&)callback {
+             certRequestInfo:(net::SSLCertRequestInfo*)certRequestInfo
+                    delegate:(scoped_ptr<content::ClientCertificateDelegate>)
+                                 delegate {
   DCHECK(browserContext);
   DCHECK(certRequestInfo);
   if ((self = [super init])) {
     observer_.reset(new SSLClientAuthObserverCocoaBridge(
-        browserContext, certRequestInfo, callback, self));
+        browserContext, certRequestInfo, delegate.Pass(), self));
   }
   return self;
 }
@@ -113,9 +127,12 @@ void ShowSSLClientCertificateSelector(
       NOTREACHED();
   }
 
-  // Finally, tell the backend which identity (or none) the user selected.
-  observer_->StopObserving();
-  observer_->CertificateSelected(cert);
+  if (!certificateSelected_) {
+    // Finally, tell the backend which identity (or none) the user selected.
+    certificateSelected_ = YES;
+    observer_->StopObserving();
+    observer_->CertificateSelected(cert);
+  }
 
   if (!closePending_)
     constrainedWindow_->CloseWebContentsModalDialog();
@@ -184,6 +201,11 @@ void ShowSSLClientCertificateSelector(
 }
 
 - (void)closeSheetWithAnimation:(BOOL)withAnimation {
+  if (!certificateSelected_) {
+    certificateSelected_ = YES;
+    observer_->StopObserving();
+    observer_->CancelCertificateSelection();

[mattm, 2015/02/14 00:32:58]

Does sheetDidEnd not have a unique returnCode for when it is closed rather than
canceled?

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

Looks like it gives the same as cancel. Sane dialogs would try those the same,
but this isn't a sane dialog. :-) Possibly we should have an explicit "no
certificate" entry in there. Though that would mean not using the native one.

I've added a comment to clarify.

+  }
   closePending_ = YES;
   overlayWindow_.reset();
   // Closing the sheet using -[NSApp endSheet:] doesn't work so use the private