From d59685a3bfcd38ec1276c28c22d1a5e1aa346d57 Mon Sep 17 00:00:00 2001 From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com> Date: Tue, 21 Oct 2025 07:28:06 +0200 Subject: [PATCH] feat: dynamic ESM import in preload without context isolation (#48488) Dynamic ESM import in non-context-isolated preload Extend `HostImportModuleWithPhaseDynamically`'s routing to support Node.js import resolution in non-context-isolated preloads through `v8_host_defined_options` length check. The length of host defined options is distinct between Blink and Node.js and we can use it to determine which resolver to use. Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: Fedor Indutny Co-authored-by: Shelley Vohr --- docs/tutorial/esm.md | 2 +- patches/chromium/.patches | 1 + ...erscriptinfo_hostdefinedoptionsindex.patch | 53 +++++++++++ shell/common/node_bindings.cc | 93 ++++++++++++++----- spec/esm-spec.ts | 34 ++++--- 5 files changed, 148 insertions(+), 35 deletions(-) create mode 100644 patches/chromium/expose_referrerscriptinfo_hostdefinedoptionsindex.patch diff --git a/docs/tutorial/esm.md b/docs/tutorial/esm.md index ae532fbba5b1..f1500eb5ff2b 100644 --- a/docs/tutorial/esm.md +++ b/docs/tutorial/esm.md @@ -32,7 +32,7 @@ This table gives a general overview of where ESM is supported and which ESM load | Main | Node.js | N/A | | | Renderer (Sandboxed) | Chromium | Unsupported | | | Renderer (Unsandboxed & Context Isolated) | Chromium | Node.js | | -| Renderer (Unsandboxed & Non Context Isolated) | Chromium | Node.js | | +| Renderer (Unsandboxed & Non Context Isolated) | Chromium | Node.js | | ## Main process diff --git a/patches/chromium/.patches b/patches/chromium/.patches index fab137a0c430..774e8391b4f7 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -140,4 +140,5 @@ chore_add_electron_objects_to_wrappablepointertag.patch chore_expose_isolate_parameter_in_script_lifecycle_observers.patch revert_partial_remove_unused_prehandlemouseevent.patch allow_electron_to_depend_on_components_os_crypt_sync.patch +expose_referrerscriptinfo_hostdefinedoptionsindex.patch chore_disable_protocol_handler_dcheck.patch diff --git a/patches/chromium/expose_referrerscriptinfo_hostdefinedoptionsindex.patch b/patches/chromium/expose_referrerscriptinfo_hostdefinedoptionsindex.patch new file mode 100644 index 000000000000..a95d1b500c0f --- /dev/null +++ b/patches/chromium/expose_referrerscriptinfo_hostdefinedoptionsindex.patch @@ -0,0 +1,53 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Fedor Indutny +Date: Wed, 24 Sep 2025 10:08:48 -0700 +Subject: Expose ReferrerScriptInfo::HostDefinedOptionsIndex + +In `shell/common/node_bindings.cc`'s +`HostImportModuleWithPhaseDynamically` we route dynamic imports to +either Node.js's or Blink's resolver based on presence of Node.js +environment, process type, etc. Exporting `HostDefinedOptionsIndex` +allows us to route based on the size of `v8_host_defined_options` data +which enables us to support dynamic imports in non-context-isolated +preload scripts. + +diff --git a/third_party/blink/renderer/bindings/core/v8/referrer_script_info.cc b/third_party/blink/renderer/bindings/core/v8/referrer_script_info.cc +index 1b797783987255622735047bd78ca0e8bb635d5e..b209c736bb80c186ed51999af1dac0a1d50fc232 100644 +--- a/third_party/blink/renderer/bindings/core/v8/referrer_script_info.cc ++++ b/third_party/blink/renderer/bindings/core/v8/referrer_script_info.cc +@@ -12,15 +12,6 @@ namespace blink { + + namespace { + +-enum HostDefinedOptionsIndex : size_t { +- kBaseURL, +- kCredentialsMode, +- kNonce, +- kParserState, +- kReferrerPolicy, +- kLength +-}; +- + // Omit storing base URL if it is same as ScriptOrigin::ResourceName(). + // Note: This improves chance of getting into a fast path in + // ReferrerScriptInfo::ToV8HostDefinedOptions. +diff --git a/third_party/blink/renderer/bindings/core/v8/referrer_script_info.h b/third_party/blink/renderer/bindings/core/v8/referrer_script_info.h +index 0119624a028bec3e53e4e402938a98fe6def1483..743865839448748fe00e3e7d5027587cb65393c9 100644 +--- a/third_party/blink/renderer/bindings/core/v8/referrer_script_info.h ++++ b/third_party/blink/renderer/bindings/core/v8/referrer_script_info.h +@@ -23,6 +23,15 @@ class CORE_EXPORT ReferrerScriptInfo { + STACK_ALLOCATED(); + + public: ++ enum HostDefinedOptionsIndex : size_t { ++ kBaseURL, ++ kCredentialsMode, ++ kNonce, ++ kParserState, ++ kReferrerPolicy, ++ kLength ++ }; ++ + ReferrerScriptInfo() {} + ReferrerScriptInfo(const KURL& base_url, + network::mojom::CredentialsMode credentials_mode, diff --git a/shell/common/node_bindings.cc b/shell/common/node_bindings.cc index 0f4ee0539285..bde0c03105cb 100644 --- a/shell/common/node_bindings.cc +++ b/shell/common/node_bindings.cc @@ -24,6 +24,7 @@ #include "base/trace_event/trace_event.h" #include "chrome/common/chrome_version.h" #include "content/public/common/content_paths.h" +#include "content/public/renderer/render_frame.h" #include "electron/buildflags/buildflags.h" #include "electron/electron_version.h" #include "electron/fuses.h" @@ -41,7 +42,9 @@ #include "shell/common/node_util.h" #include "shell/common/process_util.h" #include "shell/common/world_ids.h" +#include "third_party/blink/public/common/web_preferences/web_preferences.h" #include "third_party/blink/public/web/web_local_frame.h" +#include "third_party/blink/renderer/bindings/core/v8/referrer_script_info.h" // nogncheck #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" @@ -211,6 +214,61 @@ bool AllowWasmCodeGenerationCallback(v8::Local context, return node::AllowWasmCodeGenerationCallback(context, source); } +enum ESMHandlerPlatform { + kNone, + kNodeJS, + kBlink, +}; + +static ESMHandlerPlatform SelectESMHandlerPlatform( + v8::Local context, + v8::Local raw_host_defined_options) { + if (node::Environment::GetCurrent(context) == nullptr) { + if (electron::IsBrowserProcess() || electron::IsUtilityProcess()) + return ESMHandlerPlatform::kNone; + + return ESMHandlerPlatform::kBlink; + } + + if (!electron::IsRendererProcess()) + return ESMHandlerPlatform::kNodeJS; + + blink::WebLocalFrame* frame = blink::WebLocalFrame::FrameForContext(context); + + if (frame == nullptr) + return ESMHandlerPlatform::kBlink; + + auto prefs = content::RenderFrame::FromWebFrame(frame)->GetBlinkPreferences(); + + // If we're running with contextIsolation enabled in the renderer process, + // fall back to Blink's logic when the frame is not in the isolated world. + if (prefs.context_isolation) { + return frame->GetScriptContextWorldId(context) == + electron::WorldIDs::ISOLATED_WORLD_ID + ? ESMHandlerPlatform::kNodeJS + : ESMHandlerPlatform::kBlink; + } + + if (raw_host_defined_options.IsEmpty() || + !raw_host_defined_options->IsFixedArray()) { + return ESMHandlerPlatform::kBlink; + } + + // Since the routing is based on the `host_defined_options` length - + // make sure that Node's host defined options are different from Blink's. + static_assert( + static_cast(node::loader::HostDefinedOptions::kLength) != + blink::ReferrerScriptInfo::HostDefinedOptionsIndex::kLength); + + // Use Node.js resolver only if host options were created by it. + auto options = v8::Local::Cast(raw_host_defined_options); + if (options->Length() == node::loader::HostDefinedOptions::kLength) { + return ESMHandlerPlatform::kNodeJS; + } + + return ESMHandlerPlatform::kBlink; +} + v8::MaybeLocal HostImportModuleWithPhaseDynamically( v8::Local context, v8::Local v8_host_defined_options, @@ -218,33 +276,22 @@ v8::MaybeLocal HostImportModuleWithPhaseDynamically( v8::Local v8_specifier, v8::ModuleImportPhase import_phase, v8::Local v8_import_attributes) { - if (node::Environment::GetCurrent(context) == nullptr) { - if (electron::IsBrowserProcess() || electron::IsUtilityProcess()) - return {}; - return blink::V8Initializer::HostImportModuleWithPhaseDynamically( - context, v8_host_defined_options, v8_referrer_resource_url, - v8_specifier, import_phase, v8_import_attributes); - } - - // 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) { + switch (SelectESMHandlerPlatform(context, v8_host_defined_options)) { + case ESMHandlerPlatform::kBlink: return blink::V8Initializer::HostImportModuleWithPhaseDynamically( context, v8_host_defined_options, v8_referrer_resource_url, v8_specifier, import_phase, v8_import_attributes); - } + case ESMHandlerPlatform::kNodeJS: + // TODO: Switch to node::loader::ImportModuleDynamicallyWithPhase + // once we land the Node.js version that has it in upstream. + CHECK(import_phase == v8::ModuleImportPhase::kEvaluation); + return node::loader::ImportModuleDynamically( + context, v8_host_defined_options, v8_referrer_resource_url, + v8_specifier, v8_import_attributes); + case ESMHandlerPlatform::kNone: + default: + return {}; } - - // TODO: Switch to node::loader::ImportModuleDynamicallyWithPhase - // once we land the Node.js version that has it in upstream. - CHECK(import_phase == v8::ModuleImportPhase::kEvaluation); - return node::loader::ImportModuleDynamically( - context, v8_host_defined_options, v8_referrer_resource_url, v8_specifier, - v8_import_attributes); } v8::MaybeLocal HostImportModuleDynamically( diff --git a/spec/esm-spec.ts b/spec/esm-spec.ts index a09198ba7486..b4221c0eda0a 100644 --- a/spec/esm-spec.ts +++ b/spec/esm-spec.ts @@ -130,6 +130,17 @@ describe('esm', () => { } describe('nodeIntegration', () => { + let badFilePath = ''; + + beforeEach(async () => { + badFilePath = path.resolve(path.resolve(os.tmpdir(), 'bad-file.badjs')); + await fs.promises.writeFile(badFilePath, 'const foo = "bar";'); + }); + + afterEach(async () => { + await fs.promises.unlink(badFilePath); + }); + it('should support an esm entrypoint', async () => { const [webContents] = await loadWindowWithPreload('import { resolve } from "path"; window.resolvePath = resolve;', { nodeIntegration: true, @@ -189,6 +200,18 @@ describe('esm', () => { expect(error?.message).to.include('Failed to fetch dynamically imported module'); }); + it('should use Node.js ESM dynamic loader in the preload', async () => { + const [, preloadError] = await loadWindowWithPreload(`await import(${JSON.stringify((pathToFileURL(badFilePath)))})`, { + nodeIntegration: true, + sandbox: false, + contextIsolation: false + }); + + 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 import.meta callback handling from Node.js for Node.js modules', async () => { const result = await runFixture(path.resolve(fixturePath, 'import-meta')); expect(result.code).to.equal(0); @@ -196,17 +219,6 @@ describe('esm', () => { }); describe('with context isolation', () => { - let badFilePath = ''; - - beforeEach(async () => { - badFilePath = path.resolve(path.resolve(os.tmpdir(), 'bad-file.badjs')); - await fs.promises.writeFile(badFilePath, 'const foo = "bar";'); - }); - - afterEach(async () => { - await fs.promises.unlink(badFilePath); - }); - it('should use Node.js ESM dynamic loader in the isolated context', async () => { const [, preloadError] = await loadWindowWithPreload(`await import(${JSON.stringify((pathToFileURL(badFilePath)))})`, { nodeIntegration: true,