Browse Source

feat: set app.enableRendererProcessReuse to true by default (#22336)

* feat: set app.enableRendererProcessReuse to true by default

* chore: add context aware info to breaking changes doc

* spec: fix nodeIntegration in child windows test for rendererprocessreuse

* spec: fix remote listeners in destroyed renderers spec as the error is now async

* Update api-browser-window-spec.ts

* chore: deprecate affinity

* chore: fix docs

* spec: handle tests crashing without an exist code

* spec: update tests for new rendererprocessreuse default

* spec: with renderer process re-use we get to destroy less views
Samuel Attard 5 years ago
parent
commit
7b7def7d1e

+ 1 - 1
docs/api/browser-window.md

@@ -289,7 +289,7 @@ It creates a new `BrowserWindow` with native properties as set by the `options`.
       between the web pages even when you specified different values for them,
       including but not limited to `preload`, `sandbox` and `nodeIntegration`.
       So it is suggested to use exact same `webPreferences` for web pages with
-      the same `affinity`. _This property is experimental_
+      the same `affinity`. _Deprecated_
     * `zoomFactor` Number (optional) - The default zoom factor of the page, `3.0` represents
       `300%`. Default is `1.0`.
     * `javascript` Boolean (optional) - Enables JavaScript support. Default is `true`.

+ 62 - 62
docs/api/service-workers.md

@@ -1,62 +1,62 @@
-## Class: ServiceWorkers
-
-> Query and receive events from a sessions active service workers.
-
-Process: [Main](../glossary.md#main-process)
-
-Instances of the `ServiceWorkers` class are accessed by using `serviceWorkers` property of
-a `Session`.
-
-For example:
-
-```javascript
-const { session } = require('electron')
-
-// Get all service workers.
-console.log(session.defaultSession.serviceWorkers.getAllRunning())
-
-// Handle logs and get service worker info
-session.defaultSession.serviceWorkers.on('console-message', (event, messageDetails) => {
-  console.log(
-    'Got service worker message',
-    messageDetails,
-    'from',
-    session.defaultSession.serviceWorkers.getFromVersionID(messageDetails.versionId)
-  )
-})
-```
-
-### Instance Events
-
-The following events are available on instances of `ServiceWorkers`:
-
-#### Event: 'console-message'
-
-Returns:
-
-* `event` Event
-* `messageDetails` Object - Information about the console message
-  * `message` String - The actual console message
-  * `versionId` Number - The version ID of the service worker that sent the log message
-  * `source` String - The type of source for this message.  Can be `javascript`, `xml`, `network`, `console-api`, `storage`, `app-cache`, `rendering`, `security`, `deprecation`, `worker`, `violation`, `intervention`, `recommendation` or `other`.
-  * `level` Number - The log level, from 0 to 3.  In order it matches `verbose`, `info`, `warning` and `error`.
-  * `sourceUrl` String - The URL the message came from
-  * `lineNumber` Number - The line number of the source that triggered this console message
-
-Emitted when a service worker logs something to the console.
-
-### Instance Methods
-
-The following methods are available on instances of `ServiceWorkers`:
-
-#### `serviceWorkers.getAllRunning()`
-
-Returns `Record<Number, ServiceWorkerInfo>` - A [ServiceWorkerInfo](structures/service-worker-info.md) object where the keys are the service worker version ID and the values are the information about that service worker.
-
-#### `serviceWorkers.getFromVersionID(versionId)`
-
-* `versionId` Number
-
-Returns [`ServiceWorkerInfo`](structures/service-worker-info.md) - Information about this service worker
-
-If the service worker does not exist or is not running this method will throw an exception.
+## Class: ServiceWorkers
+
+> Query and receive events from a sessions active service workers.
+
+Process: [Main](../glossary.md#main-process)
+
+Instances of the `ServiceWorkers` class are accessed by using `serviceWorkers` property of
+a `Session`.
+
+For example:
+
+```javascript
+const { session } = require('electron')
+
+// Get all service workers.
+console.log(session.defaultSession.serviceWorkers.getAllRunning())
+
+// Handle logs and get service worker info
+session.defaultSession.serviceWorkers.on('console-message', (event, messageDetails) => {
+  console.log(
+    'Got service worker message',
+    messageDetails,
+    'from',
+    session.defaultSession.serviceWorkers.getFromVersionID(messageDetails.versionId)
+  )
+})
+```
+
+### Instance Events
+
+The following events are available on instances of `ServiceWorkers`:
+
+#### Event: 'console-message'
+
+Returns:
+
+* `event` Event
+* `messageDetails` Object - Information about the console message
+  * `message` String - The actual console message
+  * `versionId` Number - The version ID of the service worker that sent the log message
+  * `source` String - The type of source for this message.  Can be `javascript`, `xml`, `network`, `console-api`, `storage`, `app-cache`, `rendering`, `security`, `deprecation`, `worker`, `violation`, `intervention`, `recommendation` or `other`.
+  * `level` Number - The log level, from 0 to 3.  In order it matches `verbose`, `info`, `warning` and `error`.
+  * `sourceUrl` String - The URL the message came from
+  * `lineNumber` Number - The line number of the source that triggered this console message
+
+Emitted when a service worker logs something to the console.
+
+### Instance Methods
+
+The following methods are available on instances of `ServiceWorkers`:
+
+#### `serviceWorkers.getAllRunning()`
+
+Returns `Record<Number, ServiceWorkerInfo>` - A [ServiceWorkerInfo](structures/service-worker-info.md) object where the keys are the service worker version ID and the values are the information about that service worker.
+
+#### `serviceWorkers.getFromVersionID(versionId)`
+
+* `versionId` Number
+
+Returns [`ServiceWorkerInfo`](structures/service-worker-info.md) - Information about this service worker
+
+If the service worker does not exist or is not running this method will throw an exception.

+ 20 - 0
docs/breaking-changes.md

@@ -8,6 +8,14 @@ The `FIXME` string is used in code comments to denote things that should be fixe
 
 ## Planned Breaking API Changes (10.0)
 
+### Browser Window Affinity
+
+The `affinity` option when constructing a new `BrowserWindow` will be removed
+as part of our plan to more closely align with Chromiums process model for security,
+performance and maintainability.
+
+For more detailed information see [#18397](https://github.com/electron/electron/issues/18397).
+
 ### `enableRemoteModule` defaults to `false`
 
 In Electron 9, using the remote module without explicitly enabling it via the
@@ -28,6 +36,18 @@ module](https://medium.com/@nornagon/electrons-remote-module-considered-harmful-
 
 ## Planned Breaking API Changes (9.0)
 
+### Loading non-context-aware native modules in the renderer process
+
+As of Electron 9 we do not allow loading of non-context-aware native modules in
+the renderer process.  This is to improve security, performance and maintainability
+of Electron as a project.
+
+If this impacts you, you can temporarily set `app.allowRendererProcessReuse` to `false`
+to revert to the old behavior.  This flag will only be an option until Electron 11 so
+you should plan to update your native modules to be context aware.
+
+For more detailed information see [#18397](https://github.com/electron/electron/issues/18397).
+
 ### `<webview>.getWebContents()`
 
 This API, which was deprecated in Electron 8.0, is now removed.

+ 7 - 3
script/spec-runner.js

@@ -203,13 +203,17 @@ async function runMainProcessElectronTests () {
     exe = 'python'
   }
 
-  const { status } = childProcess.spawnSync(exe, runnerArgs, {
+  const { status, signal } = childProcess.spawnSync(exe, runnerArgs, {
     cwd: path.resolve(__dirname, '../..'),
     stdio: 'inherit'
   })
   if (status !== 0) {
-    const textStatus = process.platform === 'win32' ? `0x${status.toString(16)}` : status.toString()
-    console.log(`${fail} Electron tests failed with code ${textStatus}.`)
+    if (status) {
+      const textStatus = process.platform === 'win32' ? `0x${status.toString(16)}` : status.toString()
+      console.log(`${fail} Electron tests failed with code ${textStatus}.`)
+    } else {
+      console.log(`${fail} Electron tests failed with kill signal ${signal}.`)
+    }
     process.exit(1)
   }
   console.log(`${pass} Electron main process tests passed.`)

+ 1 - 1
shell/browser/electron_browser_client.h

@@ -309,7 +309,7 @@ class ElectronBrowserClient : public content::ContentBrowserClient,
 
   std::string user_agent_override_ = "";
 
-  bool disable_process_restart_tricks_ = false;
+  bool disable_process_restart_tricks_ = true;
 
   // Simple shared ID generator, used by ProxyingURLLoaderFactory and
   // ProxyingWebSocket classes.

+ 2 - 2
spec-main/api-app-spec.ts

@@ -1423,8 +1423,8 @@ describe('default behavior', () => {
   })
 
   describe('app.allowRendererProcessReuse', () => {
-    it('should default to false', () => {
-      expect(app.allowRendererProcessReuse).to.equal(false)
+    it('should default to true', () => {
+      expect(app.allowRendererProcessReuse).to.equal(true)
     })
 
     it('should cause renderer processes to get new PIDs when false', async () => {

+ 9 - 1
spec-main/api-browser-window-affinity-spec.ts

@@ -1,7 +1,7 @@
 import { expect } from 'chai'
 import * as path from 'path'
 
-import { ipcMain, BrowserWindow, WebPreferences } from 'electron'
+import { ipcMain, BrowserWindow, WebPreferences, app } from 'electron'
 import { closeWindow } from './window-helpers'
 
 describe('BrowserWindow with affinity module', () => {
@@ -10,6 +10,14 @@ describe('BrowserWindow with affinity module', () => {
   const myAffinityNameUpper = 'MYAFFINITY'
   const anotherAffinityName = 'anotherAffinity'
 
+  before(() => {
+    app.allowRendererProcessReuse = false
+  })
+
+  after(() => {
+    app.allowRendererProcessReuse = true
+  })
+
   async function createWindowWithWebPrefs (webPrefs: WebPreferences) {
     const w = new BrowserWindow({
       show: false,

+ 0 - 12
spec-main/api-browser-window-spec.ts

@@ -2406,18 +2406,6 @@ describe('BrowserWindow module', () => {
         const webPreferences = (childWebContents as any).getLastWebPreferences()
         expect(webPreferences.foo).to.equal('bar')
       })
-      it('should have nodeIntegration disabled in child windows', async () => {
-        const w = new BrowserWindow({
-          show: false,
-          webPreferences: {
-            nodeIntegration: true,
-            nativeWindowOpen: true
-          }
-        })
-        w.loadFile(path.join(fixtures, 'api', 'native-window-open-argv.html'))
-        const [, typeofProcess] = await emittedOnce(ipcMain, 'answer')
-        expect(typeofProcess).to.eql('undefined')
-      })
 
       describe('window.location', () => {
         const protocols = [

+ 9 - 1
spec-main/api-remote-spec.ts

@@ -246,9 +246,17 @@ ifdescribe(features.isRemoteModuleEnabled())('remote module', () => {
       expect(w.webContents.listenerCount('remote-handler')).to.equal(2)
       let warnMessage: string | null = null
       const originalWarn = console.warn
+      let warned: Function
+      const warnPromise = new Promise(resolve => {
+        warned = resolve
+      })
       try {
-        console.warn = (message: string) => { warnMessage = message }
+        console.warn = (message: string) => {
+          warnMessage = message
+          warned()
+        }
         w.webContents.emit('remote-handler', { sender: w.webContents })
+        await warnPromise
       } finally {
         console.warn = originalWarn
       }

+ 35 - 20
spec-main/api-web-contents-spec.ts

@@ -963,29 +963,44 @@ describe('webContents module', () => {
       w.loadFile(path.join(fixturesPath, 'pages', 'webframe-zoom.html'))
     })
 
-    it('cannot persist zoom level after navigation with webFrame', (done) => {
-      const w = new BrowserWindow({ show: false, webPreferences: { nodeIntegration: true } })
-      let initialNavigation = true
-      const source = `
-        const {ipcRenderer, webFrame} = require('electron')
-        webFrame.setZoomLevel(0.6)
-        ipcRenderer.send('zoom-level-set', webFrame.getZoomLevel())
-      `
-      w.webContents.on('did-finish-load', () => {
-        if (initialNavigation) {
-          w.webContents.executeJavaScript(source)
-        } else {
-          const zoomLevel = w.webContents.zoomLevel
-          expect(zoomLevel).to.equal(0)
+    describe('with unique domains', () => {
+      let server: http.Server
+      let serverUrl: string
+      let crossSiteUrl: string
+
+      before((done) => {
+        server = http.createServer((req, res) => {
+          setTimeout(() => res.end('hey'), 0)
+        })
+        server.listen(0, '127.0.0.1', () => {
+          serverUrl = `http://127.0.0.1:${(server.address() as AddressInfo).port}`
+          crossSiteUrl = `http://localhost:${(server.address() as AddressInfo).port}`
           done()
-        }
+        })
       })
-      ipcMain.once('zoom-level-set', (e, zoomLevel) => {
+
+      after(() => {
+        server.close()
+      })
+
+      it('cannot persist zoom level after navigation with webFrame', async () => {
+        const w = new BrowserWindow({ show: false, webPreferences: { nodeIntegration: true } })
+        const source = `
+          const {ipcRenderer, webFrame} = require('electron')
+          webFrame.setZoomLevel(0.6)
+          ipcRenderer.send('zoom-level-set', webFrame.getZoomLevel())
+        `
+        const zoomLevelPromise = emittedOnce(ipcMain, 'zoom-level-set')
+        await w.loadURL(serverUrl)
+        await w.webContents.executeJavaScript(source)
+        let [, zoomLevel] = await zoomLevelPromise
         expect(zoomLevel).to.equal(0.6)
-        w.loadFile(path.join(fixturesPath, 'pages', 'd.html'))
-        initialNavigation = false
+        const loadPromise = emittedOnce(w.webContents, 'did-finish-load')
+        await w.loadURL(crossSiteUrl)
+        await loadPromise
+        zoomLevel = w.webContents.zoomLevel
+        expect(zoomLevel).to.equal(0)
       })
-      w.loadFile(path.join(fixturesPath, 'pages', 'c.html'))
     })
   })
 
@@ -1077,7 +1092,7 @@ describe('webContents module', () => {
       const w = new BrowserWindow({ show: false })
       let rvhDeletedCount = 0
       w.webContents.once('destroyed', () => {
-        const expectedRenderViewDeletedEventCount = 3 // 1 speculative upon redirection + 2 upon window close.
+        const expectedRenderViewDeletedEventCount = 1
         expect(rvhDeletedCount).to.equal(expectedRenderViewDeletedEventCount, 'render-view-deleted wasn\'t emitted the expected nr. of times')
         done()
       })