Skip to content

Commit

Permalink
feat: set app.enableRendererProcessReuse to true by default (#22336) (#…
Browse files Browse the repository at this point in the history
…22401)

* 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
  • Loading branch information
MarshallOfSound committed Feb 27, 2020
1 parent 0d7e13d commit 7ec9b4e
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 41 deletions.
2 changes: 1 addition & 1 deletion docs/api/browser-window.md
Expand Up @@ -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`.
Expand Down
10 changes: 7 additions & 3 deletions script/spec-runner.js
Expand Up @@ -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.`)
Expand Down
2 changes: 1 addition & 1 deletion shell/browser/electron_browser_client.h
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions spec-main/api-app-spec.ts
Expand Up @@ -1418,8 +1418,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 () => {
Expand Down
10 changes: 9 additions & 1 deletion 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', () => {
Expand All @@ -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,
Expand Down
12 changes: 0 additions & 12 deletions spec-main/api-browser-window-spec.ts
Expand Up @@ -2372,18 +2372,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 = [
Expand Down
10 changes: 9 additions & 1 deletion spec-main/api-remote-spec.ts
Expand Up @@ -243,9 +243,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
}
Expand Down
55 changes: 35 additions & 20 deletions spec-main/api-web-contents-spec.ts
Expand Up @@ -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'))
})
})

Expand Down Expand Up @@ -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()
})
Expand Down

0 comments on commit 7ec9b4e

Please sign in to comment.