FIx #47262

src/vs/workbench/api/electron-browser/mainThreadTreeViews.ts

@@ -4,7 +4,6 @@
  *--------------------------------------------------------------------------------------------*/
 'use strict';
 
-import { Event, Emitter } from 'vs/base/common/event';
 import { TPromise } from 'vs/base/common/winjs.base';
 import { Disposable } from 'vs/base/common/lifecycle';
 import { ExtHostContext, MainThreadTreeViewsShape, ExtHostTreeViewsShape, MainContext, IExtHostContext } from '../node/extHost.protocol';
@@ -29,7 +28,7 @@ export class MainThreadTreeViews extends Disposable implements MainThreadTreeVie
 	}
 
 	$registerTreeViewDataProvider(treeViewId: string): void {
-		const dataProvider = this._register(new TreeViewDataProvider(treeViewId, this._proxy, this.notificationService));
+		const dataProvider = new TreeViewDataProvider(treeViewId, this._proxy, this.notificationService);
 		this._dataProviders.set(treeViewId, dataProvider);
 		const treeViewer = this.viewsService.getTreeViewer(treeViewId);
 		if (treeViewer) {
@@ -48,11 +47,14 @@ export class MainThreadTreeViews extends Disposable implements MainThreadTreeVie
 			});
 	}
 
