Reducing menubar cache verbosity (#59992)

* restructure menubar data to shrink cache

* remove old menu

* avoid storing common defaults for enabled, checked, and isNative

* remove old cache data for menubar/keybindings

src/vs/code/electron-main/keyboard.ts

@@ -5,14 +5,7 @@
 
 import * as nativeKeymap from 'native-keymap';
 import { IDisposable } from 'vs/base/common/lifecycle';
-import { IStateService } from 'vs/platform/state/common/state';
-import { Event, Emitter, once } from 'vs/base/common/event';
-import { ConfigWatcher } from 'vs/base/node/config';
-import { IUserFriendlyKeybinding } from 'vs/platform/keybinding/common/keybinding';
-import { IEnvironmentService } from 'vs/platform/environment/common/environment';
-import { ipcMain as ipc } from 'electron';
-import { IWindowsMainService } from 'vs/platform/windows/electron-main/windows';
-import { ILogService } from 'vs/platform/log/common/log';
+import { Emitter } from 'vs/base/common/event';
 
 export class KeyboardLayoutMonitor {
 
@@ -36,109 +29,4 @@ export class KeyboardLayoutMonitor {
 		}
 		return this._emitter.event(callback);
 	}
-}
-
-export interface IKeybinding {
-	id: string;
-	label: string;
-	isNative: boolean;
-}
-
-export class KeybindingsResolver {
-
-	private static readonly lastKnownKeybindingsMapStorageKey = 'lastKnownKeybindings';
-
-	private commandIds: Set<string>;
-	private keybindings: { [commandId: string]: IKeybinding };
-	private keybindingsWatcher: ConfigWatcher<IUserFriendlyKeybinding[]>;
-
-	private _onKeybindingsChanged = new Emitter<void>();
-	onKeybindingsChanged: Event<void> = this._onKeybindingsChanged.event;
-
-	constructor(
-		@IStateService private stateService: IStateService,
-		@IEnvironmentService environmentService: IEnvironmentService,
-		@IWindowsMainService private windowsMainService: IWindowsMainService,
-		@ILogService private logService: ILogService
-	) {
-		this.commandIds = new Set<string>();
-		this.keybindings = this.stateService.getItem<{ [id: string]: string; }>(KeybindingsResolver.lastKnownKeybindingsMapStorageKey) || Object.create(null);
-		this.keybindingsWatcher = new ConfigWatcher<IUserFriendlyKeybinding[]>(environmentService.appKeybindingsPath, { changeBufferDelay: 100, onError: error => this.logService.error(error) });
-
-		this.registerListeners();
-	}
-
-	private registerListeners(): void {
-
-		// Listen to resolved keybindings from window
-		ipc.on('vscode:keybindingsResolved', (event, rawKeybindings: string) => {
-			let keybindings: IKeybinding[] = [];
-			try {
-				keybindings = JSON.parse(rawKeybindings);
-			} catch (error) {
-				// Should not happen
-			}
-
-			// Fill hash map of resolved keybindings and check for changes
-			let keybindingsChanged = false;
-			let keybindingsCount = 0;
-			const resolvedKeybindings: { [commandId: string]: IKeybinding } = Object.create(null);
-			keybindings.forEach(keybinding => {
-				keybindingsCount++;
-
-				resolvedKeybindings[keybinding.id] = keybinding;
-
-				if (!this.keybindings[keybinding.id] || keybinding.label !== this.keybindings[keybinding.id].label) {
-					keybindingsChanged = true;
-				}
-			});
-
-			// A keybinding might have been unassigned, so we have to account for that too
-			if (Object.keys(this.keybindings).length !== keybindingsCount) {
-				keybindingsChanged = true;
-			}
-
-			if (keybindingsChanged) {
-				this.keybindings = resolvedKeybindings;
-				this.stateService.setItem(KeybindingsResolver.lastKnownKeybindingsMapStorageKey, this.keybindings); // keep to restore instantly after restart
-
-				this._onKeybindingsChanged.fire();
-			}
-		});
-
-		// Resolve keybindings when any first window is loaded
-		const onceOnWindowReady = once(this.windowsMainService.onWindowReady);
-		onceOnWindowReady(win => this.resolveKeybindings(win));
-
-		// Resolve keybindings again when keybindings.json changes
-		this.keybindingsWatcher.onDidUpdateConfiguration(() => this.resolveKeybindings());
-
-		// Resolve keybindings when window reloads because an installed extension could have an impact
-		this.windowsMainService.onWindowReload(() => this.resolveKeybindings());
-	}
-
-	private resolveKeybindings(win = this.windowsMainService.getLastActiveWindow()): void {
-		if (this.commandIds.size && win) {
-			const commandIds: string[] = [];
-			this.commandIds.forEach(id => commandIds.push(id));
-			win.sendWhenReady('vscode:resolveKeybindings', JSON.stringify(commandIds));
-		}
-	}
-
-	public getKeybinding(commandId: string): IKeybinding {
-		if (!commandId) {
-			return void 0;
-		}
-
-		if (!this.commandIds.has(commandId)) {
-			this.commandIds.add(commandId);
-		}
-
-		return this.keybindings[commandId];
-	}
-
-	public dispose(): void {
-		this._onKeybindingsChanged.dispose();
-		this.keybindingsWatcher.dispose();
-	}
 }
