From 56143f892998c1dc3b88701e1f1710f610e0c0cc Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Mon, 7 Dec 2020 14:52:58 -0800 Subject: [PATCH] chore: remove app.allowRendererProcessReuse --- docs/api/app.md | 13 -- docs/api/browser-window.md | 7 - docs/breaking-changes.md | 23 ++- lib/browser/api/app.ts | 6 +- shell/browser/api/electron_api_app.cc | 12 +- shell/browser/api/electron_api_app.h | 2 - .../browser/api/electron_api_web_contents.cc | 9 - shell/browser/electron_browser_client.cc | 19 +- shell/browser/electron_browser_client.h | 8 +- spec-main/api-app-spec.ts | 20 -- spec-main/api-browser-window-affinity-spec.ts | 175 ------------------ spec-main/api-browser-window-spec.ts | 38 ++-- .../api/site-instance-overrides/index.html | 1 - .../api/site-instance-overrides/main.js | 36 ---- .../api/site-instance-overrides/package.json | 4 - .../api/site-instance-overrides/preload.js | 3 - 16 files changed, 41 insertions(+), 335 deletions(-) delete mode 100644 spec-main/api-browser-window-affinity-spec.ts delete mode 100644 spec/fixtures/api/site-instance-overrides/index.html delete mode 100644 spec/fixtures/api/site-instance-overrides/main.js delete mode 100644 spec/fixtures/api/site-instance-overrides/package.json delete mode 100644 spec/fixtures/api/site-instance-overrides/preload.js diff --git a/docs/api/app.md b/docs/api/app.md index b485b339683ca..68e0371bed5d1 100644 --- a/docs/api/app.md +++ b/docs/api/app.md @@ -1477,19 +1477,6 @@ This is the user agent that will be used when no user agent is set at the app has the same user agent. Set to a custom value as early as possible in your app's initialization to ensure that your overridden value is used. -### `app.allowRendererProcessReuse` - -A `Boolean` which when `true` disables the overrides that Electron has in place -to ensure renderer processes are restarted on every navigation. The current -default value for this property is `true`. - -The intention is for these overrides to become disabled by default and then at -some point in the future this property will be removed. This property impacts -which native modules you can use in the renderer process. For more information -on the direction Electron is going with renderer process restarts and usage of -native modules in the renderer process please check out this -[Tracking Issue](https://github.com/electron/electron/issues/18397). - ### `app.runningUnderRosettaTranslation` _macOS_ _Readonly_ A `Boolean` which when `true` indicates that the app is currently running diff --git a/docs/api/browser-window.md b/docs/api/browser-window.md index aa37bd2fdf59d..bb020510c2e23 100644 --- a/docs/api/browser-window.md +++ b/docs/api/browser-window.md @@ -284,13 +284,6 @@ It creates a new `BrowserWindow` with native properties as set by the `options`. same `partition`. If there is no `persist:` prefix, the page will use an in-memory session. By assigning the same `partition`, multiple pages can share the same session. Default is the default session. - * `affinity` String (optional) - When specified, web pages with the same - `affinity` will run in the same renderer process. Note that due to reusing - the renderer process, certain `webPreferences` options will also be shared - 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`. _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/breaking-changes.md b/docs/breaking-changes.md index fd2d63aacdacd..2ff761f1de759 100644 --- a/docs/breaking-changes.md +++ b/docs/breaking-changes.md @@ -67,6 +67,21 @@ session.defaultSession.getAllExtensions() ## Planned Breaking API Changes (12.0) +### Removed: `app.allowRendererProcessReuse` + +The `app.allowRendererProcessReuse` property will be removed as part of our plan to +more closely align with Chromium's process model for security, performance and maintainability. + +For more detailed information see [#18397](https://github.com/electron/electron/issues/18397). + +### Removed: Browser Window Affinity + +The `affinity` option when constructing a new `BrowserWindow` will be removed +as part of our plan to more closely align with Chromium's process model for security, +performance and maintainability. + +For more detailed information see [#18397](https://github.com/electron/electron/issues/18397). + ### Removed: Pepper Flash support Chromium has removed support for Flash, and so we must follow suit. See @@ -215,14 +230,6 @@ Setting `{ compress: false }` in `crashReporter.start` is deprecated. Nearly all crash ingestion servers support gzip compression. This option will be removed in a future version of Electron. -### Removed: Browser Window Affinity - -The `affinity` option when constructing a new `BrowserWindow` will be removed -as part of our plan to more closely align with Chromium's process model for security, -performance and maintainability. - -For more detailed information see [#18397](https://github.com/electron/electron/issues/18397). - ### Default Changed: `enableRemoteModule` defaults to `false` In Electron 9, using the remote module without explicitly enabling it via the diff --git a/lib/browser/api/app.ts b/lib/browser/api/app.ts index 17d3cc00422f0..301c0a49d9dc9 100644 --- a/lib/browser/api/app.ts +++ b/lib/browser/api/app.ts @@ -1,7 +1,7 @@ import * as fs from 'fs'; import * as path from 'path'; -import { deprecate, Menu } from 'electron/main'; +import { Menu } from 'electron/main'; const bindings = process._linkedBinding('electron_browser_app'); const commandLine = process._linkedBinding('electron_common_command_line'); @@ -129,7 +129,3 @@ for (const name of events) { webContents.emit(name, event, ...args); }); } - -// Deprecate allowRendererProcessReuse but only if they set it to false, no need to log if -// they are setting it to true -deprecate.removeProperty({ __proto__: app } as any, 'allowRendererProcessReuse', [false]); diff --git a/shell/browser/api/electron_api_app.cc b/shell/browser/api/electron_api_app.cc index 9ffe14483f12c..1d3aaa514bc03 100644 --- a/shell/browser/api/electron_api_app.cc +++ b/shell/browser/api/electron_api_app.cc @@ -1457,13 +1457,6 @@ std::string App::GetUserAgentFallback() { return ElectronBrowserClient::Get()->GetUserAgent(); } -void App::SetBrowserClientCanUseCustomSiteInstance(bool should_disable) { - ElectronBrowserClient::Get()->SetCanUseCustomSiteInstance(should_disable); -} -bool App::CanBrowserClientUseCustomSiteInstance() { - return ElectronBrowserClient::Get()->CanUseCustomSiteInstance(); -} - #if defined(OS_MAC) bool App::MoveToApplicationsFolder(gin_helper::ErrorThrower thrower, gin::Arguments* args) { @@ -1663,10 +1656,7 @@ gin::ObjectTemplateBuilder App::GetObjectTemplateBuilder(v8::Isolate* isolate) { #endif .SetProperty("userAgentFallback", &App::GetUserAgentFallback, &App::SetUserAgentFallback) - .SetMethod("enableSandbox", &App::EnableSandbox) - .SetProperty("allowRendererProcessReuse", - &App::CanBrowserClientUseCustomSiteInstance, - &App::SetBrowserClientCanUseCustomSiteInstance); + .SetMethod("enableSandbox", &App::EnableSandbox); } const char* App::GetTypeName() { diff --git a/shell/browser/api/electron_api_app.h b/shell/browser/api/electron_api_app.h index ba6b33fdfef69..c787ec0387785 100644 --- a/shell/browser/api/electron_api_app.h +++ b/shell/browser/api/electron_api_app.h @@ -213,8 +213,6 @@ class App : public ElectronBrowserClient::Delegate, void EnableSandbox(gin_helper::ErrorThrower thrower); void SetUserAgentFallback(const std::string& user_agent); std::string GetUserAgentFallback(); - void SetBrowserClientCanUseCustomSiteInstance(bool should_disable); - bool CanBrowserClientUseCustomSiteInstance(); #if defined(OS_MAC) void SetActivationPolicy(gin_helper::ErrorThrower thrower, diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc index cec721384b43a..e587bb7d035be 100644 --- a/shell/browser/api/electron_api_web_contents.cc +++ b/shell/browser/api/electron_api_web_contents.cc @@ -2043,23 +2043,14 @@ void WebContents::Stop() { } void WebContents::GoBack() { - if (!ElectronBrowserClient::Get()->CanUseCustomSiteInstance()) { - electron::ElectronBrowserClient::SuppressRendererProcessRestartForOnce(); - } web_contents()->GetController().GoBack(); } void WebContents::GoForward() { - if (!ElectronBrowserClient::Get()->CanUseCustomSiteInstance()) { - electron::ElectronBrowserClient::SuppressRendererProcessRestartForOnce(); - } web_contents()->GetController().GoForward(); } void WebContents::GoToOffset(int offset) { - if (!ElectronBrowserClient::Get()->CanUseCustomSiteInstance()) { - electron::ElectronBrowserClient::SuppressRendererProcessRestartForOnce(); - } web_contents()->GetController().GoToOffset(offset); } diff --git a/shell/browser/electron_browser_client.cc b/shell/browser/electron_browser_client.cc index 4f3ae30a530da..231929051d14a 100644 --- a/shell/browser/electron_browser_client.cc +++ b/shell/browser/electron_browser_client.cc @@ -344,10 +344,6 @@ int GetCrashSignalFD(const base::CommandLine& command_line) { } // namespace // static -void ElectronBrowserClient::SuppressRendererProcessRestartForOnce() { - g_suppress_renderer_process_restart = true; -} - ElectronBrowserClient* ElectronBrowserClient::Get() { return g_browser_client; } @@ -609,8 +605,7 @@ void ElectronBrowserClient::OverrideWebkitPrefs( SessionPreferences::GetValidPreloads(web_contents->GetBrowserContext()); if (!preloads.empty()) prefs->preloads = preloads; - if (CanUseCustomSiteInstance()) - prefs->disable_electron_site_instance_overrides = true; + prefs->disable_electron_site_instance_overrides = true; SetFontDefaults(prefs); @@ -621,14 +616,6 @@ void ElectronBrowserClient::OverrideWebkitPrefs( } } -void ElectronBrowserClient::SetCanUseCustomSiteInstance(bool should_disable) { - disable_process_restart_tricks_ = should_disable; -} - -bool ElectronBrowserClient::CanUseCustomSiteInstance() { - return disable_process_restart_tricks_; -} - content::ContentBrowserClient::SiteInstanceForNavigationType ElectronBrowserClient::ShouldOverrideSiteInstanceForNavigation( content::RenderFrameHost* current_rfh, @@ -1723,6 +1710,10 @@ ElectronBrowserClient::GetPluginMimeTypesWithExternalHandlers( return mime_types; } +bool ElectronBrowserClient::CanUseCustomSiteInstance() { + return true; +} + content::SerialDelegate* ElectronBrowserClient::GetSerialDelegate() { if (!serial_delegate_) serial_delegate_ = std::make_unique(); diff --git a/shell/browser/electron_browser_client.h b/shell/browser/electron_browser_client.h index b47579e2c0c1d..ffa9ac925f854 100644 --- a/shell/browser/electron_browser_client.h +++ b/shell/browser/electron_browser_client.h @@ -50,9 +50,6 @@ class ElectronBrowserClient : public content::ContentBrowserClient, // Returns the WebContents for pending render processes. content::WebContents* GetWebContentsFromProcessID(int process_id); - // Don't force renderer process to restart for once. - static void SuppressRendererProcessRestartForOnce(); - NotificationPresenter* GetNotificationPresenter(); void WebNotificationAllowed(int render_process_id, @@ -82,9 +79,8 @@ class ElectronBrowserClient : public content::ContentBrowserClient, std::string GetUserAgent() override; void SetUserAgent(const std::string& user_agent); - void SetCanUseCustomSiteInstance(bool should_disable); - bool CanUseCustomSiteInstance() override; content::SerialDelegate* GetSerialDelegate() override; + bool CanUseCustomSiteInstance() override; protected: void RenderProcessWillLaunch(content::RenderProcessHost* host) override; @@ -329,8 +325,6 @@ class ElectronBrowserClient : public content::ContentBrowserClient, std::string user_agent_override_ = ""; - bool disable_process_restart_tricks_ = true; - // Simple shared ID generator, used by ProxyingURLLoaderFactory and // ProxyingWebSocket classes. uint64_t next_id_ = 0; diff --git a/spec-main/api-app-spec.ts b/spec-main/api-app-spec.ts index 10a54b64f2ad4..14034ac39e3bf 100644 --- a/spec-main/api-app-spec.ts +++ b/spec-main/api-app-spec.ts @@ -1746,26 +1746,6 @@ describe('default behavior', () => { }); }); - describe('app.allowRendererProcessReuse', () => { - it('should default to true', () => { - expect(app.allowRendererProcessReuse).to.equal(true); - }); - - it('should cause renderer processes to get new PIDs when false', async () => { - const output = await runTestApp('site-instance-overrides', 'false'); - expect(output[0]).to.be.a('number').that.is.greaterThan(0); - expect(output[1]).to.be.a('number').that.is.greaterThan(0); - expect(output[0]).to.not.equal(output[1]); - }); - - it('should cause renderer processes to keep the same PID when true', async () => { - const output = await runTestApp('site-instance-overrides', 'true'); - expect(output[0]).to.be.a('number').that.is.greaterThan(0); - expect(output[1]).to.be.a('number').that.is.greaterThan(0); - expect(output[0]).to.equal(output[1]); - }); - }); - describe('login event', () => { afterEach(closeAllWindows); let server: http.Server; diff --git a/spec-main/api-browser-window-affinity-spec.ts b/spec-main/api-browser-window-affinity-spec.ts deleted file mode 100644 index 23d2af93b5e99..0000000000000 --- a/spec-main/api-browser-window-affinity-spec.ts +++ /dev/null @@ -1,175 +0,0 @@ -import { expect } from 'chai'; -import * as path from 'path'; - -import { ipcMain, BrowserWindow, WebPreferences, app } from 'electron/main'; -import { closeWindow } from './window-helpers'; - -describe('BrowserWindow with affinity module', () => { - const fixtures = path.resolve(__dirname, '..', 'spec', 'fixtures'); - const myAffinityName = 'myAffinity'; - 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, - width: 400, - height: 400, - webPreferences: webPrefs || {} - }); - await w.loadFile(path.join(fixtures, 'api', 'blank.html')); - return w; - } - - function testAffinityProcessIds (name: string, webPreferences: WebPreferences = {}) { - describe(name, () => { - let mAffinityWindow: BrowserWindow; - before(async () => { - mAffinityWindow = await createWindowWithWebPrefs({ affinity: myAffinityName, ...webPreferences }); - }); - - after(async () => { - await closeWindow(mAffinityWindow, { assertNotWindows: false }); - mAffinityWindow = null as unknown as BrowserWindow; - }); - - it('should have a different process id than a default window', async () => { - const w = await createWindowWithWebPrefs({ ...webPreferences }); - const affinityID = mAffinityWindow.webContents.getOSProcessId(); - const wcID = w.webContents.getOSProcessId(); - - expect(affinityID).to.not.equal(wcID, 'Should have different OS process IDs'); - await closeWindow(w, { assertNotWindows: false }); - }); - - it(`should have a different process id than a window with a different affinity '${anotherAffinityName}'`, async () => { - const w = await createWindowWithWebPrefs({ affinity: anotherAffinityName, ...webPreferences }); - const affinityID = mAffinityWindow.webContents.getOSProcessId(); - const wcID = w.webContents.getOSProcessId(); - - expect(affinityID).to.not.equal(wcID, 'Should have different OS process IDs'); - await closeWindow(w, { assertNotWindows: false }); - }); - - it(`should have the same OS process id than a window with the same affinity '${myAffinityName}'`, async () => { - const w = await createWindowWithWebPrefs({ affinity: myAffinityName, ...webPreferences }); - const affinityID = mAffinityWindow.webContents.getOSProcessId(); - const wcID = w.webContents.getOSProcessId(); - - expect(affinityID).to.equal(wcID, 'Should have the same OS process ID'); - await closeWindow(w, { assertNotWindows: false }); - }); - - it(`should have the same OS process id than a window with an equivalent affinity '${myAffinityNameUpper}' (case insensitive)`, async () => { - const w = await createWindowWithWebPrefs({ affinity: myAffinityNameUpper, ...webPreferences }); - const affinityID = mAffinityWindow.webContents.getOSProcessId(); - const wcID = w.webContents.getOSProcessId(); - - expect(affinityID).to.equal(wcID, 'Should have the same OS process ID'); - await closeWindow(w, { assertNotWindows: false }); - }); - }); - } - - testAffinityProcessIds(`BrowserWindow with an affinity '${myAffinityName}'`); - testAffinityProcessIds(`BrowserWindow with an affinity '${myAffinityName}' and sandbox enabled`, { sandbox: true }); - testAffinityProcessIds(`BrowserWindow with an affinity '${myAffinityName}' and nativeWindowOpen enabled`, { nativeWindowOpen: true }); - - describe('BrowserWindow with an affinity : nodeIntegration=false', () => { - const preload = path.join(fixtures, 'module', 'send-later.js'); - const affinityWithNodeTrue = 'affinityWithNodeTrue'; - const affinityWithNodeFalse = 'affinityWithNodeFalse'; - - function testNodeIntegration (present: boolean) { - return new Promise((resolve) => { - ipcMain.once('answer', (event, typeofProcess, typeofBuffer) => { - if (present) { - expect(typeofProcess).to.not.equal('undefined'); - expect(typeofBuffer).to.not.equal('undefined'); - } else { - expect(typeofProcess).to.equal('undefined'); - expect(typeofBuffer).to.equal('undefined'); - } - resolve(); - }); - }); - } - - it('disables node integration when specified to false', async () => { - const [, w] = await Promise.all([ - testNodeIntegration(false), - createWindowWithWebPrefs({ - affinity: affinityWithNodeTrue, - preload, - nodeIntegration: false - }) - ]); - await closeWindow(w, { assertNotWindows: false }); - }); - it('allows nodeIntegration to enable in second window with the same affinity', async () => { - const [, w1] = await Promise.all([ - testNodeIntegration(false), - createWindowWithWebPrefs({ - affinity: affinityWithNodeTrue, - preload, - nodeIntegration: false - }) - ]); - const [, w2] = await Promise.all([ - testNodeIntegration(true), - createWindowWithWebPrefs({ - affinity: affinityWithNodeTrue, - preload, - nodeIntegration: true - }) - ]); - await Promise.all([ - closeWindow(w1, { assertNotWindows: false }), - closeWindow(w2, { assertNotWindows: false }) - ]); - }); - - it('enables node integration when specified to true', async () => { - const [, w] = await Promise.all([ - testNodeIntegration(true), - createWindowWithWebPrefs({ - affinity: affinityWithNodeFalse, - preload, - nodeIntegration: true - }) - ]); - await closeWindow(w, { assertNotWindows: false }); - }); - - it('allows nodeIntegration to disable in second window with the same affinity', async () => { - const [, w1] = await Promise.all([ - testNodeIntegration(true), - createWindowWithWebPrefs({ - affinity: affinityWithNodeFalse, - preload, - nodeIntegration: true - }) - ]); - const [, w2] = await Promise.all([ - testNodeIntegration(false), - createWindowWithWebPrefs({ - affinity: affinityWithNodeFalse, - preload, - nodeIntegration: false - }) - ]); - await Promise.all([ - closeWindow(w1, { assertNotWindows: false }), - closeWindow(w2, { assertNotWindows: false }) - ]); - }); - }); -}); diff --git a/spec-main/api-browser-window-spec.ts b/spec-main/api-browser-window-spec.ts index 89902c7ed808d..efc18a3454a99 100644 --- a/spec-main/api-browser-window-spec.ts +++ b/spec-main/api-browser-window-spec.ts @@ -4368,28 +4368,26 @@ describe('BrowserWindow module', () => { }); }); - describe('reloading with allowRendererProcessReuse enabled', () => { - it('does not cause Node.js module API hangs after reload', (done) => { - const w = new BrowserWindow({ - show: false, - webPreferences: { - nodeIntegration: true - } - }); - - let count = 0; - ipcMain.on('async-node-api-done', () => { - if (count === 3) { - ipcMain.removeAllListeners('async-node-api-done'); - done(); - } else { - count++; - w.reload(); - } - }); + it('reloading does not cause Node.js module API hangs after reload', (done) => { + const w = new BrowserWindow({ + show: false, + webPreferences: { + nodeIntegration: true + } + }); - w.loadFile(path.join(fixtures, 'pages', 'send-after-node.html')); + let count = 0; + ipcMain.on('async-node-api-done', () => { + if (count === 3) { + ipcMain.removeAllListeners('async-node-api-done'); + done(); + } else { + count++; + w.reload(); + } }); + + w.loadFile(path.join(fixtures, 'pages', 'send-after-node.html')); }); describe('window.webContents.focus()', () => { diff --git a/spec/fixtures/api/site-instance-overrides/index.html b/spec/fixtures/api/site-instance-overrides/index.html deleted file mode 100644 index 6c70bcfe4d48d..0000000000000 --- a/spec/fixtures/api/site-instance-overrides/index.html +++ /dev/null @@ -1 +0,0 @@ - \ No newline at end of file diff --git a/spec/fixtures/api/site-instance-overrides/main.js b/spec/fixtures/api/site-instance-overrides/main.js deleted file mode 100644 index 8bd019f7cc0dd..0000000000000 --- a/spec/fixtures/api/site-instance-overrides/main.js +++ /dev/null @@ -1,36 +0,0 @@ -const { app, BrowserWindow, ipcMain } = require('electron'); -const path = require('path'); - -process.noDeprecation = true; - -process.on('uncaughtException', (e) => { - console.error(e); - process.exit(1); -}); - -app.allowRendererProcessReuse = JSON.parse(process.argv[2]); - -const pids = []; -let win; - -ipcMain.on('pid', (event, pid) => { - pids.push(pid); - if (pids.length === 2) { - console.log(JSON.stringify(pids)); - if (win) win.close(); - app.quit(); - } else { - if (win) win.reload(); - } -}); - -app.whenReady().then(() => { - win = new BrowserWindow({ - show: false, - webPreferences: { - preload: path.resolve(__dirname, 'preload.js'), - contextIsolation: true - } - }); - win.loadFile('index.html'); -}); diff --git a/spec/fixtures/api/site-instance-overrides/package.json b/spec/fixtures/api/site-instance-overrides/package.json deleted file mode 100644 index 49fabbf14ca16..0000000000000 --- a/spec/fixtures/api/site-instance-overrides/package.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "name": "electron-test-site-instance-overrides", - "main": "main.js" -} diff --git a/spec/fixtures/api/site-instance-overrides/preload.js b/spec/fixtures/api/site-instance-overrides/preload.js deleted file mode 100644 index cfe37266b5e8b..0000000000000 --- a/spec/fixtures/api/site-instance-overrides/preload.js +++ /dev/null @@ -1,3 +0,0 @@ -const { ipcRenderer } = require('electron'); - -ipcRenderer.send('pid', process.pid);