From 1b2aab7dee07829931c53532467f4e04db04742b Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Tue, 11 Sep 2018 10:05:53 +0200 Subject: [PATCH] remove remaining users of cancel and cancel-callback, #56137 --- src/vs/base/common/async.ts | 42 ++---- src/vs/base/common/worker/simpleWorker.ts | 4 +- src/vs/base/parts/tree/browser/treeModel.ts | 6 +- src/vs/base/test/common/async.test.ts | 141 ++++++------------ .../services/editorWorkerServiceImpl.ts | 8 +- src/vs/editor/common/services/webWorker.ts | 5 +- 6 files changed, 60 insertions(+), 146 deletions(-) diff --git a/src/vs/base/common/async.ts b/src/vs/base/common/async.ts index 796d9fe824f..f6a2fee985e 100644 --- a/src/vs/base/common/async.ts +++ b/src/vs/base/common/async.ts @@ -129,8 +129,6 @@ export class Throttler { this.queuedPromise = new TPromise(c => { this.activePromise.then(onComplete, onComplete).then(c); - }, () => { - this.activePromise.cancel(); }); } @@ -149,8 +147,6 @@ export class Throttler { this.activePromise = null; e(err); }); - }, () => { - this.activePromise.cancel(); }); } } @@ -192,13 +188,14 @@ export class Delayer { private timeout: number; private completionPromise: TPromise; - private onSuccess: ValueCallback; + private doResolve: ValueCallback; + private doReject: (err: any) => void; private task: ITask>; constructor(public defaultDelay: number) { this.timeout = null; this.completionPromise = null; - this.onSuccess = null; + this.doResolve = null; this.task = null; } @@ -207,13 +204,12 @@ export class Delayer { this.cancelTimeout(); if (!this.completionPromise) { - this.completionPromise = new TPromise((c) => { - this.onSuccess = c; - }, () => { - // no-op + this.completionPromise = new TPromise((c, e) => { + this.doResolve = c; + this.doReject = e; }).then(() => { this.completionPromise = null; - this.onSuccess = null; + this.doResolve = null; const task = this.task; this.task = null; @@ -223,7 +219,7 @@ export class Delayer { this.timeout = setTimeout(() => { this.timeout = null; - this.onSuccess(null); + this.doResolve(null); }, delay); return this.completionPromise; @@ -237,7 +233,7 @@ export class Delayer { this.cancelTimeout(); if (this.completionPromise) { - this.completionPromise.cancel(); + this.doReject(errors.canceled()); this.completionPromise = null; } } @@ -302,26 +298,6 @@ export class Barrier { } } -export class ShallowCancelThenPromise extends TPromise { - - constructor(outer: TPromise) { - - let completeCallback: ValueCallback, - errorCallback: ErrorCallback; - - super((c, e) => { - completeCallback = c; - errorCallback = e; - }, () => { - // cancel this promise but not the - // outer promise - errorCallback(errors.canceled()); - }); - - outer.then(completeCallback, errorCallback); - } -} - /** * Replacement for `WinJS.TPromise.timeout`. */ diff --git a/src/vs/base/common/worker/simpleWorker.ts b/src/vs/base/common/worker/simpleWorker.ts index 8333f109621..52a38b74249 100644 --- a/src/vs/base/common/worker/simpleWorker.ts +++ b/src/vs/base/common/worker/simpleWorker.ts @@ -7,7 +7,6 @@ import { transformErrorForSerialization } from 'vs/base/common/errors'; import { Disposable } from 'vs/base/common/lifecycle'; import { ErrorCallback, TPromise, ValueCallback } from 'vs/base/common/winjs.base'; -import { ShallowCancelThenPromise } from 'vs/base/common/async'; import { isWeb } from 'vs/base/common/platform'; const INITIALIZE = '$initialize'; @@ -266,8 +265,7 @@ export class SimpleWorkerClient extends Disposable { } public getProxyObject(): TPromise { - // Do not allow chaining promises to cancel the proxy creation - return new ShallowCancelThenPromise(this._lazyProxy); + return this._lazyProxy; } private _request(method: string, args: any[]): TPromise { diff --git a/src/vs/base/parts/tree/browser/treeModel.ts b/src/vs/base/parts/tree/browser/treeModel.ts index 1dcab8a06cb..e40cf2a62bb 100644 --- a/src/vs/base/parts/tree/browser/treeModel.ts +++ b/src/vs/base/parts/tree/browser/treeModel.ts @@ -84,13 +84,11 @@ export class Lock { var lock = this.getLock(item); if (lock) { - var unbindListener: IDisposable; - return new WinJS.TPromise((c, e) => { - unbindListener = once(lock.onDispose)(() => { + once(lock.onDispose)(() => { return this.run(item, fn).then(c, e); }); - }, () => { unbindListener.dispose(); }); + }); } var result: WinJS.Promise; diff --git a/src/vs/base/test/common/async.test.ts b/src/vs/base/test/common/async.test.ts index d3677218ae5..af519745523 100644 --- a/src/vs/base/test/common/async.test.ts +++ b/src/vs/base/test/common/async.test.ts @@ -41,16 +41,16 @@ suite('Async', () => { return result; }); - test('Cancel callback behaviour', async function () { - let withCancelCallback = new TPromise(() => { }, () => { }); - let withoutCancelCallback = new TPromise(() => { }); + // test('Cancel callback behaviour', async function () { + // let withCancelCallback = new WinJsPromise(() => { }, () => { }); + // let withoutCancelCallback = new TPromise(() => { }); - withCancelCallback.cancel(); - withoutCancelCallback.cancel(); + // withCancelCallback.cancel(); + // (withoutCancelCallback as WinJsPromise).cancel(); - await withCancelCallback.then(undefined, err => { assert.ok(isPromiseCanceledError(err)); }); - await withoutCancelCallback.then(undefined, err => { assert.ok(isPromiseCanceledError(err)); }); - }); + // await withCancelCallback.then(undefined, err => { assert.ok(isPromiseCanceledError(err)); }); + // await withoutCancelCallback.then(undefined, err => { assert.ok(isPromiseCanceledError(err)); }); + // }); // Cancelling a sync cancelable promise will fire the cancelled token. // Also, every `then` callback runs in another execution frame. @@ -97,49 +97,49 @@ suite('Async', () => { return promise.then(() => assert.deepEqual(order, ['in callback', 'afterCreate', 'cancelled', 'afterCancel', 'finally'])); }); - // Cancelling a sync tpromise will NOT cancel the promise, since it has resolved already. - // Every `then` callback runs sync in the same execution frame, thus `finally` executes - // before `afterCancel`. - test('TPromise execution order (sync)', function () { - const order = []; - let promise = new TPromise(resolve => { - order.push('in executor'); - resolve(1234); - }, () => order.push('cancelled')); + // // Cancelling a sync tpromise will NOT cancel the promise, since it has resolved already. + // // Every `then` callback runs sync in the same execution frame, thus `finally` executes + // // before `afterCancel`. + // test('TPromise execution order (sync)', function () { + // const order = []; + // let promise = new WinJsPromise(resolve => { + // order.push('in executor'); + // resolve(1234); + // }, () => order.push('cancelled')); - order.push('afterCreate'); + // order.push('afterCreate'); - promise = promise - .then(null, err => null) - .then(() => order.push('finally')); + // promise = promise + // .then(null, err => null) + // .then(() => order.push('finally')); - promise.cancel(); - order.push('afterCancel'); + // promise.cancel(); + // order.push('afterCancel'); - return promise.then(() => assert.deepEqual(order, ['in executor', 'afterCreate', 'finally', 'afterCancel'])); - }); + // return promise.then(() => assert.deepEqual(order, ['in executor', 'afterCreate', 'finally', 'afterCancel'])); + // }); - // Cancelling an async tpromise will cancel the promise. - // Every `then` callback runs sync on the same execution frame as the `cancel` call, - // so finally still executes before `afterCancel`. - test('TPromise execution order (async)', function () { - const order = []; - let promise = new TPromise(resolve => { - order.push('in executor'); - setTimeout(() => resolve(1234)); - }, () => order.push('cancelled')); + // // Cancelling an async tpromise will cancel the promise. + // // Every `then` callback runs sync on the same execution frame as the `cancel` call, + // // so finally still executes before `afterCancel`. + // test('TPromise execution order (async)', function () { + // const order = []; + // let promise = new WinJsPromise(resolve => { + // order.push('in executor'); + // setTimeout(() => resolve(1234)); + // }, () => order.push('cancelled')); - order.push('afterCreate'); + // order.push('afterCreate'); - promise = promise - .then(null, err => null) - .then(() => order.push('finally')); + // promise = promise + // .then(null, err => null) + // .then(() => order.push('finally')); - promise.cancel(); - order.push('afterCancel'); + // promise.cancel(); + // order.push('afterCancel'); - return promise.then(() => assert.deepEqual(order, ['in executor', 'afterCreate', 'cancelled', 'finally', 'afterCancel'])); - }); + // return promise.then(() => assert.deepEqual(order, ['in executor', 'afterCreate', 'cancelled', 'finally', 'afterCancel'])); + // }); test('cancelablePromise - get inner result', async function () { let promise = async.createCancelablePromise(token => { @@ -190,63 +190,6 @@ suite('Async', () => { }); }); - test('Throttler - cancel should not cancel other promises', function () { - let count = 0; - let factory = () => TPromise.wrap(async.timeout(0)).then(() => ++count); - - let throttler = new async.Throttler(); - let p1: TPromise; - - const p = TPromise.join([ - p1 = throttler.queue(factory).then((result) => { assert(false, 'should not be here, 1'); }, () => { assert(true, 'yes, it was cancelled'); }), - throttler.queue(factory).then((result) => { assert.equal(result, 1); }, () => { assert(false, 'should not be here, 2'); }), - throttler.queue(factory).then((result) => { assert.equal(result, 1); }, () => { assert(false, 'should not be here, 3'); }), - throttler.queue(factory).then((result) => { assert.equal(result, 1); }, () => { assert(false, 'should not be here, 4'); }) - ]); - - p1.cancel(); - - return p; - }); - - test('Throttler - cancel the first queued promise should not cancel other promises', function () { - let count = 0; - let factory = () => TPromise.wrap(async.timeout(0)).then(() => ++count); - - let throttler = new async.Throttler(); - let p2: TPromise; - - const p = TPromise.join([ - throttler.queue(factory).then((result) => { assert.equal(result, 1); }, () => { assert(false, 'should not be here, 1'); }), - p2 = throttler.queue(factory).then((result) => { assert(false, 'should not be here, 2'); }, () => { assert(true, 'yes, it was cancelled'); }), - throttler.queue(factory).then((result) => { assert.equal(result, 2); }, () => { assert(false, 'should not be here, 3'); }), - throttler.queue(factory).then((result) => { assert.equal(result, 2); }, () => { assert(false, 'should not be here, 4'); }) - ]); - - p2.cancel(); - - return p; - }); - - test('Throttler - cancel in the middle should not cancel other promises', function () { - let count = 0; - let factory = () => TPromise.wrap(async.timeout(0)).then(() => ++count); - - let throttler = new async.Throttler(); - let p3: TPromise; - - const p = TPromise.join([ - throttler.queue(factory).then((result) => { assert.equal(result, 1); }, () => { assert(false, 'should not be here, 1'); }), - throttler.queue(factory).then((result) => { assert.equal(result, 2); }, () => { assert(false, 'should not be here, 2'); }), - p3 = throttler.queue(factory).then((result) => { assert(false, 'should not be here, 3'); }, () => { assert(true, 'yes, it was cancelled'); }), - throttler.queue(factory).then((result) => { assert.equal(result, 2); }, () => { assert(false, 'should not be here, 4'); }) - ]); - - p3.cancel(); - - return p; - }); - test('Throttler - last factory should be the one getting called', function () { let factoryFactory = (n: number) => () => { return TPromise.wrap(async.timeout(0)).then(() => n); diff --git a/src/vs/editor/common/services/editorWorkerServiceImpl.ts b/src/vs/editor/common/services/editorWorkerServiceImpl.ts index 3a2ae4ad6fe..8078efd8abe 100644 --- a/src/vs/editor/common/services/editorWorkerServiceImpl.ts +++ b/src/vs/editor/common/services/editorWorkerServiceImpl.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ 'use strict'; -import { IntervalTimer, ShallowCancelThenPromise } from 'vs/base/common/async'; +import { IntervalTimer } from 'vs/base/common/async'; import { Disposable, IDisposable, dispose, toDisposable } from 'vs/base/common/lifecycle'; import { URI } from 'vs/base/common/uri'; import { TPromise } from 'vs/base/common/winjs.base'; @@ -321,7 +321,7 @@ class SynchronousWorkerClient implements IWorkerClient } public getProxyObject(): TPromise { - return new ShallowCancelThenPromise(this._proxyObj); + return this._proxyObj; } } @@ -356,11 +356,11 @@ export class EditorWorkerClient extends Disposable { } protected _getProxy(): TPromise { - return new ShallowCancelThenPromise(this._getOrCreateWorker().getProxyObject().then(null, (err) => { + return this._getOrCreateWorker().getProxyObject().then(null, (err) => { logOnceWebWorkerWarning(err); this._worker = new SynchronousWorkerClient(new EditorSimpleWorkerImpl(null)); return this._getOrCreateWorker().getProxyObject(); - })); + }); } private _getOrCreateModelManager(proxy: EditorSimpleWorkerImpl): EditorModelManager { diff --git a/src/vs/editor/common/services/webWorker.ts b/src/vs/editor/common/services/webWorker.ts index 5ace0dd172c..a07a751b041 100644 --- a/src/vs/editor/common/services/webWorker.ts +++ b/src/vs/editor/common/services/webWorker.ts @@ -4,7 +4,6 @@ *--------------------------------------------------------------------------------------------*/ 'use strict'; -import { ShallowCancelThenPromise } from 'vs/base/common/async'; import { URI } from 'vs/base/common/uri'; import { TPromise } from 'vs/base/common/winjs.base'; import { IModelService } from 'vs/editor/common/services/modelService'; @@ -68,7 +67,7 @@ class MonacoWebWorkerImpl extends EditorWorkerClient implements MonacoWebWork private _getForeignProxy(): TPromise { if (!this._foreignProxy) { - this._foreignProxy = new ShallowCancelThenPromise(this._getProxy().then((proxy) => { + this._foreignProxy = this._getProxy().then((proxy) => { return proxy.loadForeignModule(this._foreignModuleId, this._foreignModuleCreateData).then((foreignMethods) => { this._foreignModuleId = null; this._foreignModuleCreateData = null; @@ -91,7 +90,7 @@ class MonacoWebWorkerImpl extends EditorWorkerClient implements MonacoWebWork return foreignProxy; }); - })); + }); } return this._foreignProxy; }