#58469 Move view descriptor collection to view service

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

@@ -7,7 +7,7 @@ import 'vs/css!./media/views';
 import { Disposable } from 'vs/base/common/lifecycle';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { TPromise } from 'vs/base/common/winjs.base';
-import { IViewsService, ViewsRegistry, IViewsViewlet, ViewContainer, IViewDescriptor, IViewContainersRegistry, Extensions as ViewContainerExtensions, TEST_VIEW_CONTAINER_ID, IView } from 'vs/workbench/common/views';
+import { IViewsService, ViewsRegistry, IViewsViewlet, ViewContainer, IViewDescriptor, IViewContainersRegistry, Extensions as ViewContainerExtensions, TEST_VIEW_CONTAINER_ID, IView, IViewDescriptorCollection } from 'vs/workbench/common/views';
 import { Registry } from 'vs/platform/registry/common/platform';
 import { ViewletRegistry, Extensions as ViewletExtensions } from 'vs/workbench/browser/viewlet';
 import { IStorageService, StorageScope } from 'vs/platform/storage/common/storage';
@@ -63,7 +63,7 @@ interface IViewItem {
 	active: boolean;
 }
 
-class ViewDescriptorCollection extends Disposable {
+class ViewDescriptorCollection extends Disposable implements IViewDescriptorCollection {
 
 	private contextKeys = new CounterSet<string>();
 	private items: IViewItem[] = [];
@@ -208,11 +208,11 @@ export class ContributableViewsModel extends Disposable {
 
 	constructor(
 		container: ViewContainer,
-		contextKeyService: IContextKeyService,
+		viewsService: IViewsService,
 		protected viewStates = new Map<string, IViewState>(),
 	) {
 		super();
-		const viewDescriptorCollection = this._register(new ViewDescriptorCollection(container, contextKeyService));
+		const viewDescriptorCollection = viewsService.getViewDescriptors(container);
 
 		this._register(viewDescriptorCollection.onDidChange(() => this.onDidChangeViewDescriptors(viewDescriptorCollection.viewDescriptors)));
 		this.onDidChangeViewDescriptors(viewDescriptorCollection.viewDescriptors);
@@ -407,14 +407,14 @@ export class PersistentContributableViewsModel extends ContributableViewsModel {
 	constructor(
 		container: ViewContainer,
 		viewletStateStorageId: string,
-		@IContextKeyService contextKeyService: IContextKeyService,
+		@IViewsService viewsService: IViewsService,
 		@IStorageService storageService: IStorageService,
 		@IWorkspaceContextService contextService: IWorkspaceContextService
 	) {
 		const hiddenViewsStorageId = `${viewletStateStorageId}.hidden`;
 		const viewStates = PersistentContributableViewsModel.loadViewsStates(viewletStateStorageId, hiddenViewsStorageId, storageService, contextService);
 
-		super(container, contextKeyService, viewStates);
+		super(container, viewsService, viewStates);
 
 		this.viewletStateStorageId = viewletStateStorageId;
 		this.hiddenViewsStorageId = hiddenViewsStorageId;
@@ -485,21 +485,29 @@ export class ViewsService extends Disposable implements IViewsService {
 
 	_serviceBrand: any;
 
+	private readonly viewDescriptorCollections: Map<ViewContainer, IViewDescriptorCollection>;
+
 	constructor(
 		@IInstantiationService private instantiationService: IInstantiationService,
 		@ILifecycleService private lifecycleService: ILifecycleService,
 		@IViewletService private viewletService: IViewletService,
 		@IStorageService private storageService: IStorageService,
-		@IWorkspaceContextService private workspaceContextService: IWorkspaceContextService
+		@IWorkspaceContextService private workspaceContextService: IWorkspaceContextService,
+		@IContextKeyService private contextKeyService: IContextKeyService
 	) {
 		super();
 
+		this.viewDescriptorCollections = new Map<ViewContainer, IViewDescriptorCollection>();
 		const viewContainersRegistry = Registry.as<IViewContainersRegistry>(ViewContainerExtensions.ViewContainersRegistry);
 		viewContainersRegistry.all.forEach(viewContainer => this.onDidRegisterViewContainer(viewContainer));
 		this._register(viewContainersRegistry.onDidRegister(viewContainer => this.onDidRegisterViewContainer(viewContainer)));
 		this._register(Registry.as<ViewletRegistry>(ViewletExtensions.Viewlets).onDidRegister(viewlet => this.viewletService.setViewletEnablement(viewlet.id, this.storageService.getBoolean(`viewservice.${viewlet.id}.enablement`, this.getStorageScope(), viewlet.id !== TEST_VIEW_CONTAINER_ID))));
 	}
 
+	getViewDescriptors(container: ViewContainer): IViewDescriptorCollection {
+		return this.viewDescriptorCollections.get(container);
+	}
+
 	openView(id: string, focus: boolean): TPromise<IView> {
 		const viewDescriptor = ViewsRegistry.getView(id);
 		if (viewDescriptor) {
@@ -518,6 +526,7 @@ export class ViewsService extends Disposable implements IViewsService {
 	}
 
 	private onDidRegisterViewContainer(viewContainer: ViewContainer): void {
+		this.viewDescriptorCollections.set(viewContainer, this._register(new ViewDescriptorCollection(viewContainer, this.contextKeyService)));
 		// TODO: @Joao Remove this after moving SCM Viewlet to ViewContainerViewlet - https://github.com/Microsoft/vscode/issues/49054
 		if (viewContainer.id !== SCM_VIEWLET_ID) {
 			const viewDescriptorCollection = this._register(this.instantiationService.createInstance(ViewDescriptorCollection, viewContainer));

src/vs/workbench/common/views.ts

@@ -113,6 +113,11 @@ export interface IViewDescriptor {
 	readonly hideByDefault?: boolean;
 }
 
+export interface IViewDescriptorCollection {
+	readonly onDidChange: Event<void>;
+	readonly viewDescriptors: IViewDescriptor[];
+}
+
 export interface IViewsRegistry {
 
 	readonly onViewsRegistered: Event<IViewDescriptor[]>;
@@ -213,6 +218,8 @@ export interface IViewsService {
 	_serviceBrand: any;
 
 	openView(id: string, focus?: boolean): TPromise<IView>;
+
+	getViewDescriptors(container: ViewContainer): IViewDescriptorCollection;
 }
 
 // Custom views

src/vs/workbench/parts/scm/electron-browser/scmViewlet.ts

@@ -1065,7 +1065,7 @@ export class SCMViewlet extends PanelViewlet implements IViewModel, IViewsViewle
 		this.menus = instantiationService.createInstance(SCMMenus, undefined);
 		this.menus.onDidChangeTitle(this.updateTitleArea, this, this.disposables);
 
-		this.contributedViews = new PersistentContributableViewsModel(VIEW_CONTAINER, 'scm.views', contextKeyService, storageService, contextService);
+		this.contributedViews = instantiationService.createInstance(PersistentContributableViewsModel, VIEW_CONTAINER, 'scm.views');
 		this.disposables.push(this.contributedViews);
 	}
 

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

@@ -4,14 +4,15 @@
  *--------------------------------------------------------------------------------------------*/
 
 import * as assert from 'assert';
-import { ContributableViewsModel } from 'vs/workbench/browser/parts/views/views';
-import { ViewsRegistry, IViewDescriptor, IViewContainersRegistry, Extensions as ViewContainerExtensions } from 'vs/workbench/common/views';
-import { ContextKeyService } from 'vs/platform/contextkey/browser/contextKeyService';
-import { IContextKeyService, ContextKeyExpr } from 'vs/platform/contextkey/common/contextkey';
-import { TestConfigurationService } from 'vs/platform/configuration/test/common/testConfigurationService';
+import { ContributableViewsModel, ViewsService } from 'vs/workbench/browser/parts/views/views';
+import { ViewsRegistry, IViewDescriptor, IViewContainersRegistry, Extensions as ViewContainerExtensions, IViewsService } from 'vs/workbench/common/views';
 import { IDisposable, dispose } from 'vs/base/common/lifecycle';
 import { move } from 'vs/base/common/arrays';
 import { Registry } from 'vs/platform/registry/common/platform';
+import { workbenchInstantiationService } from 'vs/workbench/test/workbenchTestServices';
+import { ContextKeyExpr, IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
+import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock';
+import { ContextKeyService } from 'vs/platform/contextkey/browser/contextKeyService';
 
 const container = Registry.as<IViewContainersRegistry>(ViewContainerExtensions.ViewContainersRegistry).registerViewContainer('test');
 
@@ -33,24 +34,28 @@ class ViewDescriptorSequence {
 }
 
 suite('ContributableViewsModel', () => {
+
+	let viewsService: IViewsService;
 	let contextKeyService: IContextKeyService;
 
 	setup(() => {
-		const configurationService = new TestConfigurationService();
-		contextKeyService = new ContextKeyService(configurationService);
+		const instantiationService: TestInstantiationService = <TestInstantiationService>workbenchInstantiationService();
+		contextKeyService = instantiationService.createInstance(ContextKeyService);
+		instantiationService.stub(IContextKeyService, contextKeyService);
+		viewsService = instantiationService.createInstance(ViewsService);
 	});
 
 	teardown(() => {
-		contextKeyService.dispose();
+		ViewsRegistry.deregisterViews(ViewsRegistry.getViews(container).map(({ id }) => id), container);
 	});
 
 	test('empty model', function () {
-		const model = new ContributableViewsModel(container, contextKeyService);
+		const model = new ContributableViewsModel(container, viewsService);
 		assert.equal(model.visibleViewDescriptors.length, 0);
 	});
 
 	test('register/unregister', function () {
-		const model = new ContributableViewsModel(container, contextKeyService);
+		const model = new ContributableViewsModel(container, viewsService);
 		const seq = new ViewDescriptorSequence(model);
 
 		assert.equal(model.visibleViewDescriptors.length, 0);
@@ -77,7 +82,7 @@ suite('ContributableViewsModel', () => {
 	});
 
 	test('when contexts', async function () {
-		const model = new ContributableViewsModel(container, contextKeyService);
+		const model = new ContributableViewsModel(container, viewsService);
 		const seq = new ViewDescriptorSequence(model);
 
 		assert.equal(model.visibleViewDescriptors.length, 0);
@@ -122,7 +127,7 @@ suite('ContributableViewsModel', () => {
 	});
 
 	test('when contexts - multiple', async function () {
-		const model = new ContributableViewsModel(container, contextKeyService);
+		const model = new ContributableViewsModel(container, viewsService);
 		const seq = new ViewDescriptorSequence(model);
 
 		const view1: IViewDescriptor = { id: 'view1', ctor: null, container, name: 'Test View 1' };
@@ -145,7 +150,7 @@ suite('ContributableViewsModel', () => {
 	});
 
 	test('when contexts - multiple 2', async function () {
-		const model = new ContributableViewsModel(container, contextKeyService);
+		const model = new ContributableViewsModel(container, viewsService);
 		const seq = new ViewDescriptorSequence(model);
 
 		const view1: IViewDescriptor = { id: 'view1', ctor: null, container, name: 'Test View 1', when: ContextKeyExpr.equals('showview1', true) };
@@ -168,7 +173,7 @@ suite('ContributableViewsModel', () => {
 	});
 
 	test('setVisible', function () {
-		const model = new ContributableViewsModel(container, contextKeyService);
+		const model = new ContributableViewsModel(container, viewsService);
 		const seq = new ViewDescriptorSequence(model);
 
 		const view1: IViewDescriptor = { id: 'view1', ctor: null, container, name: 'Test View 1', canToggleVisibility: true };
@@ -213,7 +218,7 @@ suite('ContributableViewsModel', () => {
 	});
 
 	test('move', function () {
-		const model = new ContributableViewsModel(container, contextKeyService);
+		const model = new ContributableViewsModel(container, viewsService);
 		const seq = new ViewDescriptorSequence(model);
 
 		const view1: IViewDescriptor = { id: 'view1', ctor: null, container, name: 'Test View 1' };

src/vs/workbench/test/workbenchTestServices.ts

@@ -77,6 +77,10 @@ import { Dimension } from 'vs/base/browser/dom';
 import { ILogService, LogLevel } from 'vs/platform/log/common/log';
 import { ILabelService, LabelService } from 'vs/platform/label/common/label';
 import { timeout } from 'vs/base/common/async';
+import { IViewletService } from 'vs/workbench/services/viewlet/browser/viewlet';
+import { ViewletDescriptor } from 'vs/workbench/browser/viewlet';
+import { IViewlet } from 'vs/workbench/common/viewlet';
+import { IProgressService } from 'vs/platform/progress/common/progress';
 
 export function createFileInput(instantiationService: IInstantiationService, resource: URI): FileEditorInput {
 	return instantiationService.createInstance(FileEditorInput, resource, void 0);
@@ -276,6 +280,7 @@ export function workbenchInstantiationService(): IInstantiationService {
 	const editorService = new TestEditorService();
 	instantiationService.stub(IEditorService, editorService);
 	instantiationService.stub(ICodeEditorService, new TestCodeEditorService());
+	instantiationService.stub(IViewletService, new TestViewletService());
 
 	return instantiationService;
 }
@@ -1384,6 +1389,33 @@ export class TestHashService implements IHashService {
 	}
 }
 
+export class TestViewletService implements IViewletService {
+
+	_serviceBrand: ServiceIdentifier<any>;
+
+	readonly onDidViewletRegister: Event<ViewletDescriptor> = new Emitter<ViewletDescriptor>().event;
+	onDidViewletOpen: Event<IViewlet> = new Emitter<IViewlet>().event;
+	onDidViewletClose: Event<IViewlet> = new Emitter<IViewlet>().event;
+	onDidViewletEnablementChange: Event<{ id: string, enabled: boolean }> = new Emitter<{ id: string, enabled: boolean }>().event;
+
+	openViewlet(id: string, focus?: boolean): TPromise<IViewlet> { return null; }
+
+	getActiveViewlet(): IViewlet { return null; }
+
+	getDefaultViewletId(): string { return null; }
+
+	getViewlet(id: string): ViewletDescriptor { return null; }
+
+	getAllViewlets(): ViewletDescriptor[] { return null; }
+
+	getViewlets(): ViewletDescriptor[] { return null; }
+
+	setViewletEnablement(id: string, enabled: boolean): void { }
+
+	getProgressIndicator(id: string): IProgressService { return null; }
+
+}
+
 export function getRandomTestPath(tmpdir: string, ...segments: string[]): string {
 	return paths.join(tmpdir, ...segments, generateUuid());
 }