diff --git a/default_app/default_app.ts b/default_app/default_app.ts index 538d7f8752d..6a521b9d62b 100644 --- a/default_app/default_app.ts +++ b/default_app/default_app.ts @@ -51,16 +51,17 @@ async function createWindow (backgroundColor?: string) { autoHideMenuBar: true, backgroundColor, webPreferences: { - preload: path.resolve(__dirname, 'preload.js'), + preload: url.fileURLToPath(new URL('preload.js', import.meta.url)), contextIsolation: true, - sandbox: true + sandbox: true, + nodeIntegration: false }, useContentSize: true, show: false }; if (process.platform === 'linux') { - options.icon = path.join(__dirname, 'icon.png'); + options.icon = url.fileURLToPath(new URL('icon.png', import.meta.url)); } mainWindow = new BrowserWindow(options); diff --git a/default_app/main.ts b/default_app/main.ts index be5a3c2034d..a489c8afe98 100644 --- a/default_app/main.ts +++ b/default_app/main.ts @@ -1,6 +1,7 @@ import * as electron from 'electron/main'; import * as fs from 'node:fs'; +import { Module } from 'node:module'; import * as path from 'node:path'; import * as url from 'node:url'; const { app, dialog } = electron; @@ -15,8 +16,6 @@ type DefaultAppOptions = { modules: string[]; } -const Module = require('node:module'); - // Parse command line options. const argv = process.argv.slice(1); @@ -71,10 +70,10 @@ if (nextArgIsRequire) { // Set up preload modules if (option.modules.length > 0) { - Module._preloadModules(option.modules); + (Module as any)._preloadModules(option.modules); } -function loadApplicationPackage (packagePath: string) { +async function loadApplicationPackage (packagePath: string) { // Add a flag indicating app is started from default app. Object.defineProperty(process, 'defaultApp', { configurable: false, @@ -89,11 +88,19 @@ function loadApplicationPackage (packagePath: string) { let appPath; if (fs.existsSync(packageJsonPath)) { let packageJson; + const emitWarning = process.emitWarning; try { - packageJson = require(packageJsonPath); + process.emitWarning = () => {}; + packageJson = (await import(url.pathToFileURL(packageJsonPath).toString(), { + assert: { + type: 'json' + } + })).default; } catch (e) { showErrorMessage(`Unable to parse ${packageJsonPath}\n\n${(e as Error).message}`); return; + } finally { + process.emitWarning = emitWarning; } if (packageJson.version) { @@ -112,13 +119,15 @@ function loadApplicationPackage (packagePath: string) { // Set v8 flags, deliberately lazy load so that apps that do not use this // feature do not pay the price if (packageJson.v8Flags) { - require('node:v8').setFlagsFromString(packageJson.v8Flags); + (await import('node:v8')).setFlagsFromString(packageJson.v8Flags); } appPath = packagePath; } + let filePath: string; + try { - const filePath = Module._resolveFilename(packagePath, module, true); + filePath = (Module as any)._resolveFilename(packagePath, null, true); app.setAppPath(appPath || path.dirname(filePath)); } catch (e) { showErrorMessage(`Unable to find Electron app at ${packagePath}\n\n${(e as Error).message}`); @@ -126,7 +135,7 @@ function loadApplicationPackage (packagePath: string) { } // Run the app. - Module._load(packagePath, module, true); + await import(url.pathToFileURL(filePath).toString()); } catch (e) { console.error('App threw an error during load'); console.error((e as Error).stack || e); @@ -141,16 +150,16 @@ function showErrorMessage (message: string) { } async function loadApplicationByURL (appUrl: string) { - const { loadURL } = await import('./default_app'); + const { loadURL } = await import('./default_app.js'); loadURL(appUrl); } async function loadApplicationByFile (appPath: string) { - const { loadFile } = await import('./default_app'); + const { loadFile } = await import('./default_app.js'); loadFile(appPath); } -function startRepl () { +async function startRepl () { if (process.platform === 'win32') { console.error('Electron REPL not currently supported on Windows'); process.exit(1); @@ -171,8 +180,8 @@ function startRepl () { Using: Node.js ${nodeVersion} and Electron.js ${electronVersion} `); - const { REPLServer } = require('node:repl'); - const repl = new REPLServer({ + const { start } = await import('node:repl'); + const repl = start({ prompt: '> ' }).on('exit', () => { process.exit(0); @@ -225,8 +234,8 @@ function startRepl () { const electronBuiltins = [...Object.keys(electron), 'original-fs', 'electron']; - const defaultComplete = repl.completer; - repl.completer = (line: string, callback: Function) => { + const defaultComplete: Function = repl.completer; + (repl as any).completer = (line: string, callback: Function) => { const lastSpace = line.lastIndexOf(' '); const currentSymbol = line.substring(lastSpace + 1, repl.cursor); @@ -249,11 +258,11 @@ if (option.file && !option.webdriver) { const protocol = url.parse(file).protocol; const extension = path.extname(file); if (protocol === 'http:' || protocol === 'https:' || protocol === 'file:' || protocol === 'chrome:') { - loadApplicationByURL(file); + await loadApplicationByURL(file); } else if (extension === '.html' || extension === '.htm') { - loadApplicationByFile(path.resolve(file)); + await loadApplicationByFile(path.resolve(file)); } else { - loadApplicationPackage(file); + await loadApplicationPackage(file); } } else if (option.version) { console.log('v' + process.versions.electron); @@ -262,7 +271,7 @@ if (option.file && !option.webdriver) { console.log(process.versions.modules); process.exit(0); } else if (option.interactive) { - startRepl(); + await startRepl(); } else { if (!option.noHelp) { const welcomeMessage = ` @@ -285,5 +294,5 @@ Options: console.log(welcomeMessage); } - loadApplicationByFile('index.html'); + await loadApplicationByFile('index.html'); } diff --git a/default_app/package.json b/default_app/package.json index d6c736cbc52..65fee98c59e 100644 --- a/default_app/package.json +++ b/default_app/package.json @@ -1,5 +1,6 @@ { "name": "electron", "productName": "Electron", - "main": "main.js" + "main": "main.js", + "type": "module" } diff --git a/default_app/preload.ts b/default_app/preload.ts index aedaa6e6b7a..aa8c79ea11c 100644 --- a/default_app/preload.ts +++ b/default_app/preload.ts @@ -1,4 +1,4 @@ -import { ipcRenderer, contextBridge } from 'electron/renderer'; +const { ipcRenderer, contextBridge } = require('electron/renderer'); const policy = window.trustedTypes.createPolicy('electron-default-app', { // we trust the SVG contents diff --git a/docs/tutorial/esm-limitations.md b/docs/tutorial/esm-limitations.md new file mode 100644 index 00000000000..2152b672af1 --- /dev/null +++ b/docs/tutorial/esm-limitations.md @@ -0,0 +1,38 @@ +# ESM Limitations + +This document serves to outline the limitations / differences between ESM in Electron and ESM in Node.js and Chromium. + +## ESM Support Matrix + +This table gives a general overview of where ESM is supported and most importantly which ESM loader is used. + +| | Supported | Loader | Supported in Preload | Loader in Preload | Applicable Requirements | +|-|-|-|-|-|-| +| Main Process | Yes | Node.js | N/A | N/A | | +| Sandboxed Renderer | Yes | Chromium | No | | | +| Node.js Renderer + Context Isolation | Yes | Chromium | Yes | Node.js | | +| Node.js Renderer + No Context Isolation | Yes | Chromium | Yes | Node.js | | + +## Requirements + +### You must use `await` generously in the main process to avoid race conditions + +Certain APIs in Electron (`app.setPath` for instance) are documented as needing to be called **before** the `app.on('ready')` event is emitted. When using ESM in the main process it is only guaranteed that the `ready` event hasn't been emitted while executing the side-effects of the primary import. i.e. if `index.mjs` calls `import('./set-up-paths.mjs')` at the top level the app will likely already be "ready" by the time that dynamic import resolves. To avoid this you should `await import('./set-up-paths.mjs')` at the top level of `index.mjs`. It's not just import calls you should await, if you are reading files asynchronously or performing other asynchronous actions you must await those at the top-level as well to ensure the app does not resume initialization and become ready too early. + +### Sandboxed preload scripts can't use ESM imports + +Sandboxed preload scripts are run as plain javascript without an ESM context. It is recommended that preload scripts are bundled via something like `webpack` or `vite` for performance reasons regardless, so your preload script should just be a single file that doesn't need to use ESM imports. Loading the `electron` API is still done via `require('electron')`. + +### Node.js ESM Preload Scripts will run after page load on pages with no content + +If the response body for the page is **completely** empty, i.e. `Content-Length: 0`, the preload script will not block the page load, which may result in race conditions. If this impacts you, change your response body to have _something_ in it, for example an empty `html` tag (``) or swap back to using a CommonJS preload script (`.js` or `.cjs`) which will block the page load. + +### ESM Preload Scripts must have the `.mjs` extension + +In order to load an ESM preload script it must have a `.mjs` file extension. Using `type: module` in a nearby package.json is not sufficient. Please also note the limitation above around not blocking page load if the page is empty. + +### Non-context-isolated renderers can't use dynamic Node.js ESM imports + +If your renderer process does not have `contextIsolation` enabled you can not `import()` ESM files via the Node.js module loader. This means that you can't `import('fs')` or `import('./foo')`. If you want to be able to do so you must enable context isolation. This is because in the renderer Chromium's `import()` function takes precedence and without context isolation there is no way for Electron to know which loader to route the request to. + +If you enable context isolation `import()` from the isolated preload context will use the Node.js loader and `import()` from the main context will continue using Chromium's loader. diff --git a/lib/asar/fs-wrapper.ts b/lib/asar/fs-wrapper.ts index 69a9386bde7..3423d845f80 100644 --- a/lib/asar/fs-wrapper.ts +++ b/lib/asar/fs-wrapper.ts @@ -41,8 +41,13 @@ const getOrCreateArchive = (archivePath: string) => { const asarRe = /\.asar/i; +const { getValidatedPath } = __non_webpack_require__('internal/fs/utils'); +// In the renderer node internals use the node global URL but we do not set that to be +// the global URL instance. We need to do instanceof checks against the internal URL impl +const { URL: NodeURL } = __non_webpack_require__('internal/url'); + // Separate asar package's path from full path. -const splitPath = (archivePathOrBuffer: string | Buffer) => { +const splitPath = (archivePathOrBuffer: string | Buffer | URL) => { // Shortcut for disabled asar. if (isAsarDisabled()) return { isAsar: false }; @@ -51,6 +56,9 @@ const splitPath = (archivePathOrBuffer: string | Buffer) => { if (Buffer.isBuffer(archivePathOrBuffer)) { archivePath = archivePathOrBuffer.toString(); } + if (archivePath instanceof NodeURL) { + archivePath = getValidatedPath(archivePath); + } if (typeof archivePath !== 'string') return { isAsar: false }; if (!asarRe.test(archivePath)) return { isAsar: false }; @@ -384,7 +392,13 @@ export const wrapFsWithAsar = (fs: Record) => { const { exists: nativeExists } = fs; fs.exists = function exists (pathArgument: string, callback: any) { - const pathInfo = splitPath(pathArgument); + let pathInfo: ReturnType; + try { + pathInfo = splitPath(pathArgument); + } catch { + nextTick(callback, [false]); + return; + } if (!pathInfo.isAsar) return nativeExists(pathArgument, callback); const { asarPath, filePath } = pathInfo; @@ -415,7 +429,12 @@ export const wrapFsWithAsar = (fs: Record) => { const { existsSync } = fs; fs.existsSync = (pathArgument: string) => { - const pathInfo = splitPath(pathArgument); + let pathInfo: ReturnType; + try { + pathInfo = splitPath(pathArgument); + } catch { + return false; + } if (!pathInfo.isAsar) return existsSync(pathArgument); const { asarPath, filePath } = pathInfo; diff --git a/lib/browser/init.ts b/lib/browser/init.ts index 19fc26c4f85..7f921cb8a12 100644 --- a/lib/browser/init.ts +++ b/lib/browser/init.ts @@ -182,11 +182,31 @@ const { setDefaultApplicationMenu } = require('@electron/internal/browser/defaul // menu is set before any user window is created. app.once('will-finish-launching', setDefaultApplicationMenu); +const { appCodeLoaded } = process; +delete process.appCodeLoaded; + if (packagePath) { // Finally load app's main.js and transfer control to C++. - process._firstFileName = Module._resolveFilename(path.join(packagePath, mainStartupScript), null, false); - Module._load(path.join(packagePath, mainStartupScript), Module, true); + if ((packageJson.type === 'module' && !mainStartupScript.endsWith('.cjs')) || mainStartupScript.endsWith('.mjs')) { + const { loadESM } = __non_webpack_require__('internal/process/esm_loader'); + const main = require('url').pathToFileURL(path.join(packagePath, mainStartupScript)); + loadESM(async (esmLoader: any) => { + try { + await esmLoader.import(main.toString(), undefined, Object.create(null)); + appCodeLoaded!(); + } catch (err) { + appCodeLoaded!(); + process.emit('uncaughtException', err as Error); + } + }); + } else { + // Call appCodeLoaded before just for safety, it doesn't matter here as _load is syncronous + appCodeLoaded!(); + process._firstFileName = Module._resolveFilename(path.join(packagePath, mainStartupScript), null, false); + Module._load(path.join(packagePath, mainStartupScript), Module, true); + } } else { console.error('Failed to locate a valid package to load (app, app.asar or default_app.asar)'); console.error('This normally means you\'ve damaged the Electron package somehow'); + appCodeLoaded!(); } diff --git a/lib/renderer/init.ts b/lib/renderer/init.ts index 6abdfe027db..7bd00af472d 100644 --- a/lib/renderer/init.ts +++ b/lib/renderer/init.ts @@ -1,4 +1,5 @@ import * as path from 'path'; +import { pathToFileURL } from 'url'; import { IPC_MESSAGES } from '@electron/internal/common/ipc-messages'; import type * as ipcRendererInternalModule from '@electron/internal/renderer/ipc-renderer-internal'; @@ -122,18 +123,39 @@ if (nodeIntegration) { } } -const { preloadPaths } = ipcRendererUtils.invokeSync<{ - preloadPaths: string[] -}>(IPC_MESSAGES.BROWSER_NONSANDBOX_LOAD); +const { appCodeLoaded } = process; +delete process.appCodeLoaded; -// Load the preload scripts. -for (const preloadScript of preloadPaths) { - try { - Module._load(preloadScript); - } catch (error) { - console.error(`Unable to load preload script: ${preloadScript}`); - console.error(error); +const { preloadPaths } = ipcRendererUtils.invokeSync<{ preloadPaths: string[] }>(IPC_MESSAGES.BROWSER_NONSANDBOX_LOAD); +const cjsPreloads = preloadPaths.filter(p => path.extname(p) !== '.mjs'); +const esmPreloads = preloadPaths.filter(p => path.extname(p) === '.mjs'); +if (cjsPreloads.length) { + // Load the preload scripts. + for (const preloadScript of cjsPreloads) { + try { + Module._load(preloadScript); + } catch (error) { + console.error(`Unable to load preload script: ${preloadScript}`); + console.error(error); - ipcRendererInternal.send(IPC_MESSAGES.BROWSER_PRELOAD_ERROR, preloadScript, error); + ipcRendererInternal.send(IPC_MESSAGES.BROWSER_PRELOAD_ERROR, preloadScript, error); + } } } +if (esmPreloads.length) { + const { loadESM } = __non_webpack_require__('internal/process/esm_loader'); + + loadESM(async (esmLoader: any) => { + // Load the preload scripts. + for (const preloadScript of esmPreloads) { + await esmLoader.import(pathToFileURL(preloadScript).toString(), undefined, Object.create(null)).catch((err: Error) => { + console.error(`Unable to load preload script: ${preloadScript}`); + console.error(err); + + ipcRendererInternal.send(IPC_MESSAGES.BROWSER_PRELOAD_ERROR, preloadScript, err); + }); + } + }).finally(() => appCodeLoaded!()); +} else { + appCodeLoaded!(); +} diff --git a/patches/chromium/.patches b/patches/chromium/.patches index f4fd471526b..ab4212923ab 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -119,6 +119,8 @@ expose_v8initializer_codegenerationcheckcallbackinmainthread.patch chore_patch_out_profile_methods_in_profile_selections_cc.patch add_gin_converter_support_for_arraybufferview.patch chore_defer_usb_service_getdevices_request_until_usb_service_is.patch +refactor_expose_hostimportmoduledynamically_and.patch +feat_expose_documentloader_setdefersloading_on_webdocumentloader.patch fix_remove_profiles_from_spellcheck_service.patch chore_patch_out_profile_methods_in_chrome_browser_pdf.patch chore_patch_out_profile_methods_in_titlebar_config.patch diff --git a/patches/chromium/feat_expose_documentloader_setdefersloading_on_webdocumentloader.patch b/patches/chromium/feat_expose_documentloader_setdefersloading_on_webdocumentloader.patch new file mode 100644 index 00000000000..fb7cefb0298 --- /dev/null +++ b/patches/chromium/feat_expose_documentloader_setdefersloading_on_webdocumentloader.patch @@ -0,0 +1,42 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Samuel Attard +Date: Thu, 9 Mar 2023 01:28:56 -0800 +Subject: feat: expose DocumentLoader::SetDefersLoading on WebDocumentLoader + +This allows embedders to call SetDefersLoading without reaching into Blink internals. Electron uses this to defer page loading until the preload scripts have finished executing. +This might be upstreamable? + +diff --git a/third_party/blink/public/web/web_document_loader.h b/third_party/blink/public/web/web_document_loader.h +index ff1948e649fffdc92a2db0e736c99bf4e8c06514..b165201b273c8fa9de8e66fe8ef7bfc28eee0850 100644 +--- a/third_party/blink/public/web/web_document_loader.h ++++ b/third_party/blink/public/web/web_document_loader.h +@@ -38,6 +38,7 @@ + #include "third_party/blink/public/platform/cross_variant_mojo_util.h" + #include "third_party/blink/public/platform/web_archive_info.h" + #include "third_party/blink/public/platform/web_common.h" ++#include "third_party/blink/public/platform/web_loader_freeze_mode.h" + #include "third_party/blink/public/platform/web_source_location.h" + #include "third_party/blink/public/web/web_navigation_type.h" + +@@ -62,6 +63,8 @@ class BLINK_EXPORT WebDocumentLoader { + virtual ~ExtraData() = default; + }; + ++ virtual void SetDefersLoading(WebLoaderFreezeMode) = 0; ++ + static bool WillLoadUrlAsEmpty(const WebURL&); + + // Returns the http referrer of original request which initited this load. +diff --git a/third_party/blink/renderer/core/loader/document_loader.h b/third_party/blink/renderer/core/loader/document_loader.h +index 1548bbee1845eaac75ceb9e2b2783b69084956e4..c9e8293ef2d662210cb9fc95488c6a51ffc80df8 100644 +--- a/third_party/blink/renderer/core/loader/document_loader.h ++++ b/third_party/blink/renderer/core/loader/document_loader.h +@@ -305,7 +305,7 @@ class CORE_EXPORT DocumentLoader : public GarbageCollected, + absl::optional + soft_navigation_heuristics_task_id); + +- void SetDefersLoading(LoaderFreezeMode); ++ void SetDefersLoading(LoaderFreezeMode) override; + + DocumentLoadTiming& GetTiming() { return document_load_timing_; } + diff --git a/patches/chromium/refactor_expose_hostimportmoduledynamically_and.patch b/patches/chromium/refactor_expose_hostimportmoduledynamically_and.patch new file mode 100644 index 00000000000..05ce732c6eb --- /dev/null +++ b/patches/chromium/refactor_expose_hostimportmoduledynamically_and.patch @@ -0,0 +1,75 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Samuel Attard +Date: Wed, 8 Mar 2023 13:04:21 -0800 +Subject: refactor: expose HostImportModuleDynamically and + HostGetImportMetaProperties to embedders + +This is so that Electron can blend Blink's and Node's implementations of these isolate handlers. + +diff --git a/third_party/blink/renderer/bindings/core/v8/v8_initializer.cc b/third_party/blink/renderer/bindings/core/v8/v8_initializer.cc +index b8b20d8c8340c63bd3039a0683446ef1eb4fdf0d..a0781bd3817c2e0d4be37835f0c02063b2e548f1 100644 +--- a/third_party/blink/renderer/bindings/core/v8/v8_initializer.cc ++++ b/third_party/blink/renderer/bindings/core/v8/v8_initializer.cc +@@ -606,7 +606,9 @@ bool JavaScriptCompileHintsMagicEnabledCallback( + execution_context); + } + +-v8::MaybeLocal HostImportModuleDynamically( ++} ++ ++v8::MaybeLocal V8Initializer::HostImportModuleDynamically( + v8::Local context, + v8::Local v8_host_defined_options, + v8::Local v8_referrer_resource_url, +@@ -672,7 +674,7 @@ v8::MaybeLocal HostImportModuleDynamically( + } + + // https://html.spec.whatwg.org/C/#hostgetimportmetaproperties +-void HostGetImportMetaProperties(v8::Local context, ++void V8Initializer::HostGetImportMetaProperties(v8::Local context, + v8::Local module, + v8::Local meta) { + ScriptState* script_state = ScriptState::From(context); +@@ -699,6 +701,8 @@ void HostGetImportMetaProperties(v8::Local context, + meta->CreateDataProperty(context, resolve_key, resolve_value).ToChecked(); + } + ++namespace { ++ + void InitializeV8Common(v8::Isolate* isolate) { + // Set up garbage collection before setting up anything else as V8 may trigger + // GCs during Blink setup. +@@ -718,9 +722,9 @@ void InitializeV8Common(v8::Isolate* isolate) { + SharedArrayBufferConstructorEnabledCallback); + isolate->SetJavaScriptCompileHintsMagicEnabledCallback( + JavaScriptCompileHintsMagicEnabledCallback); +- isolate->SetHostImportModuleDynamicallyCallback(HostImportModuleDynamically); ++ isolate->SetHostImportModuleDynamicallyCallback(V8Initializer::HostImportModuleDynamically); + isolate->SetHostInitializeImportMetaObjectCallback( +- HostGetImportMetaProperties); ++ V8Initializer::HostGetImportMetaProperties); + isolate->SetMetricsRecorder(std::make_shared(isolate)); + + V8ContextSnapshot::EnsureInterfaceTemplates(isolate); +diff --git a/third_party/blink/renderer/bindings/core/v8/v8_initializer.h b/third_party/blink/renderer/bindings/core/v8/v8_initializer.h +index 30a36cf16d4a8f4692ec6a13be1217212390172a..1924f7cef7f5a1f7523c00071639a6c72f9cca70 100644 +--- a/third_party/blink/renderer/bindings/core/v8/v8_initializer.h ++++ b/third_party/blink/renderer/bindings/core/v8/v8_initializer.h +@@ -77,6 +77,17 @@ class CORE_EXPORT V8Initializer { + v8::Local context, + v8::Local source); + ++ static v8::MaybeLocal HostImportModuleDynamically( ++ v8::Local context, ++ v8::Local v8_host_defined_options, ++ v8::Local v8_referrer_resource_url, ++ v8::Local v8_specifier, ++ v8::Local v8_import_assertions); ++ ++ static void HostGetImportMetaProperties(v8::Local context, ++ v8::Local module, ++ v8::Local meta); ++ + static void WasmAsyncResolvePromiseCallback( + v8::Isolate* isolate, + v8::Local context, diff --git a/patches/node/.patches b/patches/node/.patches index 4ab836918cf..ca7bf7629a1 100644 --- a/patches/node/.patches +++ b/patches/node/.patches @@ -28,6 +28,8 @@ fix_expose_the_built-in_electron_module_via_the_esm_loader.patch api_pass_oomdetails_to_oomerrorcallback.patch fix_expose_lookupandcompile_with_parameters.patch enable_crashpad_linux_node_processes.patch +fix_lazyload_fs_in_esm_loaders_to_apply_asar_patches.patch +chore_expose_importmoduledynamically_and.patch test_formally_mark_some_tests_as_flaky.patch fix_adapt_debugger_tests_for_upstream_v8_changes.patch chore_remove_--no-harmony-atomics_related_code.patch diff --git a/patches/node/chore_expose_importmoduledynamically_and.patch b/patches/node/chore_expose_importmoduledynamically_and.patch new file mode 100644 index 00000000000..94007caafda --- /dev/null +++ b/patches/node/chore_expose_importmoduledynamically_and.patch @@ -0,0 +1,103 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Samuel Attard +Date: Wed, 8 Mar 2023 13:02:17 -0800 +Subject: chore: expose ImportModuleDynamically and + HostInitializeImportMetaObjectCallback to embedders + +This also subtly changes the behavior of shouldNotRegisterESMLoader to ensure that node sets up the handlers +internally but simply avoids setting its own handlers on the Isolate. This is so that Electron can set it to +its own blended handler between Node and Blink. + +Not upstreamable. + +diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js +index f96d19969aa59a9964d947a9fd6295cf25ad3b03..15116b78f82c977bba67ce98ce57232500dfaef8 100644 +--- a/lib/internal/process/pre_execution.js ++++ b/lib/internal/process/pre_execution.js +@@ -554,7 +554,7 @@ function initializeESMLoader() { + // Create this WeakMap in js-land because V8 has no C++ API for WeakMap. + internalBinding('module_wrap').callbackMap = new SafeWeakMap(); + +- if (getEmbedderOptions().shouldNotRegisterESMLoader) return; ++ const shouldSetOnIsolate = !getEmbedderOptions().shouldNotRegisterESMLoader; + + const { + setImportModuleDynamicallyCallback, +@@ -563,8 +563,8 @@ function initializeESMLoader() { + const esm = require('internal/process/esm_loader'); + // Setup per-isolate callbacks that locate data or callbacks that we keep + // track of for different ESM modules. +- setInitializeImportMetaObjectCallback(esm.initializeImportMetaObject); +- setImportModuleDynamicallyCallback(esm.importModuleDynamicallyCallback); ++ setInitializeImportMetaObjectCallback(esm.initializeImportMetaObject, shouldSetOnIsolate); ++ setImportModuleDynamicallyCallback(esm.importModuleDynamicallyCallback, shouldSetOnIsolate); + + // Patch the vm module when --experimental-vm-modules is on. + // Please update the comments in vm.js when this block changes. +diff --git a/src/module_wrap.cc b/src/module_wrap.cc +index 0645b3ddf506df2a76f5661f0ec6bb35d5d8b94e..e0f1b2d51f3055b2250f2c0dc1dfd1048b645dd9 100644 +--- a/src/module_wrap.cc ++++ b/src/module_wrap.cc +@@ -547,7 +547,7 @@ MaybeLocal ModuleWrap::ResolveModuleCallback( + return module->module_.Get(isolate); + } + +-static MaybeLocal ImportModuleDynamically( ++MaybeLocal ImportModuleDynamically( + Local context, + Local host_defined_options, + Local resource_name, +@@ -629,12 +629,13 @@ void ModuleWrap::SetImportModuleDynamicallyCallback( + Environment* env = Environment::GetCurrent(args); + HandleScope handle_scope(isolate); + +- CHECK_EQ(args.Length(), 1); ++ CHECK_EQ(args.Length(), 2); + CHECK(args[0]->IsFunction()); + Local import_callback = args[0].As(); + env->set_host_import_module_dynamically_callback(import_callback); + +- isolate->SetHostImportModuleDynamicallyCallback(ImportModuleDynamically); ++ if (args[1]->IsBoolean() && args[1]->BooleanValue(isolate)) ++ isolate->SetHostImportModuleDynamicallyCallback(ImportModuleDynamically); + } + + void ModuleWrap::HostInitializeImportMetaObjectCallback( +@@ -665,13 +666,14 @@ void ModuleWrap::SetInitializeImportMetaObjectCallback( + Environment* env = Environment::GetCurrent(args); + Isolate* isolate = env->isolate(); + +- CHECK_EQ(args.Length(), 1); ++ CHECK_EQ(args.Length(), 2); + CHECK(args[0]->IsFunction()); + Local import_meta_callback = args[0].As(); + env->set_host_initialize_import_meta_object_callback(import_meta_callback); + +- isolate->SetHostInitializeImportMetaObjectCallback( +- HostInitializeImportMetaObjectCallback); ++ if (args[1]->IsBoolean() && args[1]->BooleanValue(isolate)) ++ isolate->SetHostInitializeImportMetaObjectCallback( ++ HostInitializeImportMetaObjectCallback); + } + + MaybeLocal ModuleWrap::SyntheticModuleEvaluationStepsCallback( +diff --git a/src/module_wrap.h b/src/module_wrap.h +index 58b233d036515c52d9bd5574c776c2ea65d2ecb1..5f7ef75480a76761c6fa62061c8700c812a3fc6f 100644 +--- a/src/module_wrap.h ++++ b/src/module_wrap.h +@@ -30,7 +30,14 @@ enum HostDefinedOptions : int { + kLength = 10, + }; + +-class ModuleWrap : public BaseObject { ++NODE_EXTERN v8::MaybeLocal ImportModuleDynamically( ++ v8::Local context, ++ v8::Local host_defined_options, ++ v8::Local resource_name, ++ v8::Local specifier, ++ v8::Local import_assertions); ++ ++class NODE_EXTERN ModuleWrap : public BaseObject { + public: + enum InternalFields { + kModuleWrapBaseField = BaseObject::kInternalFieldCount, diff --git a/patches/node/chore_update_fixtures_errors_force_colors_snapshot.patch b/patches/node/chore_update_fixtures_errors_force_colors_snapshot.patch index 1e235109321..a5294fe4176 100644 --- a/patches/node/chore_update_fixtures_errors_force_colors_snapshot.patch +++ b/patches/node/chore_update_fixtures_errors_force_colors_snapshot.patch @@ -11,7 +11,7 @@ trying to see whether or not the lines are greyed out. One possibility would be to upstream a changed test that doesn't hardcode line numbers. diff --git a/test/fixtures/errors/force_colors.snapshot b/test/fixtures/errors/force_colors.snapshot -index 0334a0b4faa3633aa8617b9538873e7f3540513b..28ba4d18fe5e3caf4d904a055550110fd4faa609 100644 +index 0334a0b4faa3633aa8617b9538873e7f3540513b..0300d397c6a5b82ef2d0feafca5f8bd746b1f5b0 100644 --- a/test/fixtures/errors/force_colors.snapshot +++ b/test/fixtures/errors/force_colors.snapshot @@ -4,11 +4,12 @@ throw new Error('Should include grayed stack trace') @@ -27,7 +27,7 @@ index 0334a0b4faa3633aa8617b9538873e7f3540513b..28ba4d18fe5e3caf4d904a055550110f + at Object..js (node:internal*modules*cjs*loader:1326:10) + at Module.load (node:internal*modules*cjs*loader:1126:32) + at node:internal*modules*cjs*loader:967:12 -+ at Function._load (node:electron*js2c*asar_bundle:756:32) ++ at Function._load (node:electron*js2c*asar_bundle:776:32) + at Function.executeUserEntryPoint [as runMain] (node:internal*modules*run_main:96:12)  at node:internal*main*run_main_module:23:47 diff --git a/patches/node/fix_expose_the_built-in_electron_module_via_the_esm_loader.patch b/patches/node/fix_expose_the_built-in_electron_module_via_the_esm_loader.patch index 406b6bbabba..336a4208f22 100644 --- a/patches/node/fix_expose_the_built-in_electron_module_via_the_esm_loader.patch +++ b/patches/node/fix_expose_the_built-in_electron_module_via_the_esm_loader.patch @@ -17,6 +17,27 @@ index 219ef03a21214deb8961044cfc18ef9c1e711b60..7749b37001f869fe565d8c450ff7ca2b }; /** +diff --git a/lib/internal/modules/esm/load.js b/lib/internal/modules/esm/load.js +index 1178f06074ec8ec02a2e7b5cf8cbe7e68b72f809..d64d4452a50bc4469b2d2fcc3251b3df31cda6ec 100644 +--- a/lib/internal/modules/esm/load.js ++++ b/lib/internal/modules/esm/load.js +@@ -116,6 +116,7 @@ function throwIfUnsupportedURLScheme(parsed, experimentalNetworkImports) { + protocol !== 'file:' && + protocol !== 'data:' && + protocol !== 'node:' && ++ protocol !== 'electron:' && + ( + !experimentalNetworkImports || + ( +@@ -124,7 +125,7 @@ function throwIfUnsupportedURLScheme(parsed, experimentalNetworkImports) { + ) + ) + ) { +- const schemes = ['file', 'data', 'node']; ++ const schemes = ['file', 'data', 'node', 'electron']; + if (experimentalNetworkImports) { + ArrayPrototypePush(schemes, 'https', 'http'); + } diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index 2cac6f2d450fab014544d15439e51575f86ccb38..ce2d851da2577e6e99980eb75337f629b38fddbf 100644 --- a/lib/internal/modules/esm/resolve.js diff --git a/patches/node/fix_lazyload_fs_in_esm_loaders_to_apply_asar_patches.patch b/patches/node/fix_lazyload_fs_in_esm_loaders_to_apply_asar_patches.patch new file mode 100644 index 00000000000..eb10cacd14e --- /dev/null +++ b/patches/node/fix_lazyload_fs_in_esm_loaders_to_apply_asar_patches.patch @@ -0,0 +1,103 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Samuel Attard +Date: Tue, 7 Mar 2023 16:17:41 -0800 +Subject: fix: lazyload fs in esm loaders to apply asar patches + +Changes { foo } from fs to just "fs.foo" so that our patching of fs is applied to esm loaders + +diff --git a/lib/internal/modules/esm/load.js b/lib/internal/modules/esm/load.js +index d64d4452a50bc4469b2d2fcc3251b3df31cda6ec..8d8f1b24fb68cce2b37e43f92aac200b4ee73cea 100644 +--- a/lib/internal/modules/esm/load.js ++++ b/lib/internal/modules/esm/load.js +@@ -20,7 +20,7 @@ const experimentalNetworkImports = + + const { Buffer: { from: BufferFrom } } = require('buffer'); + +-const { readFile: readFileAsync } = require('internal/fs/promises').exports; ++const fs = require('fs'); + const { URL } = require('internal/url'); + const { + ERR_INVALID_URL, +@@ -34,7 +34,7 @@ async function getSource(url, context) { + let responseURL = url; + let source; + if (parsed.protocol === 'file:') { +- source = await readFileAsync(parsed); ++ source = await fs.promises.readFile(parsed); + } else if (parsed.protocol === 'data:') { + const match = RegExpPrototypeExec(DATA_URL_PATTERN, parsed.pathname); + if (!match) { +diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js +index ce2d851da2577e6e99980eb75337f629b38fddbf..fed26d25b59d66ab6e5160e2e13c8eea0cb44f8d 100644 +--- a/lib/internal/modules/esm/resolve.js ++++ b/lib/internal/modules/esm/resolve.js +@@ -26,11 +26,7 @@ const { + } = primordials; + const internalFS = require('internal/fs/utils'); + const { BuiltinModule } = require('internal/bootstrap/loaders'); +-const { +- realpathSync, +- statSync, +- Stats, +-} = require('fs'); ++const fs = require('fs'); + const { getOptionValue } = require('internal/options'); + const pendingDeprecation = getOptionValue('--pending-deprecation'); + // Do not eagerly grab .manifest, it may be in TDZ +@@ -171,14 +167,14 @@ const realpathCache = new SafeMap(); + * @returns {import('fs').Stats} + */ + const tryStatSync = +- (path) => statSync(path, { throwIfNoEntry: false }) ?? new Stats(); ++ (path) => fs.statSync(path, { throwIfNoEntry: false }) ?? new fs.Stats(); + + /** + * @param {string | URL} url + * @returns {boolean} + */ + function fileExists(url) { +- return statSync(url, { throwIfNoEntry: false })?.isFile() ?? false; ++ return fs.statSync(url, { throwIfNoEntry: false })?.isFile() ?? false; + } + + /** +@@ -326,7 +322,7 @@ function finalizeResolution(resolved, base, preserveSymlinks) { + } + + if (!preserveSymlinks) { +- const real = realpathSync(path, { ++ const real = fs.realpathSync(path, { + [internalFS.realpathCacheKey]: realpathCache, + }); + const { search, hash } = resolved; +diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js +index 1ceb89da21610c703f4a18be5888373c7feaa370..347558c805c8ecd3f7ff4f6324ef7df68badc52f 100644 +--- a/lib/internal/modules/esm/translators.js ++++ b/lib/internal/modules/esm/translators.js +@@ -24,7 +24,7 @@ function lazyTypes() { + return _TYPES = require('internal/util/types'); + } + +-const { readFileSync } = require('fs'); ++const fs = require('fs'); + const { extname, isAbsolute } = require('path'); + const { + hasEsmSyntax, +@@ -131,7 +131,7 @@ translators.set('module', async function moduleStrategy(url, source, isMain) { + */ + function enrichCJSError(err, content, filename) { + if (err != null && ObjectGetPrototypeOf(err) === SyntaxErrorPrototype && +- hasEsmSyntax(content || readFileSync(filename, 'utf-8'))) { ++ hasEsmSyntax(content || fs.readFileSync(filename, 'utf-8'))) { + // Emit the warning synchronously because we are in the middle of handling + // a SyntaxError that will throw and likely terminate the process before an + // asynchronous warning would be emitted. +@@ -207,7 +207,7 @@ function cjsPreparseModuleExports(filename) { + + let source; + try { +- source = readFileSync(filename, 'utf8'); ++ source = fs.readFileSync(filename, 'utf8'); + } catch { + // Continue regardless of error. + } diff --git a/script/generate-deps-hash.js b/script/generate-deps-hash.js index 9161da0d6e9..c927fce645e 100644 --- a/script/generate-deps-hash.js +++ b/script/generate-deps-hash.js @@ -35,14 +35,19 @@ addAllFiles(path.resolve(__dirname, '../patches')); // Create Hash const hasher = crypto.createHash('SHA256'); -hasher.update(`HASH_VERSION:${HASH_VERSIONS[process.platform] || FALLBACK_HASH_VERSION}`); +const addToHashAndLog = (s) => { + console.log('Hashing:', s); + return hasher.update(s); +}; +addToHashAndLog(`HASH_VERSION:${HASH_VERSIONS[process.platform] || FALLBACK_HASH_VERSION}`); for (const file of filesToHash) { + console.log('Hashing Content:', file, crypto.createHash('SHA256').update(fs.readFileSync(file)).digest('hex')); hasher.update(fs.readFileSync(file)); } // Add the GCLIENT_EXTRA_ARGS variable to the hash const extraArgs = process.env.GCLIENT_EXTRA_ARGS || 'no_extra_args'; -hasher.update(extraArgs); +addToHashAndLog(extraArgs); const effectivePlatform = extraArgs.includes('host_os=mac') ? 'darwin' : process.platform; diff --git a/shell/browser/electron_browser_main_parts.cc b/shell/browser/electron_browser_main_parts.cc index 1bfdb82e440..6d1a8da23a6 100644 --- a/shell/browser/electron_browser_main_parts.cc +++ b/shell/browser/electron_browser_main_parts.cc @@ -286,6 +286,9 @@ void ElectronBrowserMainParts::PostEarlyInitialization() { // Load everything. node_bindings_->LoadEnvironment(node_env_.get()); + // Wait for app + node_bindings_->JoinAppCode(); + // We already initialized the feature list in PreEarlyInitialization(), but // the user JS script would not have had a chance to alter the command-line // switches at that point. Lets reinitialize it here to pick up the diff --git a/shell/browser/web_contents_preferences.cc b/shell/browser/web_contents_preferences.cc index a1d757750eb..7cc3327ff15 100644 --- a/shell/browser/web_contents_preferences.cc +++ b/shell/browser/web_contents_preferences.cc @@ -234,7 +234,6 @@ void WebContentsPreferences::SetFromDictionary( disable_blink_features_ = disable_blink_features; base::FilePath::StringType preload_path; - std::string preload_url_str; if (web_preferences.Get(options::kPreloadScript, &preload_path)) { base::FilePath preload(preload_path); if (preload.IsAbsolute()) { diff --git a/shell/common/gin_helper/locker.h b/shell/common/gin_helper/locker.h index b734d2f31cb..9ce78d701e4 100644 --- a/shell/common/gin_helper/locker.h +++ b/shell/common/gin_helper/locker.h @@ -28,6 +28,7 @@ class Locker { std::unique_ptr locker_; static bool g_is_browser_process; + static bool g_is_renderer_process; }; } // namespace gin_helper diff --git a/shell/common/node_bindings.cc b/shell/common/node_bindings.cc index afa86048f33..910237fdd65 100644 --- a/shell/common/node_bindings.cc +++ b/shell/common/node_bindings.cc @@ -26,6 +26,7 @@ #include "shell/browser/api/electron_api_app.h" #include "shell/common/api/electron_bindings.h" #include "shell/common/electron_command_line.h" +#include "shell/common/gin_converters/callback_converter.h" #include "shell/common/gin_converters/file_path_converter.h" #include "shell/common/gin_helper/dictionary.h" #include "shell/common/gin_helper/event.h" @@ -33,8 +34,11 @@ #include "shell/common/gin_helper/locker.h" #include "shell/common/gin_helper/microtasks_scope.h" #include "shell/common/mac/main_application_bundle.h" +#include "shell/common/world_ids.h" +#include "third_party/blink/public/web/web_local_frame.h" #include "third_party/blink/renderer/bindings/core/v8/v8_initializer.h" // nogncheck #include "third_party/electron_node/src/debug_utils.h" +#include "third_party/electron_node/src/module_wrap.h" #if !IS_MAS_BUILD() #include "shell/common/crash_keys.h" @@ -171,6 +175,64 @@ bool AllowWasmCodeGenerationCallback(v8::Local context, return node::AllowWasmCodeGenerationCallback(context, source); } +v8::MaybeLocal HostImportModuleDynamically( + v8::Local context, + v8::Local v8_host_defined_options, + v8::Local v8_referrer_resource_url, + v8::Local v8_specifier, + v8::Local v8_import_assertions) { + if (node::Environment::GetCurrent(context) == nullptr) { + if (electron::IsBrowserProcess() || electron::IsUtilityProcess()) + return v8::MaybeLocal(); + return blink::V8Initializer::HostImportModuleDynamically( + context, v8_host_defined_options, v8_referrer_resource_url, + v8_specifier, v8_import_assertions); + } + + // If we're running with contextIsolation enabled in the renderer process, + // fall back to Blink's logic. + if (electron::IsRendererProcess()) { + blink::WebLocalFrame* frame = + blink::WebLocalFrame::FrameForContext(context); + if (!frame || frame->GetScriptContextWorldId(context) != + electron::WorldIDs::ISOLATED_WORLD_ID) { + return blink::V8Initializer::HostImportModuleDynamically( + context, v8_host_defined_options, v8_referrer_resource_url, + v8_specifier, v8_import_assertions); + } + } + + return node::loader::ImportModuleDynamically( + context, v8_host_defined_options, v8_referrer_resource_url, v8_specifier, + v8_import_assertions); +} + +void HostInitializeImportMetaObject(v8::Local context, + v8::Local module, + v8::Local meta) { + if (node::Environment::GetCurrent(context) == nullptr) { + if (electron::IsBrowserProcess() || electron::IsUtilityProcess()) + return; + return blink::V8Initializer::HostGetImportMetaProperties(context, module, + meta); + } + + // If we're running with contextIsolation enabled in the renderer process, + // fall back to Blink's logic. + if (electron::IsRendererProcess()) { + blink::WebLocalFrame* frame = + blink::WebLocalFrame::FrameForContext(context); + if (!frame || frame->GetScriptContextWorldId(context) != + electron::WorldIDs::ISOLATED_WORLD_ID) { + return blink::V8Initializer::HostGetImportMetaProperties(context, module, + meta); + } + } + + return node::loader::ModuleWrap::HostInitializeImportMetaObjectCallback( + context, module, meta); +} + v8::ModifyCodeGenerationFromStringsResult ModifyCodeGenerationFromStrings( v8::Local context, v8::Local source, @@ -481,7 +543,8 @@ std::shared_ptr NodeBindings::CreateEnvironment( v8::Handle context, node::MultiIsolatePlatform* platform, std::vector args, - std::vector exec_args) { + std::vector exec_args, + absl::optional> on_app_code_ready) { // Feed node the path to initialization script. std::string process_type; switch (browser_env_) { @@ -529,7 +592,8 @@ std::shared_ptr NodeBindings::CreateEnvironment( node::Environment* env; uint64_t flags = node::EnvironmentFlags::kDefaultFlags | node::EnvironmentFlags::kHideConsoleWindows | - node::EnvironmentFlags::kNoGlobalSearchPaths; + node::EnvironmentFlags::kNoGlobalSearchPaths | + node::EnvironmentFlags::kNoRegisterESMLoader; if (browser_env_ == BrowserEnvironment::kRenderer || browser_env_ == BrowserEnvironment::kWorker) { @@ -541,8 +605,7 @@ std::shared_ptr NodeBindings::CreateEnvironment( // for processes that already have these defined by DOM. // Check //third_party/electron_node/lib/internal/bootstrap/node.js // for the list of overrides on globalThis. - flags |= node::EnvironmentFlags::kNoRegisterESMLoader | - node::EnvironmentFlags::kNoBrowserGlobals | + flags |= node::EnvironmentFlags::kNoBrowserGlobals | node::EnvironmentFlags::kNoCreateInspector; } @@ -635,6 +698,10 @@ std::shared_ptr NodeBindings::CreateEnvironment( } node::SetIsolateUpForNode(context->GetIsolate(), is); + context->GetIsolate()->SetHostImportModuleDynamicallyCallback( + HostImportModuleDynamically); + context->GetIsolate()->SetHostInitializeImportMetaObjectCallback( + HostInitializeImportMetaObject); gin_helper::Dictionary process(context->GetIsolate(), env->process_object()); process.SetReadOnly("type", process_type); @@ -644,6 +711,17 @@ std::shared_ptr NodeBindings::CreateEnvironment( base::PathService::Get(content::CHILD_PROCESS_EXE, &helper_exec_path); process.Set("helperExecPath", helper_exec_path); + if (browser_env_ == BrowserEnvironment::kBrowser || + browser_env_ == BrowserEnvironment::kRenderer) { + if (on_app_code_ready) { + process.SetMethod("appCodeLoaded", std::move(*on_app_code_ready)); + } else { + process.SetMethod("appCodeLoaded", + base::BindRepeating(&NodeBindings::SetAppCodeLoaded, + base::Unretained(this))); + } + } + auto env_deleter = [isolate, isolate_data, context = v8::Global{isolate, context}]( node::Environment* nenv) mutable { @@ -664,7 +742,8 @@ std::shared_ptr NodeBindings::CreateEnvironment( std::shared_ptr NodeBindings::CreateEnvironment( v8::Handle context, - node::MultiIsolatePlatform* platform) { + node::MultiIsolatePlatform* platform, + absl::optional> on_app_code_ready) { #if BUILDFLAG(IS_WIN) auto& electron_args = ElectronCommandLine::argv(); std::vector args(electron_args.size()); @@ -673,7 +752,7 @@ std::shared_ptr NodeBindings::CreateEnvironment( #else auto args = ElectronCommandLine::argv(); #endif - return CreateEnvironment(context, platform, args, {}); + return CreateEnvironment(context, platform, args, {}, on_app_code_ready); } void NodeBindings::LoadEnvironment(node::Environment* env) { @@ -716,6 +795,37 @@ void NodeBindings::StartPolling() { UvRunOnce(); } +void NodeBindings::SetAppCodeLoaded() { + app_code_loaded_ = true; +} + +void NodeBindings::JoinAppCode() { + // We can only "join" app code to the main thread in the browser process + if (browser_env_ != BrowserEnvironment::kBrowser) { + return; + } + + auto* browser = Browser::Get(); + node::Environment* env = uv_env(); + + if (!env) + return; + + v8::HandleScope handle_scope(env->isolate()); + // Enter node context while dealing with uv events. + v8::Context::Scope context_scope(env->context()); + + // Pump the event loop until we get the signal that the app code has finished + // loading + while (!app_code_loaded_ && !browser->is_shutting_down()) { + int r = uv_run(uv_loop_, UV_RUN_ONCE); + if (r == 0) { + base::RunLoop().QuitWhenIdle(); // Quit from uv. + break; + } + } +} + void NodeBindings::UvRunOnce() { node::Environment* env = uv_env(); diff --git a/shell/common/node_bindings.h b/shell/common/node_bindings.h index c6e2a56f381..4533ecd5800 100644 --- a/shell/common/node_bindings.h +++ b/shell/common/node_bindings.h @@ -11,12 +11,14 @@ #include #include "base/files/file_path.h" +#include "base/functional/callback.h" #include "base/memory/raw_ptr.h" #include "base/memory/raw_ptr_exclusion.h" #include "base/memory/weak_ptr.h" #include "gin/public/context_holder.h" #include "gin/public/gin_embedders.h" #include "shell/common/node_includes.h" +#include "third_party/abseil-cpp/absl/types/optional.h" #include "uv.h" // NOLINT(build/include_directory) #include "v8/include/v8.h" @@ -94,11 +96,15 @@ class NodeBindings { v8::Handle context, node::MultiIsolatePlatform* platform, std::vector args, - std::vector exec_args); + std::vector exec_args, + absl::optional> on_app_code_ready = + absl::nullopt); std::shared_ptr CreateEnvironment( v8::Handle context, - node::MultiIsolatePlatform* platform); + node::MultiIsolatePlatform* platform, + absl::optional> on_app_code_ready = + absl::nullopt); // Load node.js in the environment. void LoadEnvironment(node::Environment* env); @@ -134,6 +140,10 @@ class NodeBindings { NodeBindings(const NodeBindings&) = delete; NodeBindings& operator=(const NodeBindings&) = delete; + // Blocks until app code is signaled to be loaded via |SetAppCodeLoaded|. + // Only has an effect if called in the browser process + void JoinAppCode(); + protected: explicit NodeBindings(BrowserEnvironment browser_env); @@ -168,9 +178,17 @@ class NodeBindings { // Thread to poll uv events. static void EmbedThreadRunner(void* arg); + // Default callback to indicate when the node environment has finished + // initializing and the primary import chain is fully resolved and executed + void SetAppCodeLoaded(); + // Indicates whether polling thread has been created. bool initialized_ = false; + // Indicates whether the app code has finished loading + // for ESM this is async after the module is loaded + bool app_code_loaded_ = false; + // Whether the libuv loop has ended. bool embed_closed_ = false; diff --git a/shell/renderer/electron_renderer_client.cc b/shell/renderer/electron_renderer_client.cc index d33527ff847..ec57a6d9669 100644 --- a/shell/renderer/electron_renderer_client.cc +++ b/shell/renderer/electron_renderer_client.cc @@ -22,6 +22,7 @@ #include "third_party/blink/public/web/web_document.h" #include "third_party/blink/public/web/web_local_frame.h" #include "third_party/blink/renderer/core/execution_context/execution_context.h" // nogncheck +#include "third_party/blink/renderer/core/frame/web_local_frame_impl.h" // nogncheck namespace electron { @@ -61,6 +62,11 @@ void ElectronRendererClient::RunScriptsAtDocumentEnd( "document-end"); } +void ElectronRendererClient::UndeferLoad(content::RenderFrame* render_frame) { + render_frame->GetWebFrame()->GetDocumentLoader()->SetDefersLoading( + blink::LoaderFreezeMode::kNone); +} + void ElectronRendererClient::DidCreateScriptContext( v8::Handle renderer_context, content::RenderFrame* render_frame) { @@ -88,8 +94,17 @@ void ElectronRendererClient::DidCreateScriptContext( v8::Maybe initialized = node::InitializeContext(renderer_context); CHECK(!initialized.IsNothing() && initialized.FromJust()); - std::shared_ptr env = - node_bindings_->CreateEnvironment(renderer_context, nullptr); + // Before we load the node environment, let's tell blink to hold off on + // loading the body of this frame. We will undefer the load once the preload + // script has finished. This allows our preload script to run async (E.g. + // with ESM) without the preload being in a race + render_frame->GetWebFrame()->GetDocumentLoader()->SetDefersLoading( + blink::LoaderFreezeMode::kStrict); + + std::shared_ptr env = node_bindings_->CreateEnvironment( + renderer_context, nullptr, + base::BindRepeating(&ElectronRendererClient::UndeferLoad, + base::Unretained(this), render_frame)); // If we have disabled the site instance overrides we should prevent loading // any non-context aware native module. diff --git a/shell/renderer/electron_renderer_client.h b/shell/renderer/electron_renderer_client.h index 1e8318c4cae..9942fe798ab 100644 --- a/shell/renderer/electron_renderer_client.h +++ b/shell/renderer/electron_renderer_client.h @@ -35,6 +35,8 @@ class ElectronRendererClient : public RendererClientBase { content::RenderFrame* render_frame) override; private: + void UndeferLoad(content::RenderFrame* render_frame); + // content::ContentRendererClient: void RenderFrameCreated(content::RenderFrame*) override; void RunScriptsAtDocumentStart(content::RenderFrame* render_frame) override; diff --git a/spec/esm-spec.ts b/spec/esm-spec.ts new file mode 100644 index 00000000000..27a0a77f645 --- /dev/null +++ b/spec/esm-spec.ts @@ -0,0 +1,198 @@ +import { expect } from 'chai'; +import * as cp from 'node:child_process'; +import { BrowserWindow } from 'electron'; +import * as fs from 'fs-extra'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { pathToFileURL } from 'node:url'; + +const runFixture = async (appPath: string, args: string[] = []) => { + const result = cp.spawn(process.execPath, [appPath, ...args], { + stdio: 'pipe' + }); + + const stdout: Buffer[] = []; + const stderr: Buffer[] = []; + result.stdout.on('data', (chunk) => stdout.push(chunk)); + result.stderr.on('data', (chunk) => stderr.push(chunk)); + + const [code, signal] = await new Promise<[number | null, NodeJS.Signals | null]>((resolve) => { + result.on('close', (code, signal) => { + resolve([code, signal]); + }); + }); + + return { + code, + signal, + stdout: Buffer.concat(stdout).toString().trim(), + stderr: Buffer.concat(stderr).toString().trim() + }; +}; + +const fixturePath = path.resolve(__dirname, 'fixtures', 'esm'); + +describe('esm', () => { + describe('main process', () => { + it('should load an esm entrypoint', async () => { + const result = await runFixture(path.resolve(fixturePath, 'entrypoint.mjs')); + expect(result.code).to.equal(0); + expect(result.stdout).to.equal('ESM Launch, ready: false'); + }); + + it('should load an esm entrypoint based on type=module', async () => { + const result = await runFixture(path.resolve(fixturePath, 'package')); + expect(result.code).to.equal(0); + expect(result.stdout).to.equal('ESM Package Launch, ready: false'); + }); + + it('should wait for a top-level await before declaring the app ready', async () => { + const result = await runFixture(path.resolve(fixturePath, 'top-level-await.mjs')); + expect(result.code).to.equal(0); + expect(result.stdout).to.equal('Top level await, ready: false'); + }); + + it('should allow usage of pre-app-ready apis in top-level await', async () => { + const result = await runFixture(path.resolve(fixturePath, 'pre-app-ready-apis.mjs')); + expect(result.code).to.equal(0); + }); + + it('should allow use of dynamic import', async () => { + const result = await runFixture(path.resolve(fixturePath, 'dynamic.mjs')); + expect(result.code).to.equal(0); + expect(result.stdout).to.equal('Exit with app, ready: false'); + }); + }); + + describe('renderer process', () => { + let w: BrowserWindow | null = null; + const tempDirs: string[] = []; + + afterEach(async () => { + if (w) w.close(); + w = null; + while (tempDirs.length) { + await fs.remove(tempDirs.pop()!); + } + }); + + async function loadWindowWithPreload (preload: string, webPreferences: Electron.WebPreferences) { + const tmpDir = await fs.mkdtemp(path.resolve(os.tmpdir(), 'e-spec-preload-')); + tempDirs.push(tmpDir); + const preloadPath = path.resolve(tmpDir, 'preload.mjs'); + await fs.writeFile(preloadPath, preload); + + w = new BrowserWindow({ + show: false, + webPreferences: { + ...webPreferences, + preload: preloadPath + } + }); + + let error: Error | null = null; + w.webContents.on('preload-error', (_, __, err) => { + error = err; + }); + + await w.loadFile(path.resolve(fixturePath, 'empty.html')); + + return [w.webContents, error] as [Electron.WebContents, Error | null]; + } + + describe('nodeIntegration', () => { + it('should support an esm entrypoint', async () => { + const [webContents] = await loadWindowWithPreload('import { resolve } from "path"; window.resolvePath = resolve;', { + nodeIntegration: true, + sandbox: false, + contextIsolation: false + }); + + const exposedType = await webContents.executeJavaScript('typeof window.resolvePath'); + expect(exposedType).to.equal('function'); + }); + + it('should delay load until the ESM import chain is complete', async () => { + const [webContents] = await loadWindowWithPreload(`import { resolve } from "path"; + await new Promise(r => setTimeout(r, 500)); + window.resolvePath = resolve;`, { + nodeIntegration: true, + sandbox: false, + contextIsolation: false + }); + + const exposedType = await webContents.executeJavaScript('typeof window.resolvePath'); + expect(exposedType).to.equal('function'); + }); + + it('should support a top-level await fetch blocking the page load', async () => { + const [webContents] = await loadWindowWithPreload(` + const r = await fetch("package/package.json"); + window.packageJson = await r.json();`, { + nodeIntegration: true, + sandbox: false, + contextIsolation: false + }); + + const packageJson = await webContents.executeJavaScript('window.packageJson'); + expect(packageJson).to.deep.equal(require('./fixtures/esm/package/package.json')); + }); + + const hostsUrl = pathToFileURL(process.platform === 'win32' ? 'C:\\Windows\\System32\\drivers\\etc\\hosts' : '/etc/hosts'); + + describe('without context isolation', () => { + it('should use blinks dynamic loader in the main world', async () => { + const [webContents] = await loadWindowWithPreload('', { + nodeIntegration: true, + sandbox: false, + contextIsolation: false + }); + + let error: Error | null = null; + try { + await webContents.executeJavaScript(`import(${JSON.stringify(hostsUrl)})`); + } catch (err) { + error = err as Error; + } + + expect(error).to.not.equal(null); + // This is a blink specific error message + expect(error?.message).to.include('Failed to fetch dynamically imported module'); + }); + }); + + describe('with context isolation', () => { + it('should use nodes esm dynamic loader in the isolated context', async () => { + const [, preloadError] = await loadWindowWithPreload(`await import(${JSON.stringify(hostsUrl)})`, { + nodeIntegration: true, + sandbox: false, + contextIsolation: true + }); + + expect(preloadError).to.not.equal(null); + // This is a node.js specific error message + expect(preloadError!.toString()).to.include('Unknown file extension'); + }); + + it('should use blinks dynamic loader in the main world', async () => { + const [webContents] = await loadWindowWithPreload('', { + nodeIntegration: true, + sandbox: false, + contextIsolation: true + }); + + let error: Error | null = null; + try { + await webContents.executeJavaScript(`import(${JSON.stringify(hostsUrl)})`); + } catch (err) { + error = err as Error; + } + + expect(error).to.not.equal(null); + // This is a blink specific error message + expect(error?.message).to.include('Failed to fetch dynamically imported module'); + }); + }); + }); + }); +}); diff --git a/spec/fixtures/esm/dynamic.mjs b/spec/fixtures/esm/dynamic.mjs new file mode 100644 index 00000000000..32b6508d70f --- /dev/null +++ b/spec/fixtures/esm/dynamic.mjs @@ -0,0 +1,4 @@ +const { app } = await import('electron'); +const { exitWithApp } = await import('./exit.mjs'); + +exitWithApp(app); diff --git a/spec/fixtures/esm/empty.html b/spec/fixtures/esm/empty.html new file mode 100644 index 00000000000..40816a2b5a9 --- /dev/null +++ b/spec/fixtures/esm/empty.html @@ -0,0 +1 @@ +Hi \ No newline at end of file diff --git a/spec/fixtures/esm/entrypoint.mjs b/spec/fixtures/esm/entrypoint.mjs new file mode 100644 index 00000000000..38617204fb2 --- /dev/null +++ b/spec/fixtures/esm/entrypoint.mjs @@ -0,0 +1,4 @@ +import * as electron from 'electron'; + +console.log('ESM Launch, ready:', electron.app.isReady()); +process.exit(0); diff --git a/spec/fixtures/esm/exit.mjs b/spec/fixtures/esm/exit.mjs new file mode 100644 index 00000000000..6812d6df493 --- /dev/null +++ b/spec/fixtures/esm/exit.mjs @@ -0,0 +1,4 @@ +export function exitWithApp (app) { + console.log('Exit with app, ready:', app.isReady()); + process.exit(0); +} diff --git a/spec/fixtures/esm/local.mjs b/spec/fixtures/esm/local.mjs new file mode 100644 index 00000000000..8b6d60c5edc --- /dev/null +++ b/spec/fixtures/esm/local.mjs @@ -0,0 +1,3 @@ +export function add (a, b) { + return a + b; +}; diff --git a/spec/fixtures/esm/package/index.mjs b/spec/fixtures/esm/package/index.mjs new file mode 100644 index 00000000000..08c3109ef01 --- /dev/null +++ b/spec/fixtures/esm/package/index.mjs @@ -0,0 +1,4 @@ +import * as electron from 'electron'; + +console.log('ESM Package Launch, ready:', electron.app.isReady()); +process.exit(0); diff --git a/spec/fixtures/esm/package/package.json b/spec/fixtures/esm/package/package.json new file mode 100644 index 00000000000..84f117c2753 --- /dev/null +++ b/spec/fixtures/esm/package/package.json @@ -0,0 +1,4 @@ +{ + "main": "index.mjs", + "type": "module" +} diff --git a/spec/fixtures/esm/pre-app-ready-apis.mjs b/spec/fixtures/esm/pre-app-ready-apis.mjs new file mode 100644 index 00000000000..5c815b5c74e --- /dev/null +++ b/spec/fixtures/esm/pre-app-ready-apis.mjs @@ -0,0 +1,9 @@ +import * as electron from 'electron'; + +try { + electron.app.disableHardwareAcceleration(); +} catch { + process.exit(1); +} + +process.exit(0); diff --git a/spec/fixtures/esm/top-level-await.mjs b/spec/fixtures/esm/top-level-await.mjs new file mode 100644 index 00000000000..d18c33fa456 --- /dev/null +++ b/spec/fixtures/esm/top-level-await.mjs @@ -0,0 +1,7 @@ +import * as electron from 'electron'; + +// Cheeky delay +await new Promise((resolve) => setTimeout(resolve, 500)); + +console.log('Top level await, ready:', electron.app.isReady()); +process.exit(0); diff --git a/tsconfig.default_app.json b/tsconfig.default_app.json index 0c5ea603876..c4d604c9e04 100644 --- a/tsconfig.default_app.json +++ b/tsconfig.default_app.json @@ -1,7 +1,9 @@ { "extends": "./tsconfig.json", "compilerOptions": { - "rootDir": "default_app" + "rootDir": "default_app", + "module": "ESNext", + "moduleResolution": "node" }, "include": [ "default_app", diff --git a/typings/internal-ambient.d.ts b/typings/internal-ambient.d.ts index 9a94341dd14..26cff6bc760 100644 --- a/typings/internal-ambient.d.ts +++ b/typings/internal-ambient.d.ts @@ -246,6 +246,8 @@ declare namespace NodeJS { helperExecPath: string; mainModule?: NodeJS.Module | undefined; + + appCodeLoaded?: () => void; } }