send requests per provider so that a hanging provider doesn't block o…

…ther providers, #100524

src/vs/workbench/api/browser/mainThreadDecorations.ts

@@ -9,33 +9,33 @@ import { IDisposable, dispose } from 'vs/base/common/lifecycle';
 import { ExtHostContext, MainContext, IExtHostContext, MainThreadDecorationsShape, ExtHostDecorationsShape, DecorationData, DecorationRequest } from '../common/extHost.protocol';
 import { extHostNamedCustomer } from 'vs/workbench/api/common/extHostCustomers';
 import { IDecorationsService, IDecorationData } from 'vs/workbench/services/decorations/browser/decorations';
-import { values } from 'vs/base/common/collections';
 import { CancellationToken } from 'vs/base/common/cancellation';
 
 class DecorationRequestsQueue {
 
 	private _idPool = 0;
-	private _requests: { [id: number]: DecorationRequest } = Object.create(null);
-	private _resolver: { [id: number]: (data: DecorationData) => any } = Object.create(null);
+	private _requests = new Map<number, DecorationRequest>();
+	private _resolver = new Map<number, (data: DecorationData) => any>();
 
 	private _timer: any;
 
 	constructor(
-		private readonly _proxy: ExtHostDecorationsShape
+		private readonly _proxy: ExtHostDecorationsShape,
+		private readonly _handle: number
 	) {
 		//
 	}
 
-	enqueue(handle: number, uri: URI, token: CancellationToken): Promise<DecorationData> {
+	enqueue(uri: URI, token: CancellationToken): Promise<DecorationData> {
 		const id = ++this._idPool;
 		const result = new Promise<DecorationData>(resolve => {
-			this._requests[id] = { id, handle, uri };
-			this._resolver[id] = resolve;
+			this._requests.set(id, { id, uri });
+			this._resolver.set(id, resolve);
 			this._processQueue();
 		});
 		token.onCancellationRequested(() => {
-			delete this._requests[id];
-			delete this._resolver[id];
+			this._requests.delete(id);
+			this._resolver.delete(id);
 		});
 		return result;
 	}
@@ -49,15 +49,15 @@ class DecorationRequestsQueue {
 			// make request
 			const requests = this._requests;
 			const resolver = this._resolver;
-			this._proxy.$provideDecorations(values(requests), CancellationToken.None).then(data => {
+			this._proxy.$provideDecorations(this._handle, [...requests.values()], CancellationToken.None).then(data => {
 				for (const id in resolver) {
-					resolver[id](data[id]);
+					resolver.get(Number(id))!(data[Number(id)]);
 				}
 			});
 
 			// reset
-			this._requests = [];
-			this._resolver = [];
+			this._requests = new Map();
+			this._resolver = new Map();
 			this._timer = undefined;
 		}, 0);
 	}
@@ -68,14 +68,12 @@ export class MainThreadDecorations implements MainThreadDecorationsShape {
 
 	private readonly _provider = new Map<number, [Emitter<URI[]>, IDisposable]>();
 	private readonly _proxy: ExtHostDecorationsShape;
-	private readonly _requestQueue: DecorationRequestsQueue;
 
 	constructor(
 		context: IExtHostContext,
 		@IDecorationsService private readonly _decorationsService: IDecorationsService
 	) {
 		this._proxy = context.getProxy(ExtHostContext.ExtHostDecorations);
-		this._requestQueue = new DecorationRequestsQueue(this._proxy);
 	}
 
 	dispose() {
@@ -85,23 +83,23 @@ export class MainThreadDecorations implements MainThreadDecorationsShape {
 
 	$registerDecorationProvider(handle: number, label: string): void {
 		const emitter = new Emitter<URI[]>();
+		const queue = new DecorationRequestsQueue(this._proxy, handle);
 		const registration = this._decorationsService.registerDecorationsProvider({
 			label,
 			onDidChange: emitter.event,
-			provideDecorations: (uri, token) => {
-				return this._requestQueue.enqueue(handle, uri, token).then(data => {
-					if (!data) {
-						return undefined;
-					}
-					const [weight, bubble, tooltip, letter, themeColor] = data;
-					return <IDecorationData>{
-						weight: weight || 0,
-						bubble: bubble || false,
-						color: themeColor && themeColor.id,
-						tooltip,
-						letter
-					};
-				});
+			provideDecorations: async (uri, token) => {
+				const data = await queue.enqueue(uri, token);
+				if (!data) {
+					return undefined;
+				}
+				const [weight, bubble, tooltip, letter, themeColor] = data;
+				return <IDecorationData>{
+					weight: weight ?? 0,
+					bubble: bubble ?? false,
+					color: themeColor?.id,
+					tooltip,
+					letter
+				};
 			}
 		});
 		this._provider.set(handle, [emitter, registration]);

src/vs/workbench/api/common/extHost.protocol.ts

@@ -1525,15 +1525,14 @@ export interface ExtHostDebugServiceShape {
 
 export interface DecorationRequest {
 	readonly id: number;
-	readonly handle: number;
 	readonly uri: UriComponents;
 }
 
 export type DecorationData = [number, boolean, string, string, ThemeColor];
 export type DecorationReply = { [id: number]: DecorationData; };
 
 export interface ExtHostDecorationsShape {
-	$provideDecorations(requests: DecorationRequest[], token: CancellationToken): Promise<DecorationReply>;
+	$provideDecorations(handle: number, requests: DecorationRequest[], token: CancellationToken): Promise<DecorationReply>;
 }
 
 export interface ExtHostWindowShape {

src/vs/workbench/api/common/extHostDecorations.ts

@@ -52,17 +52,20 @@ export class ExtHostDecorations implements IExtHostDecorations {
 		});
 	}
 
-	$provideDecorations(requests: DecorationRequest[], token: CancellationToken): Promise<DecorationReply> {
+	async $provideDecorations(handle: number, requests: DecorationRequest[], token: CancellationToken): Promise<DecorationReply> {
+
+		if (!this._provider.has(handle)) {
+			// might have been unregistered in the meantime
+			return Object.create(null);
+		}
+
 		const result: DecorationReply = Object.create(null);
-		return Promise.all(requests.map(request => {
-			const { handle, uri, id } = request;
-			const entry = this._provider.get(handle);
-			if (!entry) {
-				// might have been unregistered in the meantime
-				return undefined;
-			}
-			const { provider, extensionId } = entry;
-			return Promise.resolve(provider.provideDecoration(URI.revive(uri), token)).then(data => {
+		const { provider, extensionId } = this._provider.get(handle)!;
+
+		await Promise.all(requests.map(async request => {
+			try {
+				const { uri, id } = request;
+				const data = await Promise.resolve(provider.provideDecoration(URI.revive(uri), token));
 				if (!data) {
 					return;
 				}
@@ -72,13 +75,12 @@ export class ExtHostDecorations implements IExtHostDecorations {
 				} catch (e) {
 					this._logService.warn(`INVALID decoration from extension '${extensionId.value}': ${e}`);
 				}
-			}, err => {
+			} catch (err) {
 				this._logService.error(err);
-			});
+			}
+		}));
 
-		})).then(() => {
-			return result;
-		});
+		return result;
 	}
 }
 

src/vs/workbench/test/browser/api/extHostDecorations.test.ts

@@ -0,0 +1,85 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import * as assert from 'assert';
+import { timeout } from 'vs/base/common/async';
+import { CancellationToken } from 'vs/base/common/cancellation';
+import { Event } from 'vs/base/common/event';
+import { URI } from 'vs/base/common/uri';
+import { mock } from 'vs/base/test/common/mock';
+import { NullLogService } from 'vs/platform/log/common/log';
+import { MainThreadDecorationsShape } from 'vs/workbench/api/common/extHost.protocol';
+import { ExtHostDecorations } from 'vs/workbench/api/common/extHostDecorations';
+import { IExtHostRpcService } from 'vs/workbench/api/common/extHostRpcService';
+import { nullExtensionDescription } from 'vs/workbench/services/extensions/common/extensions';
+
+suite('ExtHostDecorations', function () {
+
+	let mainThreadShape: MainThreadDecorationsShape;
+	let extHostDecorations: ExtHostDecorations;
+	let providers = new Set<number>();
+
+	setup(function () {
+
+		providers.clear();
+
+		mainThreadShape = new class extends mock<MainThreadDecorationsShape>() {
+			$registerDecorationProvider(handle: number) {
+				providers.add(handle);
+			}
+		};
+
+		extHostDecorations = new ExtHostDecorations(
+			new class extends mock<IExtHostRpcService>() {
+				getProxy(): any {
+					return mainThreadShape;
+				}
+			},
+			new NullLogService()
+		);
+	});
+
+	test('SCM Decorations missing #100524', async function () {
+
+		let calledA = false;
+		let calledB = false;
+
+		// never returns
+		extHostDecorations.registerDecorationProvider({
+			onDidChangeDecorations: Event.None,
+			provideDecoration() {
+				calledA = true;
+				return new Promise(() => { });
+			}
+		}, nullExtensionDescription.identifier);
+
+		// always returns
+		extHostDecorations.registerDecorationProvider({
+			onDidChangeDecorations: Event.None,
+			provideDecoration() {
+				calledB = true;
+				return new Promise(resolve => resolve({ letter: 'H', title: 'Hello' }));
+			}
+		}, nullExtensionDescription.identifier);
+
+
+		const requests = [...providers.values()].map((handle, idx) => {
+			return extHostDecorations.$provideDecorations(handle, [{ id: idx, uri: URI.parse('test:///file') }], CancellationToken.None);
+		});
+
+		assert.equal(calledA, true);
+		assert.equal(calledB, true);
+
+		assert.equal(requests.length, 2);
+		const [first, second] = requests;
+
+		const firstResult = await Promise.race([first, timeout(30).then(() => false)]);
+		assert.equal(typeof firstResult, 'boolean'); // never finishes...
+
+		const secondResult = await Promise.race([second, timeout(30).then(() => false)]);
+		assert.equal(typeof secondResult, 'object');
+	});
+
+});