chore: fix promisify helper (#16544)

* chore: fix promise deprecation helper

* fix deprecations

* update deprecation tests

lib/browser/api/app.js

@@ -36,7 +36,7 @@ Object.assign(app, {
   }
 })
 
-app.getFileIcon = deprecate.promisify(app.getFileIcon, 3)
+app.getFileIcon = deprecate.promisify(app.getFileIcon)
 
 const nativeAppMetrics = app.getAppMetrics
 app.getAppMetrics = () => {

lib/browser/api/session.js

@@ -11,10 +11,10 @@ const fromPartition = (partition) => {
   if (!session[wrappedSymbol]) {
     session[wrappedSymbol] = true
     const { cookies } = session
-    cookies.flushStore = deprecate.promisify(cookies.flushStore, 0)
+    cookies.flushStore = deprecate.promisify(cookies.flushStore)
-    cookies.get = deprecate.promisify(cookies.get, 1)
+    cookies.get = deprecate.promisify(cookies.get)
-    cookies.remove = deprecate.promisify(cookies.remove, 2)
+    cookies.remove = deprecate.promisify(cookies.remove)
-    cookies.set = deprecate.promisify(cookies.set, 1)
+    cookies.set = deprecate.promisify(cookies.set)
   }
   return session
 }
@@ -35,6 +35,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, 2)
+  this.protocol.isProtocolHandled = deprecate.promisify(this.protocol.isProtocolHandled)
   app.emit('session-created', this)
 }

lib/browser/api/web-contents.js

@@ -374,7 +374,7 @@ WebContents.prototype._init = function () {
   // render-view-deleted event, so ignore the listeners warning.
   this.setMaxListeners(0)
 
-  this.capturePage = deprecate.promisify(this.capturePage, 2)
+  this.capturePage = deprecate.promisify(this.capturePage)
 
   // Dispatch IPC messages to the ipc module.
   this.on('-ipc-message', function (event, internal, channel, args) {

lib/common/api/deprecate.js

@@ -69,27 +69,27 @@ const deprecate = {
     })
   },
 
-  promisify: (fn, cbParamIndex) => {
+  promisify: (fn) => {
     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]
+      let cb
+      if (params.length > 0 && typeof params[params.length - 1] === 'function') {
+        cb = params.pop()
+      }
       const promise = fn.apply(this, params)
-
+      if (!cb) return promise
-      if (typeof cb !== 'function') return promise
       if (process.enablePromiseAPIs) warn()
       return promise
         .then(res => {
           process.nextTick(() => {
-            cb(null, res)
+            cb.length === 2 ? cb(null, res) : cb(res)
           })
         }, err => {
-          process.nextTick(() => {
+          process.nextTick(() => cb(err))
-            cb(err)
-          })
         })
     }
   },

lib/renderer/api/desktop-capturer.js

@@ -56,4 +56,4 @@ const getSources = (options) => {
   })
 }
 
-exports.getSources = deprecate.promisify(getSources, 1)
+exports.getSources = deprecate.promisify(getSources)

spec/api-app-spec.js

@@ -859,7 +859,7 @@ describe('app module', () => {
     })
 
     // TODO(codebytere): remove when promisification is complete