-	$refresh(treeViewId: string, itemsToRefresh: { [treeItemHandle: string]: ITreeItem }): void {
+	$refresh(treeViewId: string, itemsToRefreshByHandle: { [treeItemHandle: string]: ITreeItem }): TPromise<void> {
+		const viewer = this.viewsService.getTreeViewer(treeViewId);
 		const dataProvider = this._dataProviders.get(treeViewId);
-		if (dataProvider) {
-			dataProvider.refresh(itemsToRefresh);
+		if (viewer && dataProvider) {
+			const itemsToRefresh = dataProvider.getItemsToRefresh(itemsToRefreshByHandle);
+			return viewer.refresh(itemsToRefresh.length ? itemsToRefresh : void 0);
 		}
+		return TPromise.as(null);
 	}
 
 	private registerListeners(treeViewId: string, treeViewer: ITreeViewer): void {
@@ -62,6 +64,12 @@ export class MainThreadTreeViews extends Disposable implements MainThreadTreeVie
 	}
 
 	dispose(): void {
+		this._dataProviders.forEach((dataProvider, treeViewId) => {
+			const treeViewer = this.viewsService.getTreeViewer(treeViewId);
+			if (treeViewer) {
+				treeViewer.dataProvider = null;
+			}
+		});
 		this._dataProviders.clear();
 		super.dispose();
 	}
@@ -71,12 +79,6 @@ type TreeItemHandle = string;
 
 class TreeViewDataProvider implements ITreeViewDataProvider {
 
-	private readonly _onDidChange: Emitter<ITreeItem[] | undefined | null> = new Emitter<ITreeItem[] | undefined | null>();
-	readonly onDidChange: Event<ITreeItem[] | undefined | null> = this._onDidChange.event;
-
-	private readonly _onDispose: Emitter<void> = new Emitter<void>();
-	readonly onDispose: Event<void> = this._onDispose.event;
-
 	private itemsMap: Map<TreeItemHandle, ITreeItem> = new Map<TreeItemHandle, ITreeItem>();
 
 	constructor(private treeViewId: string,
@@ -98,7 +100,7 @@ class TreeViewDataProvider implements ITreeViewDataProvider {
 			});
 	}
 
-	refresh(itemsToRefreshByHandle: { [treeItemHandle: string]: ITreeItem }) {
+	getItemsToRefresh(itemsToRefreshByHandle: { [treeItemHandle: string]: ITreeItem }): ITreeItem[] {
 		const itemsToRefresh: ITreeItem[] = [];
 		if (itemsToRefreshByHandle) {
 			for (const treeItemHandle of Object.keys(itemsToRefreshByHandle)) {
@@ -121,11 +123,7 @@ class TreeViewDataProvider implements ITreeViewDataProvider {
 				}
 			}
 		}
-		if (itemsToRefresh.length) {
-			this._onDidChange.fire(itemsToRefresh);
-		} else {
-			this._onDidChange.fire();
-		}
+		return itemsToRefresh;
 	}
 
 	private postGetChildren(elements: ITreeItem[]): ITreeItem[] {
@@ -148,8 +146,4 @@ class TreeViewDataProvider implements ITreeViewDataProvider {
 			}
 		}
 	}
-
-	dispose(): void {
-		this._onDispose.fire();
-	}
 }

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

@@ -206,7 +206,7 @@ export interface MainThreadTextEditorsShape extends IDisposable {
 
 export interface MainThreadTreeViewsShape extends IDisposable {
 	$registerTreeViewDataProvider(treeViewId: string): void;
-	$refresh(treeViewId: string, itemsToRefresh?: { [treeItemHandle: string]: ITreeItem }): void;
+	$refresh(treeViewId: string, itemsToRefresh?: { [treeItemHandle: string]: ITreeItem }): TPromise<void>;
 	$reveal(treeViewId: string, treeItem: ITreeItem, parentChain: ITreeItem[], options?: { select?: boolean }): TPromise<void>;
 }
 

src/vs/workbench/api/node/extHostTreeViews.ts

@@ -119,11 +119,25 @@ class ExtHostTreeView<T> extends Disposable {
 	private _onDidCollapseElement: Emitter<T> = this._register(new Emitter<T>());
 	readonly onDidCollapseElement: Event<T> = this._onDidCollapseElement.event;
 
+	private refreshPromise: TPromise<void> = TPromise.as(null);
+
 	constructor(private viewId: string, private dataProvider: vscode.TreeDataProvider<T>, private proxy: MainThreadTreeViewsShape, private commands: CommandsConverter) {
 		super();
 		this.proxy.$registerTreeViewDataProvider(viewId);
 		if (this.dataProvider.onDidChangeTreeData) {
-			this._register(debounceEvent<T, T[]>(this.dataProvider.onDidChangeTreeData, (last, current) => last ? [...last, current] : [current], 200)(elements => this.refresh(elements)));
+			let refreshingPromise, promiseCallback;
+			this._register(debounceEvent<T, T[]>(this.dataProvider.onDidChangeTreeData, (last, current) => {
+				if (!refreshingPromise) {
+					// New refresh has started
+					refreshingPromise = new TPromise((c, e) => promiseCallback = c);
+					this.refreshPromise = this.refreshPromise.then(() => refreshingPromise);
+				}
+				return last ? [...last, current] : [current];
+			}, 200, true)(elements => {
+				const _promiseCallback = promiseCallback;
+				refreshingPromise = null;
+				this.refresh(elements).then(() => _promiseCallback());
+			}));
 		}
 	}
 
@@ -145,9 +159,10 @@ class ExtHostTreeView<T> extends Disposable {
 
 	reveal(element: T, options?: { select?: boolean }): TPromise<void> {
 		if (typeof this.dataProvider.getParent !== 'function') {
-			return TPromise.wrapError(new Error(`Required registered TreeDataProvider to implement 'getParent' method to access 'reveal' mehtod`));
+			return TPromise.wrapError(new Error(`Required registered TreeDataProvider to implement 'getParent' method to access 'reveal' method`));
 		}
-		return this.resolveUnknownParentChain(element)
+		return this.refreshPromise
+			.then(() => this.resolveUnknownParentChain(element))
 			.then(parentChain => this.resolveTreeNode(element, parentChain[parentChain.length - 1])
 				.then(treeNode => this.proxy.$reveal(this.viewId, treeNode.item, parentChain.map(p => p.item), options)));
 	}
@@ -234,17 +249,18 @@ class ExtHostTreeView<T> extends Disposable {
 			.then(nodes => nodes.filter(n => !!n));
 	}
 
-	private refresh(elements: T[]): void {
+	private refresh(elements: T[]): TPromise<void> {
 		const hasRoot = elements.some(element => !element);
 		if (hasRoot) {
 			this.clearAll(); // clear cache
-			this.proxy.$refresh(this.viewId);
+			return this.proxy.$refresh(this.viewId);
 		} else {
 			const handlesToRefresh = this.getHandlesToRefresh(elements);
 			if (handlesToRefresh.length) {
-				this.refreshHandles(handlesToRefresh);
+				return this.refreshHandles(handlesToRefresh);
 			}
 		}
+		return TPromise.as(null);
 	}
 
 	private getHandlesToRefresh(elements: T[]): TreeItemHandle[] {

src/vs/workbench/browser/parts/views/customView.ts

@@ -5,7 +5,7 @@
 
 import 'vs/css!./media/views';
 import * as errors from 'vs/base/common/errors';
-import { IDisposable, Disposable, dispose } from 'vs/base/common/lifecycle';
+import { IDisposable, Disposable } from 'vs/base/common/lifecycle';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { TPromise } from 'vs/base/common/winjs.base';
 import * as DOM from 'vs/base/browser/dom';
@@ -111,7 +111,6 @@ class CustomTreeViewer extends Disposable implements ITreeViewer {
 	private elementsToRefresh: ITreeItem[] = [];
 
 	private _dataProvider: ITreeViewDataProvider;
-	private dataProviderDisposables: IDisposable[] = [];
 
 	private _onDidExpandItem: Emitter<ITreeItem> = this._register(new Emitter<ITreeItem>());
 	readonly onDidExpandItem: Event<ITreeItem> = this._onDidExpandItem.event;
@@ -141,11 +140,8 @@ class CustomTreeViewer extends Disposable implements ITreeViewer {
 	}
 
 	set dataProvider(dataProvider: ITreeViewDataProvider) {
-		dispose(this.dataProviderDisposables);
 		if (dataProvider) {
 			this._dataProvider = new class implements ITreeViewDataProvider {
-				onDidChange = dataProvider.onDidChange;
-				onDispose = dataProvider.onDispose;
 				getChildren(node?: ITreeItem): TPromise<ITreeItem[]> {
 					if (node && node.children) {
 						return TPromise.as(node.children);
@@ -157,8 +153,6 @@ class CustomTreeViewer extends Disposable implements ITreeViewer {
 					});
 				}
 			};
-			this._register(dataProvider.onDidChange(elements => this.refresh(elements), this, this.dataProviderDisposables));
-			this._register(dataProvider.onDispose(() => this.dataProvider = null, this, this.dataProviderDisposables));
 		} else {
 			this._dataProvider = null;
 		}

src/vs/workbench/common/views.ts

@@ -241,9 +241,6 @@ export interface ITreeItem {
 
 export interface ITreeViewDataProvider {
 
-	onDidChange: Event<ITreeItem[] | undefined | null>;
-
-	onDispose: Event<void>;
-
 	getChildren(element?: ITreeItem): TPromise<ITreeItem[]>;
+
 }
\ No newline at end of file

src/vs/workbench/test/electron-browser/api/extHostTreeViews.test.ts

@@ -31,8 +31,8 @@ suite('ExtHostTreeView', function () {
 		$registerTreeViewDataProvider(treeViewId: string): void {
 		}
 
-		$refresh(viewId: string, itemsToRefresh?: { [treeItemHandle: string]: ITreeItem }): void {
-			this.onRefresh.fire(itemsToRefresh);
+		$refresh(viewId: string, itemsToRefresh?: { [treeItemHandle: string]: ITreeItem }): TPromise<void> {
+			return TPromise.as(null).then(() => this.onRefresh.fire(itemsToRefresh));
 		}
 
 		$reveal(): TPromise<void> {
@@ -78,11 +78,7 @@ suite('ExtHostTreeView', function () {
 		testObject.createTreeView('testNodeTreeProvider', { treeDataProvider: aNodeTreeDataProvider() });
 		testObject.createTreeView('testNodeWithIdTreeProvider', { treeDataProvider: aNodeWithIdTreeDataProvider() });
 
-		testObject.$getChildren('testNodeTreeProvider').then(elements => {
-			for (const element of elements) {
-				testObject.$getChildren('testNodeTreeProvider', element.handle);
-			}
-		});
+		return loadCompleteTree('testNodeTreeProvider');
 	});
 
 	test('construct node tree', () => {
@@ -256,9 +252,13 @@ suite('ExtHostTreeView', function () {
 	});
 
 	test('refresh calls are throttled on roots', function (done) {
+		let counter = 0;
 		target.onRefresh.event(actuals => {
+			counter++;
 			assert.equal(undefined, actuals);
-			done();
+			if (counter > 1) {
+				done();
+			}
 		});
 		onDidChangeTreeNode.fire();
 		onDidChangeTreeNode.fire();
@@ -267,9 +267,18 @@ suite('ExtHostTreeView', function () {
 	});
 
 	test('refresh calls are throttled on elements', function (done) {
+		let counter = 0;
 		target.onRefresh.event(actuals => {
-			assert.deepEqual(['0/0:a', '0/0:b'], Object.keys(actuals));
-			done();
+			counter++;
+			if (counter === 1) {
+				assert.deepEqual(['0/0:a'], Object.keys(actuals));
+			}
+			if (counter === 2) {
+				assert.deepEqual(['0/0:a', '0/0:b'], Object.keys(actuals));
+			}
+			if (counter > 1) {
+				done();
+			}
 		});
 
 		onDidChangeTreeNode.fire(getNode('a'));
@@ -319,7 +328,6 @@ suite('ExtHostTreeView', function () {
 			'a/0:b': {}
 		};
 
-		onDidChangeTreeNode.fire();
 		target.onRefresh.event(() => {
 			testObject.$getChildren('testNodeTreeProvider')
 				.then(elements => {
@@ -327,6 +335,7 @@ suite('ExtHostTreeView', function () {
 					done();
 				});
 		});
+		onDidChangeTreeNode.fire();
 	});
 
 	test('tree with duplicate labels', (done) => {
@@ -361,8 +370,6 @@ suite('ExtHostTreeView', function () {
 		tree['f'] = {};
 		tree[dupItems['adup2']] = {};
 
-		onDidChangeTreeNode.fire();
-
 		target.onRefresh.event(() => {
 			testObject.$getChildren('testNodeTreeProvider')
 				.then(elements => {
@@ -376,6 +383,8 @@ suite('ExtHostTreeView', function () {
 						});
 				});
 		});
+
+		onDidChangeTreeNode.fire();
 	});
 
 	test('getChildren is not returned from cache if refreshed', (done) => {
@@ -383,7 +392,6 @@ suite('ExtHostTreeView', function () {
 			'c': {}
 		};
 
-		onDidChangeTreeNode.fire();
 		target.onRefresh.event(() => {
 			testObject.$getChildren('testNodeTreeProvider')
 				.then(elements => {
@@ -391,6 +399,8 @@ suite('ExtHostTreeView', function () {
 					done();
 				});
 		});
+
+		onDidChangeTreeNode.fire();
 	});
 
 	test('getChildren is returned from cache if not refreshed', () => {
@@ -474,6 +484,79 @@ suite('ExtHostTreeView', function () {
 			});
 	});
 
+	test('reveal after first udpate', () => {
+		const revealTarget = sinon.spy(target, '$reveal');
+		const treeView = testObject.createTreeView('treeDataProvider', { treeDataProvider: aCompleteNodeTreeDataProvider() });
+		return loadCompleteTree('treeDataProvider')
+			.then(() => {
+				tree = {
+					'a': {
+						'aa': {},
+						'ac': {}
+					},
+					'b': {
+						'ba': {},
+						'bb': {}
+					}
+				};
+				onDidChangeTreeNode.fire(getNode('a'));
+
+				return treeView.reveal({ key: 'ac' })
+					.then(() => {
+						assert.ok(revealTarget.calledOnce);
+						assert.deepEqual('treeDataProvider', revealTarget.args[0][0]);
+						assert.deepEqual({ handle: '0/0:a/0:ac', label: 'ac', collapsibleState: TreeItemCollapsibleState.None, parentHandle: '0/0:a' }, removeUnsetKeys(revealTarget.args[0][1]));
+						assert.deepEqual([{ handle: '0/0:a', label: 'a', collapsibleState: TreeItemCollapsibleState.Collapsed }], (<Array<any>>revealTarget.args[0][2]).map(arg => removeUnsetKeys(arg)));
+						assert.equal(void 0, revealTarget.args[0][3]);
+					});
+			});
+	});
+
+	test('reveal after second udpate', () => {
+		const revealTarget = sinon.spy(target, '$reveal');
+		const treeView = testObject.createTreeView('treeDataProvider', { treeDataProvider: aCompleteNodeTreeDataProvider() });
+		return loadCompleteTree('treeDataProvider')
+			.then(() => {
+				tree = {
+					'a': {
+						'aa': {},
+						'ac': {}
+					},
+					'b': {
+						'ba': {},
+						'bb': {}
+					}
+				};
+				onDidChangeTreeNode.fire(getNode('a'));
+				tree = {
+					'a': {
+						'aa': {},
+						'ac': {}
+					},
+					'b': {
+						'ba': {},
+						'bc': {}
+					}
+				};
+				onDidChangeTreeNode.fire(getNode('b'));
+
+				return treeView.reveal({ key: 'bc' })
+					.then(() => {
+						assert.ok(revealTarget.calledOnce);
+						assert.deepEqual('treeDataProvider', revealTarget.args[0][0]);
+						assert.deepEqual({ handle: '0/0:b/0:bc', label: 'bc', collapsibleState: TreeItemCollapsibleState.None, parentHandle: '0/0:b' }, removeUnsetKeys(revealTarget.args[0][1]));
+						assert.deepEqual([{ handle: '0/0:b', label: 'b', collapsibleState: TreeItemCollapsibleState.Collapsed }], (<Array<any>>revealTarget.args[0][2]).map(arg => removeUnsetKeys(arg)));
+						assert.equal(void 0, revealTarget.args[0][3]);
+					});
+			});
+	});
+
+	function loadCompleteTree(treeId, element?: string): TPromise<void> {
+		return testObject.$getChildren(treeId, element)
+			.then(elements => elements.map(e => loadCompleteTree(treeId, e.handle)))
+			.then(() => null);
+	}
+
 	function removeUnsetKeys(obj: any): any {
 		const result = {};
 		for (const key of Object.keys(obj)) {