Fixes #26151: Do not trust DOM readings immediately after calling webFrame.setZoomLevel()

src/vs/base/browser/browser.ts

@@ -15,10 +15,9 @@ class WindowManager {
 	private _fullscreen: boolean;
 
 	private _zoomLevel: number = 0;
-	private _zoomFactor: number = 0;
+	private _lastZoomLevelChangeTime: number = 0;
 
-	private _pixelRatioCache: number = 0;
-	private _pixelRatioComputed: boolean = false;
+	private _zoomFactor: number = 0;
 
 	private _onDidChangeZoomLevel: Emitter<number> = new Emitter<number>();
 	public onDidChangeZoomLevel: Event<number> = this._onDidChangeZoomLevel.event;
@@ -30,13 +29,18 @@ class WindowManager {
 		return this._zoomLevel;
 	}
 
-	public setZoomLevel(zoomLevel: number): void {
+	public getTimeSinceLastZoomLevelChanged(): number {
+		return Date.now() - this._lastZoomLevelChangeTime;
+	}
+
+	public setZoomLevel(zoomLevel: number, isTrusted: boolean): void {
 		if (this._zoomLevel === zoomLevel) {
 			return;
 		}
 
 		this._zoomLevel = zoomLevel;
-		this._pixelRatioComputed = false;
+		// See https://github.com/Microsoft/vscode/issues/26151
+		this._lastZoomLevelChangeTime = isTrusted ? 0 : Date.now();
 		this._onDidChangeZoomLevel.fire(this._zoomLevel);
 	}
 
@@ -49,14 +53,6 @@ class WindowManager {
 	}
 
 	public getPixelRatio(): number {
-		if (!this._pixelRatioComputed) {
-			this._pixelRatioCache = this._computePixelRatio();
-			this._pixelRatioComputed = true;
-		}
-		return this._pixelRatioCache;
-	}
-
-	private _computePixelRatio(): number {
 		let ctx = document.createElement('canvas').getContext('2d');
 		let dpr = window.devicePixelRatio || 1;
 		let bsr = (<any>ctx).webkitBackingStorePixelRatio ||
@@ -82,9 +78,16 @@ class WindowManager {
 }
 
 /** A zoom index, e.g. 1, 2, 3 */
+export function setZoomLevel(zoomLevel: number, isTrusted: boolean): void {
+	WindowManager.INSTANCE.setZoomLevel(zoomLevel, isTrusted);
+}
 export function getZoomLevel(): number {
 	return WindowManager.INSTANCE.getZoomLevel();
 }
+/** Returns the time (in ms) since the zoom level was changed */
+export function getTimeSinceLastZoomLevelChanged(): number {
+	return WindowManager.INSTANCE.getTimeSinceLastZoomLevelChanged();
+}
 /** The zoom scale for an index, e.g. 1, 1.2, 1.4 */
 export function getZoomFactor(): number {
 	return WindowManager.INSTANCE.getZoomFactor();
@@ -92,9 +95,6 @@ export function getZoomFactor(): number {
 export function getPixelRatio(): number {
 	return WindowManager.INSTANCE.getPixelRatio();
 }
-export function setZoomLevel(zoomLevel: number): void {
-	WindowManager.INSTANCE.setZoomLevel(zoomLevel);
-}
 export function setZoomFactor(zoomFactor: number): void {
 	WindowManager.INSTANCE.setZoomFactor(zoomFactor);
 }

src/vs/editor/browser/config/configuration.ts

@@ -102,7 +102,7 @@ class CSSBasedConfiguration extends Disposable {
 	public static INSTANCE = new CSSBasedConfiguration();
 
 	private _cache: CSSBasedConfigurationCache;
-	private _changeMonitorTimeout: number = -1;
+	private _evictUntrustedReadingsTimeout: number;
 
 	private _onDidChange = this._register(new Emitter<void>());
 	public onDidChange: Event<void> = this._onDidChange.event;
@@ -111,16 +111,44 @@ class CSSBasedConfiguration extends Disposable {
 		super();
 
 		this._cache = new CSSBasedConfigurationCache();
+		this._evictUntrustedReadingsTimeout = -1;
 	}
 
 	public dispose(): void {
-		if (this._changeMonitorTimeout !== -1) {
-			clearTimeout(this._changeMonitorTimeout);
-			this._changeMonitorTimeout = -1;
+		if (this._evictUntrustedReadingsTimeout !== -1) {
+			clearTimeout(this._evictUntrustedReadingsTimeout);
+			this._evictUntrustedReadingsTimeout = -1;
 		}
 		super.dispose();
 	}
 
+	private _writeToCache(item: BareFontInfo, value: FontInfo): void {
+		this._cache.put(item, value);
+
+		if (!value.isTrusted && this._evictUntrustedReadingsTimeout === -1) {
+			// Try reading again after some time
+			this._evictUntrustedReadingsTimeout = setTimeout(() => {
+				this._evictUntrustedReadingsTimeout = -1;
+				this._evictUntrustedReadings();
+			}, 5000);
+		}
+	}
+
+	private _evictUntrustedReadings(): void {
+		let values = this._cache.getValues();
+		let somethingRemoved = false;
+		for (let i = 0, len = values.length; i < len; i++) {
+			let item = values[i];
+			if (!item.isTrusted) {
+				somethingRemoved = true;
+				this._cache.remove(item);
+			}
+		}
+		if (somethingRemoved) {
+			this._onDidChange.fire();
+		}
+	}
+
 	public saveFontInfo(): ISerializedFontInfo[] {
 		// Only save trusted font info (that has been measured in this running instance)
 		return this._cache.getValues().filter(item => item.isTrusted);
@@ -131,25 +159,8 @@ class CSSBasedConfiguration extends Disposable {
 		// The reason for this is that a font might have been installed on the OS in the meantime.
 		for (let i = 0, len = savedFontInfo.length; i < len; i++) {
 			let fontInfo = new FontInfo(savedFontInfo[i], false);
-			this._cache.put(fontInfo, fontInfo);
+			this._writeToCache(fontInfo, fontInfo);
 		}
-
-		// Remove saved font info that does not have the trusted flag.
-		// (this forces it to be re-read).
-		setTimeout(() => {
-			let values = this._cache.getValues();
-			let somethingRemoved = false;
-			for (let i = 0, len = values.length; i < len; i++) {
-				let item = values[i];
-				if (!item.isTrusted) {
-					somethingRemoved = true;
-					this._cache.remove(item);
-				}
-			}
-			if (somethingRemoved) {
-				this._onDidChange.fire();
-			}
-		}, 5000);
 	}
 
 	public readConfiguration(bareFontInfo: BareFontInfo): FontInfo {
@@ -169,45 +180,14 @@ class CSSBasedConfiguration extends Disposable {
 					typicalFullwidthCharacterWidth: Math.max(readConfig.typicalFullwidthCharacterWidth, 5),
 					spaceWidth: Math.max(readConfig.spaceWidth, 5),
 					maxDigitWidth: Math.max(readConfig.maxDigitWidth, 5),
-				}, true);
-				this._installChangeMonitor();
+				}, false);
 			}
 
-			this._cache.put(bareFontInfo, readConfig);
+			this._writeToCache(bareFontInfo, readConfig);
 		}
 		return this._cache.get(bareFontInfo);
 	}
 
-	private _installChangeMonitor(): void {
-		if (this._changeMonitorTimeout === -1) {
-			this._changeMonitorTimeout = setTimeout(() => {
-				this._changeMonitorTimeout = -1;
-				this._monitorForChanges();
-			}, 500);
-		}
-	}
-
-	private _monitorForChanges(): void {
-		let shouldInstallChangeMonitor = false;
-		let keys = this._cache.getKeys();
-		for (let i = 0; i < keys.length; i++) {
-			let styling = keys[i];
-
-			let newValue = CSSBasedConfiguration._actualReadConfiguration(styling);
-
-			if (newValue.typicalHalfwidthCharacterWidth <= 2 || newValue.typicalFullwidthCharacterWidth <= 2 || newValue.maxDigitWidth <= 2) {
-				// We still couldn't read the CSS config
-				shouldInstallChangeMonitor = true;
-			} else {
-				this._cache.put(styling, newValue);
-				this._onDidChange.fire();
-			}
-		}
-		if (shouldInstallChangeMonitor) {
-			this._installChangeMonitor();
-		}
-	}
-
 	private static createRequest(chr: string, type: CharWidthRequestType, all: CharWidthRequest[], monospace: CharWidthRequest[]): CharWidthRequest {
 		let result = new CharWidthRequest(chr, type);
 		all.push(result);
@@ -278,6 +258,8 @@ class CSSBasedConfiguration extends Disposable {
 			}
 		}
 
+		// let's trust the zoom level only 2s after it was changed.
+		const canTrustBrowserZoomLevel = (browser.getTimeSinceLastZoomLevelChanged() > 2000);
 		return new FontInfo({
 			zoomLevel: browser.getZoomLevel(),
 			fontFamily: bareFontInfo.fontFamily,
@@ -289,7 +271,7 @@ class CSSBasedConfiguration extends Disposable {
 			typicalFullwidthCharacterWidth: typicalFullwidthCharacter.width,
 			spaceWidth: space.width,
 			maxDigitWidth: maxDigitWidth
-		}, true);
+		}, canTrustBrowserZoomLevel);
 	}
 }
 

