Browse Source

feat: convert webContents primitives to properties (#18998)

* feat: convert webContents primitives to properties

* address feedback from review
Shelley Vohr 5 years ago
parent
commit
8782d06ed6

+ 6 - 13
docs/api/modernization/property-updates.md

@@ -4,11 +4,6 @@ The Electron team is currently undergoing an initiative to convert separate gett
 
 ## Candidates
 
-* `app` module
-  * `dock`
-    * `badge`
-* `autoUpdater` module
-  * `feedUrl`
 * `BrowserWindow`
   * `fullscreen`
   * `simpleFullscreen`
@@ -20,14 +15,6 @@ The Electron team is currently undergoing an initiative to convert separate gett
   * `visibleOnAllWorkspaces`
 * `crashReporter` module
   * `uploadToServer`
-* `Session` module
-  * `preloads`
-* `webContents` module
-  * `zoomFactor`
-  * `zoomLevel`
-  * `audioMuted`
-  * `userAgent`
-  * `frameRate`
 * `webFrame` modules
   * `zoomFactor`
   * `zoomLevel`
@@ -58,3 +45,9 @@ The Electron team is currently undergoing an initiative to convert separate gett
   * `isMacTemplateImage`
 * `SystemPreferences` module
   * `appLevelAppearance`
+* `webContents` module
+  * `audioMuted`
+  * `frameRate`
+  * `userAgent`
+  * `zoomFactor`
+  * `zoomLevel`

+ 47 - 0
docs/api/web-contents.md

@@ -979,10 +979,14 @@ Returns `Boolean` - Whether the renderer process has crashed.
 
 Overrides the user agent for this web page.
 
+**[Deprecated](modernization/property-updates.md)**
+
 #### `contents.getUserAgent()`
 
 Returns `String` - The user agent for this web page.
 
+**[Deprecated](modernization/property-updates.md)**
+
 #### `contents.insertCSS(css)`
 
 * `css` String
@@ -1049,10 +1053,14 @@ Ignore application menu shortcuts while this web contents is focused.
 
 Mute the audio on the current web page.
 
+**[Deprecated](modernization/property-updates.md)**
+
 #### `contents.isAudioMuted()`
 
 Returns `Boolean` - Whether this page has been muted.
 
+**[Deprecated](modernization/property-updates.md)**
+
 #### `contents.isCurrentlyAudible()`
 
 Returns `Boolean` - Whether audio is currently playing.
@@ -1064,10 +1072,14 @@ 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.
 
+**[Deprecated](modernization/property-updates.md)**
+
 #### `contents.getZoomFactor()`
 
 Returns `Number` - the current zoom factor.
 
+**[Deprecated](modernization/property-updates.md)**
+
 #### `contents.setZoomLevel(level)`
 
 * `level` Number - Zoom level.
@@ -1077,10 +1089,14 @@ 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`.
 
+**[Deprecated](modernization/property-updates.md)**
+
 #### `contents.getZoomLevel()`
 
 Returns `Number` - the current zoom level.
 
+**[Deprecated](modernization/property-updates.md)**
+
 #### `contents.setVisualZoomLevelLimits(minimumLevel, maximumLevel)`
 
 * `minimumLevel` Number
@@ -1672,10 +1688,14 @@ Returns `Boolean` - If *offscreen rendering* is enabled returns whether it is cu
 If *offscreen rendering* is enabled sets the frame rate to the specified number.
 Only values between 1 and 60 are accepted.
 
+**[Deprecated](modernization/property-updates.md)**
+
 #### `contents.getFrameRate()`
 
 Returns `Integer` - If *offscreen rendering* is enabled returns the current frame rate.
 
+**[Deprecated](modernization/property-updates.md)**
+
 #### `contents.invalidate()`
 
 Schedules a full repaint of the window this web contents is in.
@@ -1740,6 +1760,33 @@ Returns `String` - the type of the webContent. Can be `backgroundPage`, `window`
 
 ### Instance Properties
 
+#### `contents.audioMuted`
+
+A `Boolean` property that determines whether this page is muted.
+
+#### `contents.userAgent`
+
+A `String` property that determines the user agent for this web page.
+
+#### `contents.zoomLevel`
+
+A `Number` property that determines the zoom level for this web contents.
+
+The original size is 0 and each 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.zoomFactor`
+
+A `Number` property that determines the zoom factor for this web contents.
+
+The zoom factor is the zoom percent divided by 100, so 300% = 3.0.
+
+#### `contents.frameRate`
+
+An `Integer` property that sets the frame rate of the web contents to the specified number.
+Only values between 1 and 60 are accepted.
+
+Only applicable if *offscreen rendering* is enabled.
+
 #### `contents.id`
 
 A `Integer` representing the unique ID of this WebContents.

+ 3 - 5
lib/browser/api/menu-item-roles.js

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

+ 7 - 0
lib/browser/api/web-contents.js

@@ -416,6 +416,13 @@ WebContents.prototype._init = function () {
   app.emit('web-contents-created', {}, this)
 }
 
+// Deprecations
+deprecate.fnToProperty(WebContents.prototype, 'audioMuted', '_isAudioMuted', '_setAudioMuted')
+deprecate.fnToProperty(WebContents.prototype, 'userAgent', '_getUserAgent', '_setUserAgent')
+deprecate.fnToProperty(WebContents.prototype, 'zoomLevel', '_getZoomLevel', '_setZoomLevel')
+deprecate.fnToProperty(WebContents.prototype, 'zoomFactor', '_getZoomFactor', '_setZoomFactor')
+deprecate.fnToProperty(WebContents.prototype, 'frameRate', '_getFrameRate', '_setFrameRate')
+
 // JavaScript wrapper of Debugger.
 const { Debugger } = process.electronBinding('debugger')
 Object.setPrototypeOf(Debugger.prototype, EventEmitter.prototype)

+ 20 - 10
shell/browser/api/atom_api_web_contents.cc

@@ -2388,8 +2388,10 @@ void WebContents::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("_goForward", &WebContents::GoForward)
       .SetMethod("_goToOffset", &WebContents::GoToOffset)
       .SetMethod("isCrashed", &WebContents::IsCrashed)
-      .SetMethod("setUserAgent", &WebContents::SetUserAgent)
-      .SetMethod("getUserAgent", &WebContents::GetUserAgent)
+      .SetMethod("_setUserAgent", &WebContents::SetUserAgent)
+      .SetMethod("_getUserAgent", &WebContents::GetUserAgent)
+      .SetProperty("userAgent", &WebContents::GetUserAgent,
+                   &WebContents::SetUserAgent)
       .SetMethod("savePage", &WebContents::SavePage)
       .SetMethod("openDevTools", &WebContents::OpenDevTools)
       .SetMethod("closeDevTools", &WebContents::CloseDevTools)
@@ -2400,8 +2402,10 @@ void WebContents::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("toggleDevTools", &WebContents::ToggleDevTools)
       .SetMethod("inspectElement", &WebContents::InspectElement)
       .SetMethod("setIgnoreMenuShortcuts", &WebContents::SetIgnoreMenuShortcuts)
-      .SetMethod("setAudioMuted", &WebContents::SetAudioMuted)
-      .SetMethod("isAudioMuted", &WebContents::IsAudioMuted)
+      .SetMethod("_setAudioMuted", &WebContents::SetAudioMuted)
+      .SetMethod("_isAudioMuted", &WebContents::IsAudioMuted)
+      .SetProperty("audioMuted", &WebContents::IsAudioMuted,
+                   &WebContents::SetAudioMuted)
       .SetMethod("isCurrentlyAudible", &WebContents::IsCurrentlyAudible)
       .SetMethod("undo", &WebContents::Undo)
       .SetMethod("redo", &WebContents::Redo)
@@ -2432,14 +2436,20 @@ void WebContents::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("startPainting", &WebContents::StartPainting)
       .SetMethod("stopPainting", &WebContents::StopPainting)
       .SetMethod("isPainting", &WebContents::IsPainting)
-      .SetMethod("setFrameRate", &WebContents::SetFrameRate)
-      .SetMethod("getFrameRate", &WebContents::GetFrameRate)
+      .SetMethod("_setFrameRate", &WebContents::SetFrameRate)
+      .SetMethod("_getFrameRate", &WebContents::GetFrameRate)
+      .SetProperty("frameRate", &WebContents::GetFrameRate,
+                   &WebContents::SetFrameRate)
 #endif
       .SetMethod("invalidate", &WebContents::Invalidate)
-      .SetMethod("setZoomLevel", &WebContents::SetZoomLevel)
-      .SetMethod("getZoomLevel", &WebContents::GetZoomLevel)
-      .SetMethod("setZoomFactor", &WebContents::SetZoomFactor)
-      .SetMethod("getZoomFactor", &WebContents::GetZoomFactor)
+      .SetMethod("_setZoomLevel", &WebContents::SetZoomLevel)
+      .SetMethod("_getZoomLevel", &WebContents::GetZoomLevel)
+      .SetProperty("zoomLevel", &WebContents::GetZoomLevel,
+                   &WebContents::SetZoomLevel)
+      .SetMethod("_setZoomFactor", &WebContents::SetZoomFactor)
+      .SetMethod("_getZoomFactor", &WebContents::GetZoomFactor)
+      .SetProperty("zoomFactor", &WebContents::GetZoomFactor,
+                   &WebContents::SetZoomFactor)
       .SetMethod("getType", &WebContents::GetType)
       .SetMethod("_getPreloadPaths", &WebContents::GetPreloadPaths)
       .SetMethod("getWebPreferences", &WebContents::GetWebPreferences)

+ 13 - 1
spec-main/api-session-spec.js

@@ -47,7 +47,8 @@ describe('session module', () => {
       expect(session.fromPartition('test')).to.equal(session.fromPartition('test'))
     })
 
-    it('created session is ref-counted', () => {
+    // TODO(codebytere): remove in Electron v8.0.0
+    it('created session is ref-counted (functions)', () => {
       const partition = 'test2'
       const userAgent = 'test-agent'
       const ses1 = session.fromPartition(partition)
@@ -57,6 +58,17 @@ describe('session module', () => {
       const ses2 = session.fromPartition(partition)
       expect(ses2.getUserAgent()).to.not.equal(userAgent)
     })
+
+    it('created session is ref-counted', () => {
+      const partition = 'test2'
+      const userAgent = 'test-agent'
+      const ses1 = session.fromPartition(partition)
+      ses1.userAgent = userAgent
+      expect(ses1.userAgent).to.equal(userAgent)
+      ses1.destroy()
+      const ses2 = session.fromPartition(partition)
+      expect(ses2.userAgent).to.not.equal(userAgent)
+    })
   })
 
   describe('ses.cookies', () => {

+ 23 - 0
spec/api-browser-window-spec.js

@@ -3034,6 +3034,7 @@ describe('BrowserWindow module', () => {
       })
     })
 
+    // TODO(codebytere): remove in Electron v8.0.0
     describe('window.webContents.getFrameRate()', () => {
       it('has default frame rate', (done) => {
         w.webContents.once('paint', function (event, rect, data) {
@@ -3044,6 +3045,7 @@ describe('BrowserWindow module', () => {
       })
     })
 
+    // TODO(codebytere): remove in Electron v8.0.0
     describe('window.webContents.setFrameRate(frameRate)', () => {
       it('sets custom frame rate', (done) => {
         w.webContents.on('dom-ready', () => {
@@ -3056,6 +3058,27 @@ describe('BrowserWindow module', () => {
         w.loadFile(path.join(fixtures, 'api', 'offscreen-rendering.html'))
       })
     })
+
+    describe('window.webContents.FrameRate', () => {
+      it('has default frame rate', (done) => {
+        w.webContents.once('paint', function (event, rect, data) {
+          expect(w.webContents.frameRate).to.equal(60)
+          done()
+        })
+        w.loadFile(path.join(fixtures, 'api', 'offscreen-rendering.html'))
+      })
+
+      it('sets custom frame rate', (done) => {
+        w.webContents.on('dom-ready', () => {
+          w.webContents.frameRate = 30
+          w.webContents.once('paint', function (event, rect, data) {
+            expect(w.webContents.frameRate).to.equal(30)
+            done()
+          })
+        })
+        w.loadFile(path.join(fixtures, 'api', 'offscreen-rendering.html'))
+      })
+    })
   })
 })
 

+ 30 - 15
spec/api-web-contents-spec.js

@@ -589,7 +589,7 @@ describe('webContents module', () => {
                             const {ipcRenderer, remote} = require('electron')
                             ipcRenderer.send('set-zoom', window.location.hostname)
                             ipcRenderer.on(window.location.hostname + '-zoom-set', () => {
-                              const zoomLevel = remote.getCurrentWebContents().getZoomLevel()
+                              const { zoomLevel } = remote.getCurrentWebContents()
                               ipcRenderer.send(window.location.hostname + '-zoom-level', zoomLevel)
                             })
                           </script>`
@@ -602,7 +602,8 @@ describe('webContents module', () => {
       protocol.unregisterProtocol(zoomScheme, (error) => done(error))
     })
 
-    it('can set the correct zoom level', async () => {
+    // TODO(codebytere): remove in Electron v8.0.0
+    it('can set the correct zoom level (functions)', async () => {
       try {
         await w.loadURL('about:blank')
         const zoomLevel = w.webContents.getZoomLevel()
@@ -615,11 +616,25 @@ describe('webContents module', () => {
       }
     })
 
+    it('can set the correct zoom level', async () => {
+      try {
+        await w.loadURL('about:blank')
+        const zoomLevel = w.webContents.zoomLevel
+        expect(zoomLevel).to.eql(0.0)
+        w.webContents.zoomLevel = 0.5
+        const newZoomLevel = w.webContents.zoomLevel
+        expect(newZoomLevel).to.eql(0.5)
+      } finally {
+        w.webContents.zoomLevel = 0
+      }
+    })
+
     it('can persist zoom level across navigation', (done) => {
       let finalNavigation = false
       ipcMain.on('set-zoom', (e, host) => {
         const zoomLevel = hostZoomMap[host]
-        if (!finalNavigation) w.webContents.setZoomLevel(zoomLevel)
+        if (!finalNavigation) w.webContents.zoomLevel = zoomLevel
+        console.log()
         e.sender.send(`${host}-zoom-set`)
       })
       ipcMain.on('host1-zoom-level', (e, zoomLevel) => {
@@ -645,17 +660,17 @@ describe('webContents module', () => {
         show: false
       })
       w2.webContents.on('did-finish-load', () => {
-        const zoomLevel1 = w.webContents.getZoomLevel()
+        const zoomLevel1 = w.webContents.zoomLevel
         expect(zoomLevel1).to.equal(hostZoomMap.host3)
 
-        const zoomLevel2 = w2.webContents.getZoomLevel()
+        const zoomLevel2 = w2.webContents.zoomLevel
         expect(zoomLevel1).to.equal(zoomLevel2)
         w2.setClosable(true)
         w2.close()
         done()
       })
       w.webContents.on('did-finish-load', () => {
-        w.webContents.setZoomLevel(hostZoomMap.host3)
+        w.webContents.zoomLevel = hostZoomMap.host3
         w2.loadURL(`${zoomScheme}://host3`)
       })
       w.loadURL(`${zoomScheme}://host3`)
@@ -674,10 +689,10 @@ describe('webContents module', () => {
       }, (error) => {
         if (error) return done(error)
         w2.webContents.on('did-finish-load', () => {
-          const zoomLevel1 = w.webContents.getZoomLevel()
+          const zoomLevel1 = w.webContents.zoomLevel
           expect(zoomLevel1).to.equal(hostZoomMap.host3)
 
-          const zoomLevel2 = w2.webContents.getZoomLevel()
+          const zoomLevel2 = w2.webContents.zoomLevel
           expect(zoomLevel2).to.equal(0)
           expect(zoomLevel1).to.not.equal(zoomLevel2)
 
@@ -689,7 +704,7 @@ describe('webContents module', () => {
           })
         })
         w.webContents.on('did-finish-load', () => {
-          w.webContents.setZoomLevel(hostZoomMap.host3)
+          w.webContents.zoomLevel = hostZoomMap.host3
           w2.loadURL(`${zoomScheme}://host3`)
         })
         w.loadURL(`${zoomScheme}://host3`)
@@ -707,16 +722,16 @@ describe('webContents module', () => {
         const content = `<iframe src=${url}></iframe>`
         w.webContents.on('did-frame-finish-load', (e, isMainFrame) => {
           if (!isMainFrame) {
-            const zoomLevel = w.webContents.getZoomLevel()
+            const zoomLevel = w.webContents.zoomLevel
             expect(zoomLevel).to.equal(2.0)
 
-            w.webContents.setZoomLevel(0)
+            w.webContents.zoomLevel = 0
             server.close()
             done()
           }
         })
         w.webContents.on('dom-ready', () => {
-          w.webContents.setZoomLevel(2.0)
+          w.webContents.zoomLevel = 2.0
         })
         w.loadURL(`data:text/html,${content}`)
       })
@@ -728,10 +743,10 @@ describe('webContents module', () => {
         show: false
       })
       w2.webContents.on('did-finish-load', () => {
-        const zoomLevel1 = w.webContents.getZoomLevel()
+        const zoomLevel1 = w.webContents.zoomLevel
         expect(zoomLevel1).to.equal(finalZoomLevel)
 
-        const zoomLevel2 = w2.webContents.getZoomLevel()
+        const zoomLevel2 = w2.webContents.zoomLevel
         expect(zoomLevel2).to.equal(0)
         expect(zoomLevel1).to.not.equal(zoomLevel2)
 
@@ -757,7 +772,7 @@ describe('webContents module', () => {
         if (initialNavigation) {
           w.webContents.executeJavaScript(source)
         } else {
-          const zoomLevel = w.webContents.getZoomLevel()
+          const zoomLevel = w.webContents.zoomLevel
           expect(zoomLevel).to.equal(0)
           done()
         }

+ 3 - 5
spec/ts-smoke/electron/main.ts

@@ -679,7 +679,7 @@ const template = <Electron.MenuItemConstructorOptions[]> [
         accelerator: 'CmdOrCtrl+0',
         click: (item, focusedWindow) => {
           if (focusedWindow) {
-            focusedWindow.webContents.setZoomLevel(0)
+            focusedWindow.webContents.zoomLevel = 0
           }
         }
       },
@@ -689,8 +689,7 @@ const template = <Electron.MenuItemConstructorOptions[]> [
         click: (item, focusedWindow) => {
           if (focusedWindow) {
             const { webContents } = focusedWindow
-            const zoomLevel = webContents.getZoomLevel()
-            webContents.setZoomLevel(zoomLevel + 0.5)
+            webContents.zoomLevel += 0.5
           }
         }
       },
@@ -700,8 +699,7 @@ const template = <Electron.MenuItemConstructorOptions[]> [
         click: (item, focusedWindow) => {
           if (focusedWindow) {
             const { webContents } = focusedWindow
-            const zoomLevel = webContents.getZoomLevel()
-            webContents.setZoomLevel(zoomLevel - 0.5)
+            webContents.zoomLevel -= 0.5
           }
         }
       }