voice - avoid cancellation token and disposable at the same time (#20…

…5532) * voice - avoid cancellation token and disposable at the same time * fix tests * . * .
microsoft · Feb 19, 2024 · d83cc26 · d83cc26
1 parent 26fdfc6
commit d83cc26
Show file tree

Hide file tree

Showing 8 changed files with 46 additions and 39 deletions.
diff --git a/src/vs/workbench/api/browser/mainThreadSpeech.ts b/src/vs/workbench/api/browser/mainThreadSpeech.ts
@@ -3,8 +3,7 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { CancellationTokenSource } from 'vs/base/common/cancellation';
-import { Emitter } from 'vs/base/common/event';
+import { Emitter, Event } from 'vs/base/common/event';
 import { DisposableStore, IDisposable } from 'vs/base/common/lifecycle';
 import { ILogService } from 'vs/platform/log/common/log';
 import { ExtHostContext, ExtHostSpeechShape, MainContext, MainThreadSpeechShape } from 'vs/workbench/api/common/extHost.protocol';
@@ -43,43 +42,53 @@ export class MainThreadSpeech implements MainThreadSpeechShape {
 		const registration = this.speechService.registerSpeechProvider(identifier, {
 			metadata,
 			createSpeechToTextSession: token => {
+				if (token.isCancellationRequested) {
+					return {
+						onDidChange: Event.None
+					};
+				}
+
 				const disposables = new DisposableStore();
-				const cts = new CancellationTokenSource(token);
 				const session = Math.random();
 
 				this.proxy.$createSpeechToTextSession(handle, session);
-				disposables.add(token.onCancellationRequested(() => this.proxy.$cancelSpeechToTextSession(session)));
 
 				const onDidChange = disposables.add(new Emitter<ISpeechToTextEvent>());
 				this.speechToTextSessions.set(session, { onDidChange });
 
+				disposables.add(token.onCancellationRequested(() => {
+					this.proxy.$cancelSpeechToTextSession(session);
+					this.speechToTextSessions.delete(session);
+					disposables.dispose();
+				}));
+
 				return {
-					onDidChange: onDidChange.event,
-					dispose: () => {
-						cts.dispose(true);
-						this.speechToTextSessions.delete(session);
-						disposables.dispose();
-					}
+					onDidChange: onDidChange.event
 				};
 			},
 			createKeywordRecognitionSession: token => {
+				if (token.isCancellationRequested) {
+					return {
+						onDidChange: Event.None
+					};
+				}
+
 				const disposables = new DisposableStore();
-				const cts = new CancellationTokenSource(token);
 				const session = Math.random();
 
 				this.proxy.$createKeywordRecognitionSession(handle, session);
-				disposables.add(token.onCancellationRequested(() => this.proxy.$cancelKeywordRecognitionSession(session)));
 
 				const onDidChange = disposables.add(new Emitter<IKeywordRecognitionEvent>());
 				this.keywordRecognitionSessions.set(session, { onDidChange });
 
+				disposables.add(token.onCancellationRequested(() => {
+					this.proxy.$cancelKeywordRecognitionSession(session);
+					this.keywordRecognitionSessions.delete(session);
+					disposables.dispose();
+				}));
+
 				return {
-					onDidChange: onDidChange.event,
-					dispose: () => {
-						cts.dispose(true);
-						this.keywordRecognitionSessions.delete(session);
-						disposables.dispose();
-					}
+					onDidChange: onDidChange.event
 				};
 			}
 		});

diff --git a/src/vs/workbench/contrib/chat/common/voiceChat.ts b/src/vs/workbench/contrib/chat/common/voiceChat.ts
@@ -5,7 +5,7 @@
 
 import { CancellationToken } from 'vs/base/common/cancellation';
 import { Emitter, Event } from 'vs/base/common/event';
-import { Disposable, DisposableStore, IDisposable } from 'vs/base/common/lifecycle';
+import { Disposable, DisposableStore } from 'vs/base/common/lifecycle';
 import { rtrim } from 'vs/base/common/strings';
 import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
 import { IChatAgentService } from 'vs/workbench/contrib/chat/common/chatAgents';
@@ -41,7 +41,7 @@ export interface IVoiceChatTextEvent extends ISpeechToTextEvent {
 	readonly waitingForInput?: boolean;
 }
 
-export interface IVoiceChatSession extends IDisposable {
+export interface IVoiceChatSession {
 	readonly onDidChange: Event<IVoiceChatTextEvent>;
 }
 
@@ -131,12 +131,13 @@ export class VoiceChatService extends Disposable implements IVoiceChatService {
 
 	createVoiceChatSession(token: CancellationToken, options: IVoiceChatSessionOptions): IVoiceChatSession {
 		const disposables = new DisposableStore();
+		disposables.add(token.onCancellationRequested(() => disposables.dispose()));
 
 		let detectedAgent = false;
 		let detectedSlashCommand = false;
 
 		const emitter = disposables.add(new Emitter<IVoiceChatTextEvent>());
-		const session = disposables.add(this.speechService.createSpeechToTextSession(token, 'chat'));
+		const session = this.speechService.createSpeechToTextSession(token, 'chat');
 		disposables.add(session.onDidChange(e => {
 			switch (e.status) {
 				case SpeechToTextStatus.Recognizing:
@@ -212,8 +213,7 @@ export class VoiceChatService extends Disposable implements IVoiceChatService {
 		}));
 
 		return {
-			onDidChange: emitter.event,
-			dispose: () => disposables.dispose()
+			onDidChange: emitter.event
 		};
 	}
 

diff --git a/src/vs/workbench/contrib/chat/electron-sandbox/actions/voiceChatActions.ts b/src/vs/workbench/contrib/chat/electron-sandbox/actions/voiceChatActions.ts
@@ -292,7 +292,7 @@ class VoiceChatSessions {
 
 		this.voiceChatGettingReadyKey.set(true);
 
-		const voiceChatSession = session.disposables.add(this.voiceChatService.createVoiceChatSession(cts.token, { usesAgents: controller.context !== 'inline' }));
+		const voiceChatSession = this.voiceChatService.createVoiceChatSession(cts.token, { usesAgents: controller.context !== 'inline' });
 
 		let inputValue = controller.getInput();
 

diff --git a/src/vs/workbench/contrib/chat/test/common/voiceChat.test.ts b/src/vs/workbench/contrib/chat/test/common/voiceChat.test.ts
@@ -4,10 +4,10 @@
  *--------------------------------------------------------------------------------------------*/
 
 import * as assert from 'assert';
-import { CancellationToken } from 'vs/base/common/cancellation';
+import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cancellation';
 import { Emitter, Event } from 'vs/base/common/event';
 import { IMarkdownString } from 'vs/base/common/htmlContent';
-import { DisposableStore, IDisposable } from 'vs/base/common/lifecycle';
+import { DisposableStore, IDisposable, toDisposable } from 'vs/base/common/lifecycle';
 import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';
 import { ProviderResult } from 'vs/editor/common/languages';
 import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions';
@@ -74,8 +74,7 @@ suite('VoiceChat', () => {
 
 		createSpeechToTextSession(token: CancellationToken): ISpeechToTextSession {
 			return {
-				onDidChange: emitter.event,
-				dispose: () => { }
+				onDidChange: emitter.event
 			};
 		}
 
@@ -89,12 +88,11 @@ suite('VoiceChat', () => {
 
 	let service: VoiceChatService;
 	let event: IVoiceChatTextEvent | undefined;
-	let session: ISpeechToTextSession | undefined;
 
 	function createSession(options: IVoiceChatSessionOptions) {
-		session?.dispose();
-
-		session = disposables.add(service.createVoiceChatSession(CancellationToken.None, options));
+		const cts = new CancellationTokenSource();
+		disposables.add(toDisposable(() => cts.dispose(true)));
+		const session = service.createVoiceChatSession(cts.token, options);
 		disposables.add(session.onDidChange(e => {
 			event = e;
 		}));

diff --git a/src/vs/workbench/contrib/codeEditor/browser/dictation/editorDictation.ts b/src/vs/workbench/contrib/codeEditor/browser/dictation/editorDictation.ts
@@ -231,7 +231,7 @@ export class EditorDictation extends Disposable implements IEditorContribution {
 		const cts = new CancellationTokenSource();
 		disposables.add(toDisposable(() => cts.dispose(true)));
 
-		const session = disposables.add(this.speechService.createSpeechToTextSession(cts.token));
+		const session = this.speechService.createSpeechToTextSession(cts.token);
 		disposables.add(session.onDidChange(e => {
 			if (cts.token.isCancellationRequested) {
 				return;

diff --git a/src/vs/workbench/contrib/inlineChat/electron-sandbox/inlineChatQuickVoice.ts b/src/vs/workbench/contrib/inlineChat/electron-sandbox/inlineChatQuickVoice.ts
@@ -263,7 +263,6 @@ export class InlineChatQuickVoice implements IEditorContribution {
 
 		const done = (abort: boolean) => {
 			cts.dispose(true);
-			session.dispose();
 			listener.dispose();
 			this._widget.hide();
 			this._ctxQuickChatInProgress.reset();

diff --git a/src/vs/workbench/contrib/speech/common/speechService.ts b/src/vs/workbench/contrib/speech/common/speechService.ts
@@ -33,7 +33,7 @@ export interface ISpeechToTextEvent {
 	readonly text?: string;
 }
 
-export interface ISpeechToTextSession extends IDisposable {
+export interface ISpeechToTextSession {
 	readonly onDidChange: Event<ISpeechToTextEvent>;
 }
 
@@ -48,7 +48,7 @@ export interface IKeywordRecognitionEvent {
 	readonly text?: string;
 }
 
-export interface IKeywordRecognitionSession extends IDisposable {
+export interface IKeywordRecognitionSession {
 	readonly onDidChange: Event<IKeywordRecognitionEvent>;
 }
 

diff --git a/src/vs/workbench/contrib/terminal/browser/terminalVoice.ts b/src/vs/workbench/contrib/terminal/browser/terminalVoice.ts
@@ -5,7 +5,7 @@
 
 import { RunOnceScheduler } from 'vs/base/common/async';
 import { CancellationTokenSource } from 'vs/base/common/cancellation';
-import { Disposable, DisposableStore } from 'vs/base/common/lifecycle';
+import { Disposable, DisposableStore, toDisposable } from 'vs/base/common/lifecycle';
 import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { AccessibilityVoiceSettingId, SpeechTimeoutDefault } from 'vs/workbench/contrib/accessibility/browser/accessibilityConfiguration';
@@ -87,8 +87,9 @@ export class TerminalVoiceSession extends Disposable {
 			this._sendText();
 			this.stop();
 		}, voiceTimeout));
-		this._cancellationTokenSource = this._register(new CancellationTokenSource());
-		const session = this._disposables.add(this._speechService.createSpeechToTextSession(this._cancellationTokenSource!.token));
+		this._cancellationTokenSource = new CancellationTokenSource();
+		this._register(toDisposable(() => this._cancellationTokenSource?.dispose(true)));
+		const session = this._speechService.createSpeechToTextSession(this._cancellationTokenSource?.token);
 
 		this._disposables.add(session.onDidChange((e) => {
 			if (this._cancellationTokenSource?.token.isCancellationRequested) {