From 4b18a38e9fcfe7e83fca27f0e6eb6941d0d5579d Mon Sep 17 00:00:00 2001
From: Shelley Vohr <codebytere@github.com>
Date: Mon, 10 Dec 2018 08:13:09 -0800
Subject: [PATCH] chore: simplify promisify helper  (#15952)

chore: simplify promisify helper
---
 lib/browser/api/app.js          |  5 +---
 lib/browser/api/web-contents.js |  7 ++---
 lib/common/api/deprecate.js     | 34 +++++++++++++---------
 spec/api-deprecations-spec.js   | 50 +++++++++++++++++++++++++++++++++
 4 files changed, 73 insertions(+), 23 deletions(-)

diff --git a/lib/browser/api/app.js b/lib/browser/api/app.js
index 3d7c9e43f6..297dcf1cc1 100644
--- a/lib/browser/api/app.js
+++ b/lib/browser/api/app.js
@@ -40,10 +40,7 @@ Object.assign(app, {
   }
 })
 
-const nativeGetFileIcon = app.getFileIcon
-app.getFileIcon = (path, options = {}, cb) => {
-  return deprecate.promisify(nativeGetFileIcon.call(app, path, options), cb)
-}
+app.getFileIcon = deprecate.promisify(app.getFileIcon, 2)
 
 const nativeAppMetrics = app.getAppMetrics
 app.getAppMetrics = () => {
diff --git a/lib/browser/api/web-contents.js b/lib/browser/api/web-contents.js
index dd9f9eefdc..3067c2c7fb 100644
--- a/lib/browser/api/web-contents.js
+++ b/lib/browser/api/web-contents.js
@@ -330,14 +330,11 @@ WebContents.prototype._init = function () {
   // The navigation controller.
   NavigationController.call(this, this)
 
-  // Every remote callback from renderer process would add a listenter to the
+  // Every remote callback from renderer process would add a listener to the
   // render-view-deleted event, so ignore the listeners warning.
   this.setMaxListeners(0)
 
-  const nativeCapturePage = this.capturePage
-  this.capturePage = function (rect, cb) {
-    return deprecate.promisify(nativeCapturePage.call(this, rect), cb)
-  }
+  this.capturePage = deprecate.promisify(this.capturePage, 1)
 
   // Dispatch IPC messages to the ipc module.
   this.on('ipc-message', function (event, [channel, ...args]) {
diff --git a/lib/common/api/deprecate.js b/lib/common/api/deprecate.js
index fc75d781f9..6f049bbcde 100644
--- a/lib/common/api/deprecate.js
+++ b/lib/common/api/deprecate.js
@@ -69,23 +69,29 @@ const deprecate = {
     })
   },
 
-  promisify: (promise, cb) => {
-    const oldName = `function with callbacks`
-    const newName = `function with Promises`
+  promisify: (fn, cbParamIndex) => {
+    const fnName = fn.name || 'function'
+    const oldName = `${fnName} with callbacks`
+    const newName = `${fnName} with Promises`
     const warn = warnOnce(oldName, newName)
 
-    if (typeof cb !== 'function') return promise
-    if (process.enablePromiseAPIs) warn()
-    return promise
-      .then(res => {
-        process.nextTick(() => {
-          cb(null, res)
+    return function (...params) {
+      const cb = params.splice(cbParamIndex, 1)[0]
+      const promise = fn.apply(this, params)
+
+      if (typeof cb !== 'function') return promise
+      if (process.enablePromiseAPIs) warn()
+      return promise
+        .then(res => {
+          process.nextTick(() => {
+            cb(null, res)
+          })
+        }, err => {
+          process.nextTick(() => {
+            cb(err)
+          })
         })
-      }, err => {
-        process.nextTick(() => {
-          cb(err)
-        })
-      })
+    }
   },
 
   renameProperty: (o, oldName, newName) => {
diff --git a/spec/api-deprecations-spec.js b/spec/api-deprecations-spec.js
index ccd25fcbb6..bebbf1f10a 100644
--- a/spec/api-deprecations-spec.js
+++ b/spec/api-deprecations-spec.js
@@ -117,4 +117,54 @@ describe('deprecations', () => {
       deprecate.log('this is deprecated')
     }).to.throw(/this is deprecated/)
   })
+
+  describe('promisify', () => {
+    const expected = 'Hello, world!'
+    let promiseFunc
+    let warnings
+
+    const enableCallbackWarnings = () => {
+      warnings = []
+      deprecations.setHandler(warning => { warnings.push(warning) })
+      process.enablePromiseAPIs = true
+    }
+
+    beforeEach(() => {
+      deprecations.setHandler(null)
+      process.throwDeprecation = true
+
+      promiseFunc = param => new Promise((resolve, reject) => { resolve(param) })
+    })
+
+    it('acts as a pass-through for promise-based invocations', async () => {
+      enableCallbackWarnings()
+      promiseFunc = deprecate.promisify(promiseFunc, 1)
+
+      const actual = await promiseFunc(expected)
+      expect(actual).to.equal(expected)
+      expect(warnings).to.have.lengthOf(0)
+    })
+
+    it('warns exactly once for callback-based invocations', (done) => {
+      enableCallbackWarnings()
+      promiseFunc = deprecate.promisify(promiseFunc, 1)
+
+      let callbackCount = 0
+      const invocationCount = 3
+      const callback = (error, actual) => {
+        expect(error).to.be.null()
+        expect(actual).to.equal(expected)
+        expect(warnings).to.have.lengthOf(1)
+        expect(warnings[0]).to.include('promiseFunc')
+        callbackCount += 1
+        if (callbackCount === invocationCount) {
+          done()
+        }
+      }
+
+      for (let i = 0; i < invocationCount; i += 1) {
+        promiseFunc(expected, callback)
+      }
+    })
+  })
 })