feat: promisify protocol.isProtocolHandled() (#16423)

* feat: promisify protocol

* fix base::Bind and specs

* update documentation

* make callback-compatible

* async awaitify tests

atom/browser/api/atom_api_protocol.cc

@@ -103,22 +103,29 @@ Protocol::ProtocolError Protocol::UnregisterProtocolInIO(
   return PROTOCOL_OK;
 }
 
-void Protocol::IsProtocolHandled(const std::string& scheme,
-                                 const BooleanCallback& callback) {
+bool IsProtocolHandledInIO(
+    scoped_refptr<URLRequestContextGetter> request_context_getter,
+    const std::string& scheme) {
+  bool is_handled =
+      request_context_getter->job_factory()->IsHandledProtocol(scheme);
+  return is_handled;
+}
+
+void PromiseCallback(scoped_refptr<util::Promise> promise, bool handled) {
+  promise->Resolve(handled);
+}
+
+v8::Local<v8::Promise> Protocol::IsProtocolHandled(const std::string& scheme) {
+  scoped_refptr<util::Promise> promise = new util::Promise(isolate());
   auto* getter = static_cast<URLRequestContextGetter*>(
       browser_context_->GetRequestContext());
+
   base::PostTaskWithTraitsAndReplyWithResult(
       FROM_HERE, {content::BrowserThread::IO},
-      base::Bind(&Protocol::IsProtocolHandledInIO, base::RetainedRef(getter),
-                 scheme),
-      callback);
-}
+      base::Bind(&IsProtocolHandledInIO, base::RetainedRef(getter), scheme),
+      base::Bind(&PromiseCallback, promise));
 
-// static
-bool Protocol::IsProtocolHandledInIO(
-    scoped_refptr<URLRequestContextGetter> request_context_getter,
-    const std::string& scheme) {
-  return request_context_getter->job_factory()->IsHandledProtocol(scheme);
+  return promise->GetHandle();
 }
 
 void Protocol::UninterceptProtocol(const std::string& scheme,

atom/browser/api/atom_api_protocol.h

@@ -14,6 +14,7 @@
 #include "atom/browser/api/trackable_object.h"
 #include "atom/browser/atom_browser_context.h"
 #include "atom/browser/net/atom_url_request_job_factory.h"
+#include "atom/common/promise_util.h"
 #include "base/callback.h"
 #include "base/memory/weak_ptr.h"
 #include "base/task/post_task.h"
@@ -41,7 +42,6 @@ class Protocol : public mate::TrackableObject<Protocol> {
   using Handler =
       base::Callback<void(const base::DictionaryValue&, v8::Local<v8::Value>)>;
   using CompletionCallback = base::Callback<void(v8::Local<v8::Value>)>;
-  using BooleanCallback = base::Callback<void(bool)>;
 
   static mate::Handle<Protocol> Create(v8::Isolate* isolate,
                                        AtomBrowserContext* browser_context);
@@ -136,11 +136,7 @@ class Protocol : public mate::TrackableObject<Protocol> {
       const std::string& scheme);
 
   // Whether the protocol has handler registered.
-  void IsProtocolHandled(const std::string& scheme,
-                         const BooleanCallback& callback);
-  static bool IsProtocolHandledInIO(
-      scoped_refptr<URLRequestContextGetter> request_context_getter,
-      const std::string& scheme);
+  v8::Local<v8::Promise> IsProtocolHandled(const std::string& scheme);
 
   // Replace the protocol handler with a new one.
   template <typename RequestJob>

docs/api/promisification.md

@@ -31,7 +31,6 @@ When a majority of affected functions are migrated, this flag will be enabled by
 - [ ] [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)
 - [ ] [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.getCacheSize(callback)](https://github.com/electron/electron/blob/master/docs/api/session.md#getCacheSize)
 - [ ] [ses.clearCache(callback)](https://github.com/electron/electron/blob/master/docs/api/session.md#clearCache)
 - [ ] [ses.clearStorageData([options, callback])](https://github.com/electron/electron/blob/master/docs/api/session.md#clearStorageData)
@@ -61,4 +60,5 @@ When a majority of affected functions are migrated, this flag will be enabled by
 - [ ] [webviewTag.capturePage([rect, ]callback)](https://github.com/electron/electron/blob/master/docs/api/webview-tag.md#capturePage)
 - [ ] [contents.capturePage([rect, ]callback)](https://github.com/electron/electron/blob/master/docs/api/web-contents.md#capturePage)
 - [ ] [app.getFileIcon(path[, options], callback)](https://github.com/electron/electron/blob/master/docs/api/app.md#getFileIcon)
-- [ ] [shell.openExternal(url[, options, callback])](https://github.com/electron/electron/blob/master/docs/api/shell.md#openExternal)
+- [ ] [shell.openExternal(url[, options, callback])](https://github.com/electron/electron/blob/master/docs/api/shell.md#openExternal)
+- [ ] [protocol.isProtocolHandled(scheme, callback)](https://github.com/electron/electron/blob/master/docs/api/protocol.md#isProtocolHandled)

docs/api/protocol.md

@@ -275,6 +275,15 @@ Unregisters the custom protocol of `scheme`.
 The `callback` will be called with a boolean that indicates whether there is
 already a handler for `scheme`.
 
+**[Deprecated Soon](promisification.md)**
+
+### `protocol.isProtocolHandled(scheme)`
+
+* `scheme` String
+
+Returns `Promise<Boolean>` - fulfilled with a boolean that indicates whether there is
+already a handler for `scheme`.
+
 ### `protocol.interceptFileProtocol(scheme, handler[, completion])`
 
 * `scheme` String

lib/browser/api/session.js

@@ -1,7 +1,7 @@
 'use strict'
 
 const { EventEmitter } = require('events')
-const { app } = require('electron')
+const { app, deprecate } = require('electron')
 const { fromPartition, Session, Cookies } = process.atomBinding('session')
 
 // Public API.
@@ -20,5 +20,6 @@ Object.setPrototypeOf(Session.prototype, EventEmitter.prototype)
 Object.setPrototypeOf(Cookies.prototype, EventEmitter.prototype)
 
 Session.prototype._init = function () {
+  this.protocol.isProtocolHandled = deprecate.promisify(this.protocol.isProtocolHandled, 1)
   app.emit('session-created', this)
 }

spec/api-protocol-spec.js

@@ -657,60 +657,48 @@ describe('protocol module', () => {
   })
 
   describe('protocol.isProtocolHandled', () => {
-    it('returns true for about:', (done) => {
-      protocol.isProtocolHandled('about', (result) => {
-        assert.strictEqual(result, true)
-        done()
-      })
+    it('returns true for about:', async () => {
+      const result = await protocol.isProtocolHandled('about')
+      assert.strictEqual(result, true)
     })
 
-    it('returns true for file:', (done) => {
-      protocol.isProtocolHandled('file', (result) => {
-        assert.strictEqual(result, true)
-        done()
-      })
+    it('returns true for file:', async () => {
+      const result = await protocol.isProtocolHandled('file')
+      assert.strictEqual(result, true)
     })
 
-    it('returns true for http:', (done) => {
-      protocol.isProtocolHandled('http', (result) => {
-        assert.strictEqual(result, true)
-        done()
-      })
+    it('returns true for http:', async () => {
+      const result = await protocol.isProtocolHandled('http')
+      assert.strictEqual(result, true)
     })
 
-    it('returns true for https:', (done) => {
-      protocol.isProtocolHandled('https', (result) => {
-        assert.strictEqual(result, true)
-        done()
-      })
+    it('returns true for https:', async () => {
+      const result = await protocol.isProtocolHandled('https')
+      assert.strictEqual(result, true)
     })
 
-    it('returns false when scheme is not registered', (done) => {
-      protocol.isProtocolHandled('no-exist', (result) => {
-        assert.strictEqual(result, false)
-        done()
-      })
+    it('returns false when scheme is not registered', async () => {
+      const result = await protocol.isProtocolHandled('no-exist')
+      assert.strictEqual(result, false)
     })
 
     it('returns true for custom protocol', (done) => {
       const emptyHandler = (request, callback) => callback()
-      protocol.registerStringProtocol(protocolName, emptyHandler, (error) => {
+      protocol.registerStringProtocol(protocolName, emptyHandler, async (error) => {
         assert.strictEqual(error, null)
-        protocol.isProtocolHandled(protocolName, (result) => {
-          assert.strictEqual(result, true)
-          done()
-        })
+        const result = await protocol.isProtocolHandled(protocolName)
+        assert.strictEqual(result, true)
+        done()
       })
     })
 
     it('returns true for intercepted protocol', (done) => {
       const emptyHandler = (request, callback) => callback()
-      protocol.interceptStringProtocol('http', emptyHandler, (error) => {
+      protocol.interceptStringProtocol('http', emptyHandler, async (error) => {
         assert.strictEqual(error, null)
-        protocol.isProtocolHandled('http', (result) => {
-          assert.strictEqual(result, true)
-          done()
-        })
+        const result = await protocol.isProtocolHandled('http')
+        assert.strictEqual(result, true)
+        done()
       })
     })
   })

spec/api-session-spec.js

@@ -524,14 +524,12 @@ describe('session module', () => {
       partitionProtocol.unregisterProtocol(protocolName, () => done())
     })
 
-    it('does not affect defaultSession', (done) => {
-      protocol.isProtocolHandled(protocolName, (result) => {
-        assert.strictEqual(result, false)
-        partitionProtocol.isProtocolHandled(protocolName, (result) => {
-          assert.strictEqual(result, true)
-          done()
-        })
-      })
+    it('does not affect defaultSession', async () => {
+      const result1 = await protocol.isProtocolHandled(protocolName)
+      assert.strictEqual(result1, false)
+
+      const result2 = await partitionProtocol.isProtocolHandled(protocolName)
+      assert.strictEqual(result2, true)
     })
 
     it('handles requests from partition', (done) => {