feat: make getZoomFactor/getZoomLevel sync

electron · Jan 17, 2019 · cf5d11d · cf5d11d
1 parent 32d9885
commit cf5d11d
Show file tree

Hide file tree

Showing 13 changed files with 80 additions and 128 deletions.
diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc
@@ -1940,23 +1940,24 @@ void WebContents::SetZoomFactor(double factor) {
 }
 
 double WebContents::GetZoomFactor() const {
-  auto level = GetZoomLevel();
+  double level = zoom_controller_->GetZoomLevel();
   return content::ZoomLevelToZoomFactor(level);
 }
 
 void WebContents::OnSetTemporaryZoomLevel(content::RenderFrameHost* rfh,
                                           double level,
                                           IPC::Message* reply_msg) {
   zoom_controller_->SetTemporaryZoomLevel(level);
-  double new_level = zoom_controller_->GetZoomLevel();
+  double new_level = GetZoomFactor();
   AtomFrameHostMsg_SetTemporaryZoomLevel::WriteReplyParams(reply_msg,
                                                            new_level);
   rfh->Send(reply_msg);
 }
 
 void WebContents::OnGetZoomLevel(content::RenderFrameHost* rfh,
                                  IPC::Message* reply_msg) {
-  AtomFrameHostMsg_GetZoomLevel::WriteReplyParams(reply_msg, GetZoomLevel());
+  double zoom_level = zoom_controller_->GetZoomLevel();
+  AtomFrameHostMsg_GetZoomLevel::WriteReplyParams(reply_msg, zoom_level);
   rfh->Send(reply_msg);
 }
 
@@ -2159,9 +2160,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)

diff --git a/docs/api/promisification.md b/docs/api/promisification.md
@@ -41,8 +41,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)
@@ -52,8 +50,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
 

diff --git a/docs/api/web-contents.md b/docs/api/web-contents.md
@@ -967,13 +967,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)`
 
@@ -984,13 +980,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)`
-
-* `callback` Function
-  * `zoomLevel` Number
+#### `contents.getZoomLevel()`
 
-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)`
 

diff --git a/docs/api/webview-tag.md b/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
-
-Sends a request to get current zoom factor, the `callback` will be called with
-`callback(zoomFactor)`.
+Returns `Number` - the current zoom factor.
 
-### `<webview>.getZoomLevel(callback)`
-
-* `callback` Function
-  * `zoomLevel` Number
+### `<webview>.getZoomLevel()`
 
-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)`
 

diff --git a/lib/browser/api/menu-item-roles.js b/lib/browser/api/menu-item-roles.js
@@ -156,19 +156,17 @@ const roles = {
     accelerator: 'CommandOrControl+Plus',
     nonNativeMacOSRole: true,
     webContentsMethod: (webContents) => {
-      webContents.getZoomLevel((zoomLevel) => {
-        webContents.setZoomLevel(zoomLevel + 0.5)
-      })
+      const zoomLevel = webContents.getZoomLevel()
+      webContents.setZoomLevel(zoomLevel + 0.5)
     }
   },
   zoomout: {
     label: 'Zoom Out',
     accelerator: 'CommandOrControl+-',
     nonNativeMacOSRole: true,
     webContentsMethod: (webContents) => {
-      webContents.getZoomLevel((zoomLevel) => {
-        webContents.setZoomLevel(zoomLevel - 0.5)
-      })
+      const zoomLevel = webContents.getZoomLevel()
+      webContents.setZoomLevel(zoomLevel - 0.5)
     }
   },
   // App submenu should be used for Mac only

diff --git a/lib/browser/api/web-contents.js b/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')
@@ -289,16 +290,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 +306,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.

diff --git a/lib/browser/guest-view-manager.js b/lib/browser/guest-view-manager.js
@@ -208,7 +208,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

diff --git a/lib/common/web-view-methods.js b/lib/common/web-view-methods.js
@@ -46,6 +46,8 @@ exports.syncMethods = new Set([
   'downloadURL',
   'inspectServiceWorker',
   'showDefinitionForSelection',
+  'getZoomFactor',
+  'getZoomLevel',
   'setZoomFactor',
   'setZoomLevel'
 ])
@@ -60,8 +62,6 @@ exports.asyncMethods = new Set([
   // with callback
   'capturePage',
   'executeJavaScript',
-  'getZoomFactor',
-  'getZoomLevel',
   'print',
   'printToPDF'
 ])
