From ac031bf8dedf8e3da6d918cbbc2e64a03830c639 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Wed, 30 Aug 2023 17:38:07 -0700 Subject: [PATCH] feat: I guess it's esm (#37535) * fix: allow ESM loads from within ASAR files * fix: ensure that ESM entry points finish loading before app ready * fix: allow loading ESM entrypoints via default_app * fix: allow ESM loading for renderer preloads * docs: document current known limitations of esm * chore: add patches to support blending esm handlers * refactor: use SetDefersLoading instead of JoinAppCode in renderers Blink has it's own event loop so pumping the uv loop in the renderer is not enough, luckily in blink we can suspend the loading of the frame while we do additional work. * chore: add patch to expose SetDefersLoading * fix: use fileURLToPath instead of pathname * chore: update per PR feedback * fix: fs.exists/existsSync should never throw * fix: convert path to file url before importing * fix: oops * fix: oops * Update docs/tutorial/esm-limitations.md Co-authored-by: Jeremy Rose * windows... * windows... * chore: update patches * spec: fix tests and document empty body edge case * Apply suggestions from code review Co-authored-by: Daniel Scalzi Co-authored-by: Jeremy Rose * spec: add tests for esm * spec: windows * chore: update per PR feedback * chore: update patches * Update shell/common/node_bindings.h Co-authored-by: Jeremy Rose * chore: update patches * rebase * use cjs loader by default for preload scripts * chore: fix lint * chore: update patches * chore: update patches * chore: fix patches * build: debug depshash * ? * Revert "build: debug depshash" This reverts commit 0de82523fb93f475226356b37418ce4b69acdcdf. * chore: allow electron as builtin protocol in esm loader * Revert "Revert "build: debug depshash"" This reverts commit ff86b1243ca6d05c9b3b38e0a6d717fb380343a4. * chore: fix esm doc * chore: update node patches --------- Co-authored-by: Jeremy Rose Co-authored-by: electron-patch-conflict-fixer[bot] <83340002+electron-patch-conflict-fixer[bot]@users.noreply.github.com> Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com> Co-authored-by: Daniel Scalzi --- default_app/default_app.ts | 7 +- default_app/main.ts | 49 +++-- default_app/package.json | 3 +- default_app/preload.ts | 2 +- docs/tutorial/esm-limitations.md | 38 ++++ lib/asar/fs-wrapper.ts | 25 ++- lib/browser/init.ts | 24 ++- lib/renderer/init.ts | 44 +++- patches/chromium/.patches | 2 + ...etdefersloading_on_webdocumentloader.patch | 42 ++++ ...pose_hostimportmoduledynamically_and.patch | 75 +++++++ patches/node/.patches | 2 + ...e_expose_importmoduledynamically_and.patch | 103 +++++++++ ...ixtures_errors_force_colors_snapshot.patch | 4 +- ...n_electron_module_via_the_esm_loader.patch | 21 ++ ...in_esm_loaders_to_apply_asar_patches.patch | 103 +++++++++ script/generate-deps-hash.js | 9 +- shell/browser/electron_browser_main_parts.cc | 3 + shell/browser/web_contents_preferences.cc | 1 - shell/common/gin_helper/locker.h | 1 + shell/common/node_bindings.cc | 122 ++++++++++- shell/common/node_bindings.h | 22 +- shell/renderer/electron_renderer_client.cc | 19 +- shell/renderer/electron_renderer_client.h | 2 + spec/esm-spec.ts | 198 ++++++++++++++++++ spec/fixtures/esm/dynamic.mjs | 4 + spec/fixtures/esm/empty.html | 1 + spec/fixtures/esm/entrypoint.mjs | 4 + spec/fixtures/esm/exit.mjs | 4 + spec/fixtures/esm/local.mjs | 3 + spec/fixtures/esm/package/index.mjs | 4 + spec/fixtures/esm/package/package.json | 4 + spec/fixtures/esm/pre-app-ready-apis.mjs | 9 + spec/fixtures/esm/top-level-await.mjs | 7 + tsconfig.default_app.json | 4 +- typings/internal-ambient.d.ts | 2 + 36 files changed, 910 insertions(+), 57 deletions(-) create mode 100644 docs/tutorial/esm-limitations.md create mode 100644 patches/chromium/feat_expose_documentloader_setdefersloading_on_webdocumentloader.patch create mode 100644 patches/chromium/refactor_expose_hostimportmoduledynamically_and.patch create mode 100644 patches/node/chore_expose_importmoduledynamically_and.patch create mode 100644 patches/node/fix_lazyload_fs_in_esm_loaders_to_apply_asar_patches.patch create mode 100644 spec/esm-spec.ts create mode 100644 spec/fixtures/esm/dynamic.mjs create mode 100644 spec/fixtures/esm/empty.html create mode 100644 spec/fixtures/esm/entrypoint.mjs create mode 100644 spec/fixtures/esm/exit.mjs create mode 100644 spec/fixtures/esm/local.mjs create mode 100644 spec/fixtures/esm/package/index.mjs create mode 100644 spec/fixtures/esm/package/package.json create mode 100644 spec/fixtures/esm/pre-app-ready-apis.mjs create mode 100644 spec/fixtures/esm/top-level-await.mjs 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 |
  • [You must `await` generously in the main process to avoid race conditions](#you-must-use-await-generously-in-the-main-process-to-avoid-race-conditions)
| +| Sandboxed Renderer | Yes | Chromium | No | |
  • [Sandboxed preload scripts can't use ESM imports](#sandboxed-preload-scripts-cant-use-esm-imports)
| +| Node.js Renderer + Context Isolation | Yes | Chromium | Yes | Node.js |
  • [Node.js ESM Preload Scripts will run after page load on pages with no content](#nodejs-esm-preload-scripts-will-run-after-page-load-on-pages-with-no-content)
  • [ESM Preload Scripts must have the `.mjs` extension](#esm-preload-scripts-must-have-the-mjs-extension)
| +| Node.js Renderer + No Context Isolation | Yes | Chromium | Yes | Node.js |
  • [Non-context-isolated renderers can't use dynamic Node.js ESM imports](#non-context-isolated-renderers-cant-use-dynamic-nodejs-esm-imports)
  • [ESM Preload Scripts must have the `.mjs` extension](#esm-preload-scripts-must-have-the-mjs-extension)
| + +## 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; } }