feat: promisify dialog.showCertificateTrustDialog() (#17181)

* feat: promisify dialog.showCertificateTrustDialog()

* update promisification doc

atom/browser/ui/certificate_trust.h

@@ -7,7 +7,7 @@
 
 #include <string>
 
-#include "base/callback_forward.h"
+#include "atom/common/promise_util.h"
 #include "base/memory/ref_counted.h"
 #include "net/cert/x509_certificate.h"
 
@@ -17,12 +17,10 @@ class NativeWindow;
 
 namespace certificate_trust {
 
-typedef base::Callback<void(void)> ShowTrustCallback;
+v8::Local<v8::Promise> ShowCertificateTrust(
-
+    atom::NativeWindow* parent_window,
-void ShowCertificateTrust(atom::NativeWindow* parent_window,
+    const scoped_refptr<net::X509Certificate>& cert,
-                          const scoped_refptr<net::X509Certificate>& cert,
+    const std::string& message);
-                          const std::string& message,
-                          const ShowTrustCallback& callback);
 
 }  // namespace certificate_trust
 

atom/browser/ui/certificate_trust_mac.mm

@@ -15,7 +15,7 @@
 
 @interface TrustDelegate : NSObject {
  @private
-  certificate_trust::ShowTrustCallback callback_;
+  std::unique_ptr<atom::util::Promise> promise_;
   SFCertificateTrustPanel* panel_;
   scoped_refptr<net::X509Certificate> cert_;
   SecTrustRef trust_;
@@ -23,12 +23,12 @@
   SecPolicyRef sec_policy_;
 }
 
-- (id)initWithCallback:(const certificate_trust::ShowTrustCallback&)callback
+- (id)initWithPromise:(atom::util::Promise)promise
-                 panel:(SFCertificateTrustPanel*)panel
+                panel:(SFCertificateTrustPanel*)panel
-                  cert:(const scoped_refptr<net::X509Certificate>&)cert
+                 cert:(const scoped_refptr<net::X509Certificate>&)cert
-                 trust:(SecTrustRef)trust
+                trust:(SecTrustRef)trust
-             certChain:(CFArrayRef)certChain
+            certChain:(CFArrayRef)certChain
-             secPolicy:(SecPolicyRef)secPolicy;
+            secPolicy:(SecPolicyRef)secPolicy;
 
 - (void)panelDidEnd:(NSWindow*)sheet
          returnCode:(int)returnCode
@@ -47,14 +47,14 @@
   [super dealloc];
 }
 