\ No newline at end of file

src/vs/platform/menubar/common/menubar.ts

@@ -11,13 +11,12 @@ export const IMenubarService = createDecorator<IMenubarService>('menubarService'
 export interface IMenubarService {
 	_serviceBrand: any;
 
-	updateMenubar(windowId: number, menus: IMenubarData, additionalKeybindings?: Array<IMenubarKeybinding>): TPromise<void>;
+	updateMenubar(windowId: number, menuData: IMenubarData): TPromise<void>;
 }
 
 export interface IMenubarData {
-	'Files'?: IMenubarMenu;
-	'Edit'?: IMenubarMenu;
-	[id: string]: IMenubarMenu;
+	menus: { [id: string]: IMenubarMenu };
+	keybindings: { [id: string]: IMenubarKeybinding };
 }
 
 export interface IMenubarMenu {
@@ -25,17 +24,15 @@ export interface IMenubarMenu {
 }
 
 export interface IMenubarKeybinding {
-	id: string;
 	label: string;
-	isNative: boolean;
+	isNative?: boolean; // Assumed true if missing
 }
 
 export interface IMenubarMenuItemAction {
 	id: string;
 	label: string;
-	checked: boolean;
-	enabled: boolean;
-	keybinding?: IMenubarKeybinding;
+	checked?: boolean; // Assumed false if missing
+	enabled?: boolean; // Assumed true if missing
 }
 
 export interface IMenubarMenuItemSubmenu {
@@ -54,10 +51,10 @@ export function isMenubarMenuItemSubmenu(menuItem: MenubarMenuItem): menuItem is
 	return (<IMenubarMenuItemSubmenu>menuItem).submenu !== undefined;
 }
 
-export function isMenubarMenuItemAction(menuItem: MenubarMenuItem): menuItem is IMenubarMenuItemAction {
-	return (<IMenubarMenuItemAction>menuItem).checked !== undefined || (<IMenubarMenuItemAction>menuItem).enabled !== undefined;
-}
-
 export function isMenubarMenuItemSeparator(menuItem: MenubarMenuItem): menuItem is IMenubarMenuItemSeparator {
 	return (<IMenubarMenuItemSeparator>menuItem).id === 'vscode.menubar.separator';
+}
+
+export function isMenubarMenuItemAction(menuItem: MenubarMenuItem): menuItem is IMenubarMenuItemAction {
+	return !isMenubarMenuItemSubmenu(menuItem) && !isMenubarMenuItemSeparator(menuItem);
 }
\ No newline at end of file

src/vs/platform/menubar/electron-main/menubar.ts

@@ -18,7 +18,7 @@ import { mnemonicMenuLabel as baseMnemonicLabel, unmnemonicLabel } from 'vs/base
 import { IWindowsMainService, IWindowsCountChangedEvent } from 'vs/platform/windows/electron-main/windows';
 import { IHistoryMainService } from 'vs/platform/history/common/history';
 import { IWorkspaceIdentifier, ISingleFolderWorkspaceIdentifier, isSingleFolderWorkspaceIdentifier, isWorkspaceIdentifier } from 'vs/platform/workspaces/common/workspaces';
-import { IMenubarData, IMenubarKeybinding, MenubarMenuItem, isMenubarMenuItemSeparator, isMenubarMenuItemSubmenu, isMenubarMenuItemAction } from 'vs/platform/menubar/common/menubar';
+import { IMenubarData, IMenubarKeybinding, MenubarMenuItem, isMenubarMenuItemSeparator, isMenubarMenuItemSubmenu, isMenubarMenuItemAction, IMenubarMenu } from 'vs/platform/menubar/common/menubar';
 import { URI } from 'vs/base/common/uri';
 import { ILabelService } from 'vs/platform/label/common/label';
 import { IStateService } from 'vs/platform/state/common/state';
@@ -33,8 +33,7 @@ interface IMenuItemClickHandler {
 export class Menubar {
 
 	private static readonly MAX_MENU_RECENT_ENTRIES = 10;
-	private static readonly lastKnownMenubarStorageKey = 'lastKnownMenubar';
-	private static readonly lastKnownAdditionalKeybindings = 'lastKnownAdditionalKeybindings';
+	private static readonly lastKnownMenubarStorageKey = 'lastKnownMenubarData';
 
 	private isQuitting: boolean;
 	private appMenuInstalled: boolean;
@@ -47,7 +46,7 @@ export class Menubar {
 	// TODO@sbatten Remove this when fixed upstream by Electron
 	private oldMenus: Menu[];
 
-	private menubarMenus: IMenubarData;
+	private menubarMenus: { [id: string]: IMenubarMenu };
 
 	private keybindings: { [commandId: string]: IMenubarKeybinding };
 
@@ -68,9 +67,10 @@ export class Menubar {
 
 		this.menuGC = new RunOnceScheduler(() => { this.oldMenus = []; }, 10000);
 
-		this.menubarMenus = this.stateService.getItem<IMenubarData>(Menubar.lastKnownMenubarStorageKey) || Object.create(null);
+		this.menubarMenus = Object.create(null);
+		this.keybindings = Object.create(null);
 
-		this.keybindings = this.stateService.getItem<IMenubarData>(Menubar.lastKnownAdditionalKeybindings) || Object.create(null);
+		this.restoreCachedMenubarData();
 
 		this.addFallbackHandlers();
 
@@ -83,6 +83,23 @@ export class Menubar {
 		this.registerListeners();
 	}
 
+	private restoreCachedMenubarData() {
+		// TODO@sbatten remove this at some point down the road
+		const outdatedKeys = ['lastKnownAdditionalKeybindings', 'lastKnownKeybindings', 'lastKnownMenubar'];
+		outdatedKeys.forEach(key => this.stateService.removeItem(key));
+
+		const menubarData = this.stateService.getItem<IMenubarData>(Menubar.lastKnownMenubarStorageKey);
+		if (menubarData) {
+			if (menubarData.menus) {
+				this.menubarMenus = menubarData.menus;
+			}
+
+			if (menubarData.keybindings) {
+				this.keybindings = menubarData.keybindings;
+			}
+		}
+	}
+
 	private addFallbackHandlers(): void {
 		// File Menu Items
 		this.fallbackMenuHandlers['workbench.action.files.newUntitledFile'] = () => this.windowsMainService.openNewWindow(OpenContext.MENU);
@@ -167,17 +184,12 @@ export class Menubar {
 		return enableNativeTabs;
 	}
 
-	updateMenu(menus: IMenubarData, windowId: number, additionalKeybindings?: Array<IMenubarKeybinding>) {
-		this.menubarMenus = menus;
-		if (additionalKeybindings) {
-			additionalKeybindings.forEach(keybinding => {
-				this.keybindings[keybinding.id] = keybinding;
-			});
-		}
+	updateMenu(menubarData: IMenubarData, windowId: number) {
+		this.menubarMenus = menubarData.menus;
+		this.keybindings = menubarData.keybindings;
 
 		// Save off new menu and keybindings
-		this.stateService.setItem(Menubar.lastKnownMenubarStorageKey, this.menubarMenus);
-		this.stateService.setItem(Menubar.lastKnownAdditionalKeybindings, this.keybindings);
+		this.stateService.setItem(Menubar.lastKnownMenubarStorageKey, menubarData);
 
 		this.scheduleUpdateMenu();
 	}
@@ -418,13 +430,6 @@ export class Menubar {
 					this.insertCheckForUpdatesItems(menu);
 				}
 
-				// Store the keybinding
-				if (item.keybinding) {
-					this.keybindings[item.id] = item.keybinding;
-				} else if (this.keybindings[item.id]) {
-					this.keybindings[item.id] = undefined;
-				}
-
 				if (isMacintosh) {
 					if (this.windowsMainService.getWindowCount() === 0 && this.closedLastWindow) {
 						// In the fallback scenario, we are either disabled or using a fallback handler
@@ -434,10 +439,10 @@ export class Menubar {
 							menu.append(this.createMenuItem(item.label, item.id, false, item.checked));
 						}
 					} else {
-						menu.append(this.createMenuItem(item.label, item.id, item.enabled, item.checked));
+						menu.append(this.createMenuItem(item.label, item.id, item.enabled === false ? false : true, !!item.checked));
 					}
 				} else {
-					menu.append(this.createMenuItem(item.label, item.id, item.enabled, item.checked));
+					menu.append(this.createMenuItem(item.label, item.id, item.enabled === false ? false : true, !!item.checked));
 				}
 			}
 		});
@@ -706,7 +711,7 @@ export class Menubar {
 		if (binding && binding.label) {
 
 			// if the binding is native, we can just apply it
-			if (binding.isNative) {
+			if (binding.isNative !== false) {
 				options.accelerator = binding.label;
 			}
 

src/vs/platform/menubar/electron-main/menubarService.ts

@@ -3,7 +3,7 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { IMenubarService, IMenubarData, IMenubarKeybinding } from 'vs/platform/menubar/common/menubar';
+import { IMenubarService, IMenubarData } from 'vs/platform/menubar/common/menubar';
 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';
@@ -22,11 +22,11 @@ export class MenubarService implements IMenubarService {
 		this._menubar = this.instantiationService.createInstance(Menubar);
 	}
 
-	updateMenubar(windowId: number, menus: IMenubarData, additionalKeybindings?: Array<IMenubarKeybinding>): TPromise<void> {
+	updateMenubar(windowId: number, menus: IMenubarData): TPromise<void> {
 		this.logService.trace('menubarService#updateMenubar', windowId);
 
 		if (this._menubar) {
-			this._menubar.updateMenu(menus, windowId, additionalKeybindings);
+			this._menubar.updateMenu(menus, windowId);
 		}
 
 		return TPromise.as(null);

src/vs/platform/menubar/node/menubarIpc.ts

@@ -4,7 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 import { IChannel } from 'vs/base/parts/ipc/node/ipc';
 import { TPromise } from 'vs/base/common/winjs.base';
-import { IMenubarService, IMenubarData, IMenubarKeybinding } from 'vs/platform/menubar/common/menubar';
+import { IMenubarService, IMenubarData } from 'vs/platform/menubar/common/menubar';
 import { Event } from 'vs/base/common/event';
 
 export interface IMenubarChannel extends IChannel {
@@ -22,7 +22,7 @@ export class MenubarChannel implements IMenubarChannel {
 
 	call(command: string, arg?: any): TPromise<any> {
 		switch (command) {
-			case 'updateMenubar': return this.service.updateMenubar(arg[0], arg[1], arg[2]);
+			case 'updateMenubar': return this.service.updateMenubar(arg[0], arg[1]);
 		}
 		return undefined;
 	}
@@ -34,7 +34,7 @@ export class MenubarChannelClient implements IMenubarService {
 
 	constructor(private channel: IMenubarChannel) { }
 
-	updateMenubar(windowId: number, menus: IMenubarData, additionalKeybindings?: Array<IMenubarKeybinding>): TPromise<void> {
-		return this.channel.call('updateMenubar', [windowId, menus, additionalKeybindings]);
+	updateMenubar(windowId: number, menuData: IMenubarData): TPromise<void> {
+		return this.channel.call('updateMenubar', [windowId, menuData]);
 	}
 }
\ No newline at end of file

src/vs/workbench/browser/parts/titlebar/menubarControl.ts

@@ -463,9 +463,9 @@ export class MenubarControl extends Disposable {
 			this.setupCustomMenubar();
 		} else {
 			// Send menus to main process to be rendered by Electron
-			const menubarData = {};
+			const menubarData = { menus: {}, keybindings: {} };
 			if (this.getMenubarMenus(menubarData)) {
-				this.menubarService.updateMenubar(this.windowService.getCurrentWindowId(), menubarData, this.getAdditionalKeybindings());
+				this.menubarService.updateMenubar(this.windowService.getCurrentWindowId(), menubarData);
 			}
 		}
 	}
@@ -923,25 +923,25 @@ export class MenubarControl extends Disposable {
 	private getMenubarKeybinding(id: string): IMenubarKeybinding {
 		const binding = this.keybindingService.lookupKeybinding(id);
 		if (!binding) {
-			return null;
+			return undefined;
 		}
 
 		// first try to resolve a native accelerator
 		const electronAccelerator = binding.getElectronAccelerator();
 		if (electronAccelerator) {
-			return { id, label: electronAccelerator, isNative: true };
+			return { label: electronAccelerator };
 		}
 
 		// we need this fallback to support keybindings that cannot show in electron menus (e.g. chords)
 		const acceleratorLabel = binding.getLabel();
 		if (acceleratorLabel) {
-			return { id, label: acceleratorLabel, isNative: false };
+			return { label: acceleratorLabel, isNative: false };
 		}
 
 		return null;
 	}
 
-	private populateMenuItems(menu: IMenu, menuToPopulate: IMenubarMenu) {
+	private populateMenuItems(menu: IMenu, menuToPopulate: IMenubarMenu, keybindings: { [id: string]: IMenubarKeybinding }) {
 		let groups = menu.getActions();
 		for (let group of groups) {
 			const [, actions] = group;
@@ -950,7 +950,7 @@ export class MenubarControl extends Disposable {
 
 				if (menuItem instanceof SubmenuItemAction) {
 					const submenu = { items: [] };
-					this.populateMenuItems(this.menuService.createMenu(menuItem.item.submenu, this.contextKeyService), submenu);
+					this.populateMenuItems(this.menuService.createMenu(menuItem.item.submenu, this.contextKeyService), submenu, keybindings);
 
 					let menubarSubmenuItem: IMenubarMenuItemSubmenu = {
 						id: menuItem.id,
@@ -962,13 +962,19 @@ export class MenubarControl extends Disposable {
 				} else {
 					let menubarMenuItem: IMenubarMenuItemAction = {
 						id: menuItem.id,
-						label: menuItem.label,
-						checked: menuItem.checked,
-						enabled: menuItem.enabled,
-						keybinding: this.getMenubarKeybinding(menuItem.id)
+						label: menuItem.label
 					};
 
+					if (menuItem.checked) {
+						menubarMenuItem.checked = true;
+					}
+
+					if (!menuItem.enabled) {
+						menubarMenuItem.enabled = false;
+					}
+
 					menubarMenuItem.label = this.calculateActionLabel(menubarMenuItem);
+					keybindings[menuItem.id] = this.getMenubarKeybinding(menuItem.id);
 					menuToPopulate.items.push(menubarMenuItem);
 				}
 			});
@@ -981,10 +987,10 @@ export class MenubarControl extends Disposable {
 		}
 	}
 
-	private getAdditionalKeybindings(): Array<IMenubarKeybinding> {
-		const keybindings = [];
+	private getAdditionalKeybindings(): { [id: string]: IMenubarKeybinding } {
+		const keybindings = {};
 		if (isMacintosh) {
-			keybindings.push(this.getMenubarKeybinding('workbench.action.quit'));
+			keybindings['workbench.action.quit'] = (this.getMenubarKeybinding('workbench.action.quit'));
 		}
 
 		return keybindings;
@@ -995,15 +1001,16 @@ export class MenubarControl extends Disposable {
 			return false;
 		}
 
+		menubarData.keybindings = this.getAdditionalKeybindings();
 		for (let topLevelMenuName of Object.keys(this.topLevelMenus)) {
 			const menu = this.topLevelMenus[topLevelMenuName];
 			let menubarMenu: IMenubarMenu = { items: [] };
-			this.populateMenuItems(menu, menubarMenu);
+			this.populateMenuItems(menu, menubarMenu, menubarData.keybindings);
 			if (menubarMenu.items.length === 0) {
 				// Menus are incomplete
 				return false;
 			}
-			menubarData[topLevelMenuName] = menubarMenu;
+			menubarData.menus[topLevelMenuName] = menubarMenu;
 		}
 
 		return true;