-    it('fetches a non-empty icon (callback)', () => {
+    it('fetches a non-empty icon (callback)', (done) => {
       app.getFileIcon(iconPath, (icon) => {
         expect(icon.isEmpty()).to.be.false()
         done()
@@ -875,7 +875,7 @@ describe('app module', () => {
     })
 
     // TODO(codebytere): remove when promisification is complete
-    it('fetches normal icon size by default (callback)', () => {
+    it('fetches normal icon size by default (callback)', (done) => {
       app.getFileIcon(iconPath, (icon) => {
         const size = icon.getSize()
 
@@ -903,7 +903,7 @@ describe('app module', () => {
       })
 
       // TODO(codebytere): remove when promisification is complete
-      it('fetches a normal icon (callback)', () => {
+      it('fetches a normal icon (callback)', (done) => {
         app.getFileIcon(iconPath, { size: 'normal' }, (icon) => {
           const size = icon.getSize()
 

spec/api-browser-window-spec.js

@@ -507,7 +507,7 @@ describe('BrowserWindow module', () => {
     })
 
     // TODO(codebytere): remove when promisification is complete
-    it('returns a Promise with a Buffer (callback)', () => {
+    it('returns a Promise with a Buffer (callback)', (done) => {
       w.capturePage({
         x: 0,
         y: 0,
@@ -540,16 +540,16 @@ describe('BrowserWindow module', () => {
     })
 
     // TODO(codebytere): remove when promisification is complete
-    it('preserves transparency (callback)', async () => {
+    it('preserves transparency (callback)', (done) => {
-      const w = await openTheWindow({
+      openTheWindow({
         show: false,
         width: 400,
         height: 400,
         transparent: true
-      })
+      }).then(w => {
         const p = emittedOnce(w, 'ready-to-show')
         w.loadURL('data:text/html,<html><body background-color: rgba(255,255,255,0)></body></html>')
-      await p
+        p.then(() => {
           w.show()
 
           w.capturePage((image) => {
@@ -561,6 +561,8 @@ describe('BrowserWindow module', () => {
           })
         })
       })
+    })
+  })
 
   describe('BrowserWindow.setBounds(bounds[, animate])', () => {
     it('sets the window bounds with full bounds', () => {

spec/api-deprecations-spec.js

@@ -138,7 +138,7 @@ describe('deprecations', () => {
 
     it('acts as a pass-through for promise-based invocations', async () => {
       enableCallbackWarnings()
-      promiseFunc = deprecate.promisify(promiseFunc, 1)
+      promiseFunc = deprecate.promisify(promiseFunc)
 
       const actual = await promiseFunc(expected)
       expect(actual).to.equal(expected)
@@ -147,12 +147,11 @@ describe('deprecations', () => {
 
     it('warns exactly once for callback-based invocations', (done) => {
       enableCallbackWarnings()
-      promiseFunc = deprecate.promisify(promiseFunc, 1)
+      promiseFunc = deprecate.promisify(promiseFunc)
 
       let callbackCount = 0
       const invocationCount = 3
-      const callback = (error, actual) => {
+      const callback = (actual) => {
-        expect(error).to.be.null()
         expect(actual).to.equal(expected)
         expect(warnings).to.have.lengthOf(1)
         expect(warnings[0]).to.include('promiseFunc')

spec/api-desktop-capturer-spec.js

@@ -62,9 +62,10 @@ describe('desktopCapturer', () => {
   // TODO(codebytere): remove when promisification is complete
   it('responds to subsequent calls of different options (callback)', (done) => {
     let callCount = 0
-    const callback = (error, sources) => {
+    const callback = (err, sources) => {
       callCount++
-      expect(error).to.be.null()
+      expect(err).to.be.null()
+      expect(sources).to.not.be.null()
       if (callCount === 2) done()
     }
 

spec/api-protocol-spec.js

@@ -680,14 +680,14 @@ describe('protocol module', () => {
     })
   })
 
-  describe('protocol.isProtocolHandled', (done) => {
+  describe('protocol.isProtocolHandled', () => {
     it('returns true for about:', async () => {
       const result = await protocol.isProtocolHandled('about')
       assert.strictEqual(result, true)
     })
 
     // TODO(codebytere): remove when promisification is complete
-    it('returns true for about: (callback)', () => {
+    it('returns true for about: (callback)', (done) => {
       protocol.isProtocolHandled('about', (result) => {
         assert.strictEqual(result, true)
         done()
@@ -700,7 +700,7 @@ describe('protocol module', () => {
     })
 
     // TODO(codebytere): remove when promisification is complete
-    it('returns true for file: (callback)', () => {
+    it('returns true for file: (callback)', (done) => {
       protocol.isProtocolHandled('file', (result) => {
         assert.strictEqual(result, true)
         done()
@@ -722,7 +722,7 @@ describe('protocol module', () => {
       assert.strictEqual(result, false)
     })
 
-    it('returns true for custom protocol', () => {
+    it('returns true for custom protocol', (done) => {
       const emptyHandler = (request, callback) => callback()
       protocol.registerStringProtocol(protocolName, emptyHandler, async (error) => {
         assert.strictEqual(error, null)
@@ -733,7 +733,7 @@ describe('protocol module', () => {
     })
 
     // TODO(codebytere): remove when promisification is complete
-    it('returns true for custom protocol (callback)', () => {
+    it('returns true for custom protocol (callback)', (done) => {
       const emptyHandler = (request, callback) => callback()
       protocol.registerStringProtocol(protocolName, emptyHandler, (error) => {
         assert.strictEqual(error, null)
@@ -744,7 +744,7 @@ describe('protocol module', () => {
       })
     })
 
-    it('returns true for intercepted protocol', () => {
+    it('returns true for intercepted protocol', (done) => {
       const emptyHandler = (request, callback) => callback()
       protocol.interceptStringProtocol('http', emptyHandler, async (error) => {
         assert.strictEqual(error, null)
@@ -755,7 +755,7 @@ describe('protocol module', () => {
     })
 
     // TODO(codebytere): remove when promisification is complete
-    it('returns true for intercepted protocol (callback)', () => {
+    it('returns true for intercepted protocol (callback)', (done) => {
       const emptyHandler = (request, callback) => callback()
       protocol.interceptStringProtocol('http', emptyHandler, (error) => {
         assert.strictEqual(error, null)

spec/api-session-spec.js

@@ -266,12 +266,13 @@ describe('session module', () => {
           }
           expect(error).to.be.undefined(error)
         })
+
         it('with callbacks', (done) => {
           const name = 'foo'
           const value = 'bar'
           const { cookies } = session.defaultSession
 
-          cookies.set({ url, name, value }, error => {
+          cookies.set({ url, name, value }, (error) => {
             if (error) return done(error)
             cookies.flushStore(error => done(error))
           })
@@ -279,6 +280,19 @@ describe('session module', () => {
       })
     })
 
+    describe('ses.cookies.flushStore(callback)', () => {
+      it('flushes the cookies to disk and invokes the callback when done', (done) => {
+        session.defaultSession.cookies.set({
+          url: url,
+          name: 'foo',
+          value: 'bar'
+        }, (error) => {
+          if (error) return done(error)
+          session.defaultSession.cookies.flushStore(() => done())
+        })
+      })
+    })
+
     it('should survive an app restart for persistent partition', async () => {
       const appPath = path.join(__dirname, 'fixtures', 'api', 'cookie-app')
       const electronPath = remote.getGlobal('process').execPath
@@ -735,7 +749,7 @@ describe('session module', () => {
     })
   })
 
-  describe('ses.setCertificateVerifyProc(callback)', () => {
+  describe('ses.setCertificateVerifyProc(callback)', (done) => {
     let server = null
 
     beforeEach((done) => {

spec/api-web-contents-spec.js

@@ -597,6 +597,7 @@ describe('webContents module', () => {
       }
     })
 
+    // TODO(codebytere): remove when promisification is complete
     it('can set the correct zoom level (callback)', async () => {
       try {
         await w.loadURL('about:blank')