Skip to content

Commit 996b733

Browse files
mjbvzmjbvz
authored
Adopt ensureNoDisposablesAreLeakedInTestSuite (#192221)
For #190503
1 parent 4de3a2c commit 996b733

File tree

5 files changed

+100
-68
lines changed

5 files changed

+100
-68
lines changed

‎src/vs/editor/contrib/parameterHints/test/browser/parameterHintsModel.test.ts

Lines changed: 25 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { CancellationToken } from 'vs/base/common/cancellation';
88
import { DisposableStore } from 'vs/base/common/lifecycle';
99
import { URI } from 'vs/base/common/uri';
1010
import { runWithFakedTimers } from 'vs/base/test/common/timeTravelScheduler';
11+
import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';
1112
import { Position } from 'vs/editor/common/core/position';
1213
import { Handler } from 'vs/editor/common/editorCommon';
1314
import { LanguageFeatureRegistry } from 'vs/editor/common/languageFeatureRegistry';
@@ -41,8 +42,7 @@ const emptySigHelpResult: languages.SignatureHelpResult = {
4142

4243
suite('ParameterHintsModel', () => {
4344
const disposables = new DisposableStore();
44-
45-
let registry = new LanguageFeatureRegistry<languages.SignatureHelpProvider>();
45+
let registry: LanguageFeatureRegistry<languages.SignatureHelpProvider>;
4646

4747
setup(() => {
4848
disposables.clear();
@@ -53,19 +53,28 @@ suite('ParameterHintsModel', () => {
5353
disposables.clear();
5454
});
5555

56+
ensureNoDisposablesAreLeakedInTestSuite();
57+
5658
function createMockEditor(fileContents: string) {
57-
const textModel = createTextModel(fileContents, undefined, undefined, mockFile);
58-
const editor = createTestCodeEditor(textModel, {
59+
const textModel = disposables.add(createTextModel(fileContents, undefined, undefined, mockFile));
60+
const editor = disposables.add(createTestCodeEditor(textModel, {
5961
serviceCollection: new ServiceCollection(
6062
[ITelemetryService, NullTelemetryService],
61-
[IStorageService, new InMemoryStorageService()]
63+
[IStorageService, disposables.add(new InMemoryStorageService())]
6264
)
63-
});
64-
disposables.add(textModel);
65-
disposables.add(editor);
65+
}));
6666
return editor;
6767
}
6868

69+
function getNextHint(model: ParameterHintsModel) {
70+
return new Promise<languages.SignatureHelpResult | undefined>(resolve => {
71+
const sub = disposables.add(model.onChangedHints(e => {
72+
sub.dispose();
73+
return resolve(e ? { value: e, dispose: () => { } } : undefined);
74+
}));
75+
});
76+
}
77+
6978
test('Provider should get trigger character on type', async () => {
7079
let done: () => void;
7180
const donePromise = new Promise<void>(resolve => { done = resolve; });
@@ -148,8 +157,7 @@ suite('ParameterHintsModel', () => {
148157
const triggerChar = '(';
149158

150159
const editor = createMockEditor('');
151-
const hintModel = new ParameterHintsModel(editor, registry);
152-
disposables.add(hintModel);
160+
const hintModel = disposables.add(new ParameterHintsModel(editor, registry));
153161

154162
let invokeCount = 0;
155163
disposables.add(registry.register(mockFileSelector, new class implements languages.SignatureHelpProvider {
@@ -281,7 +289,7 @@ suite('ParameterHintsModel', () => {
281289
test('Should cancel existing request when new request comes in', async () => {
282290

283291
const editor = createMockEditor('abc def');
284-
const hintsModel = new ParameterHintsModel(editor, registry);
292+
const hintsModel = disposables.add(new ParameterHintsModel(editor, registry));
285293

286294
let didRequestCancellationOf = -1;
287295
let invokeCount = 0;
@@ -293,7 +301,7 @@ suite('ParameterHintsModel', () => {
293301
provideSignatureHelp(_model: ITextModel, _position: Position, token: CancellationToken): languages.SignatureHelpResult | Promise<languages.SignatureHelpResult> {
294302
try {
295303
const count = invokeCount++;
296-
token.onCancellationRequested(() => { didRequestCancellationOf = count; });
304+
disposables.add(token.onCancellationRequested(() => { didRequestCancellationOf = count; }));
297305

298306
// retrigger on first request
299307
if (count === 0) {
@@ -330,15 +338,15 @@ suite('ParameterHintsModel', () => {
330338
assert.strictEqual(-1, didRequestCancellationOf);
331339

332340
return new Promise<void>((resolve, reject) =>
333-
hintsModel.onChangedHints(newParamterHints => {
341+
disposables.add(hintsModel.onChangedHints(newParamterHints => {
334342
try {
335343
assert.strictEqual(0, didRequestCancellationOf);
336344
assert.strictEqual('1', newParamterHints!.signatures[0].label);
337345
resolve();
338346
} catch (e) {
339347
reject(e);
340348
}
341-
}));
349+
})));
342350
});
343351
});
344352

@@ -401,8 +409,7 @@ suite('ParameterHintsModel', () => {
401409
const paramterLabel = 'parameter';
402410

403411
const editor = createMockEditor('');
404-
const model = new ParameterHintsModel(editor, registry, 5);
405-
disposables.add(model);
412+
const model = disposables.add(new ParameterHintsModel(editor, registry, 5));
406413

407414
disposables.add(registry.register(mockFileSelector, new class implements languages.SignatureHelpProvider {
408415
signatureHelpTriggerCharacters = [triggerChar];
@@ -477,8 +484,7 @@ suite('ParameterHintsModel', () => {
477484

478485
test('Quick typing should use the first trigger character', async () => {
479486
const editor = createMockEditor('');
480-
const model = new ParameterHintsModel(editor, registry, 50);
481-
disposables.add(model);
487+
const model = disposables.add(new ParameterHintsModel(editor, registry, 50));
482488

483489
const triggerCharacter = 'a';
484490

@@ -519,8 +525,7 @@ suite('ParameterHintsModel', () => {
519525
const donePromise = new Promise<void>(resolve => { done = resolve; });
520526

521527
const editor = createMockEditor('');
522-
const model = new ParameterHintsModel(editor, registry, 50);
523-
disposables.add(model);
528+
const model = disposables.add(new ParameterHintsModel(editor, registry, 50));
524529

525530
const triggerCharacter = 'a';
526531
const retriggerCharacter = 'b';
@@ -570,12 +575,3 @@ suite('ParameterHintsModel', () => {
570575
});
571576
});
572577
});
573-
574-
function getNextHint(model: ParameterHintsModel) {
575-
return new Promise<languages.SignatureHelpResult | undefined>(resolve => {
576-
const sub = model.onChangedHints(e => {
577-
sub.dispose();
578-
return resolve(e ? { value: e, dispose: () => { } } : undefined);
579-
});
580-
});
581-
}

‎src/vs/workbench/api/common/extHostWebview.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,17 @@
77

88
import { VSBuffer } from 'vs/base/common/buffer';
99
import { Emitter, Event } from 'vs/base/common/event';
10+
import { Disposable } from 'vs/base/common/lifecycle';
1011
import { Schemas } from 'vs/base/common/network';
1112
import * as objects from 'vs/base/common/objects';
1213
import { URI } from 'vs/base/common/uri';
13-
import { IExtensionDescription } from 'vs/platform/extensions/common/extensions';
1414
import { normalizeVersion, parseVersion } from 'vs/platform/extensions/common/extensionValidator';
15+
import { IExtensionDescription } from 'vs/platform/extensions/common/extensions';
1516
import { ILogService } from 'vs/platform/log/common/log';
1617
import { IExtHostApiDeprecationService } from 'vs/workbench/api/common/extHostApiDeprecationService';
1718
import { deserializeWebviewMessage, serializeWebviewMessage } from 'vs/workbench/api/common/extHostWebviewMessaging';
1819
import { IExtHostWorkspace } from 'vs/workbench/api/common/extHostWorkspace';
19-
import { asWebviewUri, webviewGenericCspSource, WebviewRemoteInfo } from 'vs/workbench/contrib/webview/common/webview';
20+
import { WebviewRemoteInfo, asWebviewUri, webviewGenericCspSource } from 'vs/workbench/contrib/webview/common/webview';
2021
import { SerializableObjectWithBuffers } from 'vs/workbench/services/extensions/common/proxyIdentifier';
2122
import type * as vscode from 'vscode';
2223
import * as extHostProtocol from './extHost.protocol';
@@ -190,7 +191,7 @@ function shouldTryRewritingOldResourceUris(extension: IExtensionDescription): bo
190191
}
191192
}
192193

193-
export class ExtHostWebviews implements extHostProtocol.ExtHostWebviewsShape {
194+
export class ExtHostWebviews extends Disposable implements extHostProtocol.ExtHostWebviewsShape {
194195

195196
private readonly _webviewProxy: extHostProtocol.MainThreadWebviewsShape;
196197

@@ -203,9 +204,19 @@ export class ExtHostWebviews implements extHostProtocol.ExtHostWebviewsShape {
203204
private readonly _logService: ILogService,
204205
private readonly _deprecationService: IExtHostApiDeprecationService,
205206
) {
207+
super();
206208
this._webviewProxy = mainContext.getProxy(extHostProtocol.MainContext.MainThreadWebviews);
207209
}
208210

211+
public override dispose(): void {
212+
super.dispose();
213+
214+
for (const webview of this._webviews.values()) {
215+
webview.dispose();
216+
}
217+
this._webviews.clear();
218+
}
219+
209220
public $onMessage(
210221
handle: extHostProtocol.WebviewHandle,
211222
jsonMessage: string,
@@ -229,7 +240,10 @@ export class ExtHostWebviews implements extHostProtocol.ExtHostWebviewsShape {
229240
const webview = new ExtHostWebview(handle, this._webviewProxy, reviveOptions(options), this.remoteInfo, this.workspace, extension, this._deprecationService);
230241
this._webviews.set(handle, webview);
231242

232-
webview._onDidDispose(() => { this._webviews.delete(handle); });
243+
const sub = webview._onDidDispose(() => {
244+
sub.dispose();
245+
this.deleteWebview(handle);
246+
});
233247

234248
return webview;
235249
}

‎src/vs/workbench/api/common/extHostWebviewPanels.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ class ExtHostWebviewPanel extends Disposable implements vscode.WebviewPanel {
169169
}
170170
}
171171

172-
export class ExtHostWebviewPanels implements extHostProtocol.ExtHostWebviewPanelsShape {
172+
export class ExtHostWebviewPanels extends Disposable implements extHostProtocol.ExtHostWebviewPanelsShape {
173173

174174
private static newHandle(): extHostProtocol.WebviewHandle {
175175
return generateUuid();
@@ -189,9 +189,17 @@ export class ExtHostWebviewPanels implements extHostProtocol.ExtHostWebviewPanel
189189
private readonly webviews: ExtHostWebviews,
190190
private readonly workspace: IExtHostWorkspace | undefined,
191191
) {
192+
super();
192193
this._proxy = mainContext.getProxy(extHostProtocol.MainContext.MainThreadWebviewPanels);
193194
}
194195

196+
public override dispose(): void {
197+
super.dispose();
198+
199+
this._webviewPanels.forEach(value => value.dispose());
200+
this._webviewPanels.clear();
201+
}
202+
195203
public createWebviewPanel(
196204
extension: IExtensionDescription,
197205
viewType: string,

‎src/vs/workbench/api/test/browser/extHostWebview.test.ts

Lines changed: 37 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -4,43 +4,71 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import * as assert from 'assert';
7+
import { DisposableStore } from 'vs/base/common/lifecycle';
78
import { Schemas } from 'vs/base/common/network';
89
import { URI } from 'vs/base/common/uri';
910
import { mock } from 'vs/base/test/common/mock';
11+
import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';
1012
import { IExtensionDescription } from 'vs/platform/extensions/common/extensions';
1113
import { NullLogService } from 'vs/platform/log/common/log';
1214
import { MainThreadWebviewManager } from 'vs/workbench/api/browser/mainThreadWebviewManager';
13-
import { IExtHostContext } from 'vs/workbench/services/extensions/common/extHostCustomers';
1415
import { NullApiDeprecationService } from 'vs/workbench/api/common/extHostApiDeprecationService';
1516
import { IExtHostRpcService } from 'vs/workbench/api/common/extHostRpcService';
1617
import { ExtHostWebviews } from 'vs/workbench/api/common/extHostWebview';
1718
import { ExtHostWebviewPanels } from 'vs/workbench/api/common/extHostWebviewPanels';
19+
import { SingleProxyRPCProtocol } from 'vs/workbench/api/test/common/testRPCProtocol';
1820
import { decodeAuthority, webviewResourceBaseHost } from 'vs/workbench/contrib/webview/common/webview';
1921
import { EditorGroupColumn } from 'vs/workbench/services/editor/common/editorGroupColumn';
22+
import { IExtHostContext } from 'vs/workbench/services/extensions/common/extHostCustomers';
2023
import type * as vscode from 'vscode';
21-
import { SingleProxyRPCProtocol } from 'vs/workbench/api/test/common/testRPCProtocol';
2224

2325
suite('ExtHostWebview', () => {
24-
26+
let disposables: DisposableStore;
2527
let rpcProtocol: (IExtHostRpcService & IExtHostContext) | undefined;
2628

2729
setup(() => {
30+
disposables = new DisposableStore();
31+
2832
const shape = createNoopMainThreadWebviews();
2933
rpcProtocol = SingleProxyRPCProtocol(shape);
3034
});
3135

36+
teardown(() => {
37+
disposables.dispose();
38+
});
39+
40+
ensureNoDisposablesAreLeakedInTestSuite();
41+
42+
function createWebview(rpcProtocol: (IExtHostRpcService & IExtHostContext) | undefined, remoteAuthority: string | undefined) {
43+
const extHostWebviews = disposables.add(new ExtHostWebviews(rpcProtocol!, {
44+
authority: remoteAuthority,
45+
isRemote: !!remoteAuthority,
46+
}, undefined, new NullLogService(), NullApiDeprecationService));
47+
48+
const extHostWebviewPanels = disposables.add(new ExtHostWebviewPanels(rpcProtocol!, extHostWebviews, undefined));
49+
50+
return disposables.add(extHostWebviewPanels.createWebviewPanel({
51+
extensionLocation: URI.from({
52+
scheme: remoteAuthority ? Schemas.vscodeRemote : Schemas.file,
53+
authority: remoteAuthority,
54+
path: '/ext/path',
55+
})
56+
} as IExtensionDescription, 'type', 'title', 1, {}));
57+
}
58+
3259
test('Cannot register multiple serializers for the same view type', async () => {
3360
const viewType = 'view.type';
3461

35-
const extHostWebviews = new ExtHostWebviews(rpcProtocol!, { authority: undefined, isRemote: false }, undefined, new NullLogService(), NullApiDeprecationService);
62+
const extHostWebviews = disposables.add(new ExtHostWebviews(rpcProtocol!, { authority: undefined, isRemote: false }, undefined, new NullLogService(), NullApiDeprecationService));
3663

37-
const extHostWebviewPanels = new ExtHostWebviewPanels(rpcProtocol!, extHostWebviews, undefined);
64+
const extHostWebviewPanels = disposables.add(new ExtHostWebviewPanels(rpcProtocol!, extHostWebviews, undefined));
3865

3966
let lastInvokedDeserializer: vscode.WebviewPanelSerializer | undefined = undefined;
4067

4168
class NoopSerializer implements vscode.WebviewPanelSerializer {
42-
async deserializeWebviewPanel(_webview: vscode.WebviewPanel, _state: any): Promise<void> {
69+
async deserializeWebviewPanel(webview: vscode.WebviewPanel, _state: any): Promise<void> {
4370
lastInvokedDeserializer = this;
71+
disposables.add(webview);
4472
}
4573
}
4674

@@ -61,12 +89,12 @@ suite('ExtHostWebview', () => {
6189
assert.strictEqual(lastInvokedDeserializer, serializerA);
6290

6391
assert.throws(
64-
() => extHostWebviewPanels.registerWebviewPanelSerializer(extension, viewType, serializerB),
92+
() => disposables.add(extHostWebviewPanels.registerWebviewPanelSerializer(extension, viewType, serializerB)),
6593
'Should throw when registering two serializers for the same view');
6694

6795
serializerARegistration.dispose();
6896

69-
extHostWebviewPanels.registerWebviewPanelSerializer(extension, viewType, serializerB);
97+
disposables.add(extHostWebviewPanels.registerWebviewPanelSerializer(extension, viewType, serializerB));
7098

7199
await extHostWebviewPanels.$deserializeWebviewPanel('x', viewType, {
72100
title: 'title',
@@ -169,30 +197,12 @@ suite('ExtHostWebview', () => {
169197
});
170198
});
171199

172-
function createWebview(rpcProtocol: (IExtHostRpcService & IExtHostContext) | undefined, remoteAuthority: string | undefined) {
173-
const extHostWebviews = new ExtHostWebviews(rpcProtocol!, {
174-
authority: remoteAuthority,
175-
isRemote: !!remoteAuthority,
176-
}, undefined, new NullLogService(), NullApiDeprecationService);
177-
178-
const extHostWebviewPanels = new ExtHostWebviewPanels(rpcProtocol!, extHostWebviews, undefined);
179-
180-
const webview = extHostWebviewPanels.createWebviewPanel({
181-
extensionLocation: URI.from({
182-
scheme: remoteAuthority ? Schemas.vscodeRemote : Schemas.file,
183-
authority: remoteAuthority,
184-
path: '/ext/path',
185-
})
186-
} as IExtensionDescription, 'type', 'title', 1, {});
187-
return webview;
188-
}
189-
190200

191201
function createNoopMainThreadWebviews() {
192202
return new class extends mock<MainThreadWebviewManager>() {
203+
$disposeWebview() { /* noop */ }
193204
$createWebviewPanel() { /* noop */ }
194205
$registerSerializer() { /* noop */ }
195206
$unregisterSerializer() { /* noop */ }
196207
};
197208
}
198-

0 commit comments

Comments
 (0)