diff --git a/src/vs/code/electron-main/app.ts b/src/vs/code/electron-main/app.ts index 0ce698fce7ffa..2c048bdec2d30 100644 --- a/src/vs/code/electron-main/app.ts +++ b/src/vs/code/electron-main/app.ts @@ -61,7 +61,6 @@ import { IMenubarService } from 'vs/platform/menubar/common/menubar'; import { MenubarService } from 'vs/platform/menubar/electron-main/menubarService'; import { MenubarChannel } from 'vs/platform/menubar/node/menubarIpc'; import { ILabelService, RegisterFormatterEvent } from 'vs/platform/label/common/label'; -import { CodeMenu } from 'vs/code/electron-main/menus'; import { hasArgs } from 'vs/platform/environment/node/argv'; import { RunOnceScheduler } from 'vs/base/common/async'; import { registerContextMenuListener } from 'vs/base/parts/contextmenu/electron-main/contextmenu'; @@ -594,23 +593,6 @@ export class CodeApplication { } } - // Install Menu - const instantiationService = accessor.get(IInstantiationService); - const configurationService = accessor.get(IConfigurationService); - - // Initial value sets the value for macOS. false indicates using Dynamic Menus - // Eventually this will not be necessary as the dynamic menu will takeover - let createNativeMenu = false; - if (platform.isLinux) { - createNativeMenu = configurationService.getValue('window.titleBarStyle') !== 'custom'; - } else if (platform.isWindows) { - createNativeMenu = configurationService.getValue('window.titleBarStyle') === 'native'; - } - - if (createNativeMenu) { - instantiationService.createInstance(CodeMenu); - } - // Jump List this.historyMainService.updateWindowsJumpList(); this.historyMainService.onRecentlyOpenedChange(() => this.historyMainService.updateWindowsJumpList()); diff --git a/src/vs/platform/menubar/electron-main/menubar.ts b/src/vs/platform/menubar/electron-main/menubar.ts index 31267a02840de..1a518e18a0fda 100644 --- a/src/vs/platform/menubar/electron-main/menubar.ts +++ b/src/vs/platform/menubar/electron-main/menubar.ts @@ -36,6 +36,11 @@ export class Menubar { private closedLastWindow: boolean; private menuUpdater: RunOnceScheduler; + private menuGC: RunOnceScheduler; + + // Array to keep menus around so that GC doesn't cause crash as explained in #55347 + // TODO@sbatten Remove this when fixed upstream by Electron + private oldMenus: Menu[]; private menubarMenus: IMenubarData; @@ -56,6 +61,8 @@ export class Menubar { ) { this.menuUpdater = new RunOnceScheduler(() => this.doUpdateMenu(), 0); + this.menuGC = new RunOnceScheduler(() => { this.oldMenus = []; }, 10000); + this.menubarMenus = this.stateService.getItem(Menubar.lastKnownMenubarStorageKey) || Object.create(null); this.keybindings = this.stateService.getItem(Menubar.lastKnownAdditionalKeybindings) || Object.create(null); @@ -64,6 +71,8 @@ export class Menubar { this.closedLastWindow = false; + this.oldMenus = []; + this.install(); this.registerListeners(); @@ -202,6 +211,12 @@ export class Menubar { } private install(): void { + // Store old menu in our array to avoid GC to collect the menu and crash. See #55347 + // TODO@sbatten Remove this when fixed upstream by Electron + const oldMenu = Menu.getApplicationMenu(); + if (oldMenu) { + this.oldMenus.push(oldMenu); + } // If we don't have a menu yet, set it to null to avoid the electron menu. // This should only happen on the first launch ever @@ -293,15 +308,6 @@ export class Menubar { menubar.append(macWindowMenuItem); } - // Preferences - if (!isMacintosh) { - const preferencesMenu = new Menu(); - const preferencesMenuItem = new MenuItem({ label: this.mnemonicLabel(nls.localize({ key: 'mPreferences', comment: ['&& denotes a mnemonic'] }, "&&Preferences")), submenu: preferencesMenu }); - - this.setMenuById(preferencesMenu, 'Preferences'); - menubar.append(preferencesMenuItem); - } - // Help const helpMenu = new Menu(); const helpMenuItem = new MenuItem({ label: this.mnemonicLabel(nls.localize({ key: 'mHelp', comment: ['&& denotes a mnemonic'] }, "&&Help")), submenu: helpMenu, role: 'help' }); @@ -314,6 +320,9 @@ export class Menubar { } else { Menu.setApplicationMenu(null); } + + // Dispose of older menus after some time + this.menuGC.schedule(); } private setMacApplicationMenu(macApplicationMenu: Electron.Menu): void { diff --git a/src/vs/platform/menubar/electron-main/menubarService.ts b/src/vs/platform/menubar/electron-main/menubarService.ts index d3d85a8dce696..79554f380b6d2 100644 --- a/src/vs/platform/menubar/electron-main/menubarService.ts +++ b/src/vs/platform/menubar/electron-main/menubarService.ts @@ -8,7 +8,6 @@ import { Menubar } from 'vs/platform/menubar/electron-main/menubar'; import { ILogService } from 'vs/platform/log/common/log'; import { TPromise } from 'vs/base/common/winjs.base'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; -import { isMacintosh } from 'vs/base/common/platform'; export class MenubarService implements IMenubarService { _serviceBrand: any; @@ -20,10 +19,7 @@ export class MenubarService implements IMenubarService { @ILogService private logService: ILogService ) { // Install Menu - // TODO@sbatten Only do this for macOS as the dynamic menus are currently stable. #55347 - if (isMacintosh) { - this._menubar = this.instantiationService.createInstance(Menubar); - } + this._menubar = this.instantiationService.createInstance(Menubar); } updateMenubar(windowId: number, menus: IMenubarData, additionalKeybindings?: Array): TPromise { diff --git a/src/vs/workbench/browser/parts/titlebar/menubarControl.ts b/src/vs/workbench/browser/parts/titlebar/menubarControl.ts index 217e3b6dde332..3c0365e941801 100644 --- a/src/vs/workbench/browser/parts/titlebar/menubarControl.ts +++ b/src/vs/workbench/browser/parts/titlebar/menubarControl.ts @@ -468,8 +468,7 @@ export class MenubarControl extends Disposable { private doSetupMenubar(): void { if (!isMacintosh && this.currentTitlebarStyleSetting === 'custom') { this.setupCustomMenubar(); - } else if (isMacintosh) { - // TODO@sbatten currently limiting dynamic native menus to macOS #55347 + } else { // Send menus to main process to be rendered by Electron const menubarData = {}; if (this.getMenubarMenus(menubarData)) {