From 7b7def7d1eb8c68c99a05756f2946eb756419036 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Mon, 24 Feb 2020 18:11:06 -0800 Subject: [PATCH] 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 --- docs/api/browser-window.md | 2 +- docs/api/service-workers.md | 124 +++++++++--------- docs/breaking-changes.md | 20 +++ script/spec-runner.js | 10 +- shell/browser/electron_browser_client.h | 2 +- spec-main/api-app-spec.ts | 4 +- spec-main/api-browser-window-affinity-spec.ts | 10 +- spec-main/api-browser-window-spec.ts | 12 -- spec-main/api-remote-spec.ts | 10 +- spec-main/api-web-contents-spec.ts | 55 +++++--- 10 files changed, 146 insertions(+), 103 deletions(-) diff --git a/docs/api/browser-window.md b/docs/api/browser-window.md index bcc58adb77..8e3798806a 100644 --- a/docs/api/browser-window.md +++ b/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`. diff --git a/docs/api/service-workers.md b/docs/api/service-workers.md index d50529b337..e2a13524bb 100644 --- a/docs/api/service-workers.md +++ b/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` - 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` - 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. diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md index a0a81b52f7..e33092acf3 100644 --- a/docs/breaking-changes.md +++ b/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). + ### `.getWebContents()` This API, which was deprecated in Electron 8.0, is now removed. diff --git a/script/spec-runner.js b/script/spec-runner.js index 4861f66eea..0a8cff3ce8 100755 --- a/script/spec-runner.js +++ b/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.`) diff --git a/shell/browser/electron_browser_client.h b/shell/browser/electron_browser_client.h index 280420dcbb..36f04a9982 100644 --- a/shell/browser/electron_browser_client.h +++ b/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. diff --git a/spec-main/api-app-spec.ts b/spec-main/api-app-spec.ts index 3ef87d069a..267988a3d7 100644 --- a/spec-main/api-app-spec.ts +++ b/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 () => { diff --git a/spec-main/api-browser-window-affinity-spec.ts b/spec-main/api-browser-window-affinity-spec.ts index 1e7f10f893..9e86c3d155 100644 --- a/spec-main/api-browser-window-affinity-spec.ts +++ b/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, diff --git a/spec-main/api-browser-window-spec.ts b/spec-main/api-browser-window-spec.ts index dfe01abf87..8b93af07be 100644 --- a/spec-main/api-browser-window-spec.ts +++ b/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 = [ diff --git a/spec-main/api-remote-spec.ts b/spec-main/api-remote-spec.ts index 87a3bc5594..6870360fbd 100644 --- a/spec-main/api-remote-spec.ts +++ b/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 } diff --git a/spec-main/api-web-contents-spec.ts b/spec-main/api-web-contents-spec.ts index ef4caab945..ca68fe612a 100644 --- a/spec-main/api-web-contents-spec.ts +++ b/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() })