diff --git a/spec/api-web-contents-spec.js b/spec/api-web-contents-spec.js
@@ -569,9 +569,8 @@ 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) => {
-                                ipcRenderer.send(window.location.hostname + '-zoom-level', zoomLevel)
-                              })
+                              const zoomLevel = remote.getCurrentWebContents().getZoomLevel()
+                              ipcRenderer.send(window.location.hostname + '-zoom-level', zoomLevel)
                             })
                           </script>`
         callback({ data: response, mimeType: 'text/html' })
@@ -586,10 +585,10 @@ describe('webContents module', () => {
     it('can set the correct zoom level', async () => {
       try {
         await w.loadURL('about:blank')
-        const zoomLevel = await new Promise(resolve => w.webContents.getZoomLevel(resolve))
+        const zoomLevel = w.webContents.getZoomLevel()
         expect(zoomLevel).to.eql(0.0)
         w.webContents.setZoomLevel(0.5)
-        const newZoomLevel = await new Promise(resolve => w.webContents.getZoomLevel(resolve))
+        const newZoomLevel = w.webContents.getZoomLevel()
         expect(newZoomLevel).to.eql(0.5)
       } finally {
         w.webContents.setZoomLevel(0)
@@ -626,15 +625,14 @@ describe('webContents module', () => {
         show: false
       })
       w2.webContents.on('did-finish-load', () => {
-        w.webContents.getZoomLevel((zoomLevel1) => {
-          assert.strictEqual(zoomLevel1, hostZoomMap.host3)
-          w2.webContents.getZoomLevel((zoomLevel2) => {
-            assert.strictEqual(zoomLevel1, zoomLevel2)
-            w2.setClosable(true)
-            w2.close()
-            done()
-          })
-        })
+        const zoomLevel1 = w.webContents.getZoomLevel()
+        assert.strictEqual(zoomLevel1, hostZoomMap.host3)
+
+        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)
@@ -656,18 +654,18 @@ describe('webContents module', () => {
       }, (error) => {
         if (error) return done(error)
         w2.webContents.on('did-finish-load', () => {
-          w.webContents.getZoomLevel((zoomLevel1) => {
-            assert.strictEqual(zoomLevel1, hostZoomMap.host3)
-            w2.webContents.getZoomLevel((zoomLevel2) => {
-              assert.strictEqual(zoomLevel2, 0)
-              assert.notStrictEqual(zoomLevel1, zoomLevel2)
-              protocol.unregisterProtocol(zoomScheme, (error) => {
-                if (error) return done(error)
-                w2.setClosable(true)
-                w2.close()
-                done()
-              })
-            })
+          const zoomLevel1 = w.webContents.getZoomLevel()
+          assert.strictEqual(zoomLevel1, hostZoomMap.host3)
+
+          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)
+            w2.close()
+            done()
           })
         })
         w.webContents.on('did-finish-load', () => {
@@ -689,12 +687,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) => {
-              assert.strictEqual(zoomLevel, 2.0)
-              w.webContents.setZoomLevel(0)
-              server.close()
-              done()
-            })
+            const zoomLevel = w.webContents.getZoomLevel()
+            assert.strictEqual(zoomLevel, 2.0)
+
+            w.webContents.setZoomLevel(0)
+            server.close()
+            done()
           }
         })
         w.webContents.on('dom-ready', () => {
@@ -710,16 +708,16 @@ describe('webContents module', () => {
         show: false
       })
       w2.webContents.on('did-finish-load', () => {
-        w.webContents.getZoomLevel((zoomLevel1) => {
-          assert.strictEqual(zoomLevel1, finalZoomLevel)
-          w2.webContents.getZoomLevel((zoomLevel2) => {
-            assert.strictEqual(zoomLevel2, 0)
-            assert.notStrictEqual(zoomLevel1, zoomLevel2)
-            w2.setClosable(true)
-            w2.close()
-            done()
-          })
-        })
+        const zoomLevel1 = w.webContents.getZoomLevel()
+        assert.strictEqual(zoomLevel1, finalZoomLevel)
+
+        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'))
@@ -739,10 +737,9 @@ describe('webContents module', () => {
         if (initialNavigation) {
           w.webContents.executeJavaScript(source, () => {})
         } else {
-          w.webContents.getZoomLevel((zoomLevel) => {
-            assert.strictEqual(zoomLevel, 0)
-            done()
-          })
+          const zoomLevel = w.webContents.getZoomLevel()
+          assert.strictEqual(zoomLevel, 0)
+          done()
         }
       })
       ipcMain.once('zoom-level-set', (e, zoomLevel) => {

diff --git a/spec/fixtures/api/execute-javascript.html b/spec/fixtures/api/execute-javascript.html
diff --git a/spec/fixtures/pages/webview-custom-zoom-level.html b/spec/fixtures/pages/webview-custom-zoom-level.html
@@ -10,17 +10,15 @@
     if (!finalNavigation && !view.canGoBack()) {
       view.setZoomLevel(2.0)
     }
-    view.getZoomLevel((zoomLevel) => {
-      view.getZoomFactor((zoomFactor) => {
-        ipcRenderer.send('webview-zoom-level', zoomLevel, zoomFactor, view.canGoBack(), finalNavigation)
-        if (!view.canGoBack() && !finalNavigation) {
-          view.src = 'zoom://host2'
-        } else if (!finalNavigation) {
-          finalNavigation = true
-          view.goBack()
-        }
-      })
-    })
+    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'
+    } else if (!finalNavigation) {
+      finalNavigation = true
+      view.goBack()
+    }
   })
 </script>
 </html>
diff --git a/spec/fixtures/pages/webview-in-page-navigate.html b/spec/fixtures/pages/webview-in-page-navigate.html
@@ -8,12 +8,10 @@
   let finalNavigation = false
   function SendZoomLevel() {
     return new Promise((resolve, reject) => {
-      view.getZoomLevel((zoomLevel) => {
-        view.getZoomFactor((zoomFactor) => {
-          ipcRenderer.send('webview-zoom-in-page', zoomLevel, zoomFactor, finalNavigation)
-          resolve()
-        })
-      })
+      const zoomLevel = view.getZoomLevel()
+      const zoomFactor = view.getZoomFactor()
+      ipcRenderer.send('webview-zoom-in-page', zoomLevel, zoomFactor, finalNavigation)
+      resolve()
     })
   }
   view.addEventListener('dom-ready', () => {