Repository navigation
refactor: move Menu.buildFromTemplate's ordering to C++ - #53875
Merged
Merged
Conversation
This was referenced Sep 13, 2026
MarshallOfSound
added this pull request to stack #53879
September 13, 2026 04:42
MarshallOfSound
added
the
target/45-x-y
MarshallOfSound
force-pushed
the
perf/menu-3-template
branch
from
September 13, 2026 05:13
587e9e3 to
d8cced8
Compare
deepak1556
reviewed
Sep 14, 2026
MarshallOfSound
force-pushed
the
perf/menu-3-template
branch
from
September 14, 2026 10:39
d8cced8 to
10f34b4
Compare
deepak1556
approved these changes
Sep 14, 2026
The before/after/beforeGroupContaining/afterGroupContaining ordering and the separator clean-up move from lib/browser/api/menu-utils.ts to shell/browser/api/electron_api_menu_template.cc. Ids are still compared with ===, and Proxy-wrapped and sparse template arrays are still accepted.
MarshallOfSound
force-pushed
the
perf/menu-3-template
branch
from
September 14, 2026 10:52
10f34b4 to
4a88992
Compare
MarshallOfSound
added
backport-check-skip
|
No Release Notes |
Contributor
|
I was unable to backport this PR to "45-x-y" cleanly; |
trop
Bot
added
needs-manual-bp/45-x-y
and removed
target/45-x-y
MarshallOfSound
added a commit
that referenced
this pull request
Sep 14, 2026
* refactor: add gin_helper and BaseWindow/WebContents helpers for native menus (#53873) - gin_helper::ConvertedValue<T>: a property store for cppgc wrappables that keeps the JS value a script assigned (for identity on read) together with its native conversion, done once at assignment. - gin_helper::Constructible: an optional FillInstanceTemplate() hook for own (instance) properties. - gin_helper::TrackableObject::GetAllNative(): the tracked instances without going through their wrappers. - BaseWindow::GetFocusedWindow() (now also backing BaseWindow.getFocusedWindow()), FromValue(), IsLive(), SetMenuNatively(). - WebContents::GetFocusedWebContents() and session(). (cherry picked from commit 1fd543f) * refactor: move the MenuItem role table to C++ (#53874) The built-in roles (default label and accelerator, registerAccelerator, the computed checked state for toggleSpellChecker, the default submenus and what each role does when activated) move from lib/browser/api/menu-item-roles.ts to shell/browser/api/electron_api_menu_roles.{h,cc}; roles now act directly on api::WebContents / NativeWindow / Browser / prefs. menu-item.ts reads the table through the menu binding. (cherry picked from commit 75bcb10) * refactor: move Menu.buildFromTemplate's ordering to C++ (#53875) The before/after/beforeGroupContaining/afterGroupContaining ordering and the separator clean-up move from lib/browser/api/menu-utils.ts to shell/browser/api/electron_api_menu_template.cc. Ids are still compared with ===, and Proxy-wrapped and sparse template arrays are still accepted. (cherry picked from commit 473519e) * perf: implement MenuItem and Menu's item handling natively (#53876) MenuItem was a JavaScript object that the native menu model called back into for every property of every item each time a menu was shown, had its accelerators registered or (on macOS) was checked for a key equivalent, and menu.insert() and click dispatch bounced between lib/browser/api and api::Menu several times per item. MenuItem is now a gin/cppgc object in shell/browser/api whose documented properties are native data properties; api::Menu holds its items natively and answers the menu model from them, and insert/append, menu.items, buildFromTemplate and click dispatch are native. The role submenus are built from native items. Showing a menu no longer enters JS; the only JS that runs is the app's own click handlers. lib/browser/api/menu-item.ts becomes a re-export of the binding; menu.ts keeps popup(), getMenuItemById() and set/getApplicationMenu(). (cherry picked from commit 6887c0e) * perf: implement Menu.popup, getMenuItemById and setApplicationMenu natively (#53877) The rest of lib/browser/api/menu.ts moves into api::Menu: popup() and closePopup() (window resolution included), getMenuItemById(), and Menu.setApplicationMenu()/getApplicationMenu(), which on Windows and Linux now sets each window's menu without a round trip through the BaseWindow wrappers. menu.ts becomes a re-export of the binding and default-menu.ts asks api::Menu whether an application menu was set. (cherry picked from commit ee3c6d7) * refactor: make ElectronMenuModel a ui::MenuModel over the menu's items (#53878) ElectronMenuModel was a ui::SimpleMenuModel: it kept its own copy of each item (type, command id, label) plus per-command-id maps for tooltip, role, custom type and badge, and asked api::Menu for everything else through a by-command-id delegate, which api::Menu answered by looking the id up in a process-wide MenuItem registry. It is now a ui::MenuModel implemented directly over ElectronMenuModel::Item pointers (api::MenuItem implements Item), so there is one list of items, the platform menu code reads item state without a delegate hop or an id lookup, and the command-id registry, the radio-group table and the SimpleMenuModel mirroring in api::Menu go away. The platform menu builders are unchanged. (cherry picked from commit 9c69d2d)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of Change
Moves
Menu.buildFromTemplate()'sbefore/after/beforeGroupContaining/afterGroupContainingordering and separator folding fromlib/browser/api/menu-utils.tstoshell/browser/api/electron_api_menu_template.cc. Same semantics (ids compared with===, Proxy-wrapped and sparse templates accepted); the sort is iterative.Perf: neutral on its own (Release, Linux x64: within noise on all menu benchmarks). Groundwork for the native
buildFromTemplate.Checklist
npm testpassesRelease Notes
Notes: none