chore: simplify promisify helper (#15952)

chore: simplify promisify helper

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 = () => {

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]) {

lib/common/api/deprecate.js

@@ -69,11 +69,16 @@ 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)
 
+    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
@@ -86,6 +91,7 @@ const deprecate = {
             cb(err)
           })
         })
+    }
   },
 
   renameProperty: (o, oldName, newName) => {

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)
+      }
+    })
+  })
 })