From e5e8ab4eea0871bc0683d894ffc9171d0d449964 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Tue, 4 May 2021 11:30:29 -0700 Subject: [PATCH] refactor: remove more dead code post render process reuse (#28983) * Overrides for window.history.* * Node environment cleanup / creation logic * Options and switches that are now static values --- lib/browser/rpc-server.ts | 16 ----------- lib/common/ipc-messages.ts | 5 ---- lib/renderer/init.ts | 3 +- lib/renderer/window-setup.ts | 27 +----------------- lib/sandboxed_renderer/init.ts | 3 +- shell/browser/electron_browser_client.cc | 1 - shell/common/options_switches.cc | 3 -- shell/common/options_switches.h | 2 -- shell/renderer/api/electron_api_web_frame.cc | 6 ---- .../electron_render_frame_observer.cc | 7 +---- shell/renderer/electron_renderer_client.cc | 28 ++++--------------- typings/internal-ambient.d.ts | 1 - 12 files changed, 10 insertions(+), 92 deletions(-) diff --git a/lib/browser/rpc-server.ts b/lib/browser/rpc-server.ts index b81b0df72708..f1cdda46b746 100644 --- a/lib/browser/rpc-server.ts +++ b/lib/browser/rpc-server.ts @@ -100,22 +100,6 @@ ipcMainUtils.handleSync(IPC_MESSAGES.BROWSER_SANDBOX_LOAD, async function (event }; }); -ipcMainInternal.on(IPC_MESSAGES.NAVIGATION_CONTROLLER_GO_BACK, function (event) { - event.sender.goBack(); -}); - -ipcMainInternal.on(IPC_MESSAGES.NAVIGATION_CONTROLLER_GO_FORWARD, function (event) { - event.sender.goForward(); -}); - -ipcMainInternal.on(IPC_MESSAGES.NAVIGATION_CONTROLLER_GO_TO_OFFSET, function (event, offset) { - event.sender.goToOffset(offset); -}); - -ipcMainInternal.on(IPC_MESSAGES.NAVIGATION_CONTROLLER_LENGTH, function (event) { - event.returnValue = event.sender.length(); -}); - ipcMainInternal.on(IPC_MESSAGES.BROWSER_PRELOAD_ERROR, function (event, preloadPath: string, error: Error) { event.sender.emit('preload-error', event, preloadPath, error); }); diff --git a/lib/common/ipc-messages.ts b/lib/common/ipc-messages.ts index 468e9ad5d40f..feb790a11bc7 100644 --- a/lib/common/ipc-messages.ts +++ b/lib/common/ipc-messages.ts @@ -29,11 +29,6 @@ export const enum IPC_MESSAGES { RENDERER_WEB_FRAME_METHOD = 'RENDERER_WEB_FRAME_METHOD', - NAVIGATION_CONTROLLER_GO_BACK = 'NAVIGATION_CONTROLLER_GO_BACK', - NAVIGATION_CONTROLLER_GO_FORWARD = 'NAVIGATION_CONTROLLER_GO_FORWARD', - NAVIGATION_CONTROLLER_GO_TO_OFFSET = 'NAVIGATION_CONTROLLER_GO_TO_OFFSET', - NAVIGATION_CONTROLLER_LENGTH = 'NAVIGATION_CONTROLLER_LENGTH', - INSPECTOR_CONFIRM = 'INSPECTOR_CONFIRM', INSPECTOR_CONTEXT_MENU = 'INSPECTOR_CONTEXT_MENU', INSPECTOR_SELECT_FILE = 'INSPECTOR_SELECT_FILE', diff --git a/lib/renderer/init.ts b/lib/renderer/init.ts index e3cb88069b1b..d3544d8987dc 100644 --- a/lib/renderer/init.ts +++ b/lib/renderer/init.ts @@ -70,7 +70,6 @@ const nodeIntegration = mainFrame.getWebPreference('nodeIntegration'); const webviewTag = mainFrame.getWebPreference('webviewTag'); const isHiddenPage = mainFrame.getWebPreference('hiddenPage'); const usesNativeWindowOpen = mainFrame.getWebPreference('nativeWindowOpen'); -const rendererProcessReuseEnabled = mainFrame.getWebPreference('disableElectronSiteInstanceOverrides'); const preloadScript = mainFrame.getWebPreference('preload'); const preloadScripts = mainFrame.getWebPreference('preloadScripts'); const guestInstanceId = mainFrame.getWebPreference('guestInstanceId') || null; @@ -97,7 +96,7 @@ switch (window.location.protocol) { default: { // Override default web functions. const { windowSetup } = require('@electron/internal/renderer/window-setup'); - windowSetup(guestInstanceId, openerId, isHiddenPage, usesNativeWindowOpen, rendererProcessReuseEnabled); + windowSetup(guestInstanceId, openerId, isHiddenPage, usesNativeWindowOpen); } } diff --git a/lib/renderer/window-setup.ts b/lib/renderer/window-setup.ts index 91cee9a840f0..08ceab841864 100644 --- a/lib/renderer/window-setup.ts +++ b/lib/renderer/window-setup.ts @@ -243,8 +243,7 @@ class BrowserWindowProxy { } export const windowSetup = ( - guestInstanceId: number, openerId: number, isHiddenPage: boolean, usesNativeWindowOpen: boolean, rendererProcessReuseEnabled: boolean -) => { + guestInstanceId: number, openerId: number, isHiddenPage: boolean, usesNativeWindowOpen: boolean) => { if (!process.sandboxed && guestInstanceId == null) { // Override default window.close. window.close = function () { @@ -319,30 +318,6 @@ export const windowSetup = ( }); } - if (!process.sandboxed && !rendererProcessReuseEnabled) { - window.history.back = function () { - ipcRendererInternal.send(IPC_MESSAGES.NAVIGATION_CONTROLLER_GO_BACK); - }; - if (contextIsolationEnabled) internalContextBridge.overrideGlobalValueFromIsolatedWorld(['history', 'back'], window.history.back); - - window.history.forward = function () { - ipcRendererInternal.send(IPC_MESSAGES.NAVIGATION_CONTROLLER_GO_FORWARD); - }; - if (contextIsolationEnabled) internalContextBridge.overrideGlobalValueFromIsolatedWorld(['history', 'forward'], window.history.forward); - - window.history.go = function (offset: number) { - ipcRendererInternal.send(IPC_MESSAGES.NAVIGATION_CONTROLLER_GO_TO_OFFSET, +offset); - }; - if (contextIsolationEnabled) internalContextBridge.overrideGlobalValueFromIsolatedWorld(['history', 'go'], window.history.go); - - const getHistoryLength = () => ipcRendererInternal.sendSync(IPC_MESSAGES.NAVIGATION_CONTROLLER_LENGTH); - Object.defineProperty(window.history, 'length', { - get: getHistoryLength, - set () {} - }); - if (contextIsolationEnabled) internalContextBridge.overrideGlobalPropertyFromIsolatedWorld(['history', 'length'], getHistoryLength); - } - if (guestInstanceId != null) { // Webview `document.visibilityState` tracks window visibility (and ignores // the actual element visibility) for backwards compatibility. diff --git a/lib/sandboxed_renderer/init.ts b/lib/sandboxed_renderer/init.ts index 63e55ac919e8..51eee794f41b 100644 --- a/lib/sandboxed_renderer/init.ts +++ b/lib/sandboxed_renderer/init.ts @@ -124,7 +124,6 @@ if (hasSwitch('unsafely-expose-electron-internals-for-testing')) { const contextIsolation = mainFrame.getWebPreference('contextIsolation'); const webviewTag = mainFrame.getWebPreference('webviewTag'); const isHiddenPage = mainFrame.getWebPreference('hiddenPage'); -const rendererProcessReuseEnabled = mainFrame.getWebPreference('disableElectronSiteInstanceOverrides'); const usesNativeWindowOpen = true; const guestInstanceId = mainFrame.getWebPreference('guestInstanceId') || null; const openerId = mainFrame.getWebPreference('openerId') || null; @@ -144,7 +143,7 @@ switch (window.location.protocol) { default: { // Override default web functions. const { windowSetup } = require('@electron/internal/renderer/window-setup'); - windowSetup(guestInstanceId, openerId, isHiddenPage, usesNativeWindowOpen, rendererProcessReuseEnabled); + windowSetup(guestInstanceId, openerId, isHiddenPage, usesNativeWindowOpen); } } diff --git a/shell/browser/electron_browser_client.cc b/shell/browser/electron_browser_client.cc index 2a73ac19c2dd..58f82ec0332a 100644 --- a/shell/browser/electron_browser_client.cc +++ b/shell/browser/electron_browser_client.cc @@ -445,7 +445,6 @@ void ElectronBrowserClient::OverrideWebkitPrefs( SessionPreferences::GetValidPreloads(web_contents->GetBrowserContext()); if (!preloads.empty()) prefs->preloads = preloads; - prefs->disable_electron_site_instance_overrides = true; SetFontDefaults(prefs); diff --git a/shell/common/options_switches.cc b/shell/common/options_switches.cc index 015916d6bea6..cdcef7efe766 100644 --- a/shell/common/options_switches.cc +++ b/shell/common/options_switches.cc @@ -181,9 +181,6 @@ const char kWebGL[] = "webgl"; // navigation. const char kNavigateOnDragDrop[] = "navigateOnDragDrop"; -const char kDisableElectronSiteInstanceOverrides[] = - "disableElectronSiteInstanceOverrides"; -const char kEnableNodeLeakageInRenderers[] = "enableNodeLeakageInRenderers"; const char kHiddenPage[] = "hiddenPage"; #if BUILDFLAG(ENABLE_BUILTIN_SPELLCHECKER) diff --git a/shell/common/options_switches.h b/shell/common/options_switches.h index a3f9b75de1a9..d0d7cee1eb3e 100644 --- a/shell/common/options_switches.h +++ b/shell/common/options_switches.h @@ -90,8 +90,6 @@ extern const char kNavigateOnDragDrop[]; extern const char kEnableWebSQL[]; extern const char kEnablePreferredSizeMode[]; -extern const char kDisableElectronSiteInstanceOverrides[]; -extern const char kEnableNodeLeakageInRenderers[]; extern const char kHiddenPage[]; #if BUILDFLAG(ENABLE_BUILTIN_SPELLCHECKER) diff --git a/shell/renderer/api/electron_api_web_frame.cc b/shell/renderer/api/electron_api_web_frame.cc index e48121369507..73db4e9063e6 100644 --- a/shell/renderer/api/electron_api_web_frame.cc +++ b/shell/renderer/api/electron_api_web_frame.cc @@ -491,9 +491,6 @@ class WebFrameRenderer : public gin::Wrappable, if (pref_name == options::kPreloadScripts) { return gin::ConvertToV8(isolate, prefs.preloads); - } else if (pref_name == options::kDisableElectronSiteInstanceOverrides) { - return gin::ConvertToV8(isolate, - prefs.disable_electron_site_instance_overrides); } else if (pref_name == options::kBackgroundColor) { return gin::ConvertToV8(isolate, prefs.background_color); } else if (pref_name == options::kOpenerID) { @@ -517,9 +514,6 @@ class WebFrameRenderer : public gin::Wrappable, return gin::ConvertToV8(isolate, prefs.node_integration); } else if (pref_name == options::kNodeIntegrationInWorker) { return gin::ConvertToV8(isolate, prefs.node_integration_in_worker); - } else if (pref_name == options::kEnableNodeLeakageInRenderers) { - // NOTE: enableNodeLeakageInRenderers is internal-only. - return gin::ConvertToV8(isolate, prefs.node_leakage_in_renderers); } else if (pref_name == options::kNodeIntegrationInSubFrames) { return gin::ConvertToV8(isolate, true); #if BUILDFLAG(ENABLE_BUILTIN_SPELLCHECKER) diff --git a/shell/renderer/electron_render_frame_observer.cc b/shell/renderer/electron_render_frame_observer.cc index 336d1f1bf7b5..7733e1b8cc32 100644 --- a/shell/renderer/electron_render_frame_observer.cc +++ b/shell/renderer/electron_render_frame_observer.cc @@ -73,15 +73,10 @@ void ElectronRenderFrameObserver::DidInstallConditionalFeatures( // DidCreateScriptContext(); bool is_main_world = IsMainWorld(world_id); bool is_main_frame = render_frame_->IsMainFrame(); - bool reuse_renderer_processes_enabled = - prefs.disable_electron_site_instance_overrides; - bool is_not_opened = !render_frame_->GetWebFrame()->Opener() || - prefs.node_leakage_in_renderers; bool allow_node_in_sub_frames = prefs.node_integration_in_sub_frames; bool should_create_isolated_context = use_context_isolation && is_main_world && - (is_main_frame || allow_node_in_sub_frames) && - (is_not_opened || reuse_renderer_processes_enabled); + (is_main_frame || allow_node_in_sub_frames); if (should_create_isolated_context) { CreateIsolatedWorldContext(); diff --git a/shell/renderer/electron_renderer_client.cc b/shell/renderer/electron_renderer_client.cc index b5f459eabea9..6599bc918c10 100644 --- a/shell/renderer/electron_renderer_client.cc +++ b/shell/renderer/electron_renderer_client.cc @@ -95,18 +95,7 @@ void ElectronRendererClient::DidCreateScriptContext( // Only load node if we are a main frame or a devtools extension // unless node support has been explicitly enabled for sub frames auto prefs = render_frame->GetBlinkPreferences(); - bool reuse_renderer_processes_enabled = - prefs.disable_electron_site_instance_overrides; - // Consider the window not "opened" if it does not have an Opener, or if a - // user has manually opted in to leaking node in the renderer - bool is_not_opened = - !render_frame->GetWebFrame()->Opener() || prefs.node_leakage_in_renderers; - // Consider this the main frame if it is both a Main Frame and it wasn't - // opened. We allow an opened main frame to have node if renderer process - // reuse is enabled as that will correctly free node environments prevent a - // leak in child windows. - bool is_main_frame = render_frame->IsMainFrame() && - (is_not_opened || reuse_renderer_processes_enabled); + bool is_main_frame = render_frame->IsMainFrame(); bool is_devtools = IsDevToolsExtension(render_frame); bool allow_node_in_subframes = prefs.node_integration_in_sub_frames; bool should_load_node = @@ -121,7 +110,7 @@ void ElectronRendererClient::DidCreateScriptContext( node_integration_initialized_ = true; node_bindings_->Initialize(); node_bindings_->PrepareMessageLoop(); - } else if (reuse_renderer_processes_enabled) { + } else { node_bindings_->PrepareMessageLoop(); } @@ -138,9 +127,7 @@ void ElectronRendererClient::DidCreateScriptContext( // If we have disabled the site instance overrides we should prevent loading // any non-context aware native module - if (reuse_renderer_processes_enabled) - env->set_force_context_aware(true); - env->set_warn_context_aware(true); + env->set_force_context_aware(true); environments_.insert(env); @@ -183,12 +170,9 @@ void ElectronRendererClient::WillReleaseScriptContext( // We also do this if we have disable electron site instance overrides to // avoid memory leaks auto prefs = render_frame->GetBlinkPreferences(); - if (prefs.node_integration_in_sub_frames || - prefs.disable_electron_site_instance_overrides) { - node::FreeEnvironment(env); - if (env == node_bindings_->uv_env()) - node::FreeIsolateData(node_bindings_->isolate_data()); - } + node::FreeEnvironment(env); + if (env == node_bindings_->uv_env()) + node::FreeIsolateData(node_bindings_->isolate_data()); // ElectronBindings is tracking node environments. electron_bindings_->EnvironmentDestroyed(env); diff --git a/typings/internal-ambient.d.ts b/typings/internal-ambient.d.ts index 40c483b2c12b..6079b445b001 100644 --- a/typings/internal-ambient.d.ts +++ b/typings/internal-ambient.d.ts @@ -103,7 +103,6 @@ declare namespace NodeJS { interface InternalWebPreferences { contextIsolation: boolean; - disableElectronSiteInstanceOverrides: boolean; guestInstanceId: number; hiddenPage: boolean; nativeWindowOpen: boolean;