Repository navigation
feat: enable code cache for custom protocols - #40544
Conversation
|
Should we enforce a good default be only allowing code cache for standard custom schemes, since the cache key relies on origin lock of the process when available. Custom protocols with null origin will have their code cache grouped together and I am not sure cache will be useful in that case since there is more chance of collision. |
b556fc1 to
976689f
Compare
I updated the API to throw when trying to enabling code cache for non-standard scheme. |
132f235 to
2aad9dd
Compare
1453c59 to
035726d
Compare
|
Release Notes Persisted
|
|
I have automatically backported this PR to "27-x-y", please check out #40708 |
|
I have automatically backported this PR to "28-x-y", please check out #40709 |
## Summary Closes #893: a V8 code cache for the editor's scripts. Main's compile cache was measured and left out, with the reasons in ADR 0023. ### What changes - **`codeCache: true` on `webforge-app://`** (`main/app-scheme.ts`). V8 keeps the code it compiled for the editor's and Help's scripts in the profile's `Code Cache`, and later starts use it. - **No `module.enableCompileCache()` for main.** It was tried, and Node's log showed every cached module accepted, but it saved 23 ms, within noise: since #892, main loads 1.4 MB at start-up. Doing it properly also needed a bootstrap entry, since main.ts's imports have all loaded by its first line. Neither is in this PR. - **The cold-start check** now records `domInteractive`, and times `DOMContentLoaded` from it, which is the issue's measure. - **Docs:** - [The start-up review](docs/reviews/2026-11-09-startup-performance.md), recommendation 2: the result. - ADR 0023: an amendment with the security note. ### Before and after The cold-start check three times with each build, alternating, on the review's laptop (Windows 11, i7-8750H), on `main` after #892, #894 and #895. These are medians of the 12 launches after the first on each profile, with the first launch in brackets, in ms. "With" here had both caches; main's share is the 23 ms. | Stage | Without | With | | --- | --- | --- | | Main's first mark, after the process starts | 387 (407) | 364 (454) | | `DOMContentLoaded` − `domInteractive` | 781 (803) | 802 (1040) | | The editor's first render, after navigation | 1922 (2033) | 1660 (2142) | | A styled canvas, after the launch | 3211 (3380) | 3036 (3477) | - **The renderer's cache helps later than the issue expected:** V8 compiles most functions only when they first run, so the saving shows in React's first render (−0.26 s), not before `DOMContentLoaded`. The issue's acceptance criterion names `DOMContentLoaded − domInteractive`, which didn't move; the review records why. - **The first launch on a new profile is about 0.1 s slower,** as it builds the cache. ### Security (ADR 0023's amendment) - **The privilege grants a page nothing.** Electron passes the scheme to Chromium's code-cache schemes and to Blink's `RegisterURLSchemeAsCodeCacheWithHashing` ([electron#40544](electron/electron#40544)). - **How entries are checked:** as for Chromium's WebUI. Blink's `ScriptCachedMetadataHandlerWithHashing` stores a SHA-256 of the script's text with each entry and checks it against the served text before using it. A mismatch is dropped and evicted, so an earlier version's entry, or one for another script, is never used. The scripts' names change with each build as well. - **The threat, someone who can write to the profile:** - The hash doesn't authenticate the compiled code, so such a writer could still plant code that runs with the preload API. - That's the trust Chromium and every Electron app's http(s) pages already place in the profile's code cache. It's accepted for the measured gain, and recorded for the next security review. - Main's compile cache would have given the same writer a path into the main process, past the asar integrity fuse, with weaker checks (V8 checks little more than the source's length), for no measurable gain. That's why it's left out. ## Testing - `pnpm check` passes. - **`app.test.ts`:** the scheme is registered with `codeCache: true`. - `pnpm e2e`, the app's project: 277 passed, 1 skipped, and 1 failed: `canvas-toolbar.spec.ts`'s window-resize test. That file also failed once on #903's run, with a different test, and passed 18 of 18 when rerun here. - `pnpm e2e:packaged`: 12 passed, including the fuses read from the binary and the editor loaded from `webforge-app://` (S5). - The update test (an installed 0.0.1 updating itself to 0.0.2 on its profile, so with 0.0.1's code cache) installs Webforge, so it runs only on CI's Windows runner (`WEBFORGE_E2E_INSTALL`). It's this PR's check that a new version doesn't use a stale cache. - **Measured by hand:** Node's `NODE_DEBUG_NATIVE=COMPILE_CACHE` log for main's cache. For the renderer, a 4.4 MB `Code Cache` after one launch, and the timings above. ## Code review (`/code-review`) - Found and changed before this PR: the first version turned on main's compile cache from a new bootstrap entry, which split `index.ts` into `index.ts` and `main.ts`. Once measured, that change was dropped with the cache. - Checked with nothing to fix: - `codeCache` is valid only with `standard`, which the scheme has. - The Help window shares the scheme, so it gets the cache too. - Nothing else in the app reads `APP_SCHEME_PRIVILEGES`. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Description of Change
Add an option in
protocol.registerSchemesAsPrivilegedto allow V8 code cache in custom schemes.Upstream Chromium issue: https://bugs.chromium.org/p/chromium/issues/detail?id=1500437.
/cc @deepak1556 and @bpasero since there will need changes in VS Code.
Fixes #27075
Checklist
npm testpassesRelease Notes
Notes: Add an option in
protocol.registerSchemesAsPrivilegedto allow V8 code cache in custom schemes.