Remove forwarding of cancel via winjs promises in rpcProtocol (#56137)

microsoft · Sep 6, 2018 · 58c3408 · 58c3408
1 parent 2b960fb
commit 58c3408
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 52 deletions.
diff --git a/src/vs/workbench/services/extensions/node/lazyPromise.ts b/src/vs/workbench/services/extensions/node/lazyPromise.ts
@@ -7,9 +7,7 @@
 import { TPromise, ValueCallback, ErrorCallback } from 'vs/base/common/winjs.base';
 import { onUnexpectedError } from 'vs/base/common/errors';
 
-export class LazyPromise implements TPromise<any> {
-
-	private _onCancel: () => void;
+export class LazyPromise implements Thenable<any> {
 
 	private _actual: TPromise<any>;
 	private _actualOk: ValueCallback;
@@ -21,26 +19,22 @@ export class LazyPromise implements TPromise<any> {
 	private _hasErr: boolean;
 	private _err: any;
 
-	private _isCanceled: boolean;
-
-	constructor(onCancel: () => void) {
-		this._onCancel = onCancel;
+	constructor() {
 		this._actual = null;
 		this._actualOk = null;
 		this._actualErr = null;
 		this._hasValue = false;
 		this._value = null;
 		this._hasErr = false;
 		this._err = null;
-		this._isCanceled = false;
 	}
 
 	private _ensureActual(): TPromise<any> {
 		if (!this._actual) {
 			this._actual = new TPromise<any>((c, e) => {
 				this._actualOk = c;
 				this._actualErr = e;
-			}, this._onCancel);
+			});
 
 			if (this._hasValue) {
 				this._actualOk(this._value);
@@ -54,7 +48,7 @@ export class LazyPromise implements TPromise<any> {
 	}
 
 	public resolveOk(value: any): void {
-		if (this._isCanceled || this._hasErr) {
+		if (this._hasValue || this._hasErr) {
 			return;
 		}
 
@@ -67,7 +61,7 @@ export class LazyPromise implements TPromise<any> {
 	}
 
 	public resolveErr(err: any): void {
-		if (this._isCanceled || this._hasValue) {
+		if (this._hasValue || this._hasErr) {
 			return;
 		}
 
@@ -84,24 +78,6 @@ export class LazyPromise implements TPromise<any> {
 	}
 
 	public then(success: any, error: any): any {
-		if (this._isCanceled) {
-			return;
-		}
-
 		return this._ensureActual().then(success, error);
 	}
-
-	public cancel(): void {
-		if (this._hasValue || this._hasErr) {
-			return;
-		}
-
-		this._isCanceled = true;
-
-		if (this._actual) {
-			this._actual.cancel();
-		} else {
-			this._onCancel();
-		}
-	}
 }
diff --git a/src/vs/workbench/services/extensions/node/rpcProtocol.ts b/src/vs/workbench/services/extensions/node/rpcProtocol.ts
@@ -96,6 +96,8 @@ export interface IRPCProtocolLogger {
 	logOutgoing(msgLength: number, req: number, initiator: RequestInitiator, str: string, data?: any): void;
 }
 
+const noop = () => { };
+
 export class RPCProtocol implements IRPCProtocol {
 
 	private readonly _protocol: IMessagePassingProtocol;
@@ -249,16 +251,17 @@ export class RPCProtocol implements IRPCProtocol {
 		}
 		const callId = String(req);
 
-		let promise: TPromise<any>;
+		let promise: Thenable<any>;
 		let cancel: () => void;
 		if (usesCancellationToken) {
 			const cancellationTokenSource = new CancellationTokenSource();
 			args.push(cancellationTokenSource.token);
 			promise = this._invokeHandler(rpcId, method, args);
 			cancel = () => cancellationTokenSource.cancel();
 		} else {
+			// cannot be cancelled
 			promise = this._invokeHandler(rpcId, method, args);
-			cancel = () => promise.cancel();
+			cancel = noop;
 		}
 
 		this._cancelInvokedHandlers[callId] = cancel;
@@ -331,7 +334,7 @@ export class RPCProtocol implements IRPCProtocol {
 		pendingReply.resolveErr(err);
 	}
 
-	private _invokeHandler(rpcId: number, methodName: string, args: any[]): TPromise<any> {
+	private _invokeHandler(rpcId: number, methodName: string, args: any[]): Thenable<any> {
 		try {
 			return TPromise.as(this._doInvokeHandler(rpcId, methodName, args));
 		} catch (err) {
@@ -351,7 +354,7 @@ export class RPCProtocol implements IRPCProtocol {
 		return method.apply(actor, args);
 	}
 
-	private _remoteCall(rpcId: number, methodName: string, args: any[]): TPromise<any> {
+	private _remoteCall(rpcId: number, methodName: string, args: any[]): Thenable<any> {
 		if (this._isDisposed) {
 			return TPromise.wrapError<any>(errors.canceled());
 		}
@@ -367,17 +370,16 @@ export class RPCProtocol implements IRPCProtocol {
 
 		const req = ++this._lastMessageId;
 		const callId = String(req);
-		const sendCancel = () => {
-			const msg = MessageIO.serializeCancel(req);
-			if (this._logger) {
-				this._logger.logOutgoing(msg.byteLength, req, RequestInitiator.LocalSide, `cancel`);
-			}
-			this._protocol.send(MessageIO.serializeCancel(req));
-		};
-		const result = new LazyPromise(sendCancel);
+		const result = new LazyPromise();
 
 		if (cancellationToken) {
-			cancellationToken.onCancellationRequested(sendCancel);
+			cancellationToken.onCancellationRequested(() => {
+				const msg = MessageIO.serializeCancel(req);
+				if (this._logger) {
+					this._logger.logOutgoing(msg.byteLength, req, RequestInitiator.LocalSide, `cancel`);
+				}
+				this._protocol.send(MessageIO.serializeCancel(req));
+			});
 		}
 
 		this._pendingRPCReplies[callId] = result;

diff --git a/src/vs/workbench/services/extensions/test/node/rpcProtocol.test.ts b/src/vs/workbench/services/extensions/test/node/rpcProtocol.test.ts
@@ -35,7 +35,7 @@ suite('RPCProtocol', () => {
 	let delegate: (a1: any, a2: any) => any;
 	let bProxy: BClass;
 	class BClass {
-		$m(a1: any, a2: any): TPromise<any> {
+		$m(a1: any, a2: any): Thenable<any> {
 			return TPromise.as(delegate.call(null, a1, a2));
 		}
 	}
@@ -108,15 +108,6 @@ suite('RPCProtocol', () => {
 		}, done);
 	});
 
-	test('cancelling a call', function () {
-		delegate = (a1: number, a2: number) => a1 + a2;
-		let p = bProxy.$m(4, 1);
-		p.then((res: number) => {
-			assert.fail('should not receive result');
-		});
-		p.cancel();
-	});
-
 	test('cancelling a call via CancellationToken before', function (done) {
 		delegate = (a1: number, a2: number) => a1 + a2;
 		let p = bProxy.$m(4, CancellationToken.Cancelled);