src/vs/editor/browser/viewParts/minimap/minimap.ts

@@ -10,7 +10,6 @@ import { ViewPart, PartFingerprint, PartFingerprints } from 'vs/editor/browser/v
 import { ViewContext } from 'vs/editor/common/view/viewContext';
 import { RenderingContext, RestrictedRenderingContext } from 'vs/editor/common/view/renderingContext';
 import { getOrCreateMinimapCharRenderer } from 'vs/editor/common/view/runtimeMinimapCharRenderer';
-import * as browser from 'vs/base/browser/browser';
 import * as dom from 'vs/base/browser/dom';
 import { MinimapCharRenderer, MinimapTokensColorTracker, Constants } from 'vs/editor/common/view/minimapCharRenderer';
 import * as editorCommon from 'vs/editor/common/editorCommon';
@@ -107,7 +106,7 @@ class MinimapOptions {
 	public readonly canvasOuterHeight: number;
 
 	constructor(configuration: editorCommon.IConfiguration) {
-		const pixelRatio = browser.getPixelRatio();
+		const pixelRatio = configuration.editor.pixelRatio;
 		const layoutInfo = configuration.editor.layoutInfo;
 
 		this.renderMinimap = layoutInfo.renderMinimap | 0;

src/vs/editor/browser/viewParts/overviewRuler/decorationsOverviewRuler.ts

@@ -59,6 +59,7 @@ export class DecorationsOverviewRuler extends ViewPart {
 			this._context.viewLayout.getScrollHeight(),
 			this._context.configuration.editor.lineHeight,
 			this._context.configuration.editor.canUseTranslate3d,
+			this._context.configuration.editor.pixelRatio,
 			DecorationsOverviewRuler.MIN_DECORATION_HEIGHT,
 			DecorationsOverviewRuler.MAX_DECORATION_HEIGHT,
 			(lineNumber: number) => this._context.viewLayout.getVerticalOffsetForLineNumber(lineNumber)
@@ -116,6 +117,10 @@ export class DecorationsOverviewRuler extends ViewPart {
 			this._overviewRuler.setCanUseTranslate3d(this._context.configuration.editor.canUseTranslate3d, false);
 		}
 
+		if (e.pixelRatio) {
+			this._overviewRuler.setPixelRatio(this._context.configuration.editor.pixelRatio, false);
+		}
+
 		if (e.viewInfo) {
 			this._renderBorder = this._context.configuration.editor.viewInfo.overviewRulerBorder;
 			this._hideCursor = this._context.configuration.editor.viewInfo.hideCursorInOverviewRuler;

src/vs/editor/browser/viewParts/overviewRuler/overviewRuler.ts

@@ -26,6 +26,7 @@ export class OverviewRuler extends ViewEventHandler implements IOverviewRuler {
 			this._context.viewLayout.getScrollHeight(),
 			this._context.configuration.editor.lineHeight,
 			this._context.configuration.editor.canUseTranslate3d,
+			this._context.configuration.editor.pixelRatio,
 			minimumHeight,
 			maximumHeight,
 			(lineNumber: number) => this._context.viewLayout.getVerticalOffsetForLineNumber(lineNumber)
@@ -51,6 +52,10 @@ export class OverviewRuler extends ViewEventHandler implements IOverviewRuler {
 			this._overviewRuler.setCanUseTranslate3d(this._context.configuration.editor.canUseTranslate3d, true);
 		}
 
+		if (e.pixelRatio) {
+			this._overviewRuler.setPixelRatio(this._context.configuration.editor.pixelRatio, true);
+		}
+
 		return true;
 	}
 

src/vs/editor/browser/viewParts/overviewRuler/overviewRulerImpl.ts

@@ -7,7 +7,6 @@
 import { FastDomNode, createFastDomNode } from 'vs/base/browser/fastDomNode';
 import { OverviewRulerLane, ThemeType } from 'vs/editor/common/editorCommon';
 import { IDisposable } from 'vs/base/common/lifecycle';
-import * as browser from 'vs/base/browser/browser';
 import { OverviewZoneManager, ColorZone, OverviewRulerZone } from 'vs/editor/common/view/overviewZoneManager';
 import { Color } from 'vs/base/common/color';
 import { OverviewRulerPosition } from 'vs/editor/common/config/editorOptions';
@@ -23,7 +22,11 @@ export class OverviewRulerImpl {
 
 	private _zoomListener: IDisposable;
 
-	constructor(canvasLeftOffset: number, cssClassName: string, scrollHeight: number, lineHeight: number, canUseTranslate3d: boolean, minimumHeight: number, maximumHeight: number, getVerticalOffsetForLine: (lineNumber: number) => number) {
+	constructor(
+		canvasLeftOffset: number, cssClassName: string, scrollHeight: number, lineHeight: number,
+		canUseTranslate3d: boolean, pixelRatio: number, minimumHeight: number, maximumHeight: number,
+		getVerticalOffsetForLine: (lineNumber: number) => number
+	) {
 		this._canvasLeftOffset = canvasLeftOffset;
 
 		this._domNode = createFastDomNode(document.createElement('canvas'));
@@ -45,15 +48,7 @@ export class OverviewRulerImpl {
 		this._zoneManager.setOuterHeight(scrollHeight);
 		this._zoneManager.setLineHeight(lineHeight);
 
-		this._zoomListener = browser.onDidChangeZoomLevel(() => {
-			this._zoneManager.setPixelRatio(browser.getPixelRatio());
-			this._domNode.setWidth(this._zoneManager.getDOMWidth());
-			this._domNode.setHeight(this._zoneManager.getDOMHeight());
-			this._domNode.domNode.width = this._zoneManager.getCanvasWidth();
-			this._domNode.domNode.height = this._zoneManager.getCanvasHeight();
-			this.render(true);
-		});
-		this._zoneManager.setPixelRatio(browser.getPixelRatio());
+		this._zoneManager.setPixelRatio(pixelRatio);
 	}
 
 	public dispose(): void {
@@ -142,6 +137,17 @@ export class OverviewRulerImpl {
 		}
 	}
 
+	public setPixelRatio(pixelRatio: number, render: boolean): void {
+		this._zoneManager.setPixelRatio(pixelRatio);
+		this._domNode.setWidth(this._zoneManager.getDOMWidth());
+		this._domNode.setHeight(this._zoneManager.getDOMHeight());
+		this._domNode.domNode.width = this._zoneManager.getCanvasWidth();
+		this._domNode.domNode.height = this._zoneManager.getCanvasHeight();
+		if (render) {
+			this.render(true);
+		}
+	}
+
 	public setZones(zones: OverviewRulerZone[], render: boolean): void {
 		this._zoneManager.setZones(zones);
 		if (render) {

src/vs/editor/common/config/editorOptions.ts

@@ -746,6 +746,7 @@ export class InternalEditorOptions {
 	readonly _internalEditorOptionsBrand: void;
 
 	readonly canUseTranslate3d: boolean;
+	readonly pixelRatio: number;
 	readonly editorClassName: string;
 	readonly lineHeight: number;
 	readonly readOnly: boolean;
@@ -769,6 +770,7 @@ export class InternalEditorOptions {
 	 */
 	constructor(source: {
 		canUseTranslate3d: boolean;
+		pixelRatio: number;
 		editorClassName: string;
 		lineHeight: number;
 		readOnly: boolean;
@@ -783,15 +785,16 @@ export class InternalEditorOptions {
 		wrappingInfo: EditorWrappingInfo;
 		contribInfo: EditorContribOptions;
 	}) {
-		this.canUseTranslate3d = Boolean(source.canUseTranslate3d);
-		this.editorClassName = String(source.editorClassName);
+		this.canUseTranslate3d = source.canUseTranslate3d;
+		this.pixelRatio = source.pixelRatio;
+		this.editorClassName = source.editorClassName;
 		this.lineHeight = source.lineHeight | 0;
-		this.readOnly = Boolean(source.readOnly);
-		this.wordSeparators = String(source.wordSeparators);
-		this.autoClosingBrackets = Boolean(source.autoClosingBrackets);
-		this.useTabStops = Boolean(source.useTabStops);
-		this.tabFocusMode = Boolean(source.tabFocusMode);
-		this.dragAndDrop = Boolean(source.dragAndDrop);
+		this.readOnly = source.readOnly;
+		this.wordSeparators = source.wordSeparators;
+		this.autoClosingBrackets = source.autoClosingBrackets;
+		this.useTabStops = source.useTabStops;
+		this.tabFocusMode = source.tabFocusMode;
+		this.dragAndDrop = source.dragAndDrop;
 		this.layoutInfo = source.layoutInfo;
 		this.fontInfo = source.fontInfo;
 		this.viewInfo = source.viewInfo;
@@ -805,6 +808,7 @@ export class InternalEditorOptions {
 	public equals(other: InternalEditorOptions): boolean {
 		return (
 			this.canUseTranslate3d === other.canUseTranslate3d
+			&& this.pixelRatio === other.pixelRatio
 			&& this.editorClassName === other.editorClassName
 			&& this.lineHeight === other.lineHeight
 			&& this.readOnly === other.readOnly
@@ -827,6 +831,7 @@ export class InternalEditorOptions {
 	public createChangeEvent(newOpts: InternalEditorOptions): IConfigurationChangedEvent {
 		return {
 			canUseTranslate3d: (this.canUseTranslate3d !== newOpts.canUseTranslate3d),
+			pixelRatio: (this.pixelRatio !== newOpts.pixelRatio),
 			editorClassName: (this.editorClassName !== newOpts.editorClassName),
 			lineHeight: (this.lineHeight !== newOpts.lineHeight),
 			readOnly: (this.readOnly !== newOpts.readOnly),
@@ -1151,6 +1156,7 @@ export interface EditorLayoutInfo {
  */
 export interface IConfigurationChangedEvent {
 	readonly canUseTranslate3d: boolean;
+	readonly pixelRatio: boolean;
 	readonly editorClassName: boolean;
 	readonly lineHeight: boolean;
 	readonly readOnly: boolean;
@@ -1626,6 +1632,7 @@ export class InternalEditorOptionsFactory {
 
 		return new InternalEditorOptions({
 			canUseTranslate3d: opts.disableTranslate3d ? false : env.canUseTranslate3d,
+			pixelRatio: env.pixelRatio,
 			editorClassName: env.editorClassName,
 			lineHeight: env.fontInfo.lineHeight,
 			readOnly: opts.readOnly,

src/vs/editor/common/config/fontInfo.ts

@@ -178,11 +178,4 @@ export class FontInfo extends BareFontInfo {
 			&& this.maxDigitWidth === other.maxDigitWidth
 		);
 	}
-
-	/**
-	 * @internal
-	 */
-	public clone(): FontInfo {
-		return new FontInfo(this, this.isTrusted);
-	}
-}
+}

src/vs/editor/common/view/viewEvents.ts

@@ -35,6 +35,7 @@ export class ViewConfigurationChangedEvent {
 	public readonly type = ViewEventType.ViewConfigurationChanged;
 
 	public readonly canUseTranslate3d: boolean;
+	public readonly pixelRatio: boolean;
 	public readonly editorClassName: boolean;
 	public readonly lineHeight: boolean;
 	public readonly readOnly: boolean;
@@ -45,6 +46,7 @@ export class ViewConfigurationChangedEvent {
 
 	constructor(source: IConfigurationChangedEvent) {
 		this.canUseTranslate3d = source.canUseTranslate3d;
+		this.pixelRatio = source.pixelRatio;
 		this.editorClassName = source.editorClassName;
 		this.lineHeight = source.lineHeight;
 		this.readOnly = source.readOnly;

src/vs/monaco.d.ts

@@ -3222,6 +3222,7 @@ declare module monaco.editor {
 	export class InternalEditorOptions {
 		readonly _internalEditorOptionsBrand: void;
 		readonly canUseTranslate3d: boolean;
+		readonly pixelRatio: number;
 		readonly editorClassName: string;
 		readonly lineHeight: number;
 		readonly readOnly: boolean;
@@ -3350,6 +3351,7 @@ declare module monaco.editor {
 	 */
 	export interface IConfigurationChangedEvent {
 		readonly canUseTranslate3d: boolean;
+		readonly pixelRatio: boolean;
 		readonly editorClassName: boolean;
 		readonly lineHeight: boolean;
 		readonly readOnly: boolean;

src/vs/workbench/electron-browser/actions.ts

@@ -242,7 +242,10 @@ export abstract class BaseZoomAction extends Action {
 		const applyZoom = () => {
 			webFrame.setZoomLevel(level);
 			browser.setZoomFactor(webFrame.getZoomFactor());
-			browser.setZoomLevel(level); // Ensure others can listen to zoom level changes
+			// See https://github.com/Microsoft/vscode/issues/26151
+			// Cannot be trusted because the webFrame might take some time
+			// until it really applies the new zoom level
+			browser.setZoomLevel(webFrame.getZoomLevel(), /*isTrusted*/false);
 		};
 
 		this.configurationEditingService.writeConfiguration(target, { key: BaseZoomAction.SETTING_KEY, value: level }).done(() => applyZoom(), error => applyZoom());

src/vs/workbench/electron-browser/main.ts

@@ -57,7 +57,10 @@ export function startup(configuration: IWindowConfiguration): TPromise<void> {
 
 	// Ensure others can listen to zoom level changes
 	browser.setZoomFactor(webFrame.getZoomFactor());
-	browser.setZoomLevel(webFrame.getZoomLevel());
+	// See https://github.com/Microsoft/vscode/issues/26151
+	// Can be trusted because we are not setting it ourselves.
+	browser.setZoomLevel(webFrame.getZoomLevel(), /*isTrusted*/true);
+
 	browser.setFullscreen(!!configuration.fullscreen);
 
 	KeyboardMapperFactory.INSTANCE._onKeyboardLayoutChanged(configuration.isISOKeyboard);

src/vs/workbench/electron-browser/window.ts

@@ -321,7 +321,10 @@ export class ElectronWindow extends Themable {
 			if (webFrame.getZoomLevel() !== newZoomLevel) {
 				webFrame.setZoomLevel(newZoomLevel);
 				browser.setZoomFactor(webFrame.getZoomFactor());
-				browser.setZoomLevel(webFrame.getZoomLevel()); // Ensure others can listen to zoom level changes
+				// See https://github.com/Microsoft/vscode/issues/26151
+				// Cannot be trusted because the webFrame might take some time
+				// until it really applies the new zoom level
+				browser.setZoomLevel(webFrame.getZoomLevel(), /*isTrusted*/false);
 			}
 		});