-- (id)initWithCallback:(const certificate_trust::ShowTrustCallback&)callback
+- (id)initWithPromise:(atom::util::Promise)promise
-                 panel:(SFCertificateTrustPanel*)panel
+                panel:(SFCertificateTrustPanel*)panel
-                  cert:(const scoped_refptr<net::X509Certificate>&)cert
+                 cert:(const scoped_refptr<net::X509Certificate>&)cert
-                 trust:(SecTrustRef)trust
+                trust:(SecTrustRef)trust
-             certChain:(CFArrayRef)certChain
+            certChain:(CFArrayRef)certChain
-             secPolicy:(SecPolicyRef)secPolicy {
+            secPolicy:(SecPolicyRef)secPolicy {
   if ((self = [super init])) {
-    callback_ = callback;
+    promise_.reset(new atom::util::Promise(std::move(promise)));
     panel_ = panel;
     cert_ = cert;
     trust_ = trust;
@@ -73,8 +73,7 @@
   // now.
   cert_db->NotifyObserversCertDBChanged();
 
-  callback_.Run();
+  promise_->Resolve();
-
   [self autorelease];
 }
 
@@ -82,10 +81,14 @@
 
 namespace certificate_trust {
 
-void ShowCertificateTrust(atom::NativeWindow* parent_window,
+v8::Local<v8::Promise> ShowCertificateTrust(
-                          const scoped_refptr<net::X509Certificate>& cert,
+    atom::NativeWindow* parent_window,
-                          const std::string& message,
+    const scoped_refptr<net::X509Certificate>& cert,
-                          const ShowTrustCallback& callback) {
+    const std::string& message) {
+  v8::Isolate* isolate = v8::Isolate::GetCurrent();
+  atom::util::Promise promise(isolate);
+  v8::Local<v8::Promise> handle = promise.GetHandle();
+
   auto* sec_policy = SecPolicyCreateBasicX509();
   auto cert_chain =
       net::x509_util::CreateSecCertificateArrayForX509Certificate(cert.get());
@@ -98,18 +101,20 @@ void ShowCertificateTrust(atom::NativeWindow* parent_window,
   auto msg = base::SysUTF8ToNSString(message);
 
   auto panel = [[SFCertificateTrustPanel alloc] init];
-  auto delegate = [[TrustDelegate alloc] initWithCallback:callback
+  auto delegate = [[TrustDelegate alloc] initWithPromise:std::move(promise)
-                                                    panel:panel
+                                                   panel:panel
-                                                     cert:cert
+                                                    cert:cert
-                                                    trust:trust
+                                                   trust:trust
-                                                certChain:cert_chain
+                                               certChain:cert_chain
-                                                secPolicy:sec_policy];
+                                               secPolicy:sec_policy];
   [panel beginSheetForWindow:window
                modalDelegate:delegate
               didEndSelector:@selector(panelDidEnd:returnCode:contextInfo:)
                  contextInfo:nil
                        trust:trust
                      message:msg];
+
+  return handle;
 }
 
 }  // namespace certificate_trust

atom/browser/ui/certificate_trust_win.cc

@@ -8,7 +8,6 @@
 
 #include <wincrypt.h>
 
-#include "base/callback.h"
 #include "net/cert/cert_database.h"
 #include "net/cert/x509_util_win.h"
 
@@ -57,14 +56,16 @@ CERT_CHAIN_PARA GetCertificateChainParameters() {
   return params;
 }
 
-void ShowCertificateTrust(atom::NativeWindow* parent_window,
+v8::Local<v8::Promise> ShowCertificateTrust(
-                          const scoped_refptr<net::X509Certificate>& cert,
+    atom::NativeWindow* parent_window,
-                          const std::string& message,
+    const scoped_refptr<net::X509Certificate>& cert,
-                          const ShowTrustCallback& callback) {
+    const std::string& message) {
+  v8::Isolate* isolate = v8::Isolate::GetCurrent();
+  atom::util::Promise promise(isolate);
+  v8::Local<v8::Promise> handle = promise.GetHandle();
+
   PCCERT_CHAIN_CONTEXT chain_context;
-
   auto cert_context = net::x509_util::CreateCertContextWithChain(cert.get());
-
   auto params = GetCertificateChainParameters();
 
   if (CertGetCertificateChain(NULL, cert_context.get(), NULL, NULL, &params,
@@ -79,7 +80,8 @@ void ShowCertificateTrust(atom::NativeWindow* parent_window,
     CertFreeCertificateChain(chain_context);
   }
 
-  callback.Run();
+  promise.Resolve();
+  return handle;
 }
 
 }  // namespace certificate_trust

docs/api/dialog.md

@@ -330,6 +330,29 @@ attached to the parent window, making it modal.
 
 On Windows the options are more limited, due to the Win32 APIs used:
 
+* The `message` argument is not used, as the OS provides its own confirmation
+   dialog.
+* The `browserWindow` argument is ignored since it is not possible to make
+   this confirmation dialog modal.
+
+**[Deprecated Soon](promisification.md)**
+
+### `dialog.showCertificateTrustDialog([browserWindow, ]options)` _macOS_ _Windows_
+
+* `browserWindow` [BrowserWindow](browser-window.md) (optional)
+* `options` Object
+  * `certificate` [Certificate](structures/certificate.md) - The certificate to trust/import.
+  * `message` String - The message to display to the user.
+
+Returns `Promise<void>` - resolves when the certificate trust dialog is shown.
+
+On macOS, this displays a modal dialog that shows a message and certificate
+information, and gives the user the option of trusting/importing the
+certificate. If you provide a `browserWindow` argument the dialog will be
+attached to the parent window, making it modal.
+
+On Windows the options are more limited, due to the Win32 APIs used:
+
 * The `message` argument is not used, as the OS provides its own confirmation
    dialog.
 * The `browserWindow` argument is ignored since it is not possible to make

docs/api/promisification.md

@@ -9,8 +9,6 @@ When a majority of affected functions are migrated, this flag will be enabled by
 ### Candidate Functions
 
 - [app.importCertificate(options, callback)](https://github.com/electron/electron/blob/master/docs/api/app.md#importCertificate)
-- [dialog.showMessageBox([browserWindow, ]options[, callback])](https://github.com/electron/electron/blob/master/docs/api/dialog.md#showMessageBox)
-- [dialog.showCertificateTrustDialog([browserWindow, ]options, callback)](https://github.com/electron/electron/blob/master/docs/api/dialog.md#showCertificateTrustDialog)
 - [contents.print([options], [callback])](https://github.com/electron/electron/blob/master/docs/api/web-contents.md#print)
 
 ### Converted Functions
@@ -34,6 +32,8 @@ When a majority of affected functions are migrated, this flag will be enabled by
 - [dialog.showSaveDialog([browserWindow, ]options[, callback])](https://github.com/electron/electron/blob/master/docs/api/dialog.md#showSaveDialog)
 - [inAppPurchase.purchaseProduct(productID, quantity, callback)](https://github.com/electron/electron/blob/master/docs/api/in-app-purchase.md#purchaseProduct)
 - [inAppPurchase.getProducts(productIDs, callback)](https://github.com/electron/electron/blob/master/docs/api/in-app-purchase.md#getProducts)
+- [dialog.showMessageBox([browserWindow, ]options[, callback])](https://github.com/electron/electron/blob/master/docs/api/dialog.md#showMessageBox)
+- [dialog.showCertificateTrustDialog([browserWindow, ]options, callback)](https://github.com/electron/electron/blob/master/docs/api/dialog.md#showCertificateTrustDialog)
 - [netLog.stopLogging([callback])](https://github.com/electron/electron/blob/master/docs/api/net-log.md#stopLogging)
 - [protocol.isProtocolHandled(scheme, callback)](https://github.com/electron/electron/blob/master/docs/api/protocol.md#isProtocolHandled)
 - [ses.clearHostResolverCache([callback])](https://github.com/electron/electron/blob/master/docs/api/session.md#clearHostResolverCache)

lib/browser/api/dialog.js

@@ -223,31 +223,27 @@ module.exports = {
     return binding.showErrorBox(...args)
   },
 
-  showCertificateTrustDialog: function (...args) {
+  showCertificateTrustDialog: function (window, options) {
-    const [window, options, callback] = parseArgs(...args)
+    if (window && window.constructor !== BrowserWindow) options = window
-
     if (options == null || typeof options !== 'object') {
       throw new TypeError('options must be an object')
     }
 
-    let { certificate, message } = options
+    const { certificate, message = '' } = options
     if (certificate == null || typeof certificate !== 'object') {
       throw new TypeError('certificate must be an object')
     }
 
-    if (message == null) {
+    if (typeof message !== 'string') throw new TypeError('message must be a string')
-      message = ''
-    } else if (typeof message !== 'string') {
-      throw new TypeError('message must be a string')
-    }
 
-    return binding.showCertificateTrustDialog(window, certificate, message, callback)
+    return binding.showCertificateTrustDialog(window, certificate, message)
   }
 }
 
 module.exports.showMessageBox = deprecate.promisify(module.exports.showMessageBox)
 module.exports.showOpenDialog = deprecate.promisify(module.exports.showOpenDialog)
 module.exports.showSaveDialog = deprecate.promisify(module.exports.showSaveDialog)
+module.exports.showCertificateTrustDialog = deprecate.promisify(module.exports.showCertificateTrustDialog)
 
 // Mark standard asynchronous functions.
 v8Util.setHiddenValue(module.exports.showMessageBox, 'asynchronous', true)