feat: make zoomLevel/zoomFactor sync (#16410)

* feat: make zoomLevel/zoomFactor sync

* update ts defs dep

atom/browser/api/atom_api_web_contents.cc

@@ -2159,9 +2159,9 @@ void WebContents::BuildPrototype(v8::Isolate* isolate,
 #endif
       .SetMethod("invalidate", &WebContents::Invalidate)
       .SetMethod("setZoomLevel", &WebContents::SetZoomLevel)
-      .SetMethod("_getZoomLevel", &WebContents::GetZoomLevel)
+      .SetMethod("getZoomLevel", &WebContents::GetZoomLevel)
       .SetMethod("setZoomFactor", &WebContents::SetZoomFactor)
-      .SetMethod("_getZoomFactor", &WebContents::GetZoomFactor)
+      .SetMethod("getZoomFactor", &WebContents::GetZoomFactor)
       .SetMethod("getType", &WebContents::GetType)
       .SetMethod("_getPreloadPath", &WebContents::GetPreloadPath)
       .SetMethod("getWebPreferences", &WebContents::GetWebPreferences)

docs/api/promisification.md

@@ -40,8 +40,6 @@ When a majority of affected functions are migrated, this flag will be enabled by
 - [ses.getBlobData(identifier, callback)](https://github.com/electron/electron/blob/master/docs/api/session.md#getBlobData)
 - [ses.clearAuthCache(options[, callback])](https://github.com/electron/electron/blob/master/docs/api/session.md#clearAuthCache)
 - [contents.executeJavaScript(code[, userGesture, callback])](https://github.com/electron/electron/blob/master/docs/api/web-contents.md#executeJavaScript)
-- [contents.getZoomFactor(callback)](https://github.com/electron/electron/blob/master/docs/api/web-contents.md#getZoomFactor)
-- [contents.getZoomLevel(callback)](https://github.com/electron/electron/blob/master/docs/api/web-contents.md#getZoomLevel)
 - [contents.hasServiceWorker(callback)](https://github.com/electron/electron/blob/master/docs/api/web-contents.md#hasServiceWorker)
 - [contents.unregisterServiceWorker(callback)](https://github.com/electron/electron/blob/master/docs/api/web-contents.md#unregisterServiceWorker)
 - [contents.print([options], [callback])](https://github.com/electron/electron/blob/master/docs/api/web-contents.md#print)
@@ -51,8 +49,6 @@ When a majority of affected functions are migrated, this flag will be enabled by
 - [webFrame.executeJavaScriptInIsolatedWorld(worldId, scripts[, userGesture, callback])](https://github.com/electron/electron/blob/master/docs/api/web-frame.md#executeJavaScriptInIsolatedWorld)
 - [webviewTag.executeJavaScript(code[, userGesture, callback])](https://github.com/electron/electron/blob/master/docs/api/webview-tag.md#executeJavaScript)
 - [webviewTag.printToPDF(options, callback)](https://github.com/electron/electron/blob/master/docs/api/webview-tag.md#printToPDF)
-- [webviewTag.getZoomFactor(callback)](https://github.com/electron/electron/blob/master/docs/api/webview-tag.md#getZoomFactor)
-- [webviewTag.getZoomLevel(callback)](https://github.com/electron/electron/blob/master/docs/api/webview-tag.md#getZoomLevel)
 
 ### Converted Functions
 

docs/api/web-contents.md

@@ -977,13 +977,9 @@ Returns `Boolean` - Whether audio is currently playing.
 Changes the zoom factor to the specified factor. Zoom factor is
 zoom percent divided by 100, so 300% = 3.0.
 
-#### `contents.getZoomFactor(callback)`
+#### `contents.getZoomFactor()`
 
-* `callback` Function
-  * `zoomFactor` Number
-
-Sends a request to get current zoom factor, the `callback` will be called with
-`callback(zoomFactor)`.
+Returns `Number` - the current zoom factor.
 
 #### `contents.setZoomLevel(level)`
 
@@ -994,13 +990,9 @@ increment above or below represents zooming 20% larger or smaller to default
 limits of 300% and 50% of original size, respectively. The formula for this is
 `scale := 1.2 ^ level`.
 
-#### `contents.getZoomLevel(callback)`
+#### `contents.getZoomLevel()`
 
-* `callback` Function
-  * `zoomLevel` Number
-
-Sends a request to get current zoom level, the `callback` will be called with
-`callback(zoomLevel)`.
+Returns `Number` - the current zoom level.
 
 #### `contents.setVisualZoomLevelLimits(minimumLevel, maximumLevel)`
 

docs/api/webview-tag.md

@@ -595,21 +595,13 @@ increment above or below represents zooming 20% larger or smaller to default
 limits of 300% and 50% of original size, respectively. The formula for this is
 `scale := 1.2 ^ level`.
 
-### `<webview>.getZoomFactor(callback)`
+### `<webview>.getZoomFactor()`
 
-* `callback` Function
-  * `zoomFactor` Number
+Returns `Number` - the current zoom factor.
 
-Sends a request to get current zoom factor, the `callback` will be called with
-`callback(zoomFactor)`.
+### `<webview>.getZoomLevel()`
 
-### `<webview>.getZoomLevel(callback)`
-
-* `callback` Function
-  * `zoomLevel` Number
-
-Sends a request to get current zoom level, the `callback` will be called with
-`callback(zoomLevel)`.
+Returns `Number` - the current zoom level.
 
 ### `<webview>.setVisualZoomLevelLimits(minimumLevel, maximumLevel)`
 

lib/browser/api/menu-item-roles.js

@@ -156,9 +156,8 @@ const roles = {
     accelerator: 'CommandOrControl+Plus',
     nonNativeMacOSRole: true,
     webContentsMethod: (webContents) => {
-      webContents.getZoomLevel((zoomLevel) => {
+      const zoomLevel = webContents.getZoomLevel()
       webContents.setZoomLevel(zoomLevel + 0.5)
-      })
     }
   },
   zoomout: {
@@ -166,9 +165,8 @@ const roles = {
     accelerator: 'CommandOrControl+-',
     nonNativeMacOSRole: true,
     webContentsMethod: (webContents) => {
-      webContents.getZoomLevel((zoomLevel) => {
+      const zoomLevel = webContents.getZoomLevel()
       webContents.setZoomLevel(zoomLevel - 0.5)
-      })
     }
   },
   // App submenu should be used for Mac only

lib/browser/api/web-contents.js

@@ -111,6 +111,7 @@ WebContents.prototype.send = function (channel, ...args) {
 
   return this._send(internal, sendToAll, channel, args)
 }
+
 WebContents.prototype.sendToAll = function (channel, ...args) {
   if (typeof channel !== 'string') {
     throw new Error('Missing required channel argument')
@@ -209,6 +210,30 @@ WebContents.prototype.executeJavaScript = function (code, hasUserGesture, callba
   }
 }
 
+// TODO(codebytere): remove when promisifications is complete
+const nativeZoomLevel = WebContents.prototype.getZoomLevel
+WebContents.prototype.getZoomLevel = function (callback) {
+  if (callback == null) {
+    return nativeZoomLevel.call(this)
+  } else {
+    process.nextTick(() => {
+      callback(nativeZoomLevel.call(this))
+    })
+  }
+}
+
+// TODO(codebytere): remove when promisifications is complete
+const nativeZoomFactor = WebContents.prototype.getZoomFactor
+WebContents.prototype.getZoomFactor = function (callback) {
+  if (callback == null) {
+    return nativeZoomFactor.call(this)
+  } else {
+    process.nextTick(() => {
+      callback(nativeZoomFactor.call(this))
+    })
+  }
+}
+
 WebContents.prototype.takeHeapSnapshot = function (filePath) {
   return new Promise((resolve, reject) => {
     const channel = `ELECTRON_TAKE_HEAP_SNAPSHOT_RESULT_${getNextId()}`
@@ -289,16 +314,6 @@ WebContents.prototype.getPrinters = function () {
   }
 }
 
-WebContents.prototype.getZoomLevel = function (callback) {
-  if (typeof callback !== 'function') {
-    throw new Error('Must pass function as an argument')
-  }
-  process.nextTick(() => {
-    const zoomLevel = this._getZoomLevel()
-    callback(zoomLevel)
-  })
-}
-
 WebContents.prototype.loadFile = function (filePath, options = {}) {
   if (typeof filePath !== 'string') {
     throw new Error('Must pass filePath as a string')
@@ -315,16 +330,6 @@ WebContents.prototype.loadFile = function (filePath, options = {}) {
   }))
 }
 
-WebContents.prototype.getZoomFactor = function (callback) {
-  if (typeof callback !== 'function') {
-    throw new Error('Must pass function as an argument')
-  }
-  process.nextTick(() => {
-    const zoomFactor = this._getZoomFactor()
-    callback(zoomFactor)
-  })
-}
-
 // Add JavaScript wrappers for WebContents class.
 WebContents.prototype._init = function () {
   // The navigation controller.

lib/browser/guest-view-manager.js

@@ -212,7 +212,7 @@ const attachGuest = function (event, embedderFrameId, elementInstanceId, guestIn
     nodeIntegration: params.nodeintegration != null ? params.nodeintegration : false,
     enableRemoteModule: params.enableremotemodule,
     plugins: params.plugins,
-    zoomFactor: embedder._getZoomFactor(),
+    zoomFactor: embedder.getZoomFactor(),
     webSecurity: !params.disablewebsecurity,
     enableBlinkFeatures: params.blinkfeatures,
     disableBlinkFeatures: params.disableblinkfeatures

lib/common/web-view-methods.js

@@ -46,6 +46,8 @@ exports.syncMethods = new Set([
   'downloadURL',
   'inspectServiceWorker',
   'showDefinitionForSelection',
+  'getZoomFactor',
+  'getZoomLevel',
   'setZoomFactor',
   'setZoomLevel'
 ])
@@ -57,8 +59,6 @@ exports.asyncCallbackMethods = new Set([
   'sendInputEvent',
   'setLayoutZoomLevelLimits',
   'setVisualZoomLevelLimits',
-  'getZoomFactor',
-  'getZoomLevel',
   'print',
   'printToPDF'
 ])

package.json

@@ -13,7 +13,7 @@
     "dotenv-safe": "^4.0.4",
     "dugite": "^1.45.0",
     "electron-docs-linter": "^2.4.0",
-    "electron-typescript-definitions": "^3.0.0",
+    "electron-typescript-definitions": "^4.0.0",
     "eslint": "^5.6.0",
     "eslint-config-standard": "^12.0.0",
     "eslint-plugin-mocha": "^5.2.0",

spec/api-web-contents-spec.js

@@ -569,10 +569,9 @@ describe('webContents module', () => {
                             const {ipcRenderer, remote} = require('electron')
                             ipcRenderer.send('set-zoom', window.location.hostname)
                             ipcRenderer.on(window.location.hostname + '-zoom-set', () => {
-                              remote.getCurrentWebContents().getZoomLevel((zoomLevel) => {
+                              const zoomLevel = remote.getCurrentWebContents().getZoomLevel()
                               ipcRenderer.send(window.location.hostname + '-zoom-level', zoomLevel)
                             })
-                            })
                           </script>`
         callback({ data: response, mimeType: 'text/html' })
       }, (error) => done(error))
@@ -584,6 +583,19 @@ describe('webContents module', () => {
     })
 
     it('can set the correct zoom level', async () => {
+      try {
+        await w.loadURL('about:blank')
+        const zoomLevel = w.webContents.getZoomLevel()
+        expect(zoomLevel).to.eql(0.0)
+        w.webContents.setZoomLevel(0.5)
+        const newZoomLevel = w.webContents.getZoomLevel()
+        expect(newZoomLevel).to.eql(0.5)
+      } finally {
+        w.webContents.setZoomLevel(0)
+      }
+    })
+
+    it('can set the correct zoom level (callback)', async () => {
       try {
         await w.loadURL('about:blank')
         const zoomLevel = await new Promise(resolve => w.webContents.getZoomLevel(resolve))
@@ -626,16 +638,15 @@ describe('webContents module', () => {
         show: false
       })
       w2.webContents.on('did-finish-load', () => {
-        w.webContents.getZoomLevel((zoomLevel1) => {
+        const zoomLevel1 = w.webContents.getZoomLevel()
         assert.strictEqual(zoomLevel1, hostZoomMap.host3)
-          w2.webContents.getZoomLevel((zoomLevel2) => {
+
+        const zoomLevel2 = w2.webContents.getZoomLevel()
         assert.strictEqual(zoomLevel1, zoomLevel2)
         w2.setClosable(true)
         w2.close()
         done()
       })
-        })
-      })
       w.webContents.on('did-finish-load', () => {
         w.webContents.setZoomLevel(hostZoomMap.host3)
         w2.loadURL(`${zoomScheme}://host3`)
@@ -656,11 +667,13 @@ describe('webContents module', () => {
       }, (error) => {
         if (error) return done(error)
         w2.webContents.on('did-finish-load', () => {
-          w.webContents.getZoomLevel((zoomLevel1) => {
+          const zoomLevel1 = w.webContents.getZoomLevel()
           assert.strictEqual(zoomLevel1, hostZoomMap.host3)
-            w2.webContents.getZoomLevel((zoomLevel2) => {
+
+          const zoomLevel2 = w2.webContents.getZoomLevel()
           assert.strictEqual(zoomLevel2, 0)
           assert.notStrictEqual(zoomLevel1, zoomLevel2)
+
           protocol.unregisterProtocol(zoomScheme, (error) => {
             if (error) return done(error)
             w2.setClosable(true)
@@ -668,8 +681,6 @@ describe('webContents module', () => {
             done()
           })
         })
-          })
-        })
         w.webContents.on('did-finish-load', () => {
           w.webContents.setZoomLevel(hostZoomMap.host3)
           w2.loadURL(`${zoomScheme}://host3`)
@@ -689,12 +700,12 @@ describe('webContents module', () => {
         const content = `<iframe src=${url}></iframe>`
         w.webContents.on('did-frame-finish-load', (e, isMainFrame) => {
           if (!isMainFrame) {
-            w.webContents.getZoomLevel((zoomLevel) => {
+            const zoomLevel = w.webContents.getZoomLevel()
             assert.strictEqual(zoomLevel, 2.0)
+
             w.webContents.setZoomLevel(0)
             server.close()
             done()
-            })
           }
         })
         w.webContents.on('dom-ready', () => {
@@ -710,17 +721,17 @@ describe('webContents module', () => {
         show: false
       })
       w2.webContents.on('did-finish-load', () => {
-        w.webContents.getZoomLevel((zoomLevel1) => {
+        const zoomLevel1 = w.webContents.getZoomLevel()
         assert.strictEqual(zoomLevel1, finalZoomLevel)
-          w2.webContents.getZoomLevel((zoomLevel2) => {
+
+        const zoomLevel2 = w2.webContents.getZoomLevel()
         assert.strictEqual(zoomLevel2, 0)
         assert.notStrictEqual(zoomLevel1, zoomLevel2)
+
         w2.setClosable(true)
         w2.close()
         done()
       })
-        })
-      })
       ipcMain.once('temporary-zoom-set', (e, zoomLevel) => {
         w2.loadFile(path.join(fixtures, 'pages', 'c.html'))
         finalZoomLevel = zoomLevel
@@ -739,10 +750,9 @@ describe('webContents module', () => {
         if (initialNavigation) {
           w.webContents.executeJavaScript(source, () => {})
         } else {
-          w.webContents.getZoomLevel((zoomLevel) => {
+          const zoomLevel = w.webContents.getZoomLevel()
           assert.strictEqual(zoomLevel, 0)
           done()
-          })
         }
       })
       ipcMain.once('zoom-level-set', (e, zoomLevel) => {

spec/fixtures/pages/webview-custom-zoom-level.html

@@ -10,8 +10,8 @@
     if (!finalNavigation && !view.canGoBack()) {
       view.setZoomLevel(2.0)
     }
-    view.getZoomLevel((zoomLevel) => {
-      view.getZoomFactor((zoomFactor) => {
+    const zoomLevel = view.getZoomLevel()
+    const zoomFactor = view.getZoomFactor()
     ipcRenderer.send('webview-zoom-level', zoomLevel, zoomFactor, view.canGoBack(), finalNavigation)
     if (!view.canGoBack() && !finalNavigation) {
       view.src = 'zoom://host2'
@@ -20,7 +20,5 @@
       view.goBack()
     }
   })
-    })
-  })
 </script>
 </html>

spec/fixtures/pages/webview-in-page-navigate.html

@@ -8,13 +8,11 @@
   let finalNavigation = false
   function SendZoomLevel() {
     return new Promise((resolve, reject) => {
-      view.getZoomLevel((zoomLevel) => {
-        view.getZoomFactor((zoomFactor) => {
+      const zoomLevel = view.getZoomLevel()
+      const zoomFactor = view.getZoomFactor()
       ipcRenderer.send('webview-zoom-in-page', zoomLevel, zoomFactor, finalNavigation)
       resolve()
     })
-      })
-    })
   }
   view.addEventListener('dom-ready', () => {
     view.setZoomLevel(2.0)

spec/fixtures/pages/webview-origin-zoom-level.html

@@ -13,8 +13,7 @@
     document.body.appendChild(view2)
   })
   view2.addEventListener('dom-ready', () => {
-    view2.getZoomLevel((level) => {
-      ipcRenderer.send('webview-origin-zoom-level', level)
-    })
+    const zoomLevel = view2.getZoomLevel()
+    ipcRenderer.send('webview-origin-zoom-level', zoomLevel)
   })
 </script>