From 3d63750fda28f8069f71d06823206857eddf56a6 Mon Sep 17 00:00:00 2001
From: Joao Moreno <joao.moreno@microsoft.com>
Date: Tue, 3 Oct 2017 15:20:21 +0200
Subject: [PATCH] prevent recursive splitview mutations

fixes #35497
---
 src/vs/base/browser/ui/splitview/splitview.ts | 35 +++++++++++++++++++
 .../browser/ui/splitview/splitview.test.ts    | 29 +++++++++++++++
 .../parts/scm/electron-browser/scmViewlet.ts  |  6 ++--
 3 files changed, 67 insertions(+), 3 deletions(-)

diff --git a/src/vs/base/browser/ui/splitview/splitview.ts b/src/vs/base/browser/ui/splitview/splitview.ts
index ba0b37167ad..b90401d437f 100644
--- a/src/vs/base/browser/ui/splitview/splitview.ts
+++ b/src/vs/base/browser/ui/splitview/splitview.ts
@@ -54,6 +54,11 @@ interface ISashDragState {
 	maxDelta: number;
 }
 
+enum State {
+	Idle,
+	Busy
+}
+
 export class SplitView implements IDisposable {
 
 	private orientation: Orientation;
@@ -63,6 +68,7 @@ export class SplitView implements IDisposable {
 	private viewItems: IViewItem[] = [];
 	private sashItems: ISashItem[] = [];
 	private sashDragState: ISashDragState;
+	private state: State = State.Idle;
 
 	get length(): number {
 		return this.viewItems.length;
@@ -78,6 +84,12 @@ export class SplitView implements IDisposable {
 	}
 
 	addView(view: IView, size: number, index = this.viewItems.length): void {
+		if (this.state !== State.Idle) {
+			throw new Error('Cant modify splitview');
+		}
+
+		this.state = State.Busy;
+
 		// Add view
 		const container = dom.$('.split-view-view');
 
@@ -125,9 +137,16 @@ export class SplitView implements IDisposable {
 
 		view.render(container, this.orientation);
 		this.relayout();
+		this.state = State.Idle;
 	}
 
 	removeView(index: number): void {
+		if (this.state !== State.Idle) {
+			throw new Error('Cant modify splitview');
+		}
+
+		this.state = State.Busy;
+
 		if (index < 0 || index >= this.viewItems.length) {
 			return;
 		}
@@ -144,9 +163,16 @@ export class SplitView implements IDisposable {
 		}
 
 		this.relayout();
+		this.state = State.Idle;
 	}
 
 	moveView(from: number, to: number): void {
+		if (this.state !== State.Idle) {
+			throw new Error('Cant modify splitview');
+		}
+
+		this.state = State.Busy;
+
 		if (from < 0 || from >= this.viewItems.length) {
 			return;
 		}
@@ -169,6 +195,7 @@ export class SplitView implements IDisposable {
 		}
 
 		this.layoutViews();
+		this.state = State.Idle;
 	}
 
 	private relayout(): void {
@@ -221,6 +248,12 @@ export class SplitView implements IDisposable {
 	}
 
 	resizeView(index: number, size: number): void {
+		if (this.state !== State.Idle) {
+			throw new Error('Cant modify splitview');
+		}
+
+		this.state = State.Busy;
+
 		if (index < 0 || index >= this.viewItems.length) {
 			return;
 		}
@@ -248,6 +281,8 @@ export class SplitView implements IDisposable {
 
 			this.resize(index - 1, deltaUp);
 		}
+
+		this.state = State.Idle;
 	}
 
 	getViewSize(index: number): number {
diff --git a/src/vs/base/test/browser/ui/splitview/splitview.test.ts b/src/vs/base/test/browser/ui/splitview/splitview.test.ts
index 85185e428b3..044af37c4eb 100644
--- a/src/vs/base/test/browser/ui/splitview/splitview.test.ts
+++ b/src/vs/base/test/browser/ui/splitview/splitview.test.ts
@@ -311,4 +311,33 @@ suite('Splitview', () => {
 		view2.dispose();
 		view1.dispose();
 	});
+
+	test('issue #35497', () => {
+		const view1 = new TestView(160, Number.POSITIVE_INFINITY);
+		const view2 = new TestView(66, 66);
+
+		const splitview = new SplitView(container);
+		splitview.layout(986);
+
+		splitview.addView(view1, 142, 0);
+		assert.equal(view1.size, 986, 'first view is stretched');
+
+		view2.onDidRender(() => {
+			assert.throws(() => splitview.resizeView(1, 922));
+			assert.throws(() => splitview.resizeView(1, 922));
+		});
+
+		splitview.addView(view2, 66, 0);
+		assert.equal(view2.size, 66, 'second view is fixed');
+		assert.equal(view1.size, 986 - 66, 'first view is collapsed');
+
+		const viewContainers = container.querySelectorAll('.split-view-view');
+		assert.equal(viewContainers.length, 2, 'there are two view containers');
+		assert.equal((viewContainers.item(0) as HTMLElement).style.height, '66px', 'second view container is 66px');
+		assert.equal((viewContainers.item(1) as HTMLElement).style.height, `${986 - 66}px`, 'first view container is 66px');
+
+		splitview.dispose();
+		view2.dispose();
+		view1.dispose();
+	});
 });
\ No newline at end of file
diff --git a/src/vs/workbench/parts/scm/electron-browser/scmViewlet.ts b/src/vs/workbench/parts/scm/electron-browser/scmViewlet.ts
index 501bdf5d32e..09fed0a3ba0 100644
--- a/src/vs/workbench/parts/scm/electron-browser/scmViewlet.ts
+++ b/src/vs/workbench/parts/scm/electron-browser/scmViewlet.ts
@@ -851,9 +851,10 @@ export class SCMViewlet extends PanelViewlet implements IViewModel {
 
 		if (shouldMainPanelBeVisible) {
 			this.mainPanel = this.instantiationService.createInstance(MainPanel, this);
-			const selectionChangeDisposable = this.mainPanel.onSelectionChange(this.onSelectionChange, this);
 			this.addPanel(this.mainPanel, this.mainPanel.minimumSize, 0);
 
+			const selectionChangeDisposable = this.mainPanel.onSelectionChange(this.onSelectionChange, this);
+
 			this.mainPanelDisposable = toDisposable(() => {
 				this.removePanel(this.mainPanel);
 				selectionChangeDisposable.dispose();
@@ -959,7 +960,7 @@ export class SCMViewlet extends PanelViewlet implements IViewModel {
 	}
 
 	protected isSingleView(): boolean {
-		return super.isSingleView() && this.repositories.length === 1;
+		return super.isSingleView() && this.repositoryPanels.length === 1;
 	}
 
 	hide(repository: ISCMRepository): void {
@@ -970,7 +971,6 @@ export class SCMViewlet extends PanelViewlet implements IViewModel {
 		this.mainPanel.hide(repository);
 	}
 
-
 	dispose(): void {
 		this.disposables = dispose(this.disposables);
 		this.mainPanelDisposable.